Improvements to findRenderable functions (#1271)

- Simplify the interfaces of findRenderableElements and findRenderableMaterialNodes, using return values instead of references to output arguments. This enables function composability and copy elision optimizations, and improves the consistency of the API across languages. - Add deprecated wrappers for the original interfaces of these functions, allowing clients to proceed without code changes. - Mark BSDF, EDF, and VDF data types as unsupported in findRenderableElements, as these cannot yet be directly rendered across all languages.
AcademySoftwareFoundation · Mar 6, 2023 · f493598 · f493598
1 parent bfdce98
commit f493598
Show file tree

Hide file tree

Showing 16 changed files with 92 additions and 95 deletions.
diff --git a/python/Scripts/generateshader.py b/python/Scripts/generateshader.py
@@ -131,7 +131,7 @@ def main():
         genoptions.targetDistanceUnit = 'meter'
 
         # Look for renderable nodes
-        nodes = mx_gen_shader.findRenderableElements(doc, False)
+        nodes = mx_gen_shader.findRenderableElements(doc)
         if not nodes:
             nodes = doc.getMaterialNodes()
             if not nodes:

diff --git a/source/JsMaterialX/JsMaterialXGenShader/JsUtil.cpp b/source/JsMaterialX/JsMaterialXGenShader/JsUtil.cpp
@@ -18,8 +18,7 @@ namespace mx = MaterialX;
 mx::ElementPtr findRenderableElement(mx::DocumentPtr doc)
 {
     mx::StringVec renderablePaths;
-    std::vector<mx::TypedElementPtr> elems;
-    mx::findRenderableElements(doc, elems);
+    std::vector<mx::TypedElementPtr> elems = mx::findRenderableElements(doc);
 
     for (mx::TypedElementPtr elem : elems)
     {
@@ -50,6 +49,5 @@ EMSCRIPTEN_BINDINGS(Util)
 {
     BIND_FUNC("isTransparentSurface", mx::isTransparentSurface, 1, 2, mx::ElementPtr, const std::string&);
 
-    ems::function("findRenderableElements", &mx::findRenderableElements);
     ems::function("findRenderableElement", &findRenderableElement);
 }
diff --git a/source/MaterialXCore/Types.cpp b/source/MaterialXCore/Types.cpp
@@ -11,6 +11,9 @@ const string DEFAULT_TYPE_STRING = "color3";
 const string FILENAME_TYPE_STRING = "filename";
 const string GEOMNAME_TYPE_STRING = "geomname";
 const string STRING_TYPE_STRING = "string";
+const string BSDF_TYPE_STRING = "BSDF";
+const string EDF_TYPE_STRING = "EDF";
+const string VDF_TYPE_STRING = "VDF";
 const string SURFACE_SHADER_TYPE_STRING = "surfaceshader";
 const string DISPLACEMENT_SHADER_TYPE_STRING = "displacementshader";
 const string VOLUME_SHADER_TYPE_STRING = "volumeshader";

diff --git a/source/MaterialXCore/Types.h b/source/MaterialXCore/Types.h
@@ -22,6 +22,9 @@ extern MX_CORE_API const string DEFAULT_TYPE_STRING;
 extern MX_CORE_API const string FILENAME_TYPE_STRING;
 extern MX_CORE_API const string GEOMNAME_TYPE_STRING;
 extern MX_CORE_API const string STRING_TYPE_STRING;
+extern MX_CORE_API const string BSDF_TYPE_STRING;
+extern MX_CORE_API const string EDF_TYPE_STRING;
+extern MX_CORE_API const string VDF_TYPE_STRING;
 extern MX_CORE_API const string SURFACE_SHADER_TYPE_STRING;
 extern MX_CORE_API const string DISPLACEMENT_SHADER_TYPE_STRING;
 extern MX_CORE_API const string VOLUME_SHADER_TYPE_STRING;

diff --git a/source/MaterialXGenShader/ShaderTranslator.cpp b/source/MaterialXGenShader/ShaderTranslator.cpp
@@ -181,9 +181,7 @@ void ShaderTranslator::translateShader(NodePtr shader, const string& destCategor
 
 void ShaderTranslator::translateAllMaterials(DocumentPtr doc, const string& destCategory)
 {
-    vector<TypedElementPtr> materialNodes;
-    std::unordered_set<ElementPtr> shaderOutputs;
-    findRenderableMaterialNodes(doc, materialNodes, false, shaderOutputs);
+    vector<TypedElementPtr> materialNodes = findRenderableMaterialNodes(doc);
     for (auto elem : materialNodes)
     {
         NodePtr materialNode = elem->asA<Node>();

diff --git a/source/MaterialXGenShader/Util.cpp b/source/MaterialXGenShader/Util.cpp
@@ -363,78 +363,62 @@ bool elementRequiresShading(ConstTypedElementPtr element)
     return colorClosures.count(elementType) > 0;
 }
 
-void findRenderableMaterialNodes(ConstDocumentPtr doc,
-                                 vector<TypedElementPtr>& elements,
-                                 bool includeReferencedGraphs,
-                                 std::unordered_set<ElementPtr>& processedSources)
+vector<TypedElementPtr> findRenderableMaterialNodes(ConstDocumentPtr doc)
 {
-    for (const NodePtr& material : doc->getMaterialNodes())
+    vector<TypedElementPtr> renderableNodes;
+    for (NodePtr materialNode : doc->getMaterialNodes())
     {
-        // Scan for any upstream shader outputs and put them on the "processed" list
-        // if we don't want to consider them for rendering.
-        vector<NodePtr> shaderNodes = getShaderNodes(material);
-        if (!shaderNodes.empty())
+        if (!getShaderNodes(materialNode).empty())
         {
-            // Push the material node only once if any shader nodes are found
-            elements.push_back(material);
-            processedSources.insert(material);
-
-            if (!includeReferencedGraphs)
-            {
-                for (NodePtr shaderNode : shaderNodes)
-                {
-                    for (InputPtr input : shaderNode->getActiveInputs())
-                    {
-                        OutputPtr outputPtr = input->getConnectedOutput();
-                        if (outputPtr && !outputPtr->hasSourceUri() && !processedSources.count(outputPtr))
-                        {
-                            processedSources.insert(outputPtr);
-                        }
-                    }
-                }
-            }
+            renderableNodes.push_back(materialNode);
         }
     }
+    return renderableNodes;
 }
 
-void findRenderableElements(ConstDocumentPtr doc, vector<TypedElementPtr>& elements, bool includeReferencedGraphs)
+vector<TypedElementPtr> findRenderableElements(ConstDocumentPtr doc)
 {
-    std::unordered_set<ElementPtr> processedSources;
-    findRenderableMaterialNodes(doc, elements, includeReferencedGraphs, processedSources);
-    if (!elements.empty())
+    vector<TypedElementPtr> renderableElements = findRenderableMaterialNodes(doc);
+    if (renderableElements.empty())
     {
-        return;
-    }
-
-    // Find all node graph outputs in the content document.
-    vector<OutputPtr> graphOutputs;
-    for (OutputPtr output : doc->getOutputs())
-    {
-        if (output->getActiveSourceUri() == doc->getActiveSourceUri())
+        // Collect all graph outputs in the content document.
+        vector<OutputPtr> graphOutputs;
+        for (NodeGraphPtr graph : doc->getNodeGraphs())
         {
-            graphOutputs.push_back(output);
+            for (OutputPtr output : graph->getOutputs())
+            {
+                if (output->getActiveSourceUri() == doc->getActiveSourceUri())
+                {
+                    graphOutputs.push_back(output);
+                }
+            }
         }
-    }
-    for (NodeGraphPtr graph : doc->getNodeGraphs())
-    {
-        for (OutputPtr output : graph->getOutputs())
+        for (OutputPtr output : doc->getOutputs())
         {
             if (output->getActiveSourceUri() == doc->getActiveSourceUri())
             {
                 graphOutputs.push_back(output);
             }
         }
-    }
 
-    // Add renderable graph outputs to the return vector.
-    for (OutputPtr output : graphOutputs)
-    {
-        NodePtr node = output->getConnectedNode();
-        if (node && node->getType() != LIGHT_SHADER_TYPE_STRING)
+        // Filter out unconnected outputs and unsupported data types.
+        const StringSet UNSUPPORTED_TYPES =
+        {
+            BSDF_TYPE_STRING,
+            EDF_TYPE_STRING,
+            VDF_TYPE_STRING,
+            LIGHT_SHADER_TYPE_STRING
+        };
+        for (OutputPtr output : graphOutputs)
         {
-            elements.push_back(output);
+            NodePtr node = output->getConnectedNode();
+            if (node && !UNSUPPORTED_TYPES.count(node->getType()))
+            {
+                renderableElements.push_back(output);
+            }
         }
     }
+    return renderableElements;
 }
 
 InputPtr getNodeDefInput(InputPtr nodeInput, const string& target)
@@ -604,4 +588,13 @@ bool hasElementAttributes(OutputPtr output, const StringVec& attributes)
     return false;
 }
 
+void findRenderableMaterialNodes(ConstDocumentPtr doc, vector<TypedElementPtr>& elements, bool, std::unordered_set<ElementPtr>&)
+{
+    elements = findRenderableMaterialNodes(doc);
+}
+void findRenderableElements(ConstDocumentPtr doc, vector<TypedElementPtr>& elements, bool)
+{
+    elements = findRenderableElements(doc);
+}
+
 MATERIALX_NAMESPACE_END
diff --git a/source/MaterialXGenShader/Util.h b/source/MaterialXGenShader/Util.h
@@ -45,22 +45,16 @@ MX_GENSHADER_API bool requiresImplementation(ConstNodeDefPtr nodeDef);
 /// Determine if a given element requires shading / lighting for rendering
 MX_GENSHADER_API bool elementRequiresShading(ConstTypedElementPtr element);
 
-/// Find any material node elements which are renderable (have input shaders)
+/// Find all renderable material nodes in the given document.
 /// @param doc Document to examine
-/// @param elements List of renderable elements (returned)
-/// @param includeReferencedGraphs Whether to check for outputs on referenced graphs
-/// @param processedSources List of elements examined.
-MX_GENSHADER_API void findRenderableMaterialNodes(ConstDocumentPtr doc,
-                                                  vector<TypedElementPtr>& elements,
-                                                  bool includeReferencedGraphs,
-                                                  std::unordered_set<ElementPtr>& processedSources);
-
-/// Find any elements which may be renderable from within a document.
-/// This includes all outputs on node graphs and shader references which are not
-/// part of any included library. Light shaders are not considered to be renderable.
-/// The option to include node graphs referened by shader references is disabled by default.
-MX_GENSHADER_API void findRenderableElements(ConstDocumentPtr doc, vector<TypedElementPtr>& elements,
-                                             bool includeReferencedGraphs = false);
+/// @return A vector of renderable material nodes.
+MX_GENSHADER_API vector<TypedElementPtr> findRenderableMaterialNodes(ConstDocumentPtr doc);
+
+/// Find all renderable elements in the given document, including material nodes if present,
+/// or graph outputs of renderable types if no material nodes are found.
+/// @param doc Document to examine
+/// @return A vector of renderable elements
+MX_GENSHADER_API vector<TypedElementPtr> findRenderableElements(ConstDocumentPtr doc);
 
 /// Given a node input, return the corresponding input within its matching nodedef.
 /// The optional target string can be used to guide the selection of nodedef declarations.
@@ -92,6 +86,13 @@ MX_GENSHADER_API NodePtr connectsToWorldSpaceNode(OutputPtr output);
 /// @param attributes Attributes to test for
 MX_GENSHADER_API bool hasElementAttributes(OutputPtr output, const StringVec& attributes);
 
+//
+// These are deprecated wrappers for older versions of the function interfaces in this module.
+// Clients using these interfaces should update them to the latest API.
+//
+MX_GENSHADER_API void findRenderableMaterialNodes(ConstDocumentPtr doc, vector<TypedElementPtr>& elements, bool, std::unordered_set<ElementPtr>&);
+MX_GENSHADER_API void findRenderableElements(ConstDocumentPtr doc, vector<TypedElementPtr>& elements, bool includeReferencedGraphs = false);
+
 MATERIALX_NAMESPACE_END
 
 #endif
diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp
@@ -667,8 +667,7 @@ auto showLabel = [](const char* label, ImColor color)
 void Graph::selectMaterial(UiNodePtr uiNode)
 {
     // find renderable element that correspond with material uiNode
-    std::vector<mx::TypedElementPtr> elems;
-    mx::findRenderableElements(_graphDoc, elems);
+    std::vector<mx::TypedElementPtr> elems = mx::findRenderableElements(_graphDoc);
     mx::TypedElementPtr typedElem = nullptr;
     for (mx::TypedElementPtr elem : elems)
     {
@@ -752,9 +751,8 @@ void Graph::setRenderMaterial(UiNodePtr node)
 void Graph::updateMaterials(mx::InputPtr input, mx::ValuePtr value)
 {
     std::string renderablePath;
-    std::vector<mx::TypedElementPtr> elems;
     mx::TypedElementPtr renderableElem;
-    mx::findRenderableElements(_graphDoc, elems);
+    std::vector<mx::TypedElementPtr> elems = mx::findRenderableElements(_graphDoc);
 
     size_t num = 0;
     int num2 = 0;

diff --git a/source/MaterialXGraphEditor/RenderView.cpp b/source/MaterialXGraphEditor/RenderView.cpp
@@ -449,8 +449,7 @@ void RenderView::updateMaterials(mx::TypedElementPtr typedElem)
         //// Create new materials.
         if (!typedElem)
         {
-            std::vector<mx::TypedElementPtr> elems;
-            mx::findRenderableElements(_document, elems);
+            std::vector<mx::TypedElementPtr> elems = mx::findRenderableElements(_document);
             if (!elems.empty())
             {
                 typedElem = elems[0];

diff --git a/source/MaterialXRenderGlsl/TextureBaker.cpp b/source/MaterialXRenderGlsl/TextureBaker.cpp
@@ -547,8 +547,7 @@ void TextureBaker::bakeAllMaterials(DocumentPtr doc, const FileSearchPath& searc
         }
     }
 
-    std::vector<TypedElementPtr> renderableMaterials;
-    findRenderableElements(doc, renderableMaterials);
+    std::vector<TypedElementPtr> renderableMaterials = findRenderableElements(doc);
 
     // Compute the UDIM set.
     ValuePtr udimSetValue = doc->getGeomPropValue(UDIM_SET_PROPERTY);

diff --git a/source/MaterialXTest/MaterialXGenShader/GenShader.cpp b/source/MaterialXTest/MaterialXGenShader/GenShader.cpp
@@ -117,8 +117,7 @@ TEST_CASE("GenShader: Graph + Nodedf Transparent Check", "[genshader]")
     mx::readFromXmlFile(doc, testPath, searchPath);
 
     // Test against nodegraphs using surface shaders
-    std::vector<mx::TypedElementPtr> testElements;
-    mx::findRenderableElements(doc, testElements);
+    std::vector<mx::TypedElementPtr> testElements = mx::findRenderableElements(doc);
     std::string failedElements;
     std::set<mx::NodeGraphPtr> testGraphs;
     for (auto testElement : testElements)
@@ -231,8 +230,7 @@ TEST_CASE("GenShader: Transparency Regression Check", "[genshader]")
         try
         {
             mx::readFromXmlFile(testDoc, testFile, searchPath);
-            std::vector<mx::TypedElementPtr> renderables;
-            mx::findRenderableElements(testDoc, renderables);
+            std::vector<mx::TypedElementPtr> renderables = mx::findRenderableElements(testDoc);
             for (auto renderable : renderables)
             {
                 mx::NodePtr node = renderable->asA<mx::Node>();

diff --git a/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp b/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp
@@ -684,7 +684,7 @@ void ShaderGeneratorTester::validate(const mx::GenOptions& generateOptions, cons
         std::vector<mx::TypedElementPtr> elements;
         try
         {
-            mx::findRenderableElements(doc, elements);
+            elements = mx::findRenderableElements(doc);
         }
         catch (mx::Exception& e)
         {

diff --git a/source/MaterialXTest/MaterialXRender/RenderUtil.cpp b/source/MaterialXTest/MaterialXRender/RenderUtil.cpp
@@ -295,7 +295,7 @@ bool ShaderRenderTester::validate(const mx::FilePath optionsFilePath)
             std::vector<mx::TypedElementPtr> elements;
             try
             {
-                mx::findRenderableElements(doc, elements);
+                elements = mx::findRenderableElements(doc);
             }
             catch (mx::Exception& e)
             {

diff --git a/source/MaterialXView/Viewer.cpp b/source/MaterialXView/Viewer.cpp
@@ -1250,13 +1250,12 @@ void Viewer::loadDocument(const mx::FilePath& filename, mx::DocumentPtr librarie
 
         // Find new renderable elements.
         mx::StringVec renderablePaths;
-        std::vector<mx::TypedElementPtr> elems;
-        std::vector<mx::NodePtr> materialNodes;
-        mx::findRenderableElements(doc, elems);
+        std::vector<mx::TypedElementPtr> elems = mx::findRenderableElements(doc);
         if (elems.empty())
         {
             throw mx::Exception("No renderable elements found in " + _materialFilename.getBaseName());
         }
+        std::vector<mx::NodePtr> materialNodes;
         for (mx::TypedElementPtr elem : elems)
         {
             mx::TypedElementPtr renderableElem = elem;

diff --git a/source/PyMaterialX/PyMaterialXCore/PyTypes.cpp b/source/PyMaterialX/PyMaterialXCore/PyTypes.cpp
@@ -134,6 +134,10 @@ void bindPyTypes(py::module& mod)
     mod.attr("DEFAULT_TYPE_STRING") = mx::DEFAULT_TYPE_STRING;
     mod.attr("FILENAME_TYPE_STRING") = mx::FILENAME_TYPE_STRING;
     mod.attr("GEOMNAME_TYPE_STRING") = mx::GEOMNAME_TYPE_STRING;
+    mod.attr("STRING_TYPE_STRING") = mx::STRING_TYPE_STRING;
+    mod.attr("BSDF_TYPE_STRING") = mx::BSDF_TYPE_STRING;
+    mod.attr("EDF_TYPE_STRING") = mx::EDF_TYPE_STRING;
+    mod.attr("VDF_TYPE_STRING") = mx::VDF_TYPE_STRING;
     mod.attr("SURFACE_SHADER_TYPE_STRING") = mx::SURFACE_SHADER_TYPE_STRING;
     mod.attr("DISPLACEMENT_SHADER_TYPE_STRING") = mx::DISPLACEMENT_SHADER_TYPE_STRING;
     mod.attr("VOLUME_SHADER_TYPE_STRING") = mx::VOLUME_SHADER_TYPE_STRING;

diff --git a/source/PyMaterialX/PyMaterialXGenShader/PyUtil.cpp b/source/PyMaterialX/PyMaterialXGenShader/PyUtil.cpp
@@ -11,11 +11,15 @@
 namespace py = pybind11;
 namespace mx = MaterialX;
 
+std::vector<mx::TypedElementPtr> findRenderableMaterialNodes(mx::ConstDocumentPtr doc)
+{
+    return mx::findRenderableMaterialNodes(doc);
+}
+
 std::vector<mx::TypedElementPtr> findRenderableElements(mx::ConstDocumentPtr doc, bool includeReferencedGraphs)
 {
-    std::vector<mx::TypedElementPtr> elements;
-    mx::findRenderableElements(doc, elements, includeReferencedGraphs);
-    return elements;
+    (void) includeReferencedGraphs;
+    return mx::findRenderableElements(doc);
 }
 
 void bindPyUtil(py::module& mod)
@@ -24,8 +28,8 @@ void bindPyUtil(py::module& mod)
     mod.def("mapValueToColor", &mx::mapValueToColor);
     mod.def("requiresImplementation", &mx::requiresImplementation);
     mod.def("elementRequiresShading", &mx::elementRequiresShading);
-    mod.def("findRenderableMaterialNodes", &mx::findRenderableMaterialNodes);
-    mod.def("findRenderableElements", &findRenderableElements);
+    mod.def("findRenderableMaterialNodes", &findRenderableMaterialNodes);
+    mod.def("findRenderableElements", &findRenderableElements, py::arg("doc"), py::arg("includeReferencedGraphs") = false);
     mod.def("getNodeDefInput", &mx::getNodeDefInput);
     mod.def("tokenSubstitution", &mx::tokenSubstitution);
     mod.def("getUdimCoordinates", &mx::getUdimCoordinates);