Add Functional NodeDef Support #2401

kwokcb · 2025-05-21T20:13:20Z

Implementation

This are the basic changes to handle the current "functional nodedef" proposal: #2355 as an
new way to form associations between a definition and it's nodegraph implementation.
This new variant is purely additive for the data model and code logic and API interface and internal logic.

Modifications.

Update node definition dictionary in the document cache to consider nodedefs with child nodegraphs (implementations). Non-child implementations have higher precedence.
Update definition query for implementations / nodegraphs to check for parent nodedef first.
Add DefintionOptions options class to which contains the option to add as child or not. Default is false. Additional arguments can be added here vs a list of args in the previous interface.
Update C++ Document::addNodeDefFromGraph() interface to take DefinitionsOptions as a new optional argument.
Add in interface to check for shared implementations in an inheritance hierarchy:
- NodeDef::getMatchingDefinitions() and NodeDef::hasSharedImplementation().
Add in conversion utility to "inline" implementations. General enough to handle any implementation but has a firewall check for now to only allow "functional nodegraphs". Very easy to remove this to accept any implementation variant.
- NodeDef::inlineImplementation()
Add in Python and Javascript wrappers.

Tests

Add a functional_nodedef.mtlx test used for unit and codegen / render tests.
Update unit tests to run definition creation test for both sibling (current) and child (new) options as defined by DefinitionOptions.
Update Javascript unit tests. Expected:
✔ Build document
✔ Create NodeDef from NodeGraph with child implementation
✔ Create NodeDef from NodeGraph with referencing implementation
Add unit tests for inlining, looking for shared implementations and inheritance queries.
Manual testing in graph editor.
- Value modifications
- Show definition

Unit Test Results

Definition Created with Sibling Reference

Details

<?xml version="1.0"?>
<materialx version="1.39">
 <nodegraph name="test_colorcorrect">
   <multiply name="AlphaGain" type="float">
     <input name="in1" type="float" nodename="inputAlpha" />
     <input name="in2" type="float" interfacename="AlphaGain_in2" />
   </multiply>
   <add name="AlphaOffset" type="float">
     <input name="in1" type="float" nodename="AlphaGain" />
     <input name="in2" type="float" interfacename="AlphaOffset_in2" />
   </add>
   <multiply name="ColorGain" type="color3">
     <input name="in1" type="color3" nodename="inputColor" />
     <input name="in2" type="color3" interfacename="ColorGain_in2" />
   </multiply>
   <add name="ColorOffset" type="color3">
     <input name="in1" type="color3" nodename="ColorGain" />
     <input name="in2" type="color3" interfacename="ColorOffset_in2" />
   </add>
   <constant name="inputColor" type="color3">
     <input name="value" type="color3" interfacename="inputColor_value" />
   </constant>
   <constant name="inputAlpha" type="float">
     <input name="value" type="float" interfacename="inputAlpha_value" />
   </constant>
   <output name="out" type="color3" nodename="ColorOffset" />
   <output name="out1" type="float" nodename="AlphaOffset" />
   <input name="AlphaGain_in2" type="float" value="0.8" uiname="AlphaGain in2" uifolder="Common" />
   <input name="AlphaOffset_in2" type="float" value="1" uiname="AlphaOffset in2" uifolder="Common" />
   <input name="ColorGain_in2" type="color3" value="0.9, 0.9, 0.9" uiname="ColorGain in2" uifolder="Common" />
   <input name="ColorOffset_in2" type="color3" value="0.379147, 0.0341412, 0.0341412" uiname="ColorOffset in2" uifolder="Common" />
   <input name="inputColor_value" type="color3" value="0.5, 0.5, 0.5" uiname="inputColor value" uifolder="Common" />
   <input name="inputAlpha_value" type="float" value="1" uiname="inputAlpha value" uifolder="Common" />
 </nodegraph>
 <nodedef name="ND_test_colorcorrect" node="test_colorcorrect" version="1.0" isdefaultversion="false" nodegroup="adjustment" uiname="test_colorcorrect Version: 1.0" doc="This is version 1 of the definition for the graph: NG_test_colorcorrect">
   <input name="AlphaGain_in2" type="float" value="0.8" uiname="AlphaGain in2" uifolder="Common" />
   <input name="AlphaOffset_in2" type="float" value="1" uiname="AlphaOffset in2" uifolder="Common" />
   <input name="ColorGain_in2" type="color3" value="0.9, 0.9, 0.9" uiname="ColorGain in2" uifolder="Common" />
   <input name="ColorOffset_in2" type="color3" value="0.379147, 0.0341412, 0.0341412" uiname="ColorOffset in2" uifolder="Common" />
   <input name="inputColor_value" type="color3" value="0.5, 0.5, 0.5" uiname="inputColor value" uifolder="Common" />
   <input name="inputAlpha_value" type="float" value="1" uiname="inputAlpha value" uifolder="Common" />
   <output name="out" type="color3" />
   <output name="out1" type="float" />
 </nodedef>
 <nodegraph name="NG_test_colorcorrect" nodedef="ND_test_colorcorrect">
   <multiply name="AlphaGain" type="float">
     <input name="in1" type="float" nodename="inputAlpha" />
     <input name="in2" type="float" interfacename="AlphaGain_in2" />
   </multiply>
   <add name="AlphaOffset" type="float">
     <input name="in1" type="float" nodename="AlphaGain" />
     <input name="in2" type="float" interfacename="AlphaOffset_in2" />
   </add>
   <multiply name="ColorGain" type="color3">
     <input name="in1" type="color3" nodename="inputColor" />
     <input name="in2" type="color3" interfacename="ColorGain_in2" />
   </multiply>
   <add name="ColorOffset" type="color3">
     <input name="in1" type="color3" nodename="ColorGain" />
     <input name="in2" type="color3" interfacename="ColorOffset_in2" />
   </add>
   <constant name="inputColor" type="color3">
     <input name="value" type="color3" interfacename="inputColor_value" />
   </constant>
   <constant name="inputAlpha" type="float">
     <input name="value" type="float" interfacename="inputAlpha_value" />
   </constant>
   <output name="out" type="color3" nodename="ColorOffset" />
   <output name="out1" type="float" nodename="AlphaOffset" />
 </nodegraph>
