Improve shader source duplication tests

This changelist improves the robustness of shader source duplication tests in MaterialXGenShader, and adds a new example material that highlights edge cases in the original logic.
autodesk-forks · Mar 26, 2024 · 685b377 · 685b377
1 parent 716660e
commit 685b377
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 17 deletions.
diff --git a/resources/Materials/TestSuite/pbrlib/surfaceshader/usd_normal_map.mtlx b/resources/Materials/TestSuite/pbrlib/surfaceshader/usd_normal_map.mtlx
@@ -0,0 +1,23 @@
+<?xml version="1.0"?>
+<materialx version="1.38" colorspace="lin_rec709">
+  <nodegraph name="NG_usd_normal_map">
+    <tiledimage name="N_tiledimage" type="vector3" nodedef="ND_tiledimage_vector3">
+      <input name="file" type="filename" value="resources/Images/mesh_wire_norm.png" />
+      <input name="uvtiling" type="vector2" value="8.0, 8.0" />
+    </tiledimage>
+    <normalmap name="N_normalmap" type="vector3" nodedef="ND_normalmap">
+      <input name="in" type="vector3" nodename="N_tiledimage" />
+      <input name="space" type="string" value="object" />
+    </normalmap>
+    <output name="out" type="vector3" nodename="N_normalmap" />
+  </nodegraph>
+  <UsdPreviewSurface name="N_surface" type="surfaceshader">
+    <input name="normal" type="vector3" output="out" nodegraph="NG_usd_normal_map" />
+    <input name="diffuseColor" type="color3" value="1, 1, 1" />
+    <input name="metallic" type="float" value="1" />
+    <input name="roughness" type="float" value="0.001" />
+  </UsdPreviewSurface>
+  <surfacematerial name="N_material" type="material">
+    <input name="surfaceshader" type="surfaceshader" nodename="N_surface" />
+  </surfacematerial>
+</materialx>
diff --git a/source/MaterialXGenShader/Nodes/SourceCodeNode.cpp b/source/MaterialXGenShader/Nodes/SourceCodeNode.cpp
@@ -77,21 +77,19 @@ void SourceCodeNode::initialize(const InterfaceElement& element, GenContext& con
 
 void SourceCodeNode::emitFunctionDefinition(const ShaderNode&, GenContext& context, ShaderStage& stage) const
 {
+    if (_inlined || _functionSource.empty() || _sourceFilename.isEmpty())
+    {
+        return;
+    }
+
     DEFINE_SHADER_STAGE(stage, Stage::PIXEL)
     {
-        // Emit function definition for non-inlined functions
-        if (!_functionSource.empty())
+        if (!stage.hasSourceDependency(_sourceFilename))
         {
-            if (!_sourceFilename.isEmpty())
-            {
-                stage.addSourceDependency(_sourceFilename);
-            }
-            if (!_inlined)
-            {
-                const ShaderGenerator& shadergen = context.getShaderGenerator();
-                shadergen.emitBlock(_functionSource, _sourceFilename, context, stage);
-                shadergen.emitLineBreak(stage);
-            }
+            const ShaderGenerator& shadergen = context.getShaderGenerator();
+            shadergen.emitBlock(_functionSource, _sourceFilename, context, stage);
+            shadergen.emitLineBreak(stage);
+            stage.addSourceDependency(_sourceFilename);
         }
     }
 }

diff --git a/source/MaterialXGenShader/ShaderStage.cpp b/source/MaterialXGenShader/ShaderStage.cpp
@@ -359,12 +359,14 @@ void ShaderStage::addInclude(const FilePath& includeFilename, const FilePath& so
     }
 }
 
+bool ShaderStage::hasSourceDependency(const FilePath& file)
+{
+    return _sourceDependencies.count(file) != 0;
+}
+
 void ShaderStage::addSourceDependency(const FilePath& file)
 {
-    if (!_sourceDependencies.count(file))
-    {
-        _sourceDependencies.insert(file);
-    }
+    _sourceDependencies.insert(file);
 }
 
 void ShaderStage::addFunctionDefinition(const ShaderNode& node, GenContext& context)

diff --git a/source/MaterialXGenShader/ShaderStage.h b/source/MaterialXGenShader/ShaderStage.h
@@ -256,7 +256,10 @@ class MX_GENSHADER_API ShaderStage
     /// Add the contents of an include file if not already present.
     void addInclude(const FilePath& includeFilename, const FilePath& sourceFilename, GenContext& context);
 
-    /// Add a source file dependency for dependency tracking purposes
+    /// Return true if this stage depends on the given source file.
+    bool hasSourceDependency(const FilePath& file);
+
+    /// Mark the given source file as a dependency of this stage.
     void addSourceDependency(const FilePath& file);
 
     /// Add a value.