Improved validation for channel strings

Also, move the ScopedFloatFormatting class to the root namespace for consistency with other RAII classes.
ppenenko · Jun 25, 2019 · db62f74 · db62f74
1 parent c7ca0d4
commit db62f74
Show file tree

Hide file tree

Showing 11 changed files with 40 additions and 35 deletions.
diff --git a/source/MaterialXCore/Element.cpp b/source/MaterialXCore/Element.cpp
@@ -589,7 +589,16 @@ bool ValueElement::validate(string* message) const
             validateRequire(valueElem != nullptr, res, message, "Interface name not found in referenced NodeDef");
             if (valueElem)
             {
-                validateRequire(valueElem->getType() == getType(), res, message, "Interface name refers to value element of a different type");
+                ConstPortElementPtr portElem = asA<PortElement>();
+                if (portElem && portElem->hasChannels())
+                {
+                    bool valid = portElem->validChannelsString(portElem->getChannels(), valueElem->getType(), getType());
+                    validateRequire(valid, res, message, "Invalid channels string for interface name");
+                }
+                else
+                {
+                    validateRequire(getType() == valueElem->getType(), res, message, "Interface name refers to value element of a different type");
+                }
             }
         }
     }

diff --git a/source/MaterialXCore/Element.h b/source/MaterialXCore/Element.h
@@ -1107,7 +1107,6 @@ class ValueElement : public TypedElement
 
   public:
     static const string VALUE_ATTRIBUTE;
-    static const string PUBLIC_NAME_ATTRIBUTE;
     static const string INTERFACE_NAME_ATTRIBUTE;
     static const string IMPLEMENTATION_NAME_ATTRIBUTE;
     static const string IMPLEMENTATION_TYPE_ATTRIBUTE;

diff --git a/source/MaterialXCore/Interface.cpp b/source/MaterialXCore/Interface.cpp
@@ -62,11 +62,7 @@ void PortElement::setConnectedNode(NodePtr node)
 NodePtr PortElement::getConnectedNode() const
 {
     ConstGraphElementPtr graph = getAncestorOfType<GraphElement>();
-    if (graph)
-    {
-        return graph->getNode(getNodeName());
-    }
-    return NodePtr();
+    return graph ? graph->getNode(getNodeName()) : nullptr;
 }
 
 bool PortElement::validate(string* message) const
@@ -90,23 +86,24 @@ bool PortElement::validate(string* message) const
                 validateRequire(output != nullptr, res, message, "Invalid output in port connection");
                 if (output)
                 {
-                    const string& outputType = output->getType();
                     if (hasChannels())
                     {
-                        validateRequire(validChannelsString(getChannels(), outputType, getType()), res, message, "Invalid channels attribute");
+                        bool valid = validChannelsString(getChannels(), output->getType(), getType());
+                        validateRequire(valid, res, message, "Invalid channels string in port connection");
                     }
                     else
                     {
-                        validateRequire(getType() == outputType, res, message, "Mismatched output type in port connection");
+                        validateRequire(getType() == output->getType(), res, message, "Mismatched types in port connection");
                     }
                 }
             }
         }
         else if (hasChannels())
         {
-            validateRequire(validChannelsString(getChannels(), connectedNode->getType(), getType()), res, message, "Invalid channels attribute");
+            bool valid = validChannelsString(getChannels(), connectedNode->getType(), getType());
+            validateRequire(valid, res, message, "Invalid channels string in port connection");
         }
-        else if(!hasChannels())
+        else
         {
             validateRequire(getType() == connectedNode->getType(), res, message, "Mismatched types in port connection");
         }

diff --git a/source/MaterialXCore/Interface.h b/source/MaterialXCore/Interface.h
@@ -176,7 +176,7 @@ class PortElement : public ValueElement
     /// source type string.
     static bool validChannelsCharacters(const string &channels, const string &sourceType);
 
-    /// Return true if the given swizzle pattern is valid for the given source
+    /// Return true if the given channels string is valid for the given source
     /// and destination type strings.
     static bool validChannelsString(const string& channels, const string& sourceType, const string& destinationType);
 

diff --git a/source/MaterialXCore/Value.cpp b/source/MaterialXCore/Value.cpp
@@ -244,15 +244,15 @@ template<class T> const T& Value::asA() const
     return typedVal->getData();
 }
 
-Value::ScopedFloatFormatting::ScopedFloatFormatting(FloatFormat format, int precision) :
+ScopedFloatFormatting::ScopedFloatFormatting(Value::FloatFormat format, int precision) :
     _format(Value::getFloatFormat()),
     _precision(Value::getFloatPrecision())
 {
     Value::setFloatFormat(format);
     Value::setFloatPrecision(precision);
 }
 
-Value::ScopedFloatFormatting::~ScopedFloatFormatting()
+ScopedFloatFormatting::~ScopedFloatFormatting()
 {
     Value::setFloatFormat(_format);
     Value::setFloatPrecision(_precision);

diff --git a/source/MaterialXCore/Value.h b/source/MaterialXCore/Value.h
@@ -101,19 +101,6 @@ class Value
         return _floatPrecision;
     }
 
