Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node editor traversal #2225

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kwokcb
Copy link
Contributor

@kwokcb kwokcb commented Feb 12, 2025

Proposal

  • Add in clickable links to traverse through pin connections (inputs / outputs)

  • This gives an easy way to move through a graph and also provide information on what is being routed in and out of a node

  • Useful for diagnosis (see bugs section below)

  • Options: Allow for output connections to be displayed with a per node toggle.

  • Did not add hotkeys as selection of what to traverse to up / downstream is ambiguous for multiple input / output connections on a node. Can consider later and hook to keys like left/right arrow for instance.

  • Aside: Add in background graph color option as it's pretty hard to see selected outlines, and better contrast with graph elements, grid etc. (--graphBackground "r,g,b")

Details

  • Currently pin connections show the type which gives no information as to what is connected to it. You can hover over the pins in the graph but this just gives you pin names w/o node names
  • Instead add in clickable buttons to traverse between pins.
  • Traversal includes selecting the centering around selected node
  • There are some existing bugs found in the code:
    • Deletion does not refresh connections properly
    • Inputs to graphs are not properly updating.
  • Prevent a possible crash when deleting. Didn't fix the issue
  • Fix positioning of "Show all inputs" for nodegraphs. (missed from a previous submission)

Results

mx_editor_traversal.mp4
  1. Fan-out from an output:
    image

  2. Upstream connections:
    image

  3. Hide output connections:
    image


Pinging @lfl-eholthouser and @jstone-lucasfilm for comments.

@jstone-lucasfilm
Copy link
Member

@kwokcb Like so many folks on the MaterialX Slack, I'm really struck by the proposed color change, and I wonder if you could separate this out into its own PR, so that we can evaluate these two distinct proposals separately.

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.

2 participants