Add new, improved raw_convert using fmt/fast_float (DoM #2220).

The old one was broken if the thousands separator was the same
as the decimal separator (e.g. if you set de_DE and change the
decimal separator to .)

6 files changed, 96 insertions, 73 deletions

diff --git a/libdcp-1.0.pc.in b/libdcp-1.0.pc.in
index a8d1007f..743c9e72 100644
--- a/libdcp-1.0.pc.in
+++ b/libdcp-1.0.pc.in
@@ -5,6 +5,6 @@ includedir=@includedir@
 Name: libdcp
 Description: DCP reading and writing library
 Version: @version@
-Requires: openssl libxml++-@xmlpp_api@ xmlsec1 libasdcp-dcpomatic xerces-c libavutil libavcodec
+Requires: openssl libxml++-@xmlpp_api@ xmlsec1 libasdcp-dcpomatic xerces-c libavutil libavcodec fmt
 Libs: @libs@
 Cflags: -I${includedir}
diff --git a/src/raw_convert.cc b/src/raw_convert.cc
index c806b45e..eeab0dad 100644
--- a/src/raw_convert.cc
+++ b/src/raw_convert.cc
@@ -33,7 +33,8 @@
 
 
 #include "raw_convert.h"
-#include "locale_convert.h"
+#include <fmt/format.h>
+#include <fast_float/fast_float.h>
 #include <boost/algorithm/string.hpp>
 
 
@@ -41,91 +42,79 @@ using std::string;
 using std::wstring;
 
 
-/** @param v Numeric value as an ASCII string */
-static
-string
-make_raw (string v)
-{
-	struct lconv* lc = localeconv ();
-	/* thousands_sep may be . so remove them before changing decimal points */
-	boost::algorithm::replace_all (v, lc->thousands_sep, "");
-	boost::algorithm::replace_all (v, lc->decimal_point, ".");
-	return v;
-}
-
-
-static
+template <>
 string
-make_local (string v)
+dcp::raw_convert(unsigned char v, int, bool)
 {
-	struct lconv* lc = localeconv ();
-	boost::algorithm::replace_all (v, ".", lc->decimal_point);
-	/* We hope it's ok not to add in thousands separators here */
-	return v;
+	return fmt::to_string(v);
 }
 
 
 template <>
 string
-dcp::raw_convert (unsigned char v, int precision, bool fixed)
+dcp::raw_convert(unsigned short int v, int, bool)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	return fmt::to_string(v);
 }
 
 
 template <>
 string
-dcp::raw_convert (unsigned short int v, int precision, bool fixed)
+dcp::raw_convert(int v, int, bool)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	return fmt::to_string(v);
 }
 
 
 template <>
 string
-dcp::raw_convert (int v, int precision, bool fixed)
+dcp::raw_convert(unsigned int v, int, bool)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	return fmt::to_string(v);
 }
 
 
 template <>
 string
-dcp::raw_convert (unsigned int v, int precision, bool fixed)
+dcp::raw_convert(long v, int, bool)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	return fmt::to_string(v);
 }
 
 
 template <>
 string
-dcp::raw_convert (long v, int precision, bool fixed)
+dcp::raw_convert(unsigned long v, int, bool)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	return fmt::to_string(v);
 }
 
 
 template <>
 string
-dcp::raw_convert (unsigned long v, int precision, bool fixed)
+dcp::raw_convert(long long v, int, bool)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	return fmt::to_string(v);
 }
 
 
 template <>
 string
-dcp::raw_convert (long long v, int precision, bool fixed)
+dcp::raw_convert(unsigned long long v, int, bool)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	return fmt::to_string(v);
 }
 
 
-template <>
-string
-dcp::raw_convert (unsigned long long v, int precision, bool fixed)
+static
+void
+make_format_string(char* buffer, int buffer_length, int precision, bool fixed)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	if (fixed) {
+		snprintf(buffer, buffer_length, "{:.%df}", precision);
+	} else {
+		snprintf(buffer, buffer_length, "{:.%d}", precision);
+	}
 }
 
 
@@ -133,7 +122,13 @@ template <>
 string
 dcp::raw_convert (float v, int precision, bool fixed)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	if (precision < 16) {
+		char format[16];
+		make_format_string(format, 16, precision, fixed);
+		return fmt::format(format, v);
+	}
+
+	return fmt::to_string(v);
 }
 
 
@@ -141,7 +136,13 @@ template <>
 string
 dcp::raw_convert (double v, int precision, bool fixed)
 {
-	return make_raw (locale_convert<string> (v, precision, fixed));
+	if (precision < 16) {
+		char format[16];
+		make_format_string(format, 16, precision, fixed);
+		return fmt::format(format, v);
+	}
+
+	return fmt::to_string(v);
 }
 
 
@@ -188,105 +189,119 @@ dcp::raw_convert (wchar_t const * v, int, bool)
 }
 
 
+template <typename T>
+T
+convert_with_fast_float(string v)
+{
+	T result;
+	auto const answer = fast_float::from_chars(v.data(), v.data() + v.size(), result);
+	if (answer.ec != std::errc()) {
+		return 0;
+	}
+
+	return result;
+}
+
+
 template <>
 unsigned char
-dcp::raw_convert(string v, int precision, bool fixed)
+dcp::raw_convert(string v, int, bool)
 {
-	return locale_convert<unsigned char> (make_local (v), precision, fixed);
+	return convert_with_fast_float<unsigned char>(v);
 }
 
 
 template <>
 unsigned short int
-dcp::raw_convert(string v, int precision, bool fixed)
+dcp::raw_convert(string v, int, bool)
 {
-	return locale_convert<unsigned short int> (make_local (v), precision, fixed);
+	return convert_with_fast_float<unsigned short int>(v);
 }
 
 
 template <>
 int
-dcp::raw_convert (string v, int precision, bool fixed)
+dcp::raw_convert(string v, int, bool)
 {
-	return locale_convert<int> (make_local (v), precision, fixed);
+	return convert_with_fast_float<int>(v);
 }
 
 
 template <>
 long
-dcp::raw_convert (string v, int precision, bool fixed)
+dcp::raw_convert(string v, int, bool)
 {
-	return locale_convert<long> (make_local (v), precision, fixed);
+	return convert_with_fast_float<long>(v);
 }
 
 
 template <>
 unsigned long
-dcp::raw_convert (string v, int precision, bool fixed)
+dcp::raw_convert(string v, int, bool)
 {
-	return locale_convert<unsigned long> (make_local (v), precision, fixed);
+	return convert_with_fast_float<unsigned long>(v);
 }
 
 
 template <>
 long long
-dcp::raw_convert (string v, int precision, bool fixed)
+dcp::raw_convert(string v, int, bool)
 {
-	return locale_convert<long long> (make_local (v), precision, fixed);
+	return convert_with_fast_float<long long>(v);
 }
 
 
 template <>
 unsigned long long
-dcp::raw_convert (string v, int precision, bool fixed)
+dcp::raw_convert(string v, int, bool)
 {
-	return locale_convert<unsigned long long> (make_local (v), precision, fixed);
+	return convert_with_fast_float<unsigned long long>(v);
 }
 
 
 template <>
 int
-dcp::raw_convert(char* v, int precision, bool fixed)
+dcp::raw_convert(char* v, int, bool)
 {
-	return locale_convert<int>(make_local (v), precision, fixed);
+	return convert_with_fast_float<int>(string(v));
 }
 
 
 template <>
 int
-dcp::raw_convert (char const * v, int precision, bool fixed)
+dcp::raw_convert(char const * v, int, bool)
 {
-	return locale_convert<int> (make_local (v), precision, fixed);
+	return convert_with_fast_float<int>(string(v));
 }
 
 
 template <>
 float
-dcp::raw_convert (string v, int precision, bool fixed)
+dcp::raw_convert(string v, int, bool)
 {
-	return locale_convert<float> (make_local (v), precision, fixed);
+	return convert_with_fast_float<float>(v);
 }
 
 
 template <>
 float
-dcp::raw_convert (char const * v, int precision, bool fixed)
+dcp::raw_convert(char const * v, int, bool)
 {
-	return locale_convert<float> (make_local (v), precision, fixed);
+	return convert_with_fast_float<float>(string(v));
 }
 
 
 template <>
 double
-dcp::raw_convert (string v, int precision, bool fixed)
+dcp::raw_convert(string v, int, bool)
 {
-	return locale_convert<double> (make_local (v), precision, fixed);
+	return convert_with_fast_float<double>(v);
 }
 
 
 template <>
 double
-dcp::raw_convert (char const * v, int precision, bool fixed)
+dcp::raw_convert(char const * v, int, bool)
 {
-	return locale_convert<double> (make_local (v), precision, fixed);
+	return convert_with_fast_float<double>(string(v));
 }
diff --git a/src/wscript b/src/wscript
index 1f0190fc..576ca748 100644
--- a/src/wscript
+++ b/src/wscript
@@ -252,6 +252,9 @@ def build(bld):
               warnings.h
               """
 
+ 
+    uselib = 'BOOST_FILESYSTEM BOOST_SIGNALS2 BOOST_DATETIME OPENSSL SIGC++ LIBXML++ OPENJPEG CXML XMLSEC1 ASDCPLIB_DCPOMATIC XERCES AVCODEC AVUTIL FMT FAST_FLOAT'
+
     # Main library
     if bld.env.STATIC:
         obj = bld(features='cxx cxxstlib')
@@ -260,7 +263,7 @@ def build(bld):
     obj.name = 'libdcp%s' % bld.env.API_VERSION
     obj.target = 'dcp%s' % bld.env.API_VERSION
     obj.export_includes = ['.']
-    obj.uselib = 'BOOST_FILESYSTEM BOOST_SIGNALS2 BOOST_DATETIME OPENSSL SIGC++ LIBXML++ OPENJPEG CXML XMLSEC1 ASDCPLIB_DCPOMATIC XERCES AVCODEC AVUTIL'
+    obj.uselib = uselib
     obj.source = source
 
     # Library for gcov
@@ -272,7 +275,7 @@ def build(bld):
         obj.name = 'libdcp%s_gcov' % bld.env.API_VERSION
         obj.target = 'dcp%s_gcov' % bld.env.API_VERSION
         obj.export_includes = ['.']
-        obj.uselib = 'BOOST_FILESYSTEM BOOST_SIGNALS2 BOOST_DATETIME OPENSSL SIGC++ LIBXML++ OPENJPEG CXML XMLSEC1 ASDCPLIB_DCPOMATIC XERCES AVCODEC AVUTIL'
+        obj.uselib = uselib
         obj.use = 'libkumu-libdcp%s libasdcp-libdcp%s' % (bld.env.API_VERSION, bld.env.API_VERSION)
         obj.source = source
         obj.cppflags = ['-fprofile-arcs', '-ftest-coverage', '-fno-inline', '-fno-default-inline', '-fno-elide-constructors', '-g', '-O0']
diff --git a/test/wscript b/test/wscript
index 13493f1e..f8286d05 100644
--- a/test/wscript
+++ b/test/wscript
@@ -51,9 +51,11 @@ def configure(conf):
     conf.env.prepend_value('LINKFLAGS', '-Lsrc')
 
 def build(bld):
+    uselib = 'BOOST_TEST BOOST_FILESYSTEM BOOST_DATETIME OPENJPEG CXML XMLSEC1 SNDFILE OPENMP ASDCPLIB_DCPOMATIC LIBXML++ OPENSSL XERCES DL AVCODEC AVUTIL FMT FAST_FLOAT'
+
     obj = bld(features='cxx cxxprogram')
     obj.name   = 'tests'
-    obj.uselib = 'BOOST_TEST BOOST_FILESYSTEM BOOST_DATETIME OPENJPEG CXML XMLSEC1 SNDFILE OPENMP ASDCPLIB_DCPOMATIC LIBXML++ OPENSSL XERCES DL AVCODEC AVUTIL'
+    obj.uselib = uselib
     obj.cppflags = ['-fno-inline', '-fno-elide-constructors', '-g', '-O0']
     if bld.env['CXX_NAME'] == 'gcc':
         obj.cppflags.append('-fno-default-inline')
@@ -126,7 +128,7 @@ def build(bld):
 
     obj = bld(features='cxx cxxprogram')
     obj.name   = 'subs_in_out'
-    obj.uselib = 'BOOST_TEST BOOST_FILESYSTEM OPENJPEG CXML OPENMP ASDCPLIB_DCPOMATIC XMLSEC1 OPENSSL DL LIBXML++ AVCODEC AVUTIL'
+    obj.uselib = uselib
     obj.cppflags = ['-fno-inline', '-fno-elide-constructors', '-g', '-O0']
     if bld.env['CXX_NAME'] == 'gcc':
         obj.cppflags.append('-fno-default-inline')
@@ -143,7 +145,7 @@ def build(bld):
 
     obj = bld(features='cxx cxxprogram')
     obj.name   = 'rewrite_subs'
-    obj.uselib = 'BOOST_TEST BOOST_FILESYSTEM OPENJPEG CXML OPENMP ASDCPLIB_DCPOMATIC XMLSEC1 OPENSSL DL LIBXML++ AVCODEC AVUTIL'
+    obj.uselib = uselib
     obj.cppflags = ['-fno-inline', '-fno-elide-constructors', '-g', '-O0']
     if bld.env['CXX_NAME'] == 'gcc':
         obj.cppflags.append('-fno-default-inline')
diff --git a/tools/wscript b/tools/wscript
index d282c88c..a5fcf545 100644
--- a/tools/wscript
+++ b/tools/wscript
@@ -32,7 +32,7 @@
 #
 
 def build(bld):
-    uselib = 'OPENJPEG CXML OPENMP ASDCPLIB_DCPOMATIC BOOST_FILESYSTEM LIBXML++ XMLSEC1 OPENSSL XERCES DL MAGICK AVCODEC AVUTIL'
+    uselib = 'OPENJPEG CXML OPENMP ASDCPLIB_DCPOMATIC BOOST_FILESYSTEM LIBXML++ XMLSEC1 OPENSSL XERCES DL MAGICK AVCODEC AVUTIL FMT FAST_FLOAT'
 
     for f in ['diff', 'info', 'verify']:
         obj = bld(features='cxx cxxprogram')
diff --git a/wscript b/wscript
index ba62d86d..1fcdd119 100644
--- a/wscript
+++ b/wscript
@@ -266,6 +266,9 @@ def configure(conf):
     conf.check_cfg(package='libavcodec', args='--cflags --libs', uselib_store='AVCODEC', mandatory=True)
     conf.check_cfg(package='libavutil', args='--cflags --libs', uselib_store='AVUTIL', mandatory=True)
 
+    conf.check_cfg(package='fmt', args='--cflags --libs', uselib_store='FMT', mandatory=True)
+    conf.check_cxx(header_name="fast_float/fast_float.h", uselib_store='FAST_FLOAT', mandatory=True)
+
     if not conf.env.DISABLE_TESTS:
         conf.recurse('test')
         if conf.options.enable_gcov: