From 07b87245afa56d24d79801a8aedda32a4ce87930 Mon Sep 17 00:00:00 2001 From: Minh Pham Date: Mon, 22 Jun 2026 23:42:55 -0400 Subject: [PATCH 1/4] Support grouping selected nodes into nested nodegraphs --- source/MaterialXCore/Interface.cpp | 18 +- source/MaterialXGraphEditor/Graph.cpp | 362 ++++++++++++++++++-- source/MaterialXGraphEditor/Graph.h | 8 + source/MaterialXTest/MaterialXCore/Node.cpp | 28 ++ 4 files changed, 390 insertions(+), 26 deletions(-) diff --git a/source/MaterialXCore/Interface.cpp b/source/MaterialXCore/Interface.cpp index e9e7bd7d80..a1ec967c96 100644 --- a/source/MaterialXCore/Interface.cpp +++ b/source/MaterialXCore/Interface.cpp @@ -149,10 +149,16 @@ bool PortElement::validate(string* message) const { bool res = true; + ConstElementPtr parent = getParent(); + ConstElementPtr scope = parent ? parent->getParent() : nullptr; NodePtr connectedNode = getConnectedNode(); if (hasNodeName() || hasOutputString()) { - NodeGraphPtr nodeGraph = resolveNameReference(getNodeName()); + NodeGraphPtr nodeGraph = resolveNameReference(getNodeName(), scope); + if (!nodeGraph) + { + nodeGraph = resolveNameReference(getNodeName()); + } if (!nodeGraph) { validateRequire(connectedNode != nullptr, res, message, "Invalid port connection"); @@ -176,7 +182,11 @@ bool PortElement::validate(string* message) const } else if (hasNodeGraphString()) { - NodeGraphPtr nodeGraph = resolveNameReference(getNodeGraphString()); + NodeGraphPtr nodeGraph = resolveNameReference(getNodeGraphString(), scope); + if (!nodeGraph) + { + nodeGraph = resolveNameReference(getNodeGraphString()); + } if (nodeGraph) { output = nodeGraph->getOutput(outputString); @@ -223,7 +233,9 @@ NodePtr Input::getConnectedNode() const // Handle inputs of compound nodegraphs. if (getParent()->isA()) { - NodePtr rootNode = getDocument()->getNode(getNodeName()); + ConstElementPtr graphParent = getParent()->getParent(); + ConstGraphElementPtr parentGraph = graphParent ? graphParent->asA() : nullptr; + NodePtr rootNode = parentGraph ? parentGraph->getNode(getNodeName()) : nullptr; if (rootNode) { return rootNode; diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp index 8e4a04422e..359d0f0924 100644 --- a/source/MaterialXGraphEditor/Graph.cpp +++ b/source/MaterialXGraphEditor/Graph.cpp @@ -105,6 +105,174 @@ std::string getUserNodeDefName(const std::string& val) return result; } +void clearPortConnection(mx::PortElementPtr port) +{ + if (!port) + { + return; + } + port->removeAttribute(mx::PortElement::NODE_NAME_ATTRIBUTE); + port->removeAttribute(mx::PortElement::NODE_GRAPH_ATTRIBUTE); + port->removeAttribute(mx::PortElement::OUTPUT_ATTRIBUTE); + port->removeAttribute(mx::ValueElement::INTERFACE_NAME_ATTRIBUTE); +} + +void copyConnectionAttributes(mx::PortElementPtr source, mx::PortElementPtr dest) +{ + if (!source || !dest) + { + return; + } + + clearPortConnection(dest); + if (source->hasNodeName()) + { + dest->setNodeName(source->getNodeName()); + } + if (source->hasNodeGraphString()) + { + dest->setNodeGraphString(source->getNodeGraphString()); + } + if (source->hasOutputString()) + { + dest->setOutputString(source->getOutputString()); + } + if (source->hasInterfaceName()) + { + dest->setInterfaceName(source->getInterfaceName()); + } + dest->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE); +} + +bool inputConnectsOutsideSelection(mx::InputPtr input, const std::unordered_set& selectedNodeNames) +{ + if (!input) + { + return false; + } + if (input->hasNodeName()) + { + return selectedNodeNames.find(input->getNodeName()) == selectedNodeNames.end(); + } + return input->hasNodeGraphString() || input->hasInterfaceName() || input->hasOutputString(); +} + +void preserveExternalUpstreamConnections(mx::NodeGraphPtr nodeGraph, const std::unordered_set& selectedNodeNames) +{ + if (!nodeGraph) + { + return; + } + + for (mx::NodePtr copiedNode : nodeGraph->getNodes()) + { + for (mx::InputPtr input : copiedNode->getInputs()) + { + if (!inputConnectsOutsideSelection(input, selectedNodeNames)) + { + continue; + } + + std::string interfaceName = nodeGraph->createValidChildName(copiedNode->getName() + "_" + input->getName()); + mx::InputPtr interfaceInput = nodeGraph->addInput(interfaceName, input->getType()); + copyConnectionAttributes(input, interfaceInput); + input->setConnectedInterfaceName(interfaceInput->getName()); + } + } +} + +std::string getBoundaryOutputType(mx::NodePtr sourceNode, mx::PortElementPtr downstreamPort) +{ + if (downstreamPort && !downstreamPort->getType().empty()) + { + return downstreamPort->getType(); + } + if (sourceNode) + { + const std::string& outputName = downstreamPort ? downstreamPort->getOutputString() : mx::EMPTY_STRING; + if (!outputName.empty()) + { + mx::OutputPtr output = sourceNode->getOutput(outputName); + if (!output && sourceNode->getNodeDef()) + { + output = sourceNode->getNodeDef()->getOutput(outputName); + } + if (output && !output->getType().empty()) + { + return output->getType(); + } + } + return sourceNode->getType(); + } + return mx::DEFAULT_TYPE_STRING; +} + +mx::OutputPtr getOrCreateBoundaryOutput(mx::NodeGraphPtr nodeGraph, + mx::NodePtr sourceNode, + mx::PortElementPtr downstreamPort, + std::map& boundaryOutputs) +{ + const std::string& sourceOutput = downstreamPort ? downstreamPort->getOutputString() : mx::EMPTY_STRING; + std::string key = sourceNode->getName() + "|" + sourceOutput; + auto existing = boundaryOutputs.find(key); + if (existing != boundaryOutputs.end()) + { + return existing->second; + } + + std::string outputBaseName = sourceNode->getName() + (sourceOutput.empty() ? "_out" : "_" + sourceOutput); + mx::OutputPtr boundaryOutput = nodeGraph->addOutput(nodeGraph->createValidChildName(outputBaseName), + getBoundaryOutputType(sourceNode, downstreamPort)); + boundaryOutput->setNodeName(sourceNode->getName()); + if (!sourceOutput.empty()) + { + boundaryOutput->setOutputString(sourceOutput); + } + boundaryOutput->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE); + boundaryOutputs[key] = boundaryOutput; + return boundaryOutput; +} + +void preserveExternalDownstreamConnections(mx::NodeGraphPtr nodeGraph, + const std::vector& nodesToGroup, + const std::unordered_set& selectedNodeNames) +{ + if (!nodeGraph) + { + return; + } + + std::map boundaryOutputs; + for (UiNodePtr uiNode : nodesToGroup) + { + mx::NodePtr sourceNode = uiNode ? uiNode->getNode() : nullptr; + if (!sourceNode) + { + continue; + } + for (mx::PortElementPtr downstreamPort : sourceNode->getDownstreamPorts()) + { + if (!downstreamPort) + { + continue; + } + + mx::ElementPtr parent = downstreamPort->getParent(); + if (parent && selectedNodeNames.find(parent->getName()) != selectedNodeNames.end()) + { + continue; + } + + mx::OutputPtr boundaryOutput = getOrCreateBoundaryOutput(nodeGraph, sourceNode, downstreamPort, boundaryOutputs); + downstreamPort->setNodeGraphString(nodeGraph->getName()); + downstreamPort->setOutputString(boundaryOutput->getName()); + downstreamPort->removeAttribute(mx::PortElement::NODE_NAME_ATTRIBUTE); + downstreamPort->removeAttribute(mx::ValueElement::INTERFACE_NAME_ATTRIBUTE); + downstreamPort->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE); + } + } +} + static void EnableSRGBCallback(const ImDrawList*, const ImDrawCmd*) { glEnable(GL_FRAMEBUFFER_SRGB); @@ -476,8 +644,10 @@ void Graph::linkGraph() void Graph::scanNestedGraphDiagnostics() { - for (mx::NodeGraphPtr ng : _graphDoc->getNodeGraphs()) + std::vector nodeGraphs = _graphDoc->getNodeGraphs(); + for (size_t i = 0; i < nodeGraphs.size(); ++i) { + mx::NodeGraphPtr ng = nodeGraphs[i]; const std::string& graphName = ng->getName(); for (mx::NodePtr node : ng->getNodes()) { @@ -486,6 +656,10 @@ void Graph::scanNestedGraphDiagnostics() addInvalidInputDiagnostic(input, node->getName(), -1, graphName, ng); } } + for (mx::NodeGraphPtr childGraph : ng->getChildrenOfType()) + { + nodeGraphs.push_back(childGraph); + } } } @@ -1354,6 +1528,7 @@ void Graph::buildUiNodeGraph(const mx::NodeGraphPtr& nodeGraphs) for (mx::ElementPtr elem : children) { mx::NodePtr node = elem->asA(); + mx::NodeGraphPtr childNodeGraph = elem->asA(); mx::InputPtr input = elem->asA(); mx::OutputPtr output = elem->asA(); std::string name = elem->getName(); @@ -1363,6 +1538,11 @@ void Graph::buildUiNodeGraph(const mx::NodeGraphPtr& nodeGraphs) currNode->setNode(node); setUiNodeInfo(currNode, node->getType(), node->getCategory()); } + else if (childNodeGraph) + { + currNode->setNodeGraph(childNodeGraph); + setUiNodeInfo(currNode, "", "nodegraph"); + } else if (input) { currNode->setInput(input); @@ -1388,9 +1568,11 @@ void Graph::buildUiNodeGraph(const mx::NodeGraphPtr& nodeGraphs) mx::ElementPtr connectingElem = edge.getConnectingElement(); mx::NodePtr upstreamNode = upstreamElem->asA(); + mx::NodeGraphPtr upstreamNodeGraph = upstreamElem->asA(); mx::InputPtr upstreamInput = upstreamElem->asA(); mx::OutputPtr upstreamOutput = upstreamElem->asA(); mx::NodePtr downstreamNode = downstreamElem->asA(); + mx::NodeGraphPtr downstreamNodeGraph = downstreamElem->asA(); mx::InputPtr downstreamInput = downstreamElem->asA(); mx::OutputPtr downstreamOutput = downstreamElem->asA(); std::string upName = upstreamElem->getName(); @@ -1401,6 +1583,10 @@ void Graph::buildUiNodeGraph(const mx::NodeGraphPtr& nodeGraphs) { upstreamType = "node"; } + else if (upstreamNodeGraph) + { + upstreamType = "nodegraph"; + } else if (upstreamInput) { upstreamType = "input"; @@ -1413,6 +1599,10 @@ void Graph::buildUiNodeGraph(const mx::NodeGraphPtr& nodeGraphs) { downstreamType = "node"; } + else if (downstreamNodeGraph) + { + downstreamType = "nodegraph"; + } else if (downstreamInput) { downstreamType = "input"; @@ -1463,15 +1653,17 @@ void Graph::buildUiNodeGraph(const mx::NodeGraphPtr& nodeGraphs) for (mx::ElementPtr elem : children) { mx::NodePtr node = elem->asA(); + mx::NodeGraphPtr childNodeGraph = elem->asA(); mx::OutputPtr output = elem->asA(); - if (node) + mx::InterfaceElementPtr interface = elem->asA(); + if (node || childNodeGraph) { - int downNum = findNode(node->getName(), "node"); + int downNum = findNode(elem->getName(), node ? "node" : "nodegraph"); if (downNum < 0) { continue; } - for (mx::InputPtr input : node->getActiveInputs()) + for (mx::InputPtr input : interface->getActiveInputs()) { int upNum = findUpstreamNode(input); if (upNum >= 0) @@ -1617,9 +1809,9 @@ void Graph::copyUiNode(UiNodePtr node) ++_state.nextUiId; if (node->getNodeGraph()) { - _graphDoc->addNodeGraph(); - std::string nodeGraphName = _graphDoc->getNodeGraphs().back()->getName(); - copyNode->setNodeGraph(_graphDoc->getNodeGraphs().back()); + mx::NodeGraphPtr nodeGraph = _state.graphElem->addChild(); + std::string nodeGraphName = nodeGraph->getName(); + copyNode->setNodeGraph(nodeGraph); copyNode->setName(nodeGraphName); copyNodeGraph(node, copyNode); } @@ -1761,6 +1953,94 @@ void Graph::copyInputs() } } +bool Graph::collectGroupableNodes(const std::vector& selectedNodes, std::vector& nodesToGroup) +{ + nodesToGroup.clear(); + + if (selectedNodes.empty()) + { + return false; + } + + std::unordered_set seenNodes; + for (ed::NodeId selected : selectedNodes) + { + int pos = findNode(int(selected.Get())); + if (pos < 0) + { + return false; + } + + UiNodePtr node = _state.nodes[pos]; + if (!node || !node->getNode()) + { + return false; + } + + if (seenNodes.insert(node->getId()).second) + { + nodesToGroup.push_back(node); + } + } + + return !nodesToGroup.empty(); +} + +void Graph::groupSelectedNodesIntoNodeGraph(const std::vector& selectedNodes) +{ + if (readOnly()) + { + _popup = true; + return; + } + + std::vector nodesToGroup; + if (!collectGroupableNodes(selectedNodes, nodesToGroup)) + { + return; + } + + savePosition(); + + std::unordered_set selectedNodeNames; + ImVec2 totalPosition(0.0f, 0.0f); + for (UiNodePtr uiNode : nodesToGroup) + { + selectedNodeNames.insert(uiNode->getName()); + ImVec2 nodePosition = ed::GetNodePosition(uiNode->getId()); + totalPosition.x += nodePosition.x; + totalPosition.y += nodePosition.y; + } + + mx::NodeGraphPtr nodeGraph = _state.graphElem->addChild(); + for (UiNodePtr uiNode : nodesToGroup) + { + mx::NodePtr sourceNode = uiNode->getNode(); + mx::NodePtr copiedNode = nodeGraph->addNode(sourceNode->getCategory(), sourceNode->getName(), sourceNode->getType()); + copiedNode->copyContentFrom(sourceNode); + } + + preserveExternalUpstreamConnections(nodeGraph, selectedNodeNames); + preserveExternalDownstreamConnections(nodeGraph, nodesToGroup, selectedNodeNames); + + for (const std::string& nodeName : selectedNodeNames) + { + _state.graphElem->removeChild(nodeName); + } + + ImVec2 averagePosition(totalPosition.x / float(nodesToGroup.size()), totalPosition.y / float(nodesToGroup.size())); + nodeGraph->setAttribute(mx::Element::XPOS_ATTRIBUTE, std::to_string(averagePosition.x / DEFAULT_NODE_SIZE.x)); + nodeGraph->setAttribute(mx::Element::YPOS_ATTRIBUTE, std::to_string(averagePosition.y / DEFAULT_NODE_SIZE.y)); + + rebuildCurrentGraph(); + linkGraph(); + restorePositions(); + _needsLayout = false; + _layoutPending = false; + _needsNavigation = false; + updateMaterials(); +} + void Graph::addNode(const std::string& category, const std::string& name, const std::string& type) { mx::NodePtr node = nullptr; @@ -1807,13 +2087,13 @@ 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 nodegraph at the current graph level. + mx::NodeGraphPtr nodeGraph = _state.graphElem->addChild(); + std::string nodeGraphName = nodeGraph->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(nodeGraph); setUiNodeInfo(nodeGraphNode, type, "nodegraph"); return; @@ -3023,6 +3303,44 @@ void Graph::initializeGraph() _state.name = materialPath.getBaseName(); } +void Graph::rebuildCurrentGraph() +{ + mx::GraphElementPtr graphElem = _state.graphElem; + bool isCompoundNodeGraph = _state.isCompoundNodeGraph; + std::string graphName = _state.name; + + _state = GraphState(); + if (graphElem == _graphDoc) + { + buildUiBaseGraph(_graphDoc); + _state.graphElem = _graphDoc; + _state.isCompoundNodeGraph = false; + if (graphName.empty()) + { + mx::FilePath materialPath(_materialFilename); + materialPath.removeExtension(); + graphName = materialPath.getBaseName(); + } + _state.name = graphName; + } + else if (mx::NodeGraphPtr nodeGraph = graphElem ? graphElem->asA() : nullptr) + { + buildUiNodeGraph(nodeGraph); + _state.graphElem = nodeGraph; + _state.isCompoundNodeGraph = isCompoundNodeGraph; + _state.name = graphName.empty() ? nodeGraph->getName() : graphName; + } + else + { + initializeGraph(); + return; + } + + _prevUiNode = nullptr; + _currUiNode = nullptr; + _currRenderNode = nullptr; +} + void Graph::loadGraphFromFile(bool prompt) { // Deselect node before loading new file @@ -3849,6 +4167,7 @@ void Graph::showHelp() const if (ImGui::TreeNode("Editing")) { ImGui::BulletText("TAB : Show popup menu to add new nodes."); + ImGui::BulletText("SHIFT-C : Group selected nodes into a node graph."); ImGui::BulletText("CTRL-C : Copy selected nodes to clipboard."); ImGui::BulletText("CTRL-V : Paste clipboard to graph."); ImGui::BulletText("CTRL-F : Find a node by name."); @@ -3898,10 +4217,6 @@ void Graph::addNodePopup(bool cursor) ImGui::InputText("##input", input, sizeof(input)); std::string subs(input); - // Input string length - // Filter extra nodes - includes inputs, outputs, groups, and node graphs - const std::string NODEGRAPH_ENTRY = "Node Graph"; - // Filter nodedefs and add to menu if matches filter for (auto node : _nodesToAdd) { @@ -3935,12 +4250,6 @@ void Graph::addNodePopup(bool cursor) std::string str(node.getName()); std::string nodeName = node.getName(); - // Disallow creating nested nodegraphs - if (_state.isCompoundNodeGraph && node.getGroup() == NODEGRAPH_ENTRY) - { - continue; - } - // Allow spaces to be used to search for node names std::replace(subs.begin(), subs.end(), ' ', '_'); @@ -4291,7 +4600,14 @@ void Graph::drawGraph(ImVec2 mousePos) // Check if keyboard shortcuts for copy/cut/paste have been used if (ed::BeginShortcut()) { - if (ed::AcceptCopy()) + bool groupSelectedNodesShortcut = io2.KeyShift && !io2.KeyCtrl && !io2.KeyAlt && !io2.KeySuper && + ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_C)) && + (!ImGui::IsPopupOpen("add node")) && (!ImGui::IsPopupOpen("search")) && !_fileDialogSave.isOpened(); + if (groupSelectedNodesShortcut) + { + groupSelectedNodesIntoNodeGraph(selectedNodes); + } + else if (ed::AcceptCopy()) { _copiedNodes.clear(); for (ed::NodeId selected : selectedNodes) @@ -4428,7 +4744,7 @@ void Graph::drawGraph(ImVec2 mousePos) // Delete selected nodes and their links if delete key is pressed // or if the shortcut for cut is used - if (ImGui::IsWindowFocused(ImGuiFocusedFlags_RootWindow)) + if (ImGui::IsWindowFocused(ImGuiFocusedFlags_RootAndChildWindows)) { bool traverseDownstream = ImGui::IsKeyReleased(ImGuiKey_RightArrow); bool traverseUpstream = ImGui::IsKeyReleased(ImGuiKey_LeftArrow); diff --git a/source/MaterialXGraphEditor/Graph.h b/source/MaterialXGraphEditor/Graph.h index 0e3ab40745..3b28cb779f 100644 --- a/source/MaterialXGraphEditor/Graph.h +++ b/source/MaterialXGraphEditor/Graph.h @@ -13,6 +13,8 @@ #include +#include + namespace ed = ax::NodeEditor; namespace mx = MaterialX; @@ -216,6 +218,9 @@ class Graph void deleteNode(UiNodePtr node); + void groupSelectedNodesIntoNodeGraph(const std::vector& selectedNodes); + bool collectGroupableNodes(const std::vector& selectedNodes, std::vector& nodesToGroup); + // Build the initial graph of a loaded document including shader, material and nodegraph node void setUiNodeInfo(UiNodePtr node, const std::string& type, const std::string& category); @@ -296,6 +301,9 @@ class Graph // Initialize the graph state from the current document. void initializeGraph(); + // Rebuild the UI state for the current graph element without changing navigation. + void rebuildCurrentGraph(); + void showHelp() const; private: diff --git a/source/MaterialXTest/MaterialXCore/Node.cpp b/source/MaterialXTest/MaterialXCore/Node.cpp index c43156a4ae..b851621f6a 100644 --- a/source/MaterialXTest/MaterialXCore/Node.cpp +++ b/source/MaterialXTest/MaterialXCore/Node.cpp @@ -385,6 +385,34 @@ TEST_CASE("Topological sort", "[nodegraph]") REQUIRE(isTopologicalOrder(elemOrder)); } +TEST_CASE("Nested compound nodegraph connections", "[nodegraph]") +{ + mx::DocumentPtr doc = mx::createDocument(); + + mx::NodeGraphPtr parentGraph = doc->addNodeGraph("parent"); + mx::NodePtr upstream = parentGraph->addNode("constant", "upstream", "float"); + mx::NodeGraphPtr nestedGraph = parentGraph->addChild("nested"); + + mx::InputPtr nestedInput = nestedGraph->addInput("in", "float"); + nestedInput->setConnectedNode(upstream); + + mx::NodePtr internal = nestedGraph->addNode("constant", "internal", "float"); + mx::InputPtr internalInput = internal->addInput("value", "float"); + internalInput->setConnectedInterfaceName(nestedInput->getName()); + + mx::OutputPtr nestedOutput = nestedGraph->addOutput("out", "float"); + nestedOutput->setConnectedNode(internal); + + mx::NodePtr downstream = parentGraph->addNode("dot", "downstream", "float"); + mx::InputPtr downstreamInput = downstream->addInput("in", "float"); + downstreamInput->setConnectedOutput(nestedOutput); + + REQUIRE(nestedInput->getConnectedNode() == upstream); + REQUIRE(downstreamInput->getConnectedOutput() == nestedOutput); + REQUIRE(downstreamInput->getConnectedNode() == internal); + REQUIRE(doc->validate()); +} + TEST_CASE("New nodegraph from output", "[nodegraph]") { // Create a document. From f44b248d407d66e9e17b84dc6e9d58281370e4f5 Mon Sep 17 00:00:00 2001 From: Minh Pham Date: Wed, 8 Jul 2026 19:00:26 -0400 Subject: [PATCH 2/4] Fix graph editor Shift+C node grouping shortcut --- source/MaterialXGraphEditor/Graph.cpp | 25 ++++++++---- source/MaterialXGraphEditor/GraphShortcuts.h | 35 +++++++++++++++++ source/MaterialXTest/CMakeLists.txt | 1 + .../MaterialXGraphEditor/CMakeLists.txt | 9 +++++ .../MaterialXGraphEditor/GraphShortcuts.cpp | 39 +++++++++++++++++++ 5 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 source/MaterialXGraphEditor/GraphShortcuts.h create mode 100644 source/MaterialXTest/MaterialXGraphEditor/CMakeLists.txt create mode 100644 source/MaterialXTest/MaterialXGraphEditor/GraphShortcuts.cpp diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp index 359d0f0924..0951fe4679 100644 --- a/source/MaterialXGraphEditor/Graph.cpp +++ b/source/MaterialXGraphEditor/Graph.cpp @@ -4,6 +4,7 @@ // #include +#include #include #include @@ -4597,17 +4598,25 @@ void Graph::drawGraph(ImVec2 mousePos) } } + GraphShortcutState groupShortcutState; + groupShortcutState.shift = io2.KeyShift; + groupShortcutState.ctrl = io2.KeyCtrl; + groupShortcutState.alt = io2.KeyAlt; + groupShortcutState.super = io2.KeySuper; + groupShortcutState.keyPressed = ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_C)); + groupShortcutState.addNodePopupOpen = ImGui::IsPopupOpen("add node"); + groupShortcutState.searchPopupOpen = ImGui::IsPopupOpen("search"); + groupShortcutState.fileDialogOpen = _fileDialogSave.isOpened() || _fileDialog.isOpened() || _fileDialogGeom.isOpened(); + if (ImGui::IsWindowFocused(ImGuiFocusedFlags_RootAndChildWindows) && + isGroupSelectedNodesShortcut(groupShortcutState)) + { + groupSelectedNodesIntoNodeGraph(selectedNodes); + } + // Check if keyboard shortcuts for copy/cut/paste have been used if (ed::BeginShortcut()) { - bool groupSelectedNodesShortcut = io2.KeyShift && !io2.KeyCtrl && !io2.KeyAlt && !io2.KeySuper && - ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_C)) && - (!ImGui::IsPopupOpen("add node")) && (!ImGui::IsPopupOpen("search")) && !_fileDialogSave.isOpened(); - if (groupSelectedNodesShortcut) - { - groupSelectedNodesIntoNodeGraph(selectedNodes); - } - else if (ed::AcceptCopy()) + if (ed::AcceptCopy()) { _copiedNodes.clear(); for (ed::NodeId selected : selectedNodes) diff --git a/source/MaterialXGraphEditor/GraphShortcuts.h b/source/MaterialXGraphEditor/GraphShortcuts.h new file mode 100644 index 0000000000..9db0d017b7 --- /dev/null +++ b/source/MaterialXGraphEditor/GraphShortcuts.h @@ -0,0 +1,35 @@ +// +// Copyright Contributors to the MaterialX Project +// SPDX-License-Identifier: Apache-2.0 +// + +/// @file +/// Shared shortcut predicates for the MaterialX Graph Editor. + +#ifndef MATERIALX_GRAPH_SHORTCUTS_H +#define MATERIALX_GRAPH_SHORTCUTS_H + +/// @struct GraphShortcutState +/// Captures the UI input state needed to evaluate graph editor shortcuts. +struct GraphShortcutState +{ + bool shift = false; + bool ctrl = false; + bool alt = false; + bool super = false; + bool keyPressed = false; + bool addNodePopupOpen = false; + bool searchPopupOpen = false; + bool fileDialogOpen = false; +}; + +/// Return true if the current shortcut state should group selected nodes. +inline bool isGroupSelectedNodesShortcut(const GraphShortcutState& state) +{ + return state.shift && state.keyPressed && + !state.ctrl && !state.alt && !state.super && + !state.addNodePopupOpen && !state.searchPopupOpen && + !state.fileDialogOpen; +} + +#endif diff --git a/source/MaterialXTest/CMakeLists.txt b/source/MaterialXTest/CMakeLists.txt index 2cb7c1330a..9deeb3365a 100644 --- a/source/MaterialXTest/CMakeLists.txt +++ b/source/MaterialXTest/CMakeLists.txt @@ -40,6 +40,7 @@ add_subdirectory(MaterialXCore) target_link_libraries(MaterialXTest MaterialXCore) add_subdirectory(MaterialXFormat) target_link_libraries(MaterialXTest MaterialXFormat) +add_subdirectory(MaterialXGraphEditor) if(MATERIALX_BUILD_PERFETTO_TRACING) target_link_libraries(MaterialXTest MaterialXTrace) endif() diff --git a/source/MaterialXTest/MaterialXGraphEditor/CMakeLists.txt b/source/MaterialXTest/MaterialXGraphEditor/CMakeLists.txt new file mode 100644 index 0000000000..f42e36e06d --- /dev/null +++ b/source/MaterialXTest/MaterialXGraphEditor/CMakeLists.txt @@ -0,0 +1,9 @@ +file(GLOB_RECURSE source "${CMAKE_CURRENT_SOURCE_DIR}/*.cpp") +file(GLOB_RECURSE headers "${CMAKE_CURRENT_SOURCE_DIR}/*.h") + +target_sources(MaterialXTest PUBLIC ${source} ${headers}) + +add_tests("${source}") + +assign_source_group("Source Files" ${source}) +assign_source_group("Header Files" ${headers}) diff --git a/source/MaterialXTest/MaterialXGraphEditor/GraphShortcuts.cpp b/source/MaterialXTest/MaterialXGraphEditor/GraphShortcuts.cpp new file mode 100644 index 0000000000..79b5890e75 --- /dev/null +++ b/source/MaterialXTest/MaterialXGraphEditor/GraphShortcuts.cpp @@ -0,0 +1,39 @@ +// +// Copyright Contributors to the MaterialX Project +// SPDX-License-Identifier: Apache-2.0 +// + +#include + +#include + +TEST_CASE("Graph editor grouping shortcut", "[grapheditor]") +{ + GraphShortcutState state; + state.shift = true; + state.keyPressed = true; + REQUIRE(isGroupSelectedNodesShortcut(state)); + + state.ctrl = true; + REQUIRE(!isGroupSelectedNodesShortcut(state)); + state.ctrl = false; + + state.alt = true; + REQUIRE(!isGroupSelectedNodesShortcut(state)); + state.alt = false; + + state.super = true; + REQUIRE(!isGroupSelectedNodesShortcut(state)); + state.super = false; + + state.addNodePopupOpen = true; + REQUIRE(!isGroupSelectedNodesShortcut(state)); + state.addNodePopupOpen = false; + + state.searchPopupOpen = true; + REQUIRE(!isGroupSelectedNodesShortcut(state)); + state.searchPopupOpen = false; + + state.fileDialogOpen = true; + REQUIRE(!isGroupSelectedNodesShortcut(state)); +} From c0f3ce9b62c4584d0cfe91a14b1bd9c7aff7f2ca Mon Sep 17 00:00:00 2001 From: Minh Pham Date: Sat, 11 Jul 2026 16:33:33 -0400 Subject: [PATCH 3/4] Limit node grouping to top-level graphs --- source/MaterialXCore/Interface.cpp | 14 +++-------- source/MaterialXGraphEditor/Graph.cpp | 5 +++- source/MaterialXTest/MaterialXCore/Node.cpp | 27 --------------------- 3 files changed, 7 insertions(+), 39 deletions(-) diff --git a/source/MaterialXCore/Interface.cpp b/source/MaterialXCore/Interface.cpp index a1ec967c96..e562fd94a0 100644 --- a/source/MaterialXCore/Interface.cpp +++ b/source/MaterialXCore/Interface.cpp @@ -149,16 +149,10 @@ bool PortElement::validate(string* message) const { bool res = true; - ConstElementPtr parent = getParent(); - ConstElementPtr scope = parent ? parent->getParent() : nullptr; NodePtr connectedNode = getConnectedNode(); if (hasNodeName() || hasOutputString()) { - NodeGraphPtr nodeGraph = resolveNameReference(getNodeName(), scope); - if (!nodeGraph) - { - nodeGraph = resolveNameReference(getNodeName()); - } + NodeGraphPtr nodeGraph = resolveNameReference(getNodeName()); if (!nodeGraph) { validateRequire(connectedNode != nullptr, res, message, "Invalid port connection"); @@ -182,7 +176,7 @@ bool PortElement::validate(string* message) const } else if (hasNodeGraphString()) { - NodeGraphPtr nodeGraph = resolveNameReference(getNodeGraphString(), scope); + NodeGraphPtr nodeGraph = resolveNameReference(getNodeGraphString()); if (!nodeGraph) { nodeGraph = resolveNameReference(getNodeGraphString()); @@ -233,9 +227,7 @@ NodePtr Input::getConnectedNode() const // Handle inputs of compound nodegraphs. if (getParent()->isA()) { - ConstElementPtr graphParent = getParent()->getParent(); - ConstGraphElementPtr parentGraph = graphParent ? graphParent->asA() : nullptr; - NodePtr rootNode = parentGraph ? parentGraph->getNode(getNodeName()) : nullptr; + NodePtr rootNode = getDocument()->getNode(getNodeName()); if (rootNode) { return rootNode; diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp index aab621965b..4a9645ace0 100644 --- a/source/MaterialXGraphEditor/Graph.cpp +++ b/source/MaterialXGraphEditor/Graph.cpp @@ -1995,7 +1995,10 @@ void Graph::groupSelectedNodesIntoNodeGraph(const std::vector& selec _popup = true; return; } - + if (_state.graphElem != _graphDoc) + { + return; + } std::vector nodesToGroup; if (!collectGroupableNodes(selectedNodes, nodesToGroup)) { diff --git a/source/MaterialXTest/MaterialXCore/Node.cpp b/source/MaterialXTest/MaterialXCore/Node.cpp index b851621f6a..6209f318f6 100644 --- a/source/MaterialXTest/MaterialXCore/Node.cpp +++ b/source/MaterialXTest/MaterialXCore/Node.cpp @@ -385,33 +385,6 @@ TEST_CASE("Topological sort", "[nodegraph]") REQUIRE(isTopologicalOrder(elemOrder)); } -TEST_CASE("Nested compound nodegraph connections", "[nodegraph]") -{ - mx::DocumentPtr doc = mx::createDocument(); - - mx::NodeGraphPtr parentGraph = doc->addNodeGraph("parent"); - mx::NodePtr upstream = parentGraph->addNode("constant", "upstream", "float"); - mx::NodeGraphPtr nestedGraph = parentGraph->addChild("nested"); - - mx::InputPtr nestedInput = nestedGraph->addInput("in", "float"); - nestedInput->setConnectedNode(upstream); - - mx::NodePtr internal = nestedGraph->addNode("constant", "internal", "float"); - mx::InputPtr internalInput = internal->addInput("value", "float"); - internalInput->setConnectedInterfaceName(nestedInput->getName()); - - mx::OutputPtr nestedOutput = nestedGraph->addOutput("out", "float"); - nestedOutput->setConnectedNode(internal); - - mx::NodePtr downstream = parentGraph->addNode("dot", "downstream", "float"); - mx::InputPtr downstreamInput = downstream->addInput("in", "float"); - downstreamInput->setConnectedOutput(nestedOutput); - - REQUIRE(nestedInput->getConnectedNode() == upstream); - REQUIRE(downstreamInput->getConnectedOutput() == nestedOutput); - REQUIRE(downstreamInput->getConnectedNode() == internal); - REQUIRE(doc->validate()); -} TEST_CASE("New nodegraph from output", "[nodegraph]") { From c66a3fa288c9780276628aad2db885b423e7035f Mon Sep 17 00:00:00 2001 From: Minh Pham Date: Thu, 23 Jul 2026 20:55:31 -0400 Subject: [PATCH 4/4] Move grouping logic into core + add tests --- source/MaterialXCore/Interface.cpp | 4 - source/MaterialXCore/Node.cpp | 213 ++++++++++++++++++++ source/MaterialXCore/Node.h | 13 ++ source/MaterialXGraphEditor/Graph.cpp | 188 +---------------- source/MaterialXTest/MaterialXCore/Node.cpp | 63 ++++++ 5 files changed, 294 insertions(+), 187 deletions(-) diff --git a/source/MaterialXCore/Interface.cpp b/source/MaterialXCore/Interface.cpp index e562fd94a0..e9e7bd7d80 100644 --- a/source/MaterialXCore/Interface.cpp +++ b/source/MaterialXCore/Interface.cpp @@ -177,10 +177,6 @@ bool PortElement::validate(string* message) const else if (hasNodeGraphString()) { NodeGraphPtr nodeGraph = resolveNameReference(getNodeGraphString()); - if (!nodeGraph) - { - nodeGraph = resolveNameReference(getNodeGraphString()); - } if (nodeGraph) { output = nodeGraph->getOutput(outputString); diff --git a/source/MaterialXCore/Node.cpp b/source/MaterialXCore/Node.cpp index f5923f337c..dcc2c26f85 100644 --- a/source/MaterialXCore/Node.cpp +++ b/source/MaterialXCore/Node.cpp @@ -9,9 +9,183 @@ #include #include +#include +#include MATERIALX_NAMESPACE_BEGIN +namespace +{ + +void clearPortConnection(PortElementPtr port) +{ + if (!port) + { + return; + } + port->removeAttribute(PortElement::NODE_NAME_ATTRIBUTE); + port->removeAttribute(PortElement::NODE_GRAPH_ATTRIBUTE); + port->removeAttribute(PortElement::OUTPUT_ATTRIBUTE); + port->removeAttribute(ValueElement::INTERFACE_NAME_ATTRIBUTE); +} + +void copyConnectionAttributes(PortElementPtr source, PortElementPtr dest) +{ + if (!source || !dest) + { + return; + } + + clearPortConnection(dest); + if (source->hasNodeName()) + { + dest->setNodeName(source->getNodeName()); + } + if (source->hasNodeGraphString()) + { + dest->setNodeGraphString(source->getNodeGraphString()); + } + if (source->hasOutputString()) + { + dest->setOutputString(source->getOutputString()); + } + if (source->hasInterfaceName()) + { + dest->setInterfaceName(source->getInterfaceName()); + } + dest->removeAttribute(ValueElement::VALUE_ATTRIBUTE); +} + +bool inputConnectsOutsideSelection(InputPtr input, const std::unordered_set& selectedNodeNames) +{ + if (!input) + { + return false; + } + if (input->hasNodeName()) + { + return selectedNodeNames.find(input->getNodeName()) == selectedNodeNames.end(); + } + return input->hasNodeGraphString() || input->hasInterfaceName() || input->hasOutputString(); +} + +void preserveExternalUpstreamConnections(NodeGraphPtr nodeGraph, const std::unordered_set& selectedNodeNames) +{ + if (!nodeGraph) + { + return; + } + + for (NodePtr copiedNode : nodeGraph->getNodes()) + { + for (InputPtr input : copiedNode->getInputs()) + { + if (!inputConnectsOutsideSelection(input, selectedNodeNames)) + { + continue; + } + + string interfaceName = nodeGraph->createValidChildName(copiedNode->getName() + "_" + input->getName()); + InputPtr interfaceInput = nodeGraph->addInput(interfaceName, input->getType()); + copyConnectionAttributes(input, interfaceInput); + input->setConnectedInterfaceName(interfaceInput->getName()); + } + } +} + +string getBoundaryOutputType(NodePtr sourceNode, PortElementPtr downstreamPort) +{ + if (downstreamPort && !downstreamPort->getType().empty()) + { + return downstreamPort->getType(); + } + if (sourceNode) + { + const string& outputName = downstreamPort ? downstreamPort->getOutputString() : EMPTY_STRING; + if (!outputName.empty()) + { + OutputPtr output = sourceNode->getOutput(outputName); + if (!output && sourceNode->getNodeDef()) + { + output = sourceNode->getNodeDef()->getOutput(outputName); + } + if (output && !output->getType().empty()) + { + return output->getType(); + } + } + return sourceNode->getType(); + } + return DEFAULT_TYPE_STRING; +} + +OutputPtr getOrCreateBoundaryOutput(NodeGraphPtr nodeGraph, + NodePtr sourceNode, + PortElementPtr downstreamPort, + std::map& boundaryOutputs) +{ + const string& sourceOutput = downstreamPort ? downstreamPort->getOutputString() : EMPTY_STRING; + string key = sourceNode->getName() + "|" + sourceOutput; + auto existing = boundaryOutputs.find(key); + if (existing != boundaryOutputs.end()) + { + return existing->second; + } + + string outputBaseName = sourceNode->getName() + (sourceOutput.empty() ? "_out" : "_" + sourceOutput); + OutputPtr boundaryOutput = nodeGraph->addOutput(nodeGraph->createValidChildName(outputBaseName), + getBoundaryOutputType(sourceNode, downstreamPort)); + boundaryOutput->setNodeName(sourceNode->getName()); + if (!sourceOutput.empty()) + { + boundaryOutput->setOutputString(sourceOutput); + } + boundaryOutput->removeAttribute(ValueElement::VALUE_ATTRIBUTE); + boundaryOutputs[key] = boundaryOutput; + return boundaryOutput; +} + +void preserveExternalDownstreamConnections(NodeGraphPtr nodeGraph, + const vector& nodesToGroup, + const std::unordered_set& selectedNodeNames) +{ + if (!nodeGraph) + { + return; + } + + std::map boundaryOutputs; + for (NodePtr sourceNode : nodesToGroup) + { + if (!sourceNode) + { + continue; + } + for (PortElementPtr downstreamPort : sourceNode->getDownstreamPorts()) + { + if (!downstreamPort) + { + continue; + } + + ElementPtr parent = downstreamPort->getParent(); + if (parent && selectedNodeNames.find(parent->getName()) != selectedNodeNames.end()) + { + continue; + } + + OutputPtr boundaryOutput = getOrCreateBoundaryOutput(nodeGraph, sourceNode, downstreamPort, boundaryOutputs); + downstreamPort->setNodeGraphString(nodeGraph->getName()); + downstreamPort->setOutputString(boundaryOutput->getName()); + downstreamPort->removeAttribute(PortElement::NODE_NAME_ATTRIBUTE); + downstreamPort->removeAttribute(ValueElement::INTERFACE_NAME_ATTRIBUTE); + downstreamPort->removeAttribute(ValueElement::VALUE_ATTRIBUTE); + } + } +} + +} // anonymous namespace + const string Backdrop::CONTAINS_ATTRIBUTE = "contains"; const string Backdrop::WIDTH_ATTRIBUTE = "width"; const string Backdrop::HEIGHT_ATTRIBUTE = "height"; @@ -403,6 +577,45 @@ void GraphElement::flattenSubgraphs(const string& target, NodePredicate filter) } } +NodeGraphPtr GraphElement::createNodeGraphFromNodes(const vector& nodes, const string& name) +{ + // Filter to valid direct-child nodes and collect their names. + std::unordered_set selectedNodeNames; + vector validNodes; + for (const NodePtr& node : nodes) + { + if (node && node->getParent() == getSelf() && selectedNodeNames.insert(node->getName()).second) + { + validNodes.push_back(node); + } + } + if (validNodes.empty()) + { + return nullptr; + } + + // Create the nodegraph and copy the selected nodes into it. + NodeGraphPtr nodeGraph = addChild(name); + for (const NodePtr& sourceNode : validNodes) + { + NodePtr copiedNode = nodeGraph->addNode(sourceNode->getCategory(), sourceNode->getName(), sourceNode->getType()); + copiedNode->copyContentFrom(sourceNode); + } + + // Preserve boundary connections. Downstream preservation reads the original + // nodes' downstream ports, so it must run before the originals are removed. + preserveExternalUpstreamConnections(nodeGraph, selectedNodeNames); + preserveExternalDownstreamConnections(nodeGraph, validNodes, selectedNodeNames); + + // Remove the original nodes from this graph element. + for (const string& nodeName : selectedNodeNames) + { + removeChild(nodeName); + } + + return nodeGraph; +} + ElementVec GraphElement::topologicalSort() const { // Calculate a topological order of the children, using Kahn's algorithm diff --git a/source/MaterialXCore/Node.h b/source/MaterialXCore/Node.h index d6700b45fb..d30fe69e9a 100644 --- a/source/MaterialXCore/Node.h +++ b/source/MaterialXCore/Node.h @@ -307,6 +307,19 @@ class MX_CORE_API GraphElement : public InterfaceElement /// should be included and excluded from this process. void flattenSubgraphs(const string& target = EMPTY_STRING, NodePredicate filter = nullptr); + /// Create a new child NodeGraph containing the given nodes, moving them out + /// of this graph element. Connections between grouped nodes are preserved; + /// connections from outside the group into a grouped node become nodegraph + /// interface inputs, and consumers outside the group are rewired to + /// nodegraph outputs. + /// @param nodes The nodes to group. Entries that are null or are not direct + /// children of this graph element are ignored. + /// @param name An optional name for the new NodeGraph. + /// @return A shared pointer to the new NodeGraph, or nullptr if no valid + /// nodes were provided. + NodeGraphPtr createNodeGraphFromNodes(const vector& nodes, + const string& name = EMPTY_STRING); + /// Return a vector of all children (nodes and outputs) sorted in /// topological order. ElementVec topologicalSort() const; diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp index 4a9645ace0..0aeed56634 100644 --- a/source/MaterialXGraphEditor/Graph.cpp +++ b/source/MaterialXGraphEditor/Graph.cpp @@ -106,174 +106,6 @@ std::string getUserNodeDefName(const std::string& val) return result; } -void clearPortConnection(mx::PortElementPtr port) -{ - if (!port) - { - return; - } - port->removeAttribute(mx::PortElement::NODE_NAME_ATTRIBUTE); - port->removeAttribute(mx::PortElement::NODE_GRAPH_ATTRIBUTE); - port->removeAttribute(mx::PortElement::OUTPUT_ATTRIBUTE); - port->removeAttribute(mx::ValueElement::INTERFACE_NAME_ATTRIBUTE); -} - -void copyConnectionAttributes(mx::PortElementPtr source, mx::PortElementPtr dest) -{ - if (!source || !dest) - { - return; - } - - clearPortConnection(dest); - if (source->hasNodeName()) - { - dest->setNodeName(source->getNodeName()); - } - if (source->hasNodeGraphString()) - { - dest->setNodeGraphString(source->getNodeGraphString()); - } - if (source->hasOutputString()) - { - dest->setOutputString(source->getOutputString()); - } - if (source->hasInterfaceName()) - { - dest->setInterfaceName(source->getInterfaceName()); - } - dest->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE); -} - -bool inputConnectsOutsideSelection(mx::InputPtr input, const std::unordered_set& selectedNodeNames) -{ - if (!input) - { - return false; - } - if (input->hasNodeName()) - { - return selectedNodeNames.find(input->getNodeName()) == selectedNodeNames.end(); - } - return input->hasNodeGraphString() || input->hasInterfaceName() || input->hasOutputString(); -} - -void preserveExternalUpstreamConnections(mx::NodeGraphPtr nodeGraph, const std::unordered_set& selectedNodeNames) -{ - if (!nodeGraph) - { - return; - } - - for (mx::NodePtr copiedNode : nodeGraph->getNodes()) - { - for (mx::InputPtr input : copiedNode->getInputs()) - { - if (!inputConnectsOutsideSelection(input, selectedNodeNames)) - { - continue; - } - - std::string interfaceName = nodeGraph->createValidChildName(copiedNode->getName() + "_" + input->getName()); - mx::InputPtr interfaceInput = nodeGraph->addInput(interfaceName, input->getType()); - copyConnectionAttributes(input, interfaceInput); - input->setConnectedInterfaceName(interfaceInput->getName()); - } - } -} - -std::string getBoundaryOutputType(mx::NodePtr sourceNode, mx::PortElementPtr downstreamPort) -{ - if (downstreamPort && !downstreamPort->getType().empty()) - { - return downstreamPort->getType(); - } - if (sourceNode) - { - const std::string& outputName = downstreamPort ? downstreamPort->getOutputString() : mx::EMPTY_STRING; - if (!outputName.empty()) - { - mx::OutputPtr output = sourceNode->getOutput(outputName); - if (!output && sourceNode->getNodeDef()) - { - output = sourceNode->getNodeDef()->getOutput(outputName); - } - if (output && !output->getType().empty()) - { - return output->getType(); - } - } - return sourceNode->getType(); - } - return mx::DEFAULT_TYPE_STRING; -} - -mx::OutputPtr getOrCreateBoundaryOutput(mx::NodeGraphPtr nodeGraph, - mx::NodePtr sourceNode, - mx::PortElementPtr downstreamPort, - std::map& boundaryOutputs) -{ - const std::string& sourceOutput = downstreamPort ? downstreamPort->getOutputString() : mx::EMPTY_STRING; - std::string key = sourceNode->getName() + "|" + sourceOutput; - auto existing = boundaryOutputs.find(key); - if (existing != boundaryOutputs.end()) - { - return existing->second; - } - - std::string outputBaseName = sourceNode->getName() + (sourceOutput.empty() ? "_out" : "_" + sourceOutput); - mx::OutputPtr boundaryOutput = nodeGraph->addOutput(nodeGraph->createValidChildName(outputBaseName), - getBoundaryOutputType(sourceNode, downstreamPort)); - boundaryOutput->setNodeName(sourceNode->getName()); - if (!sourceOutput.empty()) - { - boundaryOutput->setOutputString(sourceOutput); - } - boundaryOutput->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE); - boundaryOutputs[key] = boundaryOutput; - return boundaryOutput; -} - -void preserveExternalDownstreamConnections(mx::NodeGraphPtr nodeGraph, - const std::vector& nodesToGroup, - const std::unordered_set& selectedNodeNames) -{ - if (!nodeGraph) - { - return; - } - - std::map boundaryOutputs; - for (UiNodePtr uiNode : nodesToGroup) - { - mx::NodePtr sourceNode = uiNode ? uiNode->getNode() : nullptr; - if (!sourceNode) - { - continue; - } - for (mx::PortElementPtr downstreamPort : sourceNode->getDownstreamPorts()) - { - if (!downstreamPort) - { - continue; - } - - mx::ElementPtr parent = downstreamPort->getParent(); - if (parent && selectedNodeNames.find(parent->getName()) != selectedNodeNames.end()) - { - continue; - } - - mx::OutputPtr boundaryOutput = getOrCreateBoundaryOutput(nodeGraph, sourceNode, downstreamPort, boundaryOutputs); - downstreamPort->setNodeGraphString(nodeGraph->getName()); - downstreamPort->setOutputString(boundaryOutput->getName()); - downstreamPort->removeAttribute(mx::PortElement::NODE_NAME_ATTRIBUTE); - downstreamPort->removeAttribute(mx::ValueElement::INTERFACE_NAME_ATTRIBUTE); - downstreamPort->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE); - } - } -} - static void EnableSRGBCallback(const ImDrawList*, const ImDrawCmd*) { glEnable(GL_FRAMEBUFFER_SRGB); @@ -2007,30 +1839,20 @@ void Graph::groupSelectedNodesIntoNodeGraph(const std::vector& selec savePosition(); - std::unordered_set selectedNodeNames; + std::vector nodes; ImVec2 totalPosition(0.0f, 0.0f); for (UiNodePtr uiNode : nodesToGroup) { - selectedNodeNames.insert(uiNode->getName()); + nodes.push_back(uiNode->getNode()); ImVec2 nodePosition = ed::GetNodePosition(uiNode->getId()); totalPosition.x += nodePosition.x; totalPosition.y += nodePosition.y; } - mx::NodeGraphPtr nodeGraph = _state.graphElem->addChild(); - for (UiNodePtr uiNode : nodesToGroup) - { - mx::NodePtr sourceNode = uiNode->getNode(); - mx::NodePtr copiedNode = nodeGraph->addNode(sourceNode->getCategory(), sourceNode->getName(), sourceNode->getType()); - copiedNode->copyContentFrom(sourceNode); - } - - preserveExternalUpstreamConnections(nodeGraph, selectedNodeNames); - preserveExternalDownstreamConnections(nodeGraph, nodesToGroup, selectedNodeNames); - - for (const std::string& nodeName : selectedNodeNames) + mx::NodeGraphPtr nodeGraph = _state.graphElem->createNodeGraphFromNodes(nodes); + if (!nodeGraph) { - _state.graphElem->removeChild(nodeName); + return; } ImVec2 averagePosition(totalPosition.x / float(nodesToGroup.size()), totalPosition.y / float(nodesToGroup.size())); diff --git a/source/MaterialXTest/MaterialXCore/Node.cpp b/source/MaterialXTest/MaterialXCore/Node.cpp index 6209f318f6..875bea2ae9 100644 --- a/source/MaterialXTest/MaterialXCore/Node.cpp +++ b/source/MaterialXTest/MaterialXCore/Node.cpp @@ -961,3 +961,66 @@ TEST_CASE("Set Name Global", "[node, nodegraph]") } } } + +TEST_CASE("Group nodes into nodegraph", "[nodegraph]") +{ + mx::DocumentPtr doc = mx::createDocument(); + + // External upstream source (not grouped). + mx::NodePtr src = doc->addNode("constant", "src", "float"); + + // Nodes to group: a -> b (internal link). + mx::NodePtr a = doc->addNode("add", "a", "float"); + mx::InputPtr aIn = a->addInput("in1", "float"); + aIn->setConnectedNode(src); // external upstream into the group + + mx::NodePtr b = doc->addNode("multiply", "b", "float"); + mx::InputPtr bIn = b->addInput("in1", "float"); + bIn->setConnectedNode(a); // internal link within the group + + // Two external downstream consumers of b (not grouped). + mx::NodePtr c1 = doc->addNode("add", "c1", "float"); + mx::InputPtr c1In = c1->addInput("in1", "float"); + c1In->setConnectedNode(b); + mx::NodePtr c2 = doc->addNode("add", "c2", "float"); + mx::InputPtr c2In = c2->addInput("in1", "float"); + c2In->setConnectedNode(b); + + mx::NodeGraphPtr ng = doc->createNodeGraphFromNodes({ a, b }); + REQUIRE(ng != nullptr); + + // Originals moved out of the document, copies present in the nodegraph. + REQUIRE(!doc->getNode("a")); + REQUIRE(!doc->getNode("b")); + REQUIRE(ng->getNode("a")); + REQUIRE(ng->getNode("b")); + + // Internal link preserved inside the nodegraph. + REQUIRE(ng->getNode("b")->getInput("in1")->getConnectedNode() == ng->getNode("a")); + + // Upstream boundary: a's input now references an interface input that + // connects to the external source node. + mx::InputPtr aCopyIn = ng->getNode("a")->getInput("in1"); + REQUIRE(!aCopyIn->getInterfaceName().empty()); + REQUIRE(!aCopyIn->hasNodeName()); + mx::InputPtr iface = ng->getInput(aCopyIn->getInterfaceName()); + REQUIRE(iface != nullptr); + REQUIRE(iface->getNodeName() == "src"); + + // Downstream boundary: both consumers rewired to the nodegraph, sharing a + // single boundary output (fan-out dedup). + REQUIRE(c1In->getNodeGraphString() == ng->getName()); + REQUIRE(c2In->getNodeGraphString() == ng->getName()); + REQUIRE(!c1In->hasNodeName()); + REQUIRE(!c1In->getOutputString().empty()); + REQUIRE(c1In->getOutputString() == c2In->getOutputString()); + REQUIRE(ng->getOutput(c1In->getOutputString()) != nullptr); + + REQUIRE(doc->validate()); +} + +TEST_CASE("Group nodes with empty selection", "[nodegraph]") +{ + mx::DocumentPtr doc = mx::createDocument(); + REQUIRE(doc->createNodeGraphFromNodes({}) == nullptr); +}