Text format (e.g. IridasCube) parsing optimizations

share/cmake/utils/CompilerFlags.cmake

@@ -62,6 +62,9 @@ if(USE_MSVC)
         )
     endif()
 
+    # Make MSVC compiler report correct __cplusplus version (otherwise reports 199711L)
+    set(PLATFORM_COMPILE_OPTIONS "${PLATFORM_COMPILE_OPTIONS};/Zc:__cplusplus")
+
     # Explicitely specify the default warning level i.e. /W3.
     # Note: Do not use /Wall (i.e. /W4) which adds 'informational' warnings.
     set(PLATFORM_COMPILE_OPTIONS "${PLATFORM_COMPILE_OPTIONS};/W3")

src/OpenColorIO/LookParse.cpp

@@ -17,13 +17,13 @@ void LookParseResult::Token::parse(const std::string & str)
 {
     // Assert no commas, colons, or | in str.
 
-    if(StringUtils::StartsWith(str, "+"))
+    if(StringUtils::StartsWith(str, '+'))
     {
         name = StringUtils::LeftTrim(str, '+');
         dir = TRANSFORM_DIR_FORWARD;
     }
     // TODO: Handle --
-    else if(StringUtils::StartsWith(str, "-"))
+    else if(StringUtils::StartsWith(str, '-'))
     {
         name = StringUtils::LeftTrim(str, '-');
         dir = TRANSFORM_DIR_INVERSE;

src/OpenColorIO/ParseUtils.cpp

@@ -672,13 +672,13 @@ bool nextline(std::istream &istream, std::string &line)
         {
             line.resize(line.size() - 1);
         }
-        if(!StringUtils::Trim(line).empty())
+        if(!StringUtils::IsEmptyOrWhiteSpace(line))
         {
             return true;
         }
     }
 
-    line = "";
+    line.clear();
     return false;
 }
 

src/OpenColorIO/fileformats/FileFormat3DL.cpp

@@ -263,11 +263,11 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             if(lineParts.empty()) continue;
             if (lineParts.size() > 0)
             {
-                if (StringUtils::StartsWith(lineParts[0], "#"))
+                if (StringUtils::StartsWith(lineParts[0], '#'))
                 {
                     continue;
                 }
-                if (StringUtils::StartsWith(lineParts[0], "<"))
+                if (StringUtils::StartsWith(lineParts[0], '<'))
                 {
                     // Format error: reject files that could be
                     // formatted as xml.

src/OpenColorIO/fileformats/FileFormatIridasCube.cpp

@@ -180,7 +180,7 @@ LocalFileFormat::read(std::istream & istream,
         {
             ++lineNumber;
             // All lines starting with '#' are comments
-            if (StringUtils::StartsWith(line,"#")) continue;
+            if (StringUtils::StartsWith(line,'#')) continue;
 
             line = StringUtils::Lower(StringUtils::Trim(line));
 
@@ -310,10 +310,10 @@ LocalFileFormat::read(std::istream & istream,
 
         do
         {
-            line = StringUtils::Trim(line);
+            line = StringUtils::LeftTrim(line);
 
             // All lines starting with '#' are comments
-            if (StringUtils::StartsWith(line,"#")) continue;
+            if (StringUtils::StartsWith(line,'#')) continue;
 
             if (line.empty()) continue;
 

src/OpenColorIO/fileformats/FileFormatIridasItx.cpp

@@ -146,7 +146,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
         {
             ++lineNumber;
             // All lines starting with '#' are comments
-            if(StringUtils::StartsWith(line,"#")) continue;
+            if(StringUtils::StartsWith(line,'#')) continue;
 
             // Strip, lowercase, and split the line
             parts = StringUtils::SplitByWhiteSpaces(StringUtils::Lower(StringUtils::Trim(line)));

src/OpenColorIO/fileformats/FileFormatPandora.cpp

@@ -129,7 +129,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             if(parts.empty()) continue;
 
             // Skip all lines starting with '#'
-            if(StringUtils::StartsWith(parts[0],"#")) continue;
+            if(StringUtils::StartsWith(parts[0],'#')) continue;
 
             if(parts[0] == "channel")
             {

src/OpenColorIO/fileformats/FileFormatResolveCube.cpp

@@ -296,7 +296,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             ++lineNumber;
 
             // All lines starting with '#' are comments
-            if(StringUtils::StartsWith(line,"#"))
+            if(StringUtils::StartsWith(line,'#'))
             {
                 if(headerComplete)
                 {

src/OpenColorIO/fileformats/FileFormatSpi1D.cpp

@@ -168,7 +168,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
                 }
             }
         }
-        while (istream.good() && !StringUtils::StartsWith(headerLine,"{"));
+        while (istream.good() && !StringUtils::StartsWith(headerLine,'{'));
     }
 
     if (version == -1)

src/OpenColorIO/fileformats/FileFormatTruelight.cpp

@@ -124,7 +124,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             if(parts.empty()) continue;
 
             // Parse header metadata (which starts with #)
-            if(StringUtils::StartsWith(parts[0],"#"))
+            if(StringUtils::StartsWith(parts[0],'#'))
             {
                 if(parts.size() < 2) continue;
 

src/OpenColorIO/fileformats/FileFormatVF.cpp

@@ -129,7 +129,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
 
             if(parts.empty()) continue;
 
-            if(StringUtils::StartsWith(parts[0],"#")) continue;
+            if(StringUtils::StartsWith(parts[0],'#')) continue;
 
             if(!in3d)
             {

src/utils/NumberUtils.h

@@ -4,6 +4,22 @@
 #ifndef INCLUDED_NUMBERUTILS_H
 #define INCLUDED_NUMBERUTILS_H
 
+// With C++17, we can use <charconv> from_chars.
+//
+// That has advantage of not dealing with locale / errno,
+// which might involve locks or thread local storage accesses.
+//
+// Note that it is not enough to check __cplusplus version,
+// since even if compiler reports C++17 it might not implement
+// from_chars for floats. Checking __cpp_lib_to_chars works
+// correctly and the check starts passing with gcc 11, clang 12,
+// msvc 2019 (16.4). It correctly does not pass on apple clang 15,
+// since it does not implement from_chars for floats.
+#if __cpp_lib_to_chars >= 201611L
+#define USE_CHARCONV_FROM_CHARS
+#include <charconv>
+#endif
+
 #if defined(_MSC_VER)
 #define really_inline __forceinline
 #else
@@ -54,14 +70,56 @@ struct from_chars_result
 
 static const Locale loc;
 
+#ifdef USE_CHARCONV_FROM_CHARS
+really_inline bool from_chars_is_space(char c) noexcept
+{
+    return c == ' ' || c == '\n' || c == '\t' || c == '\r' || c == '\v' || c == '\f';
+}
+
+// skip prefix whitespace and "+"
+really_inline const char* from_chars_skip_prefix(const char* first, const char* last) noexcept
+{
+    while (first < last && from_chars_is_space(first[0]))
+    {
+        ++first;
+    }
+    if (first < last && first[0] == '+')
+    {
+        ++first;
+    }
+    return first;
+}
+
+really_inline bool from_chars_hex_prefix(const char*& first, const char* last) noexcept
+{
+    if (first + 2 < last && first[0] == '0' && (first[1] == 'x' || first[1] == 'X'))
+    {
+        first += 2;
+        return true;
+    }
+    return false;
+}
+#endif
+
 really_inline from_chars_result from_chars(const char *first, const char *last, double &value) noexcept
 {
-    errno = 0;
     if (!first || !last || first == last)
     {
         return {first, std::errc::invalid_argument};
     }
 
+#ifdef USE_CHARCONV_FROM_CHARS
+    first = from_chars_skip_prefix(first, last);
+    std::chars_format fmt = std::chars_format::general;
+    if (from_chars_hex_prefix(first, last))
+    {
+        fmt = std::chars_format::hex;
+    }
+    std::from_chars_result res = std::from_chars(first, last, value, fmt);
+    return from_chars_result{ res.ptr, res.ec };
+#else
+
+    errno = 0;
     char * endptr = nullptr;
 
     double
@@ -88,16 +146,28 @@ really_inline from_chars_result from_chars(const char *first, const char *last,
     {
         return {first, std::errc::argument_out_of_domain};
     }
+#endif
 }
 
 really_inline from_chars_result from_chars(const char *first, const char *last, float &value) noexcept
 {
-    errno = 0;
     if (!first || !last || first == last)
     {
         return {first, std::errc::invalid_argument};
     }
 
+#ifdef USE_CHARCONV_FROM_CHARS
+    first = from_chars_skip_prefix(first, last);
+    std::chars_format fmt = std::chars_format::general;
+    if (from_chars_hex_prefix(first, last))
+    {
+        fmt = std::chars_format::hex;
+    }
+    std::from_chars_result res = std::from_chars(first, last, value, fmt);
+    return from_chars_result{ res.ptr, res.ec };
+#else
+
+    errno = 0;
     char *endptr = nullptr;
 
     float
@@ -132,16 +202,28 @@ really_inline from_chars_result from_chars(const char *first, const char *last,
     {
         return {first, std::errc::argument_out_of_domain};
     }
+#endif
 }
 
 really_inline from_chars_result from_chars(const char *first, const char *last, long int &value) noexcept
 {
-    errno = 0;
     if (!first || !last || first == last)
     {
         return {first, std::errc::invalid_argument};
     }
 
+#ifdef USE_CHARCONV_FROM_CHARS
+    first = from_chars_skip_prefix(first, last);
+    int base = 10;
+    if (from_chars_hex_prefix(first, last))
+    {
+        base = 16;
+    }
+    std::from_chars_result res = std::from_chars(first, last, value, base);
+    return from_chars_result{ res.ptr, res.ec };
+#else
+
+    errno = 0;
     char *endptr = nullptr;
 
     long int
@@ -170,6 +252,7 @@ really_inline from_chars_result from_chars(const char *first, const char *last,
     {
         return {first, std::errc::argument_out_of_domain};
     }
+#endif
 }
 } // namespace NumberUtils
 } // namespace OCIO_NAMESPACE

src/utils/StringUtils.h

@@ -44,6 +44,13 @@ inline unsigned char Upper(unsigned char c)
     }
 }
 
+// Checks if character is whitespace (space, tab, newline or other
+// non-printable char). Does not take locale into account.
+inline bool IsSpace(unsigned char c)
+{
+    return c <= ' ';
+}
+
 // Return the lower case string.
 inline std::string Lower(std::string str)
 {
@@ -95,6 +102,13 @@ inline bool StartsWith(const std::string & str, const std::string & prefix)
     return str.size() >= prefix.size() && 0 == str.compare(0, prefix.size(), prefix);
 }
 
+// Return true if the string starts with the prefix character.
+// Note: The comparison is case sensitive.
+inline bool StartsWith(const std::string& str, char prefix)
+{
+    return !str.empty() && str.front() == prefix;
+}
+
 // Starting from the left, trim the character.
 inline std::string LeftTrim(std::string str, char c)
 {
@@ -106,7 +120,7 @@ inline std::string LeftTrim(std::string str, char c)
 // Starting from the left, trim all the space characters i.e. space, tabulation, etc.
 inline std::string LeftTrim(std::string str)
 {
-    const auto it = std::find_if(str.begin(), str.end(), [](char ch) { return !std::isspace(static_cast<unsigned char>(ch)); });
+    const auto it = std::find_if(str.begin(), str.end(), [](char ch) { return !IsSpace(ch); });
     str.erase(str.begin(), it);
     return str;
 }
@@ -123,7 +137,7 @@ inline std::string RightTrim(std::string str, char c)
 inline std::string RightTrim(std::string str)
 {
     const auto it =
-        std::find_if(str.rbegin(), str.rend(), [](char ch) { return !std::isspace(static_cast<unsigned char>(ch)); });
+        std::find_if(str.rbegin(), str.rend(), [](char ch) { return !IsSpace(ch); });
     str.erase(it.base(), str.end());
     return str;
 }
@@ -148,6 +162,12 @@ inline void Trim(StringVec & list)
     }
 }
 
+// Checks if string is empty or white-space only.
+inline bool IsEmptyOrWhiteSpace(const std::string& str)
+{
+    return std::find_if(str.begin(), str.end(), [](char ch) { return !IsSpace(ch); }) == str.end();
+}
+
 // Split a string content using an arbitrary separator.
 inline StringVec Split(const std::string & str, char separator)
 {

tests/cpu/fileformats/FileFormatIridasCube_tests.cpp

@@ -161,6 +161,43 @@ OCIO_ADD_TEST(FileFormatIridasCube, read_failure)
     }
 }
 
+OCIO_ADD_TEST(FileFormatIridasCube, whitespace_handling)
+{
+    const std::string SAMPLE =
+        "# comment\n"
+        "# comment with trailing space  \n"
+        "# next up various forms of empty lines\n"
+        "\n"
+        "   \n"
+        "   \t  \n"
+        "# whitespace before keywords or after data should be supported\n"
+        "  LUT_3D_SIZE \t 2  \t\n"
+        "\t \tDOMAIN_MIN    0.25    0.5    0.75\n"
+        "\n"
+        "DOMAIN_MAX\t1.5\t2.5\t3.5\n"
+
+        "0.0 0.0 0.0\n"
+        "# comments in between data should be ignored\n"
+        "   1.0    0.0 \t 0.0\n"
+        "0.0 1.0 0.0    \n"
+        "     1.0 1.0 0.0\n"
+        "    \n"
+        "0.0 0.0 1.0\n"
+        "1.0 0.0 1.0\n"
+        "0.0 1.0 1.0\n"
+        "1.0 1.0 1.0\n"
+        "   \n"
+        "\n";
+
+    OCIO::LocalCachedFileRcPtr file;
+    OCIO_CHECK_NO_THROW(file = ReadIridasCube(SAMPLE));
+    OCIO_CHECK_EQUAL(file->domain_min[0], 0.25f);
+    OCIO_CHECK_EQUAL(file->domain_min[2], 0.75f);
+    OCIO_CHECK_EQUAL(file->domain_max[0], 1.5f);
+    OCIO_CHECK_EQUAL(file->domain_max[2], 3.5f);
+    OCIO_CHECK_EQUAL(file->lut3D->getArray().getLength(), 2);
+}
+
 OCIO_ADD_TEST(FileFormatIridasCube, no_shaper)
 {
     // check baker output