Skip to content

Add hotkey to group nodes into a node graph#3002

Open
minhpham1810 wants to merge 5 commits into
AcademySoftwareFoundation:mainfrom
minhpham1810:issue-2018-group-nodes-nodegraph
Open

Add hotkey to group nodes into a node graph#3002
minhpham1810 wants to merge 5 commits into
AcademySoftwareFoundation:mainfrom
minhpham1810:issue-2018-group-nodes-nodegraph

Conversation

@minhpham1810

@minhpham1810 minhpham1810 commented Jul 8, 2026

Copy link
Copy Markdown

Summary

Adds the MaterialX Graph Editor Shift+C node-grouping shortcut so it actually triggers when graph nodes are selected.

When graph nodes are selected, the shortcut creates a new nodegraph, moves the selected nodes into it, and preserves relevant boundary connections so the resulting nodegraph remains usable without manually reconnecting links.

Changes

  • Add Shift+C handling for grouping selected Graph Editor nodes into a nodegraph.
  • Create a new nodegraph from the current selection.
  • Preserve internal links between grouped nodes.
  • Preserve external upstream links through nodegraph interface inputs.
  • Preserve external downstream links through nodegraph outputs.
  • Keep copy/cut/paste handling in the existing node editor shortcut path.
  • Add a small testable shortcut predicate for grouping shortcut conditions.
  • Add MaterialXTest coverage for the grouping shortcut state.

Scope Note

This PR currently includes support for grouping within nested nodegraphs, which required changes in MaterialXCore connection resolution. Based on maintainer feedback, I’m happy to narrow this PR to top-level document grouping only and move nested nodegraph support into a follow-up PR. (This feature is currently limited, now nodes can only be grouped at the top level of the document.)

Testing

  • git diff --check
  • MaterialXTest "Graph editor grouping shortcut"
  • cmake --build build --target MaterialXGraphEditor

Related Issue

Fixes #2018

@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 8, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

@minhpham1810 minhpham1810 changed the title Issue 2018 group nodes into nodegraph hotkey Add hot key to group nodes into a node graph Jul 8, 2026
@minhpham1810 minhpham1810 changed the title Add hot key to group nodes into a node graph Add hotkey to group nodes into a node graph Jul 8, 2026
@minhpham1810 minhpham1810 marked this pull request as draft July 9, 2026 00:16
@minhpham1810 minhpham1810 marked this pull request as ready for review July 9, 2026 00:17
@jstone-lucasfilm

Copy link
Copy Markdown
Member

This looks very promising, thanks @minhpham1810, and take a look at the comment above for instructions on resolving the EasyCLA errors.

@minhpham1810 minhpham1810 force-pushed the issue-2018-group-nodes-nodegraph branch from 7a4a73e to 027125f Compare July 9, 2026 03:55
@minhpham1810 minhpham1810 marked this pull request as draft July 9, 2026 03:59
@minhpham1810 minhpham1810 force-pushed the issue-2018-group-nodes-nodegraph branch from 027125f to c344edb Compare July 9, 2026 04:06
@minhpham1810 minhpham1810 marked this pull request as ready for review July 9, 2026 04:11
@minhpham1810

Copy link
Copy Markdown
Author

@jstone-lucasfilm I just signed the CLA and changed my commit email, please have a look.

@jstone-lucasfilm

Copy link
Copy Markdown
Member

Thanks for the excellent work here, @minhpham1810. The connection-preservation logic is more thorough than I'd expected, and the hard parts are done well.

This PR has grown into something larger than the summary suggests, though, so before we dive into code review I'd like to align on scope. Two recommended next steps would be:

  1. Update the PR description to reflect the full change. It currently reads as a Shift+C shortcut fix, but it implements the whole grouping feature along with some MaterialXCore changes.
  2. The key decision is nested nodegraphs. When this was first scoped, @kwokcb suggested the editor guard against nesting; this PR adds support for it instead, which is what motivates the Interface.cpp changes. I'd suggest we start with grouping at the top document level only, which would cover the original use case, keep us on well-supported ground, and require no MaterialXCore changes. We can then take up nested nodegraphs (and the resolution work they require) as a deliberate follow-up.

Once we've settled the scope, I'm glad to go deep on the grouping logic itself. Thanks again for the strong contribution!

@minhpham1810

Copy link
Copy Markdown
Author

@jstone-lucasfilm Thank you so much for the detailed feedback! I admitted that I want a bit out of scope in this PR, so I reverted the changes I made in MaterialXCore, as well as editing the safeguard for top-level document node grouping. Please let me know if there is any other parts that I should look at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Graph Editor: Group nodes to create node graph

2 participants