Improve material name handling in shader generation (AcademySoftwareFoundation#2804)

Convert names of shader graph nodes early to identifiers to increase the chance of a name without suffixes. This allows to render all materials from the testsuite and examples with MDL without the need to skip some of them.

Author: jreichel-nvidia

source/MaterialXGenMdl/MdlShaderGenerator.cpp

@@ -186,9 +186,8 @@ ShaderPtr MdlShaderGenerator::generate(const string& name, ElementPtr element, G
     const ShaderGraphOutputSocket* outputSocket = graph.getOutputSocket(0);
     emitString("export material ", stage);
 
-    // Begin shader signature. Note that makeIdentifier() will sanitize the name.
+    // Begin shader signature. Note that the function name is already sanitized.
     string functionName = shader->getName();
-    _syntax->makeIdentifier(functionName, graph.getIdentifierMap());
     setFunctionName(functionName, stage);
     emitLine(functionName, stage, false);
     emitScopeBegin(stage, Syntax::PARENTHESES);

source/MaterialXGenOsl/OslShaderGenerator.cpp

@@ -80,9 +80,8 @@ ShaderPtr OslShaderGenerator::generate(const string& name, ElementPtr element, G
         emitString("shader ", stage);
     }
 
-    // Begin shader signature. Note that makeIdentifier() will sanitize the name.
+    // Begin shader signature. Note that the function name is already sanitized.
     string functionName = shader->getName();
-    _syntax->makeIdentifier(functionName, graph.getIdentifierMap());
     setFunctionName(functionName, stage);
     emitLine(functionName, stage, false);
 

source/MaterialXGenShader/ShaderGraph.cpp

@@ -18,10 +18,12 @@ MATERIALX_NAMESPACE_BEGIN
 // ShaderGraph methods
 //
 
-ShaderGraph::ShaderGraph(const ShaderGraph* parent, const string& name, ConstDocumentPtr document) :
-    ShaderNode(parent, name),
+ShaderGraph::ShaderGraph(const ShaderGraph* parent, const string& name, ConstDocumentPtr document,
+                         GenContext& context)
+  : ShaderNode(parent, name),
     _document(document)
 {
+    context.getShaderGenerator().getSyntax().makeIdentifier(_name, getIdentifierMap());
 }
 
 void ShaderGraph::addInputSockets(const InterfaceElement& elem, GenContext& context)
@@ -436,7 +438,7 @@ ShaderGraphPtr ShaderGraph::create(const ShaderGraph* parent, const NodeGraph& n
 
     string graphName = nodeGraph.getName();
     context.getShaderGenerator().getSyntax().makeValidName(graphName);
-    ShaderGraphPtr graph = std::make_shared<ShaderGraph>(parent, graphName, nodeGraph.getDocument());
+    ShaderGraphPtr graph = std::make_shared<ShaderGraph>(parent, graphName, nodeGraph.getDocument(), context);
 
     // Clear classification
     graph->_classification = 0;
@@ -495,7 +497,7 @@ ShaderGraphPtr ShaderGraph::create(const ShaderGraph* parent, const string& name
             throw ExceptionShaderGenError("Given output '" + output->getName() + "' has no interface valid for shader generation");
         }
 
-        graph = std::make_shared<ShaderGraph>(parent, name, element->getDocument());
+        graph = std::make_shared<ShaderGraph>(parent, name, element->getDocument(), context);
 
         // Clear classification
         graph->_classification = 0;
@@ -531,7 +533,7 @@ ShaderGraphPtr ShaderGraph::create(const ShaderGraph* parent, const string& name
             throw ExceptionShaderGenError("Could not find a nodedef for node '" + node->getName() + "'");
         }
 
-        graph = std::make_shared<ShaderGraph>(parent, name, element->getDocument());
+        graph = std::make_shared<ShaderGraph>(parent, name, element->getDocument(), context);
 
         // Create input sockets
         graph->addInputSockets(*nodeDef, context);

source/MaterialXGenShader/ShaderGraph.h

@@ -44,7 +44,8 @@ class MX_GENSHADER_API ShaderGraph : public ShaderNode
 {
   public:
     /// Constructor.
-    ShaderGraph(const ShaderGraph* parent, const string& name, ConstDocumentPtr document);
+    ShaderGraph(const ShaderGraph* parent, const string& name, ConstDocumentPtr document,
+                GenContext& context);
 
     /// Destructor.
     virtual ~ShaderGraph() { }

source/MaterialXTest/MaterialXRenderMdl/RenderMdl.cpp

@@ -42,17 +42,6 @@ class MdlShaderRenderTester : public RenderUtil::ShaderRenderTester
                      const std::string& outputPath = ".",
                      mx::ImageVec* imageVec = nullptr) override;
 
-    void addSkipFiles() override
-    {
-        // The command-line assembled in runRenderer() assumes that the simple name of the MDL
-        // material is the same as "shaderName", but for this file the MDL material name gets a "1"
-        // suffix. Communicating the generated name from the stage to the shader could help, but in
-        // general it is probably better to give the main object a predictable name and use suffixes
-        // for internal objects.
-        _skipFiles.insert("translucent_bsdf.mtlx");
-        _skipFiles.insert("blur.mtlx");
-    }
-
     mx::DocumentPtr _last_doc;
 };
 

source/MaterialXTest/README.md

@@ -67,6 +67,7 @@ When rendering tests are enabled through the `MATERIALX_TEST_RENDER` option, the
     - Set the following build options to enable MDL support:
         - `MATERIALX_MDL_SDK_DIR`: Path to the MDL SDK directory (containing 'include', 'lib', etc.).
     - MDL versions 1.6 and later are supported.
+    - For installations via vcpkg: the core `mdl-sdk` package is sufficient for syntax checks via `mdlc`. The `df_vulkan` test render binary needs the package features `dds`, `openimageio`, and `df-vulkan`.
 - `MSL`:
     - Metal Shading Language (MSL) 2.0 and later are supported.
     - Minimum tested operating system version is macOS Catalina 10.15.7