</materialx>

Definition Created with Child

Details

<?xml version="1.0"?>
<materialx version="1.39">
  <nodegraph name="test_colorcorrect">
    <multiply name="AlphaGain" type="float">
      <input name="in1" type="float" nodename="inputAlpha" />
      <input name="in2" type="float" interfacename="AlphaGain_in2" />
    </multiply>
    <add name="AlphaOffset" type="float">
      <input name="in1" type="float" nodename="AlphaGain" />
      <input name="in2" type="float" interfacename="AlphaOffset_in2" />
    </add>
    <multiply name="ColorGain" type="color3">
      <input name="in1" type="color3" nodename="inputColor" />
      <input name="in2" type="color3" interfacename="ColorGain_in2" />
    </multiply>
    <add name="ColorOffset" type="color3">
      <input name="in1" type="color3" nodename="ColorGain" />
      <input name="in2" type="color3" interfacename="ColorOffset_in2" />
    </add>
    <constant name="inputColor" type="color3">
      <input name="value" type="color3" interfacename="inputColor_value" />
    </constant>
    <constant name="inputAlpha" type="float">
      <input name="value" type="float" interfacename="inputAlpha_value" />
    </constant>
    <output name="out" type="color3" nodename="ColorOffset" />
    <output name="out1" type="float" nodename="AlphaOffset" />
    <input name="AlphaGain_in2" type="float" value="0.8" uiname="AlphaGain in2" uifolder="Common" />
    <input name="AlphaOffset_in2" type="float" value="1" uiname="AlphaOffset in2" uifolder="Common" />
    <input name="ColorGain_in2" type="color3" value="0.9, 0.9, 0.9" uiname="ColorGain in2" uifolder="Common" />
    <input name="ColorOffset_in2" type="color3" value="0.379147, 0.0341412, 0.0341412" uiname="ColorOffset in2" uifolder="Common" />
    <input name="inputColor_value" type="color3" value="0.5, 0.5, 0.5" uiname="inputColor value" uifolder="Common" />
    <input name="inputAlpha_value" type="float" value="1" uiname="inputAlpha value" uifolder="Common" />
  </nodegraph>
  <nodedef name="ND_test_colorcorrect" node="test_colorcorrect" version="1.0" isdefaultversion="false" nodegroup="adjustment" uiname="test_colorcorrect Version: 1.0" doc="This is version 1 of the definition for the graph: NG_test_colorcorrect">
    <nodegraph name="NG_test_colorcorrect">
      <multiply name="AlphaGain" type="float">
        <input name="in1" type="float" nodename="inputAlpha" />
        <input name="in2" type="float" interfacename="AlphaGain_in2" />
      </multiply>
      <add name="AlphaOffset" type="float">
        <input name="in1" type="float" nodename="AlphaGain" />
        <input name="in2" type="float" interfacename="AlphaOffset_in2" />
      </add>
      <multiply name="ColorGain" type="color3">
        <input name="in1" type="color3" nodename="inputColor" />
        <input name="in2" type="color3" interfacename="ColorGain_in2" />
      </multiply>
      <add name="ColorOffset" type="color3">
        <input name="in1" type="color3" nodename="ColorGain" />
        <input name="in2" type="color3" interfacename="ColorOffset_in2" />
      </add>
      <constant name="inputColor" type="color3">
        <input name="value" type="color3" interfacename="inputColor_value" />
      </constant>
      <constant name="inputAlpha" type="float">
        <input name="value" type="float" interfacename="inputAlpha_value" />
      </constant>
      <output name="out" type="color3" nodename="ColorOffset" />
      <output name="out1" type="float" nodename="AlphaOffset" />
    </nodegraph>
    <input name="AlphaGain_in2" type="float" value="0.8" uiname="AlphaGain in2" uifolder="Common" />
    <input name="AlphaOffset_in2" type="float" value="1" uiname="AlphaOffset in2" uifolder="Common" />
    <input name="ColorGain_in2" type="color3" value="0.9, 0.9, 0.9" uiname="ColorGain in2" uifolder="Common" />
    <input name="ColorOffset_in2" type="color3" value="0.379147, 0.0341412, 0.0341412" uiname="ColorOffset in2" uifolder="Common" />
    <input name="inputColor_value" type="color3" value="0.5, 0.5, 0.5" uiname="inputColor value" uifolder="Common" />
    <input name="inputAlpha_value" type="float" value="1" uiname="inputAlpha value" uifolder="Common" />
    <output name="out" type="color3" />
    <output name="out1" type="float" />
  </nodedef>
