diff --git a/resources/Materials/Examples/UsdPreviewSurface/usd_preview_surface_brass_tiled.mtlx b/resources/Materials/Examples/UsdPreviewSurface/usd_preview_surface_brass_tiled.mtlx index 85f6f3e139..a250136dab 100644 --- a/resources/Materials/Examples/UsdPreviewSurface/usd_preview_surface_brass_tiled.mtlx +++ b/resources/Materials/Examples/UsdPreviewSurface/usd_preview_surface_brass_tiled.mtlx @@ -1,5 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/source/MaterialXGenHw/HwImplementation.cpp b/source/MaterialXGenHw/HwImplementation.cpp index 8c572eb0b5..830f55eaa2 100644 --- a/source/MaterialXGenHw/HwImplementation.cpp +++ b/source/MaterialXGenHw/HwImplementation.cpp @@ -10,7 +10,6 @@ MATERIALX_NAMESPACE_BEGIN const string HwImplementation::SPACE = "space"; const string HwImplementation::INDEX = "index"; -const string HwImplementation::GEOMPROP = "geomprop"; namespace { diff --git a/source/MaterialXGenHw/HwImplementation.h b/source/MaterialXGenHw/HwImplementation.h index 66c20f7c18..802fd1e355 100644 --- a/source/MaterialXGenHw/HwImplementation.h +++ b/source/MaterialXGenHw/HwImplementation.h @@ -37,7 +37,6 @@ class MX_GENHW_API HwImplementation : public ShaderNodeImpl /// Internal string constants static const string SPACE; static const string INDEX; - static const string GEOMPROP; }; MATERIALX_NAMESPACE_END diff --git a/source/MaterialXGenHw/Nodes/HwGeomPropValueNode.cpp b/source/MaterialXGenHw/Nodes/HwGeomPropValueNode.cpp index 9fb78fe7d5..4bd11647de 100644 --- a/source/MaterialXGenHw/Nodes/HwGeomPropValueNode.cpp +++ b/source/MaterialXGenHw/Nodes/HwGeomPropValueNode.cpp @@ -13,17 +13,77 @@ MATERIALX_NAMESPACE_BEGIN +static const string GEOMPROP = "geomprop"; + ShaderNodeImplPtr HwGeomPropValueNode::create() { return std::make_shared(); } -void HwGeomPropValueNode::createVariables(const ShaderNode& node, GenContext&, Shader& shader) const +// Find the ShaderInput that contains the actual name of the geomprop to read +static const ShaderInput* getGeomPropValueInput(const ShaderNode& node, const GenContext& context) { + // We should always have the "geomprop" input locally on this node const ShaderInput* geomPropInput = node.getInput(GEOMPROP); + if (!geomPropInput) + { + throw ExceptionShaderGenError("No 'geomprop' input found on geompropvalue node '" + node.getName() + "'."); + } + + // Depth into the compound instance stack, measured from the innermost + // instance. Incremented each time we cross a graph boundary so that nested + // compounds resolve to the correct enclosing instance. + size_t depth = 0; + + while (geomPropInput) + { + // If it has a local value set, then we've found the ShaderInput and can return that + ConstValuePtr value = geomPropInput->getValue(); + if (value) + { + return geomPropInput; + } + + // If we don't have a local value, then we may have an incoming connection from a containing NodeGraph + // Note : the "geomprop" input on the base node is "uniform" so it cannot be driven by the output from + // another node. + const ShaderOutput* upstreamPort = geomPropInput->getConnection(); + if (!upstreamPort) + { + return nullptr; + } + + const ShaderNode* upstreamNode = upstreamPort->getNode(); + if (!upstreamNode || !upstreamNode->isAGraph()) + { + return nullptr; + } + + // We've reached a graph input socket. Step out to the matching compound + // instance, walking the instance stack outward in tandem with the graph + // boundary we just crossed, so that nested compounds resolve to the + // correct instance rather than always the innermost one. + const ShaderNode* compoundInstance = context.getCompoundInstanceNode(depth++); + if (!compoundInstance) + { + return nullptr; + } + + // Consider the input on the compound instance to be the new potential + // source for the value of "geomprop". The next iteration either finds a + // value here or continues climbing. + geomPropInput = compoundInstance->getInput(upstreamPort->getName()); + } + + return nullptr; +} + +void HwGeomPropValueNode::createVariables(const ShaderNode& node, GenContext& context, Shader& shader) const +{ + const ShaderInput* geomPropInput = getGeomPropValueInput(node, context); if (!geomPropInput || !geomPropInput->getValue()) { - throw ExceptionShaderGenError("No 'geomprop' parameter found on geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); + throw ExceptionShaderGenError("No 'geomprop' value found for geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); } const string geomProp = geomPropInput->getValue()->getValueString(); const ShaderOutput* output = node.getOutput(); @@ -39,10 +99,10 @@ void HwGeomPropValueNode::emitFunctionCall(const ShaderNode& node, GenContext& c { const HwShaderGenerator& shadergen = static_cast(context.getShaderGenerator()); - const ShaderInput* geomPropInput = node.getInput(GEOMPROP); + const ShaderInput* geomPropInput = getGeomPropValueInput(node, context); if (!geomPropInput) { - throw ExceptionShaderGenError("No 'geomprop' parameter found on geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); + throw ExceptionShaderGenError("No 'geomprop' value found for geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); } const string geomname = geomPropInput->getValue()->getValueString(); const string variable = HW::T_IN_GEOMPROP + "_" + geomname; @@ -76,12 +136,12 @@ ShaderNodeImplPtr HwGeomPropValueNodeAsUniform::create() return std::make_shared(); } -void HwGeomPropValueNodeAsUniform::createVariables(const ShaderNode& node, GenContext&, Shader& shader) const +void HwGeomPropValueNodeAsUniform::createVariables(const ShaderNode& node, GenContext& context, Shader& shader) const { - const ShaderInput* geomPropInput = node.getInput(GEOMPROP); + const ShaderInput* geomPropInput = getGeomPropValueInput(node, context); if (!geomPropInput || !geomPropInput->getValue()) { - throw ExceptionShaderGenError("No 'geomprop' parameter found on geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); + throw ExceptionShaderGenError("No 'geomprop' value found for geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); } const string geomProp = geomPropInput->getValue()->getValueString(); ShaderStage& ps = shader.getStage(Stage::PIXEL); @@ -94,10 +154,10 @@ void HwGeomPropValueNodeAsUniform::emitFunctionCall(const ShaderNode& node, GenC DEFINE_SHADER_STAGE(stage, Stage::PIXEL) { const ShaderGenerator& shadergen = context.getShaderGenerator(); - const ShaderInput* geomPropInput = node.getInput(GEOMPROP); + const ShaderInput* geomPropInput = getGeomPropValueInput(node, context); if (!geomPropInput) { - throw ExceptionShaderGenError("No 'geomprop' parameter found on geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); + throw ExceptionShaderGenError("No 'geomprop' value found for geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); } const string attrName = geomPropInput->getValue()->getValueString(); shadergen.emitLineBegin(stage); diff --git a/source/MaterialXGenShader/GenContext.h b/source/MaterialXGenShader/GenContext.h index 0e5e87804b..d8061ece99 100644 --- a/source/MaterialXGenShader/GenContext.h +++ b/source/MaterialXGenShader/GenContext.h @@ -127,6 +127,27 @@ class MX_GENSHADER_API GenContext return _parentNodes; } + /// Push a compound instance node onto the stack. + void pushCompoundInstanceNode(const ShaderNode* node) + { + _compoundInstanceNodes.push_back(node); + } + + /// Pop the current compound instance node from the stack. + void popCompoundInstanceNode() + { + _compoundInstanceNodes.pop_back(); + } + + /// Return the compound instance node at the given depth from the + /// innermost instance (depth 0), or nullptr if the depth is out of range. + const ShaderNode* getCompoundInstanceNode(size_t depth = 0) const + { + return depth < _compoundInstanceNodes.size() + ? _compoundInstanceNodes[_compoundInstanceNodes.size() - 1 - depth] + : nullptr; + } + /// Add user data to the context to make it /// available during shader generator. void pushUserData(const string& name, GenUserDataPtr data) @@ -218,6 +239,7 @@ class MX_GENSHADER_API GenContext std::unordered_map _outputSuffix; vector _parentNodes; + vector _compoundInstanceNodes; ApplicationVariableHandler _applicationVariableHandler; }; @@ -237,6 +259,29 @@ class MX_GENSHADER_API ScopedSetVariableName string _oldName; }; +/// A RAII class for tracking the active compound instance node on the +/// GenContext stack, ensuring the node is popped even if shader generation +/// throws while the compound is being processed. +class MX_GENSHADER_API ScopedSetCompoundInstanceNode +{ + public: + /// Constructor pushing a compound instance node onto the context stack. + ScopedSetCompoundInstanceNode(GenContext& context, const ShaderNode* node) : + _context(context) + { + _context.pushCompoundInstanceNode(node); + } + + /// Destructor popping the compound instance node from the context stack. + ~ScopedSetCompoundInstanceNode() + { + _context.popCompoundInstanceNode(); + } + + private: + GenContext& _context; +}; + MATERIALX_NAMESPACE_END #endif // MATERIALX_GENCONTEXT_H diff --git a/source/MaterialXGenShader/Nodes/CompoundNode.cpp b/source/MaterialXGenShader/Nodes/CompoundNode.cpp index f25dbbdf7b..68a8f9a97e 100644 --- a/source/MaterialXGenShader/Nodes/CompoundNode.cpp +++ b/source/MaterialXGenShader/Nodes/CompoundNode.cpp @@ -56,8 +56,10 @@ void CompoundNode::initialize(const InterfaceElement& element, GenContext& conte _hash = std::hash{}(_functionName); } -void CompoundNode::createVariables(const ShaderNode&, GenContext& context, Shader& shader) const +void CompoundNode::createVariables(const ShaderNode& node, GenContext& context, Shader& shader) const { + ScopedSetCompoundInstanceNode scopedInstance(context, &node); + // Gather shader inputs from all child nodes for (const ShaderNode* childNode : _rootGraph->getNodes()) { @@ -74,6 +76,8 @@ void CompoundNode::emitFunctionDefinition(const ShaderNode& node, GenContext& co { const ShaderGenerator& shadergen = context.getShaderGenerator(); + ScopedSetCompoundInstanceNode scopedInstance(context, &node); + // Emit functions for all child nodes shadergen.emitFunctionDefinitions(*_rootGraph, context, stage); @@ -160,6 +164,7 @@ void CompoundNode::emitFunctionCall(const ShaderNode& node, GenContext& context, MX_TRACE_SCOPE(Tracing::Category::ShaderGen, _functionName.c_str()); const ShaderGenerator& shadergen = context.getShaderGenerator(); + ScopedSetCompoundInstanceNode scopedInstance(context, &node); DEFINE_SHADER_STAGE(stage, Stage::VERTEX) { diff --git a/source/MaterialXRender/CgltfLoader.cpp b/source/MaterialXRender/CgltfLoader.cpp index 062c811b66..6a733ea229 100644 --- a/source/MaterialXRender/CgltfLoader.cpp +++ b/source/MaterialXRender/CgltfLoader.cpp @@ -491,6 +491,7 @@ bool CgltfLoader::load(const FilePath& filePath, MeshList& meshList, bool texcoo texcoordStream = mesh->generateTextureCoordinates(positionStream); mesh->addStream(texcoordStream); } + mesh->addStreamAlias("i_" + MeshStream::GEOMETRY_PROPERTY_ATTRIBUTE + "_st", texcoordStream->getName()); if (!vec4TangentStream) { MeshStreamPtr tangentStream = mesh->generateTangents(positionStream, normalStream, texcoordStream); diff --git a/source/MaterialXRender/Mesh.h b/source/MaterialXRender/Mesh.h index e0fa3d7001..9ab6c01293 100644 --- a/source/MaterialXRender/Mesh.h +++ b/source/MaterialXRender/Mesh.h @@ -273,7 +273,8 @@ class MX_RENDER_API Mesh return _sourceUri; } - /// Get a mesh stream by name + /// Get a mesh stream by name. If no stream matches directly, the name is + /// resolved through any registered stream aliases. /// @param name Name of stream /// @return Reference to a mesh stream if found MeshStreamPtr getStream(const string& name) const @@ -285,9 +286,28 @@ class MX_RENDER_API Mesh return stream; } } + auto it = _streamAliases.find(name); + if (it != _streamAliases.end()) + { + for (const auto& stream : _streams) + { + if (stream->getName() == it->second) + { + return stream; + } + } + } return MeshStreamPtr(); } + /// Register an alias so that getStream(alias) resolves to the stream + /// named target. Used to expose a single underlying stream under multiple + /// shader-input names (e.g. mapping the USD "st" primvar to UV0). + void addStreamAlias(const string& alias, const string& target) + { + _streamAliases[alias] = target; + } + /// Get a mesh stream by type and index /// @param type Type of stream /// @param index Index of stream @@ -440,6 +460,7 @@ class MX_RENDER_API Mesh float _sphereRadius; MeshStreamList _streams; + std::unordered_map _streamAliases; size_t _vertexCount; vector _partitions; }; diff --git a/source/MaterialXRender/TinyObjLoader.cpp b/source/MaterialXRender/TinyObjLoader.cpp index b01af1ebf9..2310b05727 100644 --- a/source/MaterialXRender/TinyObjLoader.cpp +++ b/source/MaterialXRender/TinyObjLoader.cpp @@ -174,6 +174,7 @@ bool TinyObjLoader::load(const FilePath& filePath, MeshList& meshList, bool texc texcoordStream = mesh->generateTextureCoordinates(positionStream); } mesh->addStream(texcoordStream); + mesh->addStreamAlias("i_" + MeshStream::GEOMETRY_PROPERTY_ATTRIBUTE + "_st", texcoordStream->getName()); MeshStreamPtr tangentStream = mesh->generateTangents(positionStream, normalStream, texcoordStream); if (tangentStream) diff --git a/source/MaterialXTest/MaterialXGenShader/GenShader.cpp b/source/MaterialXTest/MaterialXGenShader/GenShader.cpp index c4381ba7f8..bec76de032 100644 --- a/source/MaterialXTest/MaterialXGenShader/GenShader.cpp +++ b/source/MaterialXTest/MaterialXGenShader/GenShader.cpp @@ -452,3 +452,94 @@ TEST_CASE("GenShader: Track Application Variables", "[genshader]") } #endif } + +// A geompropvalue node wrapped inside a nodegraph (e.g. UsdPrimvarReader) receives +// the name of the geometric property to read through the graph interface rather than +// as a local value. This verifies that hardware shader generation resolves that name +// across more than one level of nested compounds, so the geometric stream is bound. +TEST_CASE("GenShader: Nested geompropvalue interface", "[genshader]") +{ + // "outer_primvar" wraps "inner_primvar", which wraps a geompropvalue node. The + // "geomprop" name is threaded down through both graph interfaces from the top + // level value "st", so resolution must climb two compound instances to find it. + std::string testDocumentString = + " \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + \ + "; + + mx::FileSearchPath searchPath = mx::getDefaultDataSearchPath(); + mx::DocumentPtr libraries = mx::createDocument(); + mx::loadLibraries({ "libraries" }, searchPath, libraries); + + mx::DocumentPtr testDoc = mx::createDocument(); + mx::readFromXmlString(testDoc, testDocumentString); + testDoc->setDataLibrary(libraries); + + mx::NodeGraphPtr graph = testDoc->getNodeGraph("NG_main"); + REQUIRE(graph); + mx::OutputPtr output = graph->getOutput("out"); + REQUIRE(output); + + // The geomprop name "st" resolves through both graph interfaces to the bound + // vertex attribute "i_geomprop_st" only when nested compounds are climbed in + // tandem with the instance stack. + const std::string expectedAttribute = mx::HW::IN_GEOMPROP + "_st"; + + // Only the hardware backends use HwGeomPropValueNode; OSL and MDL do not. +#ifdef MATERIALX_BUILD_GEN_GLSL + { + mx::GenContext context(mx::GlslShaderGenerator::create()); + context.registerSourceCodeSearchPath(searchPath); + mx::ShaderPtr shader = context.getShaderGenerator().generate("NestedPrimvar", output, context); + REQUIRE(shader); + const std::string source = shader->getSourceCode(mx::Stage::VERTEX) + shader->getSourceCode(mx::Stage::PIXEL); + CHECK(source.find(expectedAttribute) != std::string::npos); + } +#endif +#ifdef MATERIALX_BUILD_GEN_MSL + { + mx::GenContext context(mx::MslShaderGenerator::create()); + context.registerSourceCodeSearchPath(searchPath); + mx::ShaderPtr shader = context.getShaderGenerator().generate("NestedPrimvar", output, context); + REQUIRE(shader); + const std::string source = shader->getSourceCode(mx::Stage::VERTEX) + shader->getSourceCode(mx::Stage::PIXEL); + CHECK(source.find(expectedAttribute) != std::string::npos); + } +#endif +#ifdef MATERIALX_BUILD_GEN_SLANG + { + mx::GenContext context(mx::SlangShaderGenerator::create()); + context.registerSourceCodeSearchPath(searchPath); + mx::ShaderPtr shader = context.getShaderGenerator().generate("NestedPrimvar", output, context); + REQUIRE(shader); + const std::string source = shader->getSourceCode(mx::Stage::VERTEX) + shader->getSourceCode(mx::Stage::PIXEL); + CHECK(source.find(expectedAttribute) != std::string::npos); + } +#endif +}