diff --git a/resources/Materials/TestSuite/stdlib/nodegraphs/nested_graphs.mtlx b/resources/Materials/TestSuite/stdlib/nodegraphs/nested_graphs.mtlx
new file mode 100644
index 0000000000..c105850675
--- /dev/null
+++ b/resources/Materials/TestSuite/stdlib/nodegraphs/nested_graphs.mtlx
@@ -0,0 +1,37 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
\ No newline at end of file
diff --git a/resources/Materials/TestSuite/stdlib/nodegraphs/nexted_graph_2.mtlx b/resources/Materials/TestSuite/stdlib/nodegraphs/nexted_graph_2.mtlx
new file mode 100644
index 0000000000..bafa8d5cf3
--- /dev/null
+++ b/resources/Materials/TestSuite/stdlib/nodegraphs/nexted_graph_2.mtlx
@@ -0,0 +1,36 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/source/MaterialXCore/Document.h b/source/MaterialXCore/Document.h
index ab01bbbc11..f3f4891b14 100644
--- a/source/MaterialXCore/Document.h
+++ b/source/MaterialXCore/Document.h
@@ -84,38 +84,6 @@ class MX_CORE_API Document : public GraphElement
/// @param library The data library to be imported.
void importLibrary(const ConstDocumentPtr& library);
- /// @}
- /// @name NodeGraph Elements
- /// @{
-
- /// Add a NodeGraph to the document.
- /// @param name The name of the new NodeGraph.
- /// If no name is specified, then a unique name will automatically be
- /// generated.
- /// @return A shared pointer to the new NodeGraph.
- NodeGraphPtr addNodeGraph(const string& name = EMPTY_STRING)
- {
- return addChild(name);
- }
-
- /// Return the NodeGraph, if any, with the given name.
- NodeGraphPtr getNodeGraph(const string& name) const
- {
- return getChildOfType(name);
- }
-
- /// Return a vector of all NodeGraph elements in the document.
- vector getNodeGraphs() const
- {
- return getChildrenOfType();
- }
-
- /// Remove the NodeGraph, if any, with the given name.
- void removeNodeGraph(const string& name)
- {
- removeChildOfType(name);
- }
-
/// Return a vector of all port elements that match the given node name.
/// Port elements support spatially-varying upstream connections to
/// nodes, and include both Input and Output elements.
diff --git a/source/MaterialXCore/Element.cpp b/source/MaterialXCore/Element.cpp
index 8c6404dbb4..cb0dabb42d 100644
--- a/source/MaterialXCore/Element.cpp
+++ b/source/MaterialXCore/Element.cpp
@@ -813,6 +813,7 @@ bool ValueElement::validate(string* message) const
if (hasInterfaceName())
{
validateRequire(isA() || isA(), res, message, "Only input and token elements support interface names");
+
ConstGraphElementPtr graph = getAncestorOfType();
if (graph && graph == getParent())
{
diff --git a/source/MaterialXCore/Interface.cpp b/source/MaterialXCore/Interface.cpp
index e9e7bd7d80..4e352cd0ca 100644
--- a/source/MaterialXCore/Interface.cpp
+++ b/source/MaterialXCore/Interface.cpp
@@ -87,13 +87,17 @@ OutputPtr PortElement::getConnectedOutput() const
ConstElementPtr parent = getParent();
ConstElementPtr scope = parent ? parent->getParent() : nullptr;
- // Look for a nodegraph output.
+ // Look for a nodegraph output. Need to check parent, grandparent and document (root) scope.
if (hasNodeGraphString())
{
NodeGraphPtr nodeGraph = resolveNameReference(getNodeGraphString(), scope);
if (!nodeGraph)
{
- nodeGraph = resolveNameReference(getNodeGraphString());
+ nodeGraph = resolveNameReference(getNodeGraphString(), parent);
+ if (!nodeGraph)
+ {
+ nodeGraph = resolveNameReference(getNodeGraphString());
+ }
}
if (nodeGraph)
{
@@ -110,6 +114,12 @@ OutputPtr PortElement::getConnectedOutput() const
}
}
}
+
+ // Handle direct nodegraph -> nodegraph output connections
+ while (result && result->hasNodeGraphString())
+ {
+ result = result->getConnectedOutput();
+ }
}
// Look for a node output.
else if (hasNodeName())
@@ -149,42 +159,60 @@ bool PortElement::validate(string* message) const
{
bool res = true;
- NodePtr connectedNode = getConnectedNode();
- if (hasNodeName() || hasOutputString())
+ const string& outputString = getOutputString();
+ InterfaceElementPtr connectedElement = nullptr;
+ NodePtr connectedNode = nullptr;
+ NodeGraphPtr connectedGraph = nullptr;
+ if (hasNodeName())
{
- NodeGraphPtr nodeGraph = resolveNameReference(getNodeName());
- if (!nodeGraph)
+ connectedElement = connectedNode = getConnectedNode();
+ }
+ else if (hasNodeGraphString())
+ {
+ const string& nodeGraphString = getNodeGraphString();
+ connectedGraph = resolveNameReference(nodeGraphString);
+ if (!connectedGraph)
{
- validateRequire(connectedNode != nullptr, res, message, "Invalid port connection");
+ ConstElementPtr parent = getParent();
+ if (parent)
+ {
+ connectedGraph = resolveNameReference(nodeGraphString, parent);
+ if (!connectedGraph)
+ {
+ ConstElementPtr grandparent = parent->getParent();
+ connectedGraph = resolveNameReference(nodeGraphString, grandparent);
+ }
+ }
}
+ connectedElement = connectedGraph;
+ validateRequire(connectedGraph != nullptr, res, message,
+ "Nodegraph '" + nodeGraphString + "' not found for connection");
}
- if (connectedNode)
+
+ if (connectedElement)
{
- const string& outputString = getOutputString();
if (!outputString.empty())
{
- OutputPtr output;
- if (hasNodeName())
+ OutputPtr output = nullptr;
+ if (connectedNode)
{
NodeDefPtr nodeDef = connectedNode->getNodeDef();
output = nodeDef ? nodeDef->getOutput(outputString) : OutputPtr();
if (output)
{
validateRequire(connectedNode->getType() == MULTI_OUTPUT_TYPE_STRING, res, message,
- "Multi-output type expected in port connection");
+ "Multi-output type expected in port connection'");
}
}
- else if (hasNodeGraphString())
+ else if (connectedGraph)
{
- NodeGraphPtr nodeGraph = resolveNameReference(getNodeGraphString());
- if (nodeGraph)
+ output = connectedGraph->getOutput(outputString);
+ validateRequire(output != nullptr, res, message,
+ "Nodegraph output '" + outputString + "' not found for connection");
+ if (connectedGraph->getNodeDef())
{
- output = nodeGraph->getOutput(outputString);
- if (nodeGraph->getNodeDef())
- {
- validateRequire(nodeGraph->getOutputCount() > 1, res, message,
- "Multi-output type expected in port connection");
- }
+ validateRequire(connectedGraph->getOutputCount() > 1, res, message,
+ "Multi-output type expected in port connection");
}
}
else
@@ -196,14 +224,44 @@ bool PortElement::validate(string* message) const
if (output)
{
- validateRequire(getType() == output->getType(), res, message, "Mismatched types in port connection");
+ validateRequire(getType() == output->getType(), res, message, "Mismatched types in port connection:" +
+ getType() + " versus " + output->getType());
}
}
- else if (connectedNode->getType() != MULTI_OUTPUT_TYPE_STRING)
+
+ // Check first output. Special case multioutput nodes which are not supposed to have an output
+ // versus nodegraphs which must have an output specified.
+ else
{
- validateRequire(getType() == connectedNode->getType(), res, message, "Mismatched types in port connection");
+ OutputPtr output = nullptr;
+ string outputType = EMPTY_STRING;
+
+ if (connectedNode)
+ {
+ outputType = connectedNode->getType();
+ }
+ else if (connectedGraph)
+ {
+ std::vector outputs = connectedGraph->getOutputs();
+ validateRequire(!outputs.empty(), res, message, "Nodegraph has no outputs for port connection");
+ if (!outputs.empty())
+ {
+ output = outputs[0];
+ outputType = output->getType();
+ }
+ }
+
+ if (!outputType.empty())
+ {
+ if (outputType != MULTI_OUTPUT_TYPE_STRING)
+ {
+ validateRequire(getType() == outputType, res, message, "Mismatched types in port connection:" +
+ getType() + " versus " + outputType);
+ }
+ }
}
}
+
return ValueElement::validate(message) && res;
}
@@ -275,11 +333,50 @@ InputPtr Input::getInterfaceInput() const
{
if (hasInterfaceName())
{
+ const string& interfaceName = getInterfaceName();
ConstGraphElementPtr graph = getAncestorOfType();
- if (graph)
+ if (graph && graph == getParent())
+ {
+ // This element is a direct child of a graph element, so its
+ // interface name references a value element in the parent scope.
+ ConstElementPtr graphParent = graph->getParent();
+ graph = graphParent ? graphParent->getAncestorOfType() : nullptr;
+ }
+ ConstNodeGraphPtr nodeGraph = graph ? graph->asA() : nullptr;
+ if (!nodeGraph)
{
return graph->getInput(getInterfaceName());
}
+ if (nodeGraph)
+ {
+ // If this is a functional nodegraph, the input is defined by the nodegraph's nodedef
+ NodeDefPtr nodedef = nodeGraph->getNodeDef();
+ if (nodedef)
+ {
+ return nodedef->getInput(interfaceName);
+ }
+ // Otherwise traverse through interface element connections recursivley.
+ else
+ {
+ InputPtr returnVal = nodeGraph->getInput(interfaceName);
+ if (returnVal)
+ {
+ if (returnVal->hasInterfaceName())
+ {
+ InputPtr val = returnVal->getInterfaceInput();
+ if (val)
+ {
+ returnVal = val;
+ }
+ return returnVal;
+ }
+ else
+ {
+ return returnVal;
+ }
+ }
+ }
+ }
}
return nullptr;
}
diff --git a/source/MaterialXCore/Node.h b/source/MaterialXCore/Node.h
index d6700b45fb..b5770b65e7 100644
--- a/source/MaterialXCore/Node.h
+++ b/source/MaterialXCore/Node.h
@@ -294,6 +294,38 @@ class MX_CORE_API GraphElement : public InterfaceElement
removeChildOfType(name);
}
+ /// @}
+ /// @name NodeGraph Elements
+ /// @{
+
+ /// Add a NodeGraph to the document.
+ /// @param name The name of the new NodeGraph.
+ /// If no name is specified, then a unique name will automatically be
+ /// generated.
+ /// @return A shared pointer to the new NodeGraph.
+ NodeGraphPtr addNodeGraph(const string& name = EMPTY_STRING)
+ {
+ return addChild(name);
+ }
+
+ /// Return the NodeGraph, if any, with the given name.
+ NodeGraphPtr getNodeGraph(const string& name) const
+ {
+ return getChildOfType(name);
+ }
+
+ /// Return a vector of all NodeGraph elements in the document.
+ vector getNodeGraphs() const
+ {
+ return getChildrenOfType();
+ }
+
+ /// Remove the NodeGraph, if any, with the given name.
+ void removeNodeGraph(const string& name)
+ {
+ removeChildOfType(name);
+ }
+
/// @}
/// @name Utility
/// @{
diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp
index 9f505375fa..314b9a251a 100644
--- a/source/MaterialXGraphEditor/Graph.cpp
+++ b/source/MaterialXGraphEditor/Graph.cpp
@@ -297,15 +297,51 @@ ed::PinId Graph::getOutputPin(UiNodePtr node, UiNodePtr upNode, UiPinPtr input)
{
if (upNode->getNodeGraph() != nullptr)
{
- // For nodegraph need to get the correct output pin according to the names of the output nodes
+ // For nodegraph, resolve the upstream output from either an input port
+ // or an output port so links into output nodes are handled as well.
mx::OutputPtr output;
- if (input->getUiNode()->getNode())
+ const std::string desiredOutputName = input->getInput() ?
+ input->getInput()->getOutputString() :
+ (input->getOutput() ? input->getOutput()->getOutputString() : mx::EMPTY_STRING);
+ if (input->getInput())
{
- output = input->getUiNode()->getNode()->getConnectedOutput(input->getName());
+ output = input->getInput()->getConnectedOutput();
}
- else if (input->getUiNode()->getNodeGraph())
+ else if (input->getOutput())
{
- output = input->getUiNode()->getNodeGraph()->getConnectedOutput(input->getName());
+ output = input->getOutput()->getConnectedOutput();
+ }
+
+ // If no explicit output was specified on a nodegraph connection,
+ // resolve by output attribute name first, then default to the first
+ // declared output for single-output graphs.
+ if (!output)
+ {
+ const std::string& nodeGraphName = input->getInput() ?
+ input->getInput()->getNodeGraphString() :
+ (input->getOutput() ? input->getOutput()->getNodeGraphString() : mx::EMPTY_STRING);
+ const std::string& outputName = input->getInput() ?
+ input->getInput()->getOutputString() :
+ (input->getOutput() ? input->getOutput()->getOutputString() : mx::EMPTY_STRING);
+
+ if (!nodeGraphName.empty() && upNode->getNodeGraph() && upNode->getNodeGraph()->getName() == nodeGraphName)
+ {
+ const auto& outputs = upNode->getOutputPins();
+ if (!outputName.empty())
+ {
+ for (UiPinPtr outPin : outputs)
+ {
+ if (outPin->getName() == outputName)
+ {
+ return outPin->getPinId();
+ }
+ }
+ }
+ if (!outputs.empty())
+ {
+ return outputs[0]->getPinId();
+ }
+ }
}
if (output)
@@ -318,6 +354,34 @@ ed::PinId Graph::getOutputPin(UiNodePtr node, UiNodePtr upNode, UiPinPtr input)
return outputs->getPinId();
}
}
+
+ // If the connected output resolves deeper than this nodegraph,
+ // prefer the explicit output attribute from the referencing port.
+ if (!desiredOutputName.empty())
+ {
+ for (UiPinPtr outputs : upNode->getOutputPins())
+ {
+ if (outputs->getName() == desiredOutputName)
+ {
+ return outputs->getPinId();
+ }
+ }
+ }
+
+ // If a referenced output name cannot be resolved in the UI pins,
+ // fall back to the first output to keep links renderable.
+ const auto& outputs = upNode->getOutputPins();
+ if (!outputs.empty())
+ {
+ return outputs[0]->getPinId();
+ }
+ }
+
+ // If no upstream output was explicitly resolved, default to first output.
+ const auto& outputs = upNode->getOutputPins();
+ if (!outputs.empty())
+ {
+ return outputs[0]->getPinId();
}
return ed::PinId();
}
@@ -476,17 +540,36 @@ void Graph::linkGraph()
void Graph::scanNestedGraphDiagnostics()
{
+ //
+ // Kick off a recursive scan from each top-level graph. The previous version
+ // only scanned one level deep and could miss deeper nested nodegraphs.
for (mx::NodeGraphPtr ng : _graphDoc->getNodeGraphs())
{
- const std::string& graphName = ng->getName();
- for (mx::NodePtr node : ng->getNodes())
+ scanNestedGraphDiagnostics(ng, ng->getName());
+ }
+}
+
+void Graph::scanNestedGraphDiagnostics(mx::NodeGraphPtr ng, const std::string& graphPath)
+{
+ //
+ // Validate all node inputs in the current graph level, and record a path
+ // label so diagnostics can point back into nested graph hierarchies.
+ for (mx::NodePtr node : ng->getNodes())
+ {
+ for (mx::InputPtr input : node->getInputs())
{
- for (mx::InputPtr input : node->getInputs())
- {
- addInvalidInputDiagnostic(input, node->getName(), -1, graphName, ng);
- }
+ addInvalidInputDiagnostic(input, node->getName(), -1, graphPath, ng);
}
}
+
+ //
+ // Recurse into child nodegraphs so diagnostics include arbitrarily nested
+ // subgraphs, not only direct children.
+ for (mx::NodeGraphPtr child : ng->getNodeGraphs())
+ {
+ std::string childPath = graphPath.empty() ? child->getName() : graphPath + "/" + child->getName();
+ scanNestedGraphDiagnostics(child, childPath);
+ }
}
void Graph::connectLinks()
@@ -1253,7 +1336,7 @@ void Graph::createNodeUIList(mx::DocumentPtr doc)
addExtraNodes();
}
-void Graph::buildUiBaseGraph(mx::DocumentPtr doc)
+void Graph::buildUiBaseGraph(mx::GraphElementPtr doc)
{
std::vector nodeGraphs = doc->getNodeGraphs();
std::vector inputNodes = doc->getActiveInputs();
@@ -1273,7 +1356,10 @@ void Graph::buildUiBaseGraph(mx::DocumentPtr doc)
setUiNodeInfo(currNode, node->getType(), node->getCategory());
}
- // Create UiNodes for the nodegraph
+ //
+ // Top-level build only creates top-level nodegraph nodes.
+ // Nested nodegraphs are created when diving into their parent graph,
+ // preventing accidental flattening into the document view.
for (mx::NodeGraphPtr nodeGraph : nodeGraphs)
{
if (!includeElement(nodeGraph))
@@ -1332,161 +1418,97 @@ void Graph::buildUiBaseGraph(mx::DocumentPtr doc)
void Graph::buildUiNodeGraph(const mx::NodeGraphPtr& nodeGraphs)
{
- if (nodeGraphs)
+ //
+ // Use uniform, explicit construction order
+ // (inputs -> nodes -> nested nodegraphs -> outputs) for the current graph
+ // level, then wires edges in dedicated passes.
+ if (!nodeGraphs)
{
- mx::NodeGraphPtr nodeGraph = nodeGraphs;
- std::vector children = nodeGraph->topologicalSort();
- mx::NodeDefPtr nodeDef = nodeGraph->getNodeDef();
-
- // Create input nodes
- if (nodeDef)
- {
- std::vector inputs = nodeDef->getActiveInputs();
+ return;
+ }
- for (mx::InputPtr input : inputs)
- {
- auto currNode = std::make_shared(input->getName(), _state.nextUiId);
- currNode->setInput(input);
- setUiNodeInfo(currNode, input->getType(), input->getCategory());
- }
- }
+ mx::NodeGraphPtr nodeGraph = nodeGraphs;
+ mx::ElementPredicate includeElement = getElementPredicate();
- // Search node graph children to create uiNodes
- for (mx::ElementPtr elem : children)
- {
- mx::NodePtr node = elem->asA();
- mx::InputPtr input = elem->asA();
- mx::OutputPtr output = elem->asA();
- std::string name = elem->getName();
- auto currNode = std::make_shared(name, _state.nextUiId);
- if (node)
- {
- currNode->setNode(node);
- setUiNodeInfo(currNode, node->getType(), node->getCategory());
- }
- else if (input)
- {
- currNode->setInput(input);
- setUiNodeInfo(currNode, input->getType(), input->getCategory());
- }
- else if (output)
- {
- currNode->setOutput(output);
- setUiNodeInfo(currNode, output->getType(), output->getCategory());
- }
- }
+ //
+ // Use the graph's active interface inputs directly so compound graphs and
+ // nested graphs show their actual editable interface at this level.
+ for (mx::InputPtr input : nodeGraph->getActiveInputs())
+ {
+ if (!includeElement(input))
+ continue;
+ auto currNode = std::make_shared(input->getName(), _state.nextUiId);
+ currNode->setInput(input);
+ setUiNodeInfo(currNode, input->getType(), input->getCategory());
+ }
- // Write out all connections.
- std::set processedEdges;
- for (mx::OutputPtr output : nodeGraph->getOutputs())
- {
- for (mx::Edge edge : output->traverseGraph())
- {
- if (!processedEdges.count(edge))
- {
- mx::ElementPtr upstreamElem = edge.getUpstreamElement();
- mx::ElementPtr downstreamElem = edge.getDownstreamElement();
- mx::ElementPtr connectingElem = edge.getConnectingElement();
-
- mx::NodePtr upstreamNode = upstreamElem->asA();
- mx::InputPtr upstreamInput = upstreamElem->asA();
- mx::OutputPtr upstreamOutput = upstreamElem->asA();
- mx::NodePtr downstreamNode = downstreamElem->asA();
- mx::InputPtr downstreamInput = downstreamElem->asA();
- mx::OutputPtr downstreamOutput = downstreamElem->asA();
- std::string upName = upstreamElem->getName();
- std::string downName = downstreamElem->getName();
- std::string upstreamType;
- std::string downstreamType;
- if (upstreamNode)
- {
- upstreamType = "node";
- }
- else if (upstreamInput)
- {
- upstreamType = "input";
- }
- else if (upstreamOutput)
- {
- upstreamType = "output";
- }
- if (downstreamNode)
- {
- downstreamType = "node";
- }
- else if (downstreamInput)
- {
- downstreamType = "input";
- }
- else if (downstreamOutput)
- {
- downstreamType = "output";
- }
- int upNum = findNode(upName, upstreamType);
- int downNum = findNode(downName, downstreamType);
+ // Create regular node instances in this graph level.
+ for (mx::NodePtr node : nodeGraph->getNodes())
+ {
+ if (!includeElement(node))
+ continue;
+ auto currNode = std::make_shared(node->getName(), _state.nextUiId);
+ currNode->setNode(node);
+ setUiNodeInfo(currNode, node->getType(), node->getCategory());
+ }
- // Create edges for output nodes (no connecting input)
- if (downNum >= 0 && upNum >= 0 && _state.nodes[downNum]->getOutput())
- {
- createEdge(_state.nodes[upNum], _state.nodes[downNum], nullptr);
- }
- else if (connectingElem && upNum >= 0 && downNum >= 0)
- {
- mx::InputPtr connectingInput = connectingElem->asA();
- if (connectingInput)
- {
- createEdge(_state.nodes[upNum], _state.nodes[downNum], connectingInput);
- }
- }
+ //
+ // Explicitly create child nodegraph nodes so nested graph structure is
+ // represented in the UI and can be entered by the user.
+ for (mx::NodeGraphPtr subGraph : nodeGraph->getNodeGraphs())
+ {
+ if (!includeElement(subGraph))
+ continue;
+ auto currNode = std::make_shared(subGraph->getName(), _state.nextUiId);
+ currNode->setNodeGraph(subGraph);
+ setUiNodeInfo(currNode, "", "nodegraph");
+ }
- // Connect input nodes for upstream node
- if (upstreamNode && upNum >= 0)
- {
- for (mx::InputPtr input : upstreamNode->getActiveInputs())
- {
- if (input->hasInterfaceName())
- {
- int newUp = findNode(input->getInterfaceName(), "input");
- if (newUp >= 0)
- {
- createEdge(_state.nodes[newUp], _state.nodes[upNum], input);
- }
- }
- }
- }
+ // Create output nodes in this graph level.
+ for (mx::OutputPtr output : nodeGraph->getOutputs())
+ {
+ if (!includeElement(output))
+ continue;
+ auto currNode = std::make_shared(output->getName(), _state.nextUiId);
+ currNode->setOutput(output);
+ setUiNodeInfo(currNode, output->getType(), output->getCategory());
+ }
- processedEdges.insert(edge);
- }
- }
+ //
+ // Build incoming edges from each interface element's active inputs.
+ // This handles node inputs and nested nodegraph inputs with the same logic.
+ for (size_t i = 0; i < _state.nodes.size(); i++)
+ {
+ UiNodePtr& uiNode = _state.nodes[i];
+ mx::ElementPtr elem = uiNode->getElement();
+ mx::InterfaceElementPtr interface = elem ? elem->asA() : nullptr;
+ if (!interface)
+ {
+ continue;
}
- // Second pass to catch all of the connections that aren't part of an output
- for (mx::ElementPtr elem : children)
+ int downNum = static_cast(i);
+ for (mx::InputPtr input : interface->getActiveInputs())
{
- mx::NodePtr node = elem->asA();
- mx::OutputPtr output = elem->asA();
- if (node)
- {
- int downNum = findNode(node->getName(), "node");
- if (downNum < 0)
- {
- continue;
- }
- for (mx::InputPtr input : node->getActiveInputs())
- {
- int upNum = findUpstreamNode(input);
- if (upNum >= 0)
- {
- createEdge(_state.nodes[upNum], _state.nodes[downNum], input);
- }
- }
- }
- else if (output)
+ if (!includeElement(input))
+ continue;
+ int upNum = findUpstreamNode(input);
+ if (upNum >= 0)
{
- createEdgeForOutput(output);
+ createEdge(_state.nodes[upNum], _state.nodes[downNum], input);
}
}
}
+
+ //
+ // Build output edges in a separate pass so outputs connected through either
+ // nodename or output references are handled consistently.
+ for (mx::OutputPtr output : nodeGraph->getOutputs())
+ {
+ if (!includeElement(output))
+ continue;
+ createEdgeForOutput(output);
+ }
}
int Graph::findNode(const std::string& name, const std::string& type)
@@ -1600,6 +1622,16 @@ bool Graph::createEdge(UiNodePtr upNode, UiNodePtr downNode, mx::InputPtr connec
void Graph::createEdgeForOutput(mx::OutputPtr output)
{
+ //
+ // Outputs can be connected in two valid forms:
+ // 1) output -> connected node
+ // 2) output -> specific connected output (including from a nodegraph)
+ // Handle both so nested nodegraph output wiring is visible in the UI.
+ if (!output)
+ {
+ return;
+ }
+
mx::NodePtr connectedNode = output->getConnectedNode();
if (connectedNode)
{
@@ -1609,6 +1641,55 @@ void Graph::createEdgeForOutput(mx::OutputPtr output)
{
createEdge(_state.nodes[upNum], _state.nodes[downNum], nullptr);
}
+ return;
+ }
+
+ //
+ // If no direct node connection exists, fall back to connected-output
+ // references and resolve their parent element type.
+ mx::OutputPtr connectedOutput = output->getConnectedOutput();
+ if (!connectedOutput)
+ {
+ // Handle output connections expressed as nodegraph references without
+ // an explicit output name (valid for single-output nodegraphs).
+ const mx::string& nodeGraphName = output->getNodeGraphString();
+ if (!nodeGraphName.empty())
+ {
+ int upNum = findNode(nodeGraphName, "nodegraph");
+ int downNum = findNode(output->getName(), "output");
+ if (upNum >= 0 && downNum >= 0)
+ {
+ createEdge(_state.nodes[upNum], _state.nodes[downNum], nullptr);
+ }
+ }
+ return;
+ }
+
+ int upNum = -1;
+ mx::ElementPtr parent = connectedOutput->getParent();
+ if (mx::NodePtr upstreamNode = parent ? parent->asA() : nullptr)
+ {
+ upNum = findNode(upstreamNode->getName(), "node");
+ }
+ else if (mx::NodeGraphPtr upstreamGraph = parent ? parent->asA() : nullptr)
+ {
+ upNum = findNode(upstreamGraph->getName(), "nodegraph");
+ }
+
+ // If parent-based resolution fails, fall back to explicit nodegraph attribute.
+ if (upNum < 0)
+ {
+ const mx::string& nodeGraphName = output->getNodeGraphString();
+ if (!nodeGraphName.empty())
+ {
+ upNum = findNode(nodeGraphName, "nodegraph");
+ }
+ }
+
+ int downNum = findNode(output->getName(), "output");
+ if (upNum >= 0 && downNum >= 0)
+ {
+ createEdge(_state.nodes[upNum], _state.nodes[downNum], nullptr);
}
}
@@ -1618,9 +1699,13 @@ void Graph::copyUiNode(UiNodePtr node)
++_state.nextUiId;
if (node->getNodeGraph())
{
- _graphDoc->addNodeGraph();
- std::string nodeGraphName = _graphDoc->getNodeGraphs().back()->getName();
- copyNode->setNodeGraph(_graphDoc->getNodeGraphs().back());
+ //
+ // Create copied nodegraphs under the currently edited graph scope.
+ // Previously this was always created at document scope, which broke
+ // nested graph hierarchy during copy/paste.
+ mx::NodeGraphPtr newNodeGraph = _state.graphElem->addNodeGraph();
+ std::string nodeGraphName = newNodeGraph->getName();
+ copyNode->setNodeGraph(newNodeGraph);
copyNode->setName(nodeGraphName);
copyNodeGraph(node, copyNode);
}
@@ -1663,7 +1748,10 @@ void Graph::copyNodeGraph(UiNodePtr origGraph, UiNodePtr copyGraph)
std::vector inputs = copyGraph->getNodeGraph()->getActiveInputs();
for (mx::InputPtr input : inputs)
{
- std::string newName = _graphDoc->createValidChildName(input->getName());
+ //
+ // Resolve unique input names against the copied graph itself, not the
+ // document root. This keeps copied nested graphs self-consistent.
+ std::string newName = copyGraph->getNodeGraph()->createValidChildName(input->getName());
input->setName(newName);
}
}
@@ -1808,13 +1896,16 @@ void Graph::addNode(const std::string& category, const std::string& name, const
}
else if (category == "nodegraph")
{
- // Create new mx::NodeGraph and set as current node graph
- _graphDoc->addNodeGraph();
- std::string nodeGraphName = _graphDoc->getNodeGraphs().back()->getName();
+ // Create a new nodegraph as a child of the currently edited graph.
+ //
+ // Use the current graph scope so adding a nodegraph inside another
+ // nodegraph preserves intended nesting.
+ mx::NodeGraphPtr newNodeGraph = _state.graphElem->addNodeGraph();
+ std::string nodeGraphName = newNodeGraph->getName();
auto nodeGraphNode = std::make_shared(nodeGraphName, int(++_state.nextUiId));
// Set mx::Nodegraph as node graph for uiNode
- nodeGraphNode->setNodeGraph(_graphDoc->getNodeGraphs().back());
+ nodeGraphNode->setNodeGraph(newNodeGraph);
setUiNodeInfo(nodeGraphNode, type, "nodegraph");
return;
@@ -2350,7 +2441,7 @@ std::vector Graph::createNodes(bool nodegraph)
ImGui::GetWindowDrawList()->AddRectFilled(
ImGui::GetCursorScreenPos() + ImVec2(-hdrPadL, 3),
ImGui::GetCursorScreenPos() + ImVec2(ed::GetNodeSize(node->getId()).x - hdrPadL - 2.f * hdrInset, ImGui::GetTextLineHeight() + hdrPadB),
- ImColor(ImColor(35, 35, 35, 255)), 0);
+ ImColor(ImColor(135, 206, 250, 255)), 0);
ImGui::Indent(hdrTextIndent);
ImGui::Text("%s", node->getName().c_str());
ImGui::Unindent(hdrTextIndent);
@@ -2688,7 +2779,80 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId)
{
if (_state.nodes[downNode]->getOutput() != nullptr)
{
- _state.nodes[downNode]->getOutput()->setConnectedNode(_state.nodes[upNode]->getNode());
+ //
+ // Output elements in a parent graph can connect to either:
+ // - a node in the same graph
+ // - a specific output on a nested nodegraph
+ // Serialize using connected-output when needed so the exported
+ // MaterialX preserves nodegraph/output attributes correctly.
+ mx::OutputPtr downOutput = _state.nodes[downNode]->getOutput();
+ if (uiUpNode->getNodeGraph())
+ {
+ mx::OutputPtr upstreamOutput = nullptr;
+ for (UiPinPtr outPin : uiUpNode->getOutputPins())
+ {
+ if (outPin->getPinId() == outputPinId)
+ {
+ upstreamOutput = uiUpNode->getNodeGraph()->getOutput(outPin->getName());
+ break;
+ }
+ }
+
+ // If no explicit output is selected, default to first output.
+ if (!upstreamOutput)
+ {
+ if (!uiUpNode->getOutputPins().empty())
+ {
+ upstreamOutput = uiUpNode->getNodeGraph()->getOutput(uiUpNode->getOutputPins()[0]->getName());
+ }
+ }
+
+ if (upstreamOutput)
+ {
+ downOutput->setConnectedOutput(upstreamOutput);
+ }
+ }
+ else if (uiUpNode->getNode())
+ {
+ mx::NodePtr upstreamNode = uiUpNode->getNode();
+ mx::NodeDefPtr upstreamNodeDef = upstreamNode->getNodeDef();
+ const bool isMultiOutput = upstreamNodeDef ? upstreamNodeDef->getOutputs().size() > 1 : false;
+
+ if (!isMultiOutput)
+ {
+ downOutput->setConnectedNode(upstreamNode);
+ }
+ else
+ {
+ mx::OutputPtr upstreamOutput = nullptr;
+ for (UiPinPtr outPin : uiUpNode->getOutputPins())
+ {
+ if (outPin->getPinId() == outputPinId)
+ {
+ upstreamOutput = upstreamNode->getOutput(outPin->getName());
+ break;
+ }
+ }
+
+ // If no explicit output is selected, default to first output.
+ if (!upstreamOutput)
+ {
+ if (!uiUpNode->getOutputPins().empty())
+ {
+ upstreamOutput = upstreamNode->getOutput(uiUpNode->getOutputPins()[0]->getName());
+ }
+ }
+
+ if (upstreamOutput)
+ {
+ downOutput->setConnectedOutput(upstreamOutput);
+ }
+ else
+ {
+ downOutput->setConnectedNode(upstreamNode);
+ }
+ }
+ }
}
// Create new edge and set edge information.
@@ -2805,7 +2969,12 @@ void Graph::deleteLinkInfo(int startAttr, int endAttr)
if ((int) pin->getPinId().Get() == endAttr)
{
removeEdge(downNode, upNode, pin);
+ // Clear all serialized connection forms for outputs.
+ // Output links may be encoded as nodename, nodegraph+output,
+ // or nodename+output for multioutput nodes.
_state.nodes[downNode]->getOutput()->removeAttribute("nodename");
+ _state.nodes[downNode]->getOutput()->removeAttribute("nodegraph");
+ _state.nodes[downNode]->getOutput()->removeAttribute("output");
for (UiPinPtr connect : pin->getConnections())
{
pin->deleteConnection(connect);
@@ -3973,10 +4142,10 @@ void Graph::addNodePopup(bool cursor)
std::string nodeName = node.getName();
// Disallow creating nested nodegraphs
- if (_state.isCompoundNodeGraph && node.getGroup() == NODEGRAPH_ENTRY)
- {
- continue;
- }
+ //if (_state.isCompoundNodeGraph && node.getGroup() == NODEGRAPH_ENTRY)
+ //{
+ // continue;
+ //}
// Allow spaces to be used to search for node names
std::replace(subs.begin(), subs.end(), ' ', '_');
diff --git a/source/MaterialXGraphEditor/Graph.h b/source/MaterialXGraphEditor/Graph.h
index 49da4dd951..6a432dec6f 100644
--- a/source/MaterialXGraphEditor/Graph.h
+++ b/source/MaterialXGraphEditor/Graph.h
@@ -132,7 +132,7 @@ class Graph
void createNodeUIList(mx::DocumentPtr doc);
// Build UiNode nodegraph upon loading a document
- void buildUiBaseGraph(mx::DocumentPtr doc);
+ void buildUiBaseGraph(mx::GraphElementPtr doc);
// Build UiNode node graph upon diving into a nodegraph node
void buildUiNodeGraph(const mx::NodeGraphPtr& nodeGraphs);
@@ -146,6 +146,11 @@ class Graph
// Walk all NodeGraph elements in _graphDoc and append invalid-connection diagnostics.
void scanNestedGraphDiagnostics();
+ // Recursive helper used by scanNestedGraphDiagnostics.
+ // Walks nested NodeGraph hierarchies while preserving a slash-delimited
+ // graph path for UI diagnostics.
+ void scanNestedGraphDiagnostics(mx::NodeGraphPtr ng, const std::string& graphPath);
+
// If the given connected input fails core validation, append a diagnostic and return true.
bool addInvalidInputDiagnostic(mx::InputPtr input, const std::string& nodeName,
int uiNodeId, const std::string& graphPath,
diff --git a/source/MaterialXGraphEditor/UiNode.cpp b/source/MaterialXGraphEditor/UiNode.cpp
index 4b85d8431c..451f0de3d0 100644
--- a/source/MaterialXGraphEditor/UiNode.cpp
+++ b/source/MaterialXGraphEditor/UiNode.cpp
@@ -149,7 +149,8 @@ void UiNode::buildUiTokenMap()
std::string inputValue = input->getValueString();
if (inputValue.empty() && input->hasInterfaceName())
- inputValue = input->getInterfaceInput()->getValueString(); // Get value from referenced interface
+ if (input->getInterfaceInput())
+ inputValue = input->getInterfaceInput()->getValueString(); // Get value from referenced interface
for (const auto& entry : inputTokens)
{
diff --git a/source/MaterialXTest/MaterialXCore/Node.cpp b/source/MaterialXTest/MaterialXCore/Node.cpp
index c43156a4ae..d7d0b35d3a 100644
--- a/source/MaterialXTest/MaterialXCore/Node.cpp
+++ b/source/MaterialXTest/MaterialXCore/Node.cpp
@@ -699,6 +699,94 @@ TEST_CASE("Organization", "[nodegraph]")
CHECK(nodeGraph->getBackdrops().empty());
}
+TEST_CASE("Nested Nodegraphs", "[nodegraph]")
+{
+ mx::DocumentPtr doc = mx::createDocument();
+
+ // Create 2 level nesting of graphs
+ mx::NodeGraphPtr level1 = doc->addNodeGraph("level1_graph");
+ mx::NodeGraphPtr level2 = level1->addNodeGraph("level2_graph");
+ REQUIRE(level2);
+ mx::NodeGraphPtr level3 = level2->addNodeGraph("level3_graph");
+ REQUIRE(level3);
+
+ // Add an input connection between the graph inputs at the levels 1 and 2
+ // (An interface connection)
+ mx::InputPtr level1Input = level1->addInput("level1_input", "color3");
+ mx::InputPtr level2Input = level2->addInput("level2_input", "color3");
+ level2Input->setInterfaceName("level1_input");
+ mx::InputPtr interfaceInput = level2Input->getInterfaceInput();
+ REQUIRE(interfaceInput);
+ REQUIRE(interfaceInput->getNamePath() == level1Input->getNamePath());
+
+ // Add an input connection between the graphs inputs at level 1, 2 and 3
+ // Recursive interfacename search
+ mx::InputPtr level1Input2 = level1->addInput("level1_input2", "color3");
+ mx::InputPtr level2Input2 = level2->addInput("level2_input2", "color3");
+ level2Input2->setInterfaceName(level1Input2->getName());
+ mx::InputPtr level3Input = level3->addInput("level3_input", "color3");
+ level3Input->setInterfaceName(level2Input2->getName());
+ interfaceInput = level3Input->getInterfaceInput();
+ REQUIRE(interfaceInput->getNamePath() == level1Input2->getNamePath());
+
+ // Add output connection from child nodegraph to node inside nodegraph
+ mx::OutputPtr level2Output = level2->addOutput("level2_output", "color3");
+ mx::NodePtr level1Multiply = level1->addNode("multiply", "color3");
+ mx::InputPtr level1MultiplyInput = level1Multiply->addInput("in1", "color3");
+ level1MultiplyInput->setNodeGraphString(level2->getName());
+ level1MultiplyInput->setOutputString(level2Output->getName());
+ mx::OutputPtr outputPtr = level1MultiplyInput->getConnectedOutput();
+ REQUIRE(outputPtr->getNamePath() == level2Output->getNamePath());
+
+ // Check for connected node inside level2
+ mx::NodePtr level2Add = level2->addNode("add", "color3");
+ level2Output->setNodeName(level2Add->getName());
+ mx::NodePtr connectedNode = level1MultiplyInput->getConnectedNode();
+ REQUIRE(connectedNode->getNamePath() == level2Add->getNamePath());
+ REQUIRE(outputPtr->getNamePath() == level2Output->getNamePath());
+
+ // Also connect to level 1 output
+ mx::OutputPtr leve1Output = level1->addOutput("leve1_output", "color3");
+ leve1Output->setNodeGraphString(level2->getName());
+ level1MultiplyInput->setOutputString(level2Output->getName());
+ outputPtr = level1MultiplyInput->getConnectedOutput();
+ REQUIRE(outputPtr->getNamePath() == level2Output->getNamePath());
+
+ // Check transitive outputs
+ mx::OutputPtr level3Output = level3->addOutput("level3_output", "color3");
+
+ mx::OutputPtr level2Output2 = level2->addOutput("level2_output2", "color3");
+ level2Output2->setNodeGraphString(level3->getName());
+ level2Output2->setOutputString(level3Output->getName());
+
+ mx::OutputPtr connectedOutput = level2Output2->getConnectedOutput();
+ REQUIRE(connectedOutput);
+ REQUIRE(connectedOutput->getNamePath() == level3Output->getNamePath());
+
+ mx::OutputPtr level1Output2 = level1->addOutput("level1_output2", "color3");
+ level1Output2->setNodeGraphString(level2->getName());
+ level1Output2->setOutputString(level2Output2->getName());
+
+ connectedOutput = level1Output2->getConnectedOutput();
+ REQUIRE(connectedOutput);
+ REQUIRE(connectedOutput->getNamePath() == level3Output->getNamePath());
+
+ mx::OutputPtr docOutput = doc->addOutput("doc_output", "color3");
+ docOutput->setNodeGraphString(level1->getName());
+ docOutput->setOutputString(level1Output2->getName());
+ REQUIRE(connectedOutput->getNamePath() == level3Output->getNamePath());
+
+ std::string errors;
+ bool valid = doc->validate(&errors);
+ if (!valid)
+ {
+ std::string xmlString = mx::writeToXmlString(doc);
+ INFO("Graph:\n" + xmlString + "\n");
+ INFO("Validation errors:" + errors);
+ }
+ REQUIRE(valid);
+}
+
TEST_CASE("Node Definition Creation", "[nodedef]")
{
mx::FileSearchPath searchPath = mx::getDefaultDataSearchPath();