From 8a237a330fda67a5548183274d18064ba8b67e6d Mon Sep 17 00:00:00 2001 From: Bernard Kwok Date: Fri, 23 Aug 2024 13:18:30 -0400 Subject: [PATCH] - Review changes: - Simplify logic. - Remove precision clamping. - Add JS wrapper. --- .../JsMaterialXCore/JsDocument.cpp | 1 + source/JsMaterialX/JsMaterialXCore/JsUtil.cpp | 2 + source/MaterialXCore/Document.cpp | 4 +- source/MaterialXCore/Document.h | 3 +- source/MaterialXCore/Util.cpp | 48 +++++++++---------- source/MaterialXCore/Util.h | 2 +- .../MaterialXTest/MaterialXCore/CoreUtil.cpp | 17 +++---- .../MaterialXTest/MaterialXCore/Document.cpp | 2 +- 8 files changed, 41 insertions(+), 38 deletions(-) diff --git a/source/JsMaterialX/JsMaterialXCore/JsDocument.cpp b/source/JsMaterialX/JsMaterialXCore/JsDocument.cpp index ec4881732d..3d584184e5 100644 --- a/source/JsMaterialX/JsMaterialXCore/JsDocument.cpp +++ b/source/JsMaterialX/JsMaterialXCore/JsDocument.cpp @@ -103,6 +103,7 @@ EMSCRIPTEN_BINDINGS(document) .function("hasColorManagementConfig", &mx::Document::hasColorManagementConfig) .function("getColorManagementConfig", &mx::Document::getColorManagementConfig) .function("invalidateCache", &mx::Document::invalidateCache) + .function("normalizeValueStrings", &mx::Document::normalizeValueStrings) .class_property("CATEGORY", &mx::Document::CATEGORY) .class_property("CMS_ATTRIBUTE", &mx::Document::CMS_ATTRIBUTE) .class_property("CMS_CONFIG_ATTRIBUTE", &mx::Document::CMS_CONFIG_ATTRIBUTE); diff --git a/source/JsMaterialX/JsMaterialXCore/JsUtil.cpp b/source/JsMaterialX/JsMaterialXCore/JsUtil.cpp index 1bc54ad1b8..da6b488337 100644 --- a/source/JsMaterialX/JsMaterialXCore/JsUtil.cpp +++ b/source/JsMaterialX/JsMaterialXCore/JsUtil.cpp @@ -47,4 +47,6 @@ EMSCRIPTEN_BINDINGS(util) ems::function("splitNamePath", &mx::splitNamePath); ems::function("createNamePath", &mx::createNamePath); ems::function("parentNamePath", &mx::parentNamePath); + + ems::function("normalizeNumericString", &mx::normalizeNumericString); } diff --git a/source/MaterialXCore/Document.cpp b/source/MaterialXCore/Document.cpp index 10002aedf3..780d20abc7 100644 --- a/source/MaterialXCore/Document.cpp +++ b/source/MaterialXCore/Document.cpp @@ -402,7 +402,7 @@ void Document::invalidateCache() _cache->valid = false; } -void Document::normalizeValueStrings(unsigned int numericPrecision) +void Document::normalizeValueStrings() { // For now this will only normalize numeric strings StringSet numericType = { "float", "vector2", "vector3", "vector4", "color3", "color4" }; @@ -420,7 +420,7 @@ void Document::normalizeValueStrings(unsigned int numericPrecision) continue; } - string normalizeString = normalizeNumericString(originalString, numericPrecision); + string normalizeString = normalizeNumericString(originalString); if (normalizeString != originalString) { valueElem->setValueString(normalizeString); diff --git a/source/MaterialXCore/Document.h b/source/MaterialXCore/Document.h index afeec7f336..c40b8b4856 100644 --- a/source/MaterialXCore/Document.h +++ b/source/MaterialXCore/Document.h @@ -664,8 +664,7 @@ class MX_CORE_API Document : public GraphElement void invalidateCache(); /// Normalize value strings for all numeric values attributes - /// @param numericPrecision Precision to use for floating point numbers. Default is 6. - void normalizeValueStrings(unsigned int numericPrecision = 6); + void normalizeValueStrings(); /// @} diff --git a/source/MaterialXCore/Util.cpp b/source/MaterialXCore/Util.cpp index a6bdd686f0..c2e982fc03 100644 --- a/source/MaterialXCore/Util.cpp +++ b/source/MaterialXCore/Util.cpp @@ -181,47 +181,47 @@ string parentNamePath(const string& namePath) return EMPTY_STRING; } -string normalizeNumericString(const string& str, unsigned int precision) +std::string normalizeNumericString(const std::string& str) { std::stringstream ss(str); - std::stringstream result; + std::string result; std::string token; - while (std::getline(ss, token, ',')) - { + while (std::getline(ss, token, ',')) { // Remove leading and trailing spaces token.erase(0, token.find_first_not_of(' ')); token.erase(token.find_last_not_of(' ') + 1); - // Convert the string to a double (automatically removes leading zeros) - double number = std::stod(token); + // Remove leading zeros + token.erase(0, token.find_first_not_of('0')); + + // Preserve 0 values + if (token.empty() || token[0] == '.') { + token = "0" + token; + } - // Format the number with the specified precision - result << std::fixed << std::setprecision(precision) << number; + // If the token contains a decimal point, remove trailing zeros + size_t decimalPos = token.find('.'); + if (decimalPos != std::string::npos) { + token.erase(token.find_last_not_of('0') + 1); - // Remove unnecessary trailing zeros - std::string formattedNumber = result.str(); - formattedNumber.erase(formattedNumber.find_last_not_of('0') + 1, std::string::npos); - if (formattedNumber.back() == '.') - { - formattedNumber.pop_back(); // Remove the trailing dot if there's no decimal part left + // If the token ends with a decimal point after removing trailing zeros, remove it + if (token.back() == '.') { + token.pop_back(); + } } - // Append the formatted number to the result - result.str(EMPTY_STRING); // Clear the stringstream for the next number - result.clear(); - result << formattedNumber << ","; + // Append the formatted token to the result with a comma + result += token + ","; } - std::string output = result.str(); - // Remove the last comma - if (!output.empty()) - { - output.pop_back(); + if (!result.empty()) { + result.pop_back(); } - return output; + return result; } + MATERIALX_NAMESPACE_END diff --git a/source/MaterialXCore/Util.h b/source/MaterialXCore/Util.h index ad4ff37479..da0a062cfd 100644 --- a/source/MaterialXCore/Util.h +++ b/source/MaterialXCore/Util.h @@ -71,7 +71,7 @@ MX_CORE_API string parentNamePath(const string& namePath); /// Normalize a string containing a numeric value. This can /// either be a single number or an array of comma seperated numbers. -MX_CORE_API string normalizeNumericString(const string& str, unsigned int precision = 6); +MX_CORE_API string normalizeNumericString(const string& str); MATERIALX_NAMESPACE_END diff --git a/source/MaterialXTest/MaterialXCore/CoreUtil.cpp b/source/MaterialXTest/MaterialXCore/CoreUtil.cpp index d4ed712e43..925f846677 100644 --- a/source/MaterialXTest/MaterialXCore/CoreUtil.cpp +++ b/source/MaterialXTest/MaterialXCore/CoreUtil.cpp @@ -41,14 +41,15 @@ TEST_CASE("String utilities", "[coreutil]") std::string inputScalar1 = " 00.1000 "; std::string resultScalar1 = mx::normalizeNumericString(inputScalar1); REQUIRE(resultScalar1 == "0.1"); - std::string inputScalar2 = "0.1234567890"; - std::string resultScalar2 = mx::normalizeNumericString(inputScalar2, 3); - REQUIRE(resultScalar2 == "0.123"); - std::string resultScalar3 = mx::normalizeNumericString(inputScalar2, 12); - REQUIRE(resultScalar3 == "0.123456789"); - std::string inputScalar3 = "0.12345678901234567890"; - std::string resultScalar4 = mx::normalizeNumericString(inputScalar3, 9); - REQUIRE(resultScalar4 == "0.123456789"); + std::string inputScalar2 = ".000000"; + std::string resultScalar2 = mx::normalizeNumericString(inputScalar2); + REQUIRE(resultScalar2 == "0"); + std::string inputScalar3 = "000."; + std::string resultScalar3 = mx::normalizeNumericString(inputScalar3); + REQUIRE(resultScalar3 == "0"); + std::string inputScalar4 = "000.01"; + std::string resultScalar4 = mx::normalizeNumericString(inputScalar4); + REQUIRE(resultScalar4 == "0.01"); std::string inputVector1 = "1.0, 2.0, 0000.231"; std::string inputVector2 = "0001.2000, 0000.00010"; diff --git a/source/MaterialXTest/MaterialXCore/Document.cpp b/source/MaterialXTest/MaterialXCore/Document.cpp index 8e18d5e706..f8904fc9b9 100644 --- a/source/MaterialXTest/MaterialXCore/Document.cpp +++ b/source/MaterialXTest/MaterialXCore/Document.cpp @@ -119,7 +119,7 @@ TEST_CASE("Document", "[document]") REQUIRE(doc->validate()); } -TEST_CASE("Document equivalence", "[document2]") +TEST_CASE("Document equivalence", "[document]") { mx::DocumentPtr doc = mx::createDocument(); std::multimap inputMap;