</materialx>

Graph Editor : Std Library Definition Using Child Nodegraph

func_nodedef_safepower.mp4

Graph Editor: Created Definition Using Child Nodegraph

Instance

Definition Expanded

kwokcb · 2025-05-21T20:23:18Z

source/MaterialXCore/Document.cpp

+            // So append them to the end of the implementation list assocaited 
+            // with any existing nodedef entry (or create a new one if does not exist).
+            //
+            for (const auto& [nodedefKey, appendImplementations] : nonFuncNodeDefMap)


Search order is strictly set here for definitions: functional defs first, all others after. Usage priority is decided elsewhere but generally it's a "first found" logic (per target).

For this initial check-in, we only look for child nodegraphs. Child implementations could be added if / when desired.

I think we should add the child implementations as well - so we retain the symmetry between nodegraph and implementation elements.

If I'm understanding your comments here correctly, I think you're saying that nodegraph element defined inside a nodedef (a functional def), would be used ahead of any externally defined nodegraph element.

If so, I might suggest that we reverse this ordering. I think I would like to be able to append an additional file to my search path, or create a new nodegraph element locally in my document to replace the existing behavior of a given node definition. But if we're giving priority to the internal nodegraph, then I think I would never be able to override the implementation.

Hopefully that makes sense, or else sorry if I misunderstood what you're thinking about order of precedence.