-    /// RAII class for scoped setting of float formatting.
-    /// Flags are reset when the object goes out of scope.
-    class ScopedFloatFormatting
-    {
-      public:
-        ScopedFloatFormatting(FloatFormat format, int precision = 6);
-        ~ScopedFloatFormatting();
-
-      private:
-        FloatFormat _format;
-        int _precision;
-    };
-
   protected:
     template <class T> friend class ValueRegistry;
 
@@ -186,6 +173,19 @@ template <class T> class TypedValue : public Value
     T _data;
 };
 
+/// @class ScopedFloatFormatting
+/// An RAII class for controlling the float formatting of values.
+class ScopedFloatFormatting
+{
+  public:
+    ScopedFloatFormatting(Value::FloatFormat format, int precision = 6);
+    ~ScopedFloatFormatting();
+
+  private:
+    Value::FloatFormat _format;
+    int _precision;
+};
+
 /// @class ExceptionTypeError
 /// An exception that is thrown when a type mismatch is encountered.
 class ExceptionTypeError : public Exception

diff --git a/source/MaterialXGenGlsl/GlslShaderGenerator.cpp b/source/MaterialXGenGlsl/GlslShaderGenerator.cpp
@@ -259,7 +259,7 @@ ShaderPtr GlslShaderGenerator::generate(const string& name, ElementPtr element,
     // Turn on fixed float formatting to make sure float values are
     // emitted with a decimal point and not as integers, and to avoid
     // any scientific notation which isn't supported by all OpenGL targets.
-    Value::ScopedFloatFormatting fmt(Value::FloatFormatFixed);
+    ScopedFloatFormatting fmt(Value::FloatFormatFixed);
 
     // Emit code for vertex shader stage
     ShaderStage& vs = shader->getStage(Stage::VERTEX);

diff --git a/source/MaterialXGenOsl/OslSyntax.cpp b/source/MaterialXGenOsl/OslSyntax.cpp
@@ -198,7 +198,7 @@ class OSLMatrix3TypeSyntax : public AggregateTypeSyntax
 
     string getValue(const Value& value, bool uniform) const
     {
-        Value::ScopedFloatFormatting fmt(Value::FloatFormatFixed, 3);
+        ScopedFloatFormatting fmt(Value::FloatFormatFixed, 3);
         StringVec values = splitString(value.getValueString(), ",");
         return getValue(values, uniform);
     }

diff --git a/source/MaterialXTest/GenGlsl.cpp b/source/MaterialXTest/GenGlsl.cpp
@@ -29,7 +29,7 @@ TEST_CASE("GenShader: GLSL Syntax Check", "[genglsl]")
     REQUIRE(syntax->getOutputTypeName(mx::Type::BSDF) == "out BSDF");
 
     // Set fixed precision with one digit
-    mx::Value::ScopedFloatFormatting format(mx::Value::FloatFormatFixed, 1);
+    mx::ScopedFloatFormatting format(mx::Value::FloatFormatFixed, 1);
 
     std::string value;
     value = syntax->getDefaultValue(mx::Type::FLOAT);

diff --git a/source/MaterialXTest/GenOsl.cpp b/source/MaterialXTest/GenOsl.cpp
@@ -37,7 +37,7 @@ TEST_CASE("GenShader: OSL Syntax", "[genosl]")
     REQUIRE(syntax->getOutputTypeName(mx::Type::BSDF) == "output BSDF");
 
     // Set fixed precision with one digit
-    mx::Value::ScopedFloatFormatting format(mx::Value::FloatFormatFixed, 1);
+    mx::ScopedFloatFormatting format(mx::Value::FloatFormatFixed, 1);
 
     std::string value;
     value = syntax->getDefaultValue(mx::Type::FLOAT);

diff --git a/source/MaterialXTest/Value.cpp b/source/MaterialXTest/Value.cpp
@@ -53,16 +53,16 @@ TEST_CASE("Value strings", "[value]")
     // Convert from data values to value strings
     // using the various float formattings.
     {
-        mx::Value::ScopedFloatFormatting fmt(mx::Value::FloatFormatFixed, 3);
+        mx::ScopedFloatFormatting fmt(mx::Value::FloatFormatFixed, 3);
         REQUIRE(mx::toValueString(0.1234f) == "0.123");
         REQUIRE(mx::toValueString(mx::Color3(1.0f)) == "1.000, 1.000, 1.000");
     }
     {
-        mx::Value::ScopedFloatFormatting fmt(mx::Value::FloatFormatScientific, 2);
+        mx::ScopedFloatFormatting fmt(mx::Value::FloatFormatScientific, 2);
         REQUIRE(mx::toValueString(0.1234f) == "1.23e-01");
     }
     {
-        mx::Value::ScopedFloatFormatting fmt(mx::Value::FloatFormatDefault, 2);
+        mx::ScopedFloatFormatting fmt(mx::Value::FloatFormatDefault, 2);
         REQUIRE(mx::toValueString(0.1234f) == "0.12");
     }