Implement locale-agnostic number parsing

This commit adds Daniel Lemire's fast_float library as an external dependency, and applies it to our uses of sscanf with %f, strtod, and istringstream. Additionally, for parsing integer numbers, we implement a from_chars shim that forwards the call to strtol_l along with a statically initialized locale constant. The usage of fast_float is warded by a new OCIO_USE_FAST_FLOAT configuration variable; if disabled, an identical approach to integers is followed. See: Daniel Lemire, Number Parsing at a Gigabyte per Second, Software: Pratice and Experience 51 (8), 2021. <https://arxiv.org/abs/2101.11408> Fixes AcademySoftwareFoundation#297 Fixes AcademySoftwareFoundation#379 Fixes AcademySoftwareFoundation#1322 Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark · Nov 10, 2021 · 76a23ac · 76a23ac
1 parent 64dccad
commit 76a23ac
Show file tree

Hide file tree

Showing 14 changed files with 383 additions and 65 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -147,7 +147,7 @@ option(OCIO_USE_WINDOWS_UNICODE "On Windows only, compile with Unicode support"
 
 option(OCIO_USE_SSE "Specify whether to enable SSE CPU performance optimizations" ON)
 option(OCIO_USE_OPENEXR_HALF "Specify whether to use an OpenEXR/IlmBase install of the Half library (<=v2.5) instead of the newer Imath library (>=v3.0)" OFF)
-
+option(OCIO_USE_FAST_FLOAT "Specify whether to use the fastfloat library to parse floating point numbers" ON)
 
 ###############################################################################
 # GPU configuration

diff --git a/share/cmake/modules/FindExtPackages.cmake b/share/cmake/modules/FindExtPackages.cmake
@@ -56,6 +56,12 @@ else()
     set(OCIO_USE_IMATH_HALF "0" CACHE STRING "Whether 'half' type will be sourced from the Imath library (>=v3.0)" FORCE)
 endif()
 
+# fast_float
+# https://github.com/fastfloat/fast_float
+if (OCIO_USE_FAST_FLOAT)
+    find_package(FastFloat 3.2.0 REQUIRED)
+endif()
+
 if(OCIO_BUILD_APPS)
 
     # NOTE: Depending of the compiler version lcms2 2.2 does not compile with 

diff --git a/share/cmake/modules/FindFastFloat.cmake b/share/cmake/modules/FindFastFloat.cmake
@@ -0,0 +1,128 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright Contributors to the OpenColorIO Project.
+#
+# Locate or install the fast_float library
+#
+# Variables defined by this module:
+#   fast_float_FOUND - If FALSE, do not try to include fast_float.h
+#   fast_float_INCLUDE_DIR - Where to find fast_float.h
+#   fast_float_VERSION - The version of the library (if available)
+#
+# Targets defined by this module:
+#   fast_float::fast_float - IMPORTED target, if found
+#
+# The library is include-only, there is no associated binary.
+#
+# If fast_float is not installed in a standard path, you can use the 
+# fast_float_ROOT variable to tell CMake where to find it. If it is not found 
+# and OCIO_INSTALL_EXT_PACKAGES is set to MISSING or ALL, fast_float will be 
+# downloaded, built, and statically-linked into libOpenColorIO at build time.
+#
+
+###############################################################################
+### Try to find package ###
+
+if(NOT OCIO_INSTALL_EXT_PACKAGES STREQUAL ALL)
+    set(_FastFloat_REQUIRED_VARS FastFloat_INCLUDE_DIR)
+
+    if(NOT DEFINED FastFloat_ROOT)
+        # Search for FastFloatConfig.cmake
+        # Do notice that the CMake Config module is "FastFloat"
+        # while the library is "fast_float"
+        # (capital letters replaced by lowercase and underscore)
+        find_package(FastFloat ${FastFloat_FIND_VERSION} CONFIG QUIET)
+    endif()
+
+    # There is no pkg-config support on fast_float.
+
+    # Override REQUIRED if package can be installed
+    if(OCIO_INSTALL_EXT_PACKAGES STREQUAL MISSING)
+        set(FastFloat_FIND_REQUIRED FALSE)
+    endif()
+
+    include(FindPackageHandleStandardArgs)
+    find_package_handle_standard_args(FastFloat
+        REQUIRED_VARS 
+            ${_FastFloat_REQUIRED_VARS}
+        VERSION_VAR
+            FastFloat_VERSION
+    )
+endif()
+
+###############################################################################
+### Create target
+
+if (NOT TARGET fast_float::fast_float)
+    add_library(fast_float::fast_float INTERFACE IMPORTED GLOBAL)
+    set(_FastFloat_TARGET_CREATE TRUE)
+endif()
+
+###############################################################################
+### Install package from source ###
+
+if(NOT FastFloat_FOUND)
+    include(ExternalProject)
+    include(GNUInstallDirs)
+
+    set(_EXT_DIST_ROOT "${CMAKE_BINARY_DIR}/ext/dist")
+    set(_EXT_BUILD_ROOT "${CMAKE_BINARY_DIR}/ext/build")
+
+    # Set find_package standard args
+    set(FastFloat_FOUND TRUE)
+    set(FastFloat_VERSION ${FastFloat_FIND_VERSION})
+    set(FastFloat_INCLUDE_DIR "${_EXT_DIST_ROOT}/${CMAKE_INSTALL_INCLUDEDIR}")
+
+    # This library is include-only. No need to add further flags.
+
+    if(_FastFloat_TARGET_CREATE)
+        set(FastFloat_CMAKE_ARGS
+            ${FastFloat_CMAKE_ARGS}
+            -DCMAKE_POSITION_INDEPENDENT_CODE=ON
+            -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
+            -DCMAKE_CXX_FLAGS=${FastFloat_CXX_FLAGS}
+            -DCMAKE_CXX_STANDARD=${CMAKE_CXX_STANDARD}
+            -DCMAKE_INSTALL_MESSAGE=${CMAKE_INSTALL_MESSAGE}
+            -DCMAKE_INSTALL_PREFIX=${_EXT_DIST_ROOT}
+            -DCMAKE_OBJECT_PATH_MAX=${CMAKE_OBJECT_PATH_MAX}
+            -DFASTFLOAT_TEST=OFF
+            -DFASTFLOAT_SANITIZE=OFF
+        )
+
+        if(CMAKE_TOOLCHAIN_FILE)
+            set(FastFloat_CMAKE_ARGS
+                ${FastFloat_CMAKE_ARGS} -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE})
+        endif()
+
+        if(APPLE)
+            set(FastFloat_CMAKE_ARGS
+                ${FastFloat_CMAKE_ARGS} -DCMAKE_OSX_DEPLOYMENT_TARGET=${CMAKE_OSX_DEPLOYMENT_TARGET})
+        endif()
+
+        # Hack to let imported target be built from ExternalProject_Add
+        file(MAKE_DIRECTORY ${FastFloat_INCLUDE_DIR})
+
+        ExternalProject_Add(FastFloat_install
+            GIT_REPOSITORY "https://github.com/fastfloat/fast_float.git"
+            GIT_TAG "v${FastFloat_VERSION}"
+            GIT_CONFIG advice.detachedHead=false
+            GIT_SHALLOW TRUE
+            PREFIX "${_EXT_BUILD_ROOT}/fast_float"
+            CMAKE_ARGS ${FastFloat_CMAKE_ARGS}
+            EXCLUDE_FROM_ALL TRUE
+        )
+
+        add_dependencies(fast_float::fast_float FastFloat_install)
+        message(STATUS "Installing fast_float: ${FastFloat_INCLUDE_DIR} (version \"${FastFloat_VERSION}\")")
+    endif()
+endif()
+
+###############################################################################
+### Configure target ###
+
+if(_FastFloat_TARGET_CREATE)
+    set_target_properties(fast_float::fast_float PROPERTIES
+        INTERFACE_INCLUDE_DIRECTORIES ${FastFloat_INCLUDE_DIR}
+    )
+
+    mark_as_advanced(FastFloat_INCLUDE_DIR FastFloat_VERSION)
+endif()
diff --git a/src/OpenColorIO/CMakeLists.txt b/src/OpenColorIO/CMakeLists.txt
@@ -227,6 +227,7 @@ target_link_libraries(OpenColorIO
         ${OCIO_HALF_LIB}
         pystring::pystring
         sampleicc::sampleicc
+        utils::from_chars
         utils::strings
         yaml-cpp
 )

diff --git a/src/OpenColorIO/ParseUtils.cpp b/src/OpenColorIO/ParseUtils.cpp
@@ -11,6 +11,7 @@
 #include "ParseUtils.h"
 #include "Platform.h"
 #include "utils/StringUtils.h"
+#include "utils/NumberUtils.h"
 
 
 namespace OCIO_NAMESPACE
@@ -526,6 +527,7 @@ static constexpr int DOUBLE_DECIMALS = 16;
 std::string FloatToString(float value)
 {
     std::ostringstream pretty;
+    pretty.imbue(std::locale::classic());
     pretty.precision(FLOAT_DECIMALS);
     pretty << value;
     return pretty.str();
@@ -536,6 +538,7 @@ std::string FloatVecToString(const float * fval, unsigned int size)
     if(size<=0) return "";
 
     std::ostringstream pretty;
+    pretty.imbue(std::locale::classic());
     pretty.precision(FLOAT_DECIMALS);
     for(unsigned int i=0; i<size; ++i)
     {
@@ -550,9 +553,9 @@ bool StringToFloat(float * fval, const char * str)
 {
     if(!str) return false;
 
-    std::istringstream inputStringstream(str);
     float x;
-    if(!(inputStringstream >> x))
+    const auto result = NumberUtils::from_chars(str, str + strlen(str), x);
+    if (!result)
     {
         return false;
     }
@@ -567,6 +570,7 @@ bool StringToInt(int * ival, const char * str, bool failIfLeftoverChars)
     if(!ival) return false;
 
     std::istringstream i(str);
+    i.imbue(std::locale::classic());
     char c=0;
     if (!(i >> *ival) || (failIfLeftoverChars && i.get(c))) return false;
     return true;
@@ -575,6 +579,7 @@ bool StringToInt(int * ival, const char * str, bool failIfLeftoverChars)
 std::string DoubleToString(double value)
 {
     std::ostringstream pretty;
+    pretty.imbue(std::locale::classic());
     pretty.precision(DOUBLE_DECIMALS);
     pretty << value;
     return pretty.str();
@@ -585,6 +590,7 @@ std::string DoubleVecToString(const double * val, unsigned int size)
     if (size <= 0) return "";
 
     std::ostringstream pretty;
+    pretty.imbue(std::locale::classic());
     pretty.precision(DOUBLE_DECIMALS);
     for (unsigned int i = 0; i<size; ++i)
     {
@@ -601,10 +607,10 @@ bool StringVecToFloatVec(std::vector<float> &floatArray, const StringUtils::Stri
 
     for(unsigned int i=0; i<lineParts.size(); i++)
     {
-        std::istringstream inputStringstream(lineParts[i]);
         float x;
-        if(!(inputStringstream >> x))
-        {
+        const char *str = lineParts[i].c_str();
+        const auto result = NumberUtils::from_chars(str, str + strlen(str), x);
+        if (!result) {
             return false;
         }
         floatArray[i] = x;

diff --git a/src/OpenColorIO/fileformats/FileFormatHDL.cpp b/src/OpenColorIO/fileformats/FileFormatHDL.cpp
@@ -37,6 +37,7 @@
 #include "ParseUtils.h"
 #include "transforms/FileTransform.h"
 #include "utils/StringUtils.h"
+#include "utils/NumberUtils.h"
 
 
 namespace OCIO_NAMESPACE
@@ -186,12 +187,11 @@ readLuts(std::istream& istream,
             // - StringToFloat took 3879 (avg nanoseconds per value)
             // - stdtod took 169 nanoseconds
             char* endptr = 0;
-            float v = static_cast<float>(strtod(word.c_str(), &endptr));
+            float v;
+            bool result = NumberUtils::from_chars(word.c_str(), word.c_str() + word.size(), v);
 
-            if(!*endptr)
+            if(result)
             {
-                // Since each word should contain a single
-                // float value, the pointer should be null
                 lutValues[lutname].push_back(v);
             }
             else

diff --git a/src/OpenColorIO/fileformats/FileFormatIridasLook.cpp b/src/OpenColorIO/fileformats/FileFormatIridasLook.cpp
@@ -16,6 +16,7 @@
 #include "pystring/pystring.h"
 #include "transforms/FileTransform.h"
 #include "utils/StringUtils.h"
+#include "utils/NumberUtils.h"
 
 
 /*
@@ -417,9 +418,11 @@ class XMLParserHelper
             std::string size_clean = pystring::strip(size_raw, "'\" "); // strip quotes and space
 
             char* endptr = 0;
-            int size_3d = static_cast<int>(strtol(size_clean.c_str(), &endptr, 10));
+            long int size_3d;
+
+            const bool result = NumberUtils::from_chars(size_clean.c_str(), size_clean.c_str() + size_clean.size(), size_3d);
 
-            if (*endptr)
+            if (!result)
             {
                 // strtol didn't consume entire string, there was
                 // remaining data, thus did not contain a single integer
@@ -428,7 +431,7 @@ class XMLParserHelper
                 os << "'. Expected quoted integer";
                 pImpl->Throw(os.str().c_str());
             }
-            pImpl->m_lutSize = size_3d;
+            pImpl->m_lutSize = static_cast<int>(size_3d);
         }
         else if (pImpl->m_data)
         {

diff --git a/src/OpenColorIO/fileformats/FileFormatSpi1D.cpp b/src/OpenColorIO/fileformats/FileFormatSpi1D.cpp
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: BSD-3-Clause
 // Copyright Contributors to the OpenColorIO Project.
 
+#include <cmath>
 #include <cstdio>
 #include <sstream>
 
@@ -13,7 +14,7 @@
 #include "Platform.h"
 #include "transforms/FileTransform.h"
 #include "utils/StringUtils.h"
-
+#include "utils/NumberUtils.h"
 
 /*
 Version 1
@@ -124,10 +125,25 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             }
             else if(StringUtils::StartsWith(headerLine, "From"))
             {
-                if (sscanf(lineBuffer, "From %f %f", &from_min, &from_max) != 2)
+                char fromMinS[64] = "";
+                char fromMaxS[64] = "";
+#ifdef _WIN32
+                if (sscanf(lineBuffer, "From %s %s", fromMinS, 64, fromMaxS, 64) != 2)
+#else
+                if (sscanf(lineBuffer, "From %s %s", fromMinS, fromMaxS) != 2)
+#endif
                 {
                     ThrowErrorMessage("Invalid 'From' Tag", currentLine, headerLine);
                 }
+                else
+                {
+                    const auto fromMinAnswer = NumberUtils::from_chars(fromMinS, fromMinS +64, from_min);
+                    const auto fromMaxAnswer = NumberUtils::from_chars(fromMaxS, fromMaxS + 64, from_max);
+
+                    if (!fromMinAnswer || !fromMaxAnswer) {
+                        ThrowErrorMessage("Invalid 'From' Tag", currentLine, headerLine);
+                    }
+                }
             }
             else if(StringUtils::StartsWith(headerLine, "Components"))
             {
@@ -179,7 +195,6 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
 
         int lineCount=0;
 
-        StringUtils::StringVec inputLUT;
         std::vector<float> values;
 
         while (istream.good())
@@ -192,10 +207,17 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
 
             if (line.length() != 0)
             {
-                inputLUT = StringUtils::SplitByWhiteSpaces(StringUtils::Trim(lineBuffer));
                 values.clear();
-                if (!StringVecToFloatVec(values, inputLUT)
-                    || components != (int)values.size())
+
+                char inputLUT[4][64] = {"", "", "", ""};
+#ifdef _WIN32
+                if (sscanf(lineBuffer, "%s %s %s %63s", inputLUT[0], 64,
+                           inputLUT[1], 64, inputLUT[2], 64, inputLUT[3],
+                           64) != components)
+#else
+                if (sscanf(lineBuffer, "%s %s %s %63s", inputLUT[0],
+                           inputLUT[1], inputLUT[2], inputLUT[3]) != components)
+#endif
                 {
                     std::ostringstream os;
                     os << "Malformed LUT line. Expecting a ";
@@ -209,6 +231,26 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
                     ThrowErrorMessage("Too many entries found", currentLine, "");
                 }
 
+                values.resize(components);
+
+                for (int i = 0; i < components; i++) {
+                  float v = NAN;
+                  const auto result = NumberUtils::from_chars(
+                      inputLUT[i], inputLUT[i] + strlen(inputLUT[i]), v);
+
+                    if (!result)
+                    {
+                        std::ostringstream os;
+                        os << "Malformed LUT line. Could not convert component";
+                        os << i << " to a floating point number.";
+
+                        ThrowErrorMessage("Malformed LUT line", currentLine,
+                                            line);
+                    }
+
+                    values[i] = v;
+                }
+
                 // If 1 component is specified, use x1 x1 x1.
                 if (components == 1)
                 {