This was a remaining question, and I think your logic makes sense. It also addresses a previous concern from Doug about how to extend implementation associations.

If there is no overlap then order does not matter.

If there is then we can give non-embedded nodegraph implementation a higher priority by adding them to the implementation list first (so that they are found first when looking for implementations).

Note that this does no affect being able to get all the implementations back.

kwokcb · 2025-05-21T20:25:10Z

source/MaterialXCore/Node.cpp

 NodeDefPtr NodeGraph::getNodeDef() const
 {
-    NodeDefPtr nodedef = resolveNameReference<NodeDef>(getNodeDefString());
+    ElementPtr parent = getSelfNonConst()->getParent();


"Backwards" search looks for parent definitions now.

kwokcb · 2025-05-21T20:25:30Z

source/MaterialXCore/Definition.cpp


 NodeDefPtr Implementation::getNodeDef() const
 {
+    ElementPtr parent = getSelfNonConst()->getParent();


"Backwards" search looks for parent definitions now.

kwokcb · 2025-05-21T20:26:43Z

libraries/stdlib/stdlib_defs.mtlx

    <input name="in2" type="float" value="1.0" />
    <output name="out" type="float" defaultinput="in1" />
+
+    <nodegraph name="NG_safepower_float">


Any / or all graphs could reside here. Both new and old mechanism still work.

I will revert this to not break for anyone parsing this directly.

ld-kerley

I think this looks good to me.

+1 to reverting the data library change for now - until we have socialized this change and validated its not going to inconvenience anyone if we start to update the data library.

I left one outstanding question about order of precedence - but maybe Ive got the wrong end of the stick....

ld-kerley · 2025-10-24T19:36:50Z

source/MaterialXCore/Document.cpp

+            // So append them to the end of the implementation list assocaited 
+            // with any existing nodedef entry (or create a new one if does not exist).
+            //
+            for (const auto& [nodedefKey, appendImplementations] : nonFuncNodeDefMap)


I think we should add the child implementations as well - so we retain the symmetry between nodegraph and implementation elements.

ld-kerley · 2025-10-24T19:44:28Z

source/MaterialXCore/Document.cpp

+            // So append them to the end of the implementation list assocaited 
+            // with any existing nodedef entry (or create a new one if does not exist).
+            //
+            for (const auto& [nodedefKey, appendImplementations] : nonFuncNodeDefMap)


If I'm understanding your comments here correctly, I think you're saying that nodegraph element defined inside a nodedef (a functional def), would be used ahead of any externally defined nodegraph element.

If so, I might suggest that we reverse this ordering. I think I would like to be able to append an additional file to my search path, or create a new nodegraph element locally in my document to replace the existing behavior of a given node definition. But if we're giving priority to the internal nodegraph, then I think I would never be able to override the implementation.

Hopefully that makes sense, or else sorry if I misunderstood what you're thinking about order of precedence.

kwokcb · 2025-10-24T21:09:02Z

I think this looks good to me.

+1 to reverting the data library change for now - until we have socialized this change and validated its not going to inconvenience anyone if we start to update the data library.

I left one outstanding question about order of precedence - but maybe Ive got the wrong end of the stick....

I will put in a unit test for precedence, remove the stdlib change and add a specific test file for codegen / render tests. Thx.

- Update precedence logic so that non-functional implementations have higher priority. - Revert std lib sample changes. - Add in functional_nodedef.mtlx test file - Tests for implementation search of child nodegraph - Tests for precedence by adding a non-functional implementation graph afterwards.

kwokcb

Review changes made.

kwokcb · 2025-10-25T04:23:37Z

source/MaterialXTest/MaterialXCore/Node.cpp

+        REQUIRE(referenceNodeGraph != nullptr);
+        if (referenceNodeGraph)
+        {
+            referenceNodeGraph->setNodeDefString(nodeDefName);


Superscede the child nodegraph impl with one that is not a child. As all changes cause a nodedef->impl cache rebuild this allows for this dynamic behaviour.

kwokcb · 2025-10-25T04:24:08Z

source/MaterialXTest/MaterialXCore/Node.cpp

+        std::string nodeDefName = nodeDef->getName();
+        std::string nodeGraphName = "NG_" + nodeDefName.substr(3); // Remove the 'ND_' prefix
+
+        mx::InterfaceElementPtr implementation = nodeDef->getImplementation();


Basic child nodegraph implementation search test.

Fix warnings as errors in test.

…xtensive testing.

kwokcb · 2025-12-13T17:38:19Z

FYI: @ashwinbhat, after talking with @ld-kerley, I've made it so the logic is easy to extend to handling non nodegraph implementation encapsulation as a future next step. A new spec proposal issue will cover what is required there.

Initial test.

f8ceb63

kwokcb mentioned this pull request May 21, 2025

Specification Proposal : "Functional" Node Definitions #2355

Open

kwokcb commented May 21, 2025

View reviewed changes

kwokcb added 7 commits May 22, 2025 07:53

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

a9cc157

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

b96d4cd

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

814e524

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

179a6d6

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

36a154f

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

cf7d0f2

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

8ef6904

kwokcb requested review from ashwinbhat and ld-kerley September 1, 2025 16:04

kwokcb added 7 commits September 3, 2025 07:03

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

c52561b

Add create options and unit test.

cad97bc

Add doc for DefinitionsOptions arg on addNodeDefFromGraph().

5ac34e5

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

4dc3ee1

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

7add695

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

df75cae

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

f3b5985

kwokcb changed the title ~~Function NodeDef Prototype~~ Add Function NodeDef Support Oct 23, 2025

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

cb430ab

kwokcb marked this pull request as ready for review October 23, 2025 17:24

ld-kerley reviewed Oct 24, 2025

View reviewed changes

kwokcb added 2 commits October 24, 2025 23:01

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

ece55fc

kwokcb changed the title ~~Add Function NodeDef Support~~ Add Functional NodeDef Support Oct 25, 2025

kwokcb removed the request for review from ashwinbhat October 25, 2025 04:19

kwokcb commented Oct 25, 2025

View reviewed changes

kwokcb requested a review from ld-kerley October 25, 2025 04:24

kwokcb added 14 commits October 25, 2025 00:30

Fix Windows test warnings.

8caf533

Fix warnings as errors in test.

Add Javascript bindings + unit test.

4057ff9

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

7e12bf1

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

231bcce

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

6fc3a7b

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

d999875

Merge branch 'main' into functional_nodedefs

aa3ca63

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

d24b4f1

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

fc97603

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

5f62af2

Merge branch 'AcademySoftwareFoundation:main' into functional_nodedefs

8905df3

Merge branch 'main' into functional_nodedefs

f481f6f

Merge remote-tracking branch 'upstream/main' into functional_nodedefs

0018a27

Add inlining, inheritance, and shared implementation logic and more e…

dd352e6

…xtensive testing.

kwokcb requested a review from ashwinbhat December 13, 2025 17:36

kwokcb added 5 commits December 17, 2025 08:50

Merge remote-tracking branch 'upstream/main' into functional_nodedefs

a3ed34d

Auto sync.

3e70748

Merge remote-tracking branch 'upstream/main' into functional_nodedefs

d6346f8

Fix merge.

f9a5603

Merge remote-tracking branch 'aswf/main' into functional_nodedefs

f235733

Add Functional NodeDef Support #2401

Are you sure you want to change the base?

Add Functional NodeDef Support #2401

Uh oh!

Conversation

kwokcb commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Implementation

Modifications.

Tests

Unit Test Results

Definition Created with Sibling Reference

Definition Created with Child

Graph Editor : Std Library Definition Using Child Nodegraph

Graph Editor: Created Definition Using Child Nodegraph

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ld-kerley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kwokcb commented Oct 24, 2025

Uh oh!

kwokcb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kwokcb commented Dec 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kwokcb commented May 21, 2025 •

edited

Loading