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

Update API and Traversal to Support Nested NodeGraphs #1790

Closed

Conversation

kwokcb
Copy link
Contributor

@kwokcb kwokcb commented Apr 24, 2024

Issue

Even though the 1.39 specification notes that nodegraphs may be nested the current API and traversal mechanism implementation does not reflect this.

Changes

  • Move the nodegraph parenting API from Document to GraphElement thus allow for parenting from the derived class NodeGraph
  • Update traversal and connection logic to make connections to nested nodegraphs be supported.
  • Updated API. Should be no API compatibility issues as the interfaces are still available at Document level, but are now available at NodeGraph level.

Tests

  • New nested nodegraph tests added to unit test suite.
graph BT
    subgraph parentNG
    style parentNG_parentNGInput  fill:#09D, color:#FFF
    parentNG_parentNGInput([parentNGInput:1, 1, 0.2])
    style parentNG_parentNGInput2  fill:#09D, color:#FFF
    parentNG_parentNGInput2([parentNGInput2:0.2, 1, 1])
    style parentNG_parentNGInput3  fill:#09D, color:#FFF
    parentNG_parentNGInput3([parentNGInput3:1, 0.2, 1])
    style parentNG_parentNGOutput1  fill:#0C0, color:#FFF
    parentNG_parentNGOutput1([parentNGOutput1])
    style parentNG_parentNGOutput2  fill:#0C0, color:#FFF
    parentNG_parentNGOutput2([parentNGOutput2])
    parentNG_parentmultiplyNode[parentmultiplyNode]
    
    subgraph parentNG_childNG
    style parentNG_childNG_childNGInput  fill:#09D, color:#FFF
    parentNG_childNG_childNGInput([childNGInput])
    style parentNG_childNG_childNGInput2  fill:#09D, color:#FFF
    parentNG_childNG_childNGInput2([childNGInput2])
    parentNG_childNG_multiplyNode[multiplyNode]
    parentNG_childNG_childNGOutput[childNGOutput]
    end
    
    end

nestd_graph_shader[nestd_graph_shader]
    style nested_graph_material  fill:#090, color:#FFF
    nested_graph_material([nested_graph_material])
    nestd_graph_shader2[nestd_graph_shader2]
    style nested_graph_material2  fill:#090, color:#FFF
    nested_graph_material2([nested_graph_material2])
    
    parentNG_parentNGInput --> parentNG_childNG_childNGInput
    parentNG_parentNGInput2 --> parentNG_childNG_childNGInput2
    parentNG_childNG_childNGInput --"in1"--> parentNG_childNG_multiplyNode
    parentNG_childNG_childNGInput2 --"in2"--> parentNG_childNG_multiplyNode
    parentNG_childNG_multiplyNode --> parentNG_childNG_childNGOutput
    parentNG_childNG_childNGOutput--"in1"--> parentNG_parentmultiplyNode
    parentNG_parentNGInput3 --"in2"--> parentNG_parentmultiplyNode
    parentNG_childNG_childNGOutput--> parentNG_parentNGOutput1
    parentNG_parentmultiplyNode --> parentNG_parentNGOutput2
    parentNG_parentNGOutput1 --"base_color"--> nestd_graph_shader
    nestd_graph_shader --"surfaceshader"--> nested_graph_material
    parentNG_parentNGOutput2 --"base_color"--> nestd_graph_shader2
    nestd_graph_shader2 --"surfaceshader"--> nested_graph_material2
Loading
  • Ran RTS with GLSL and OSL locally.

@jstone-lucasfilm
Copy link
Member

@kwokcb We've now merged development work on MaterialX 1.39 back to the main branch of MaterialX, in preparation for the final weeks of development on the 1.39.0 release. When you have a chance, could you retarget this pull request back to the main branch as well?

@jstone-lucasfilm
Copy link
Member

@kwokcb If you'd like to keep this proposal moving forward, make sure to retarget it to to the main branch, and resolve any merge conflicts that may be present!

@jstone-lucasfilm
Copy link
Member

@kwokcb I'll close out this pull request for now, since it's framed in terms of a branch that we're planning to delete, but feel free to propose a similar pull request for the main branch in the future!

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.

None yet

2 participants