Specification Proposal : "Functional" Node Definitions

[kwokcb]

Proposal for "Functional" Node Definition

Proposal

Introduce a syntax to reduce the complexity of specifying and maintaining a definition and it's corresponding implementation graph.

• This makes packaging of definitions simpler, including for the case of embedding definitions within working documents.
• This is additive and can be done incrementally for either new or existing definitions.
• This could be considered for any proposal to introduce definitions into UsdShade for OpenUSD.
• This can also be considered as an useful "consolidation" step for any future proposal for type-less definitions.

We will use the term functional node definition for this definition variant.

Scope

This only affects definitions using node graph implementations. All other implementation variants are not affected.

Affected areas would include:

• Validation
• Implementation lookup
• Code generation
• Definition organization

Background Complexity

Currently nodegraph (functional graph) is associated nodedef (definition declaration).

This separation is useful in the case where here can be multiple implementations for a given definition, but is not required when there is only one implementation (a).

The following are the details of implementation logic which a user must handle properly:

• This creates a implementation split for a logically atomic entity into two parts, where the following validation logic must be adhered to:

  1. The definition has the interface inputs and outputs without connections as there is no implementation
  2. The graph has the node graph implementation, without interface inputs and outputs with connections to nodes in the graph implementation.
     • Connections to internal node inputs from definition inputs are allowed but cannot be defined on the graph.
     • The output on the graph and the definition must match by name, and type. Outputs can specify a default input which can only be defined on the definition.
  3. Specifies the association "backwards" from the graph to the corresponding definition via a nodedef meta-data attribute on the graph.

• The concept of a compound nodegraph and functional nodegraph must be handled by the user due to the above logic rules.

  • A specific check is required to see if a nodedef meta-data attribute is defined on the nodegraph.
  • This is required to allow different actions to be performed on a compound graph versus a functional graph. For example, the former is mutable but the latter is not.
  • The interface for the functional graph must be manually kept in sync with it's definition. If the graph is changed without modifying it's corresponding definition then this can make the entire definition invalid.
  • This check is also required to avoid mixing definitions and non-definitions within the same "working" document. For example it is not recommended to embe the standard library definitions within a working document.

• By convention, the standard library separates the functional nodegraph and definition into two files making it harder to match one with the other. When publishing (creating) definitions this splits an otherwise "atomic" grouping into 2 places.

_{(a) It is still possible to have implementations per target, but the node graph implementation is considered the "default" one when no such overrides are defined. Note that this mechanism would still be supported with the current syntax and implementation logic.}

Proposal

The proposal is allow the implementation of a functional graph to reside directly within the definition (nodedef) making this an "atomic" entity.

Changes:

• Outputs would specify connections to internal node outputs for data routing. If specified then the graph should reside within the definition.
• Outputs can still specify default values or inputs.
• Additional functional node graphs are only allowed if they are for a specific target. (Note this may not be currently allowed by the spec.)

Example: The following is an example of one of the safepower definitions:

Current Specificaition

• Definition

  <nodedef name="ND_safepower_float" node="safepower" nodegroup="math">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <output name="out" type="float" defaultinput="in1" />
  </nodedef>

• Functional Graph

 <nodegraph name="NG_safepower_float" nodedef="ND_safepower_float">
    <sign name="sign_in1" type="float">
      <input name="in" type="float" interfacename="in1" />
    </sign>
    <absval name="abs_in1" type="float">
      <input name="in" type="float" interfacename="in1" />
    </absval>
    <power name="power" type="float">
      <input name="in1" type="float" nodename="abs_in1" />
      <input name="in2" type="float" interfacename="in2" />
    </power>
    <multiply name="safepower" type="float">
      <input name="in1" type="float" nodename="sign_in1" />
      <input name="in2" type="float" nodename="power" />
    </multiply>
    <output name="out" type="float" nodename="safepower" />
  </nodegraph>

New Specification

  <nodedef name="ND_safepower_float" node="safepower" nodegroup="math">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <output name="out" type="float"  defaultinput="in1" /> <!-- defaults stay here -->

     <nodegraph name="NG_safe_power_float"> <!-- back reference to nodedef removed -->
        <sign name="sign_in1" type="float">
          <input name="in" type="float" interfacename="in1" />
        </sign>
        <absval name="abs_in1" type="float">
          <input name="in" type="float" interfacename="in1" />
        </absval>
        <power name="power" type="float">
          <input name="in1" type="float" nodename="abs_in1" />
          <input name="in2" type="float" interfacename="in2" />
        </power>
        <multiply name="safepower" type="float">
          <input name="in1" type="float" nodename="sign_in1" />
          <input name="in2" type="float" nodename="power" />
        </multiply>
        <output name="out" type="float" nodename="safepower"  /> <!-- output routing stay's here -->
     </nodegraph>
</nodedef>

flowchart LR
    subgraph Current
        ND[Nodedef: ND_safepower_float]
        NG[Nodegraph: NG_safepower_float]
        ND -->|Referenced by nodedef attribute| NG
        ND_inputs[Inputs/Outputs] --> ND
        NG_nodes[Internal Nodes & Routing] --> NG
    end

    subgraph Functional Node Definition
        FND[Nodedef with embedded Nodegraph]
        FND_inputs[Inputs/Outputs] --> FND
        FND_nodes[Internal Nodes & Routing] --> FND
    end

    style ND fill:#151,stroke:#111,stroke-width:2px
    style NG fill:#131,stroke:#111,stroke-width:2px
    style FND fill:#151,stroke:#111,stroke-width:2px

With the new specification

• No search is required to find if and where the implementation reside as all content is specified by a single entity.
• Interfaces and graph node references reside within the same context.
• No additional functional graph is required to be specified and maintained.
• Validation checking would be simplified especially for connectivity checking.

Implementation Logic

• Accessors to get the definition from an nodegraph / implementation would search for a parent definition first,. All existing logic can remain and thus have secondary priority.
• The cache used to keep definition -> list of implementations would need to scan for implementaiton children parented under a definition. These entries would be added before other existing logic and thus be given higher priority.
  • Note that the existing cache building allows for duplicate implementations already as it does not check for implementation name nor target usage. The existing search logic finds the first suitable implementation. This would not change.
• There is a convenience function to create definitions from nodegraphs. This can add in logic to create definitions with child implementations. The option could either default to child implementation or not. For backwards compatibility it can be to no create children and be switch to create children in a later release where behaviour is allowed to change.

These are relatively small additive changes in isolated areas of the code. API callers would not see any difference (except for the creator convenience function).

[kwokcb]

@ashwinbhat, @ld-kerley : This is the basic equivalent to the current proposal for glTF which is merging functional graph into a nodedef to create a functional nodedef for lack of a better term. Thanks.

[ashwinbhat]

The main benefit of this proposal is that implementation and definition are contained so it does reduce the complexity you highlighted. Would it be possible to further extend your proposal to embed implementation and hopefully help with solidifying the implementation specification. Would something like this help towards this?

Example of a nodedef with an embedded nodegraph implementation (similar to the proposal but in a implementation)

<nodedef name="ND_safepower_float" node="safepower" nodegroup="math">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <output name="out" type="float" nodename="safepower" defaultinput="in1" />
    <implementation name="IM_safepower_float" nodedef="NG_safepower_float">
		<sign name="sign_in1" type="float">
		  <input name="in" type="float" interfacename="in1" />
		</sign>
		<absval name="abs_in1" type="float">
		  <input name="in" type="float" interfacename="in1" />
		</absval>
		<power name="power" type="float">
		  <input name="in1" type="float" nodename="abs_in1" />
		  <input name="in2" type="float" interfacename="in2" />
		</power>
		<multiply name="safepower" type="float">
		  <input name="in1" type="float" nodename="sign_in1" />
		  <input name="in2" type="float" nodename="power" />
		</multiply>
  </implementation> 
</nodedef>

Example of a nodedef with a sourcecode implementation

<nodedef name="ND_safesqrt_float" node="safesqrt" nodegroup="math">
    <input name="in1" type="float" value="0.0" />
	  <implementation name="IM_safesqrt_float_glsl" nodedef="ND_safesqrt_float" file="mx_sqrt_float.glsl" function="mx_sqrt_float" target="genglsl">
		<input name="default" type="float" implname="default_value" />
	  </implementation>
	  <implementation name="IM_safesqrt_float_osl" nodedef="ND_safesqrt_float" file="mx_sqrt_float.osl" function="mx_sqrt_float" target="osl">
		<input name="default" type="float" implname="default_value" />
	  </implementation>
      <implementation name="IM_safesqrt_float_msl" nodedef="ND_safesqrt_float" target="genmsl" sourcecode="{sqrt({value})}" />
    <output name="out" type="float" nodename="safesqrt" defaultinput="in1" />
</nodedef>

[ld-kerley]

Thanks @kwokcb for putting this altogether so thoroughly. I think it looks like a great step forwards, and will actually greatly simplify a side-experiment I have to try and get generic types working, I actually think in the future that alone will be a great justification for this sort of unification.

When I read the first example given by @kwokcb - my brain immediately wanted to add some structure too - I think in a similar way to @ashwinbhat. I think having the <output> at the bottom makes the original node definition component a little harder to parse visually, I wanted it immediately under the <input> tags. I was wondering what you thought of having an explicit <interface> section?

<nodedef name="ND_safepower_float" node="safepower" nodegroup="math">
    <interface>
        <input name="in1" type="float" value="0.0" />
        <input name="in2" type="float" value="1.0" />
        <output name="out" type="float" nodename="safepower" defaultinput="in1" />
    </interface>
    <implementation>
        <sign name="sign_in1" type="float">
	    <input name="in" type="float" interfacename="in1" />
	</sign>
	<absval name="abs_in1" type="float">
	    <input name="in" type="float" interfacename="in1" />
	</absval>
	<power name="power" type="float">
	    <input name="in1" type="float" nodename="abs_in1" />
	    <input name="in2" type="float" interfacename="in2" />
	</power>
	<multiply name="safepower" type="float">
	    <input name="in1" type="float" nodename="sign_in1" />
	    <input name="in2" type="float" nodename="power" />
	</multiply>
    </implementation> 
</nodedef>

It might also be nice to consider dropping the requirement for specifying the name attribute for these sub-container elements. We already know the node definition has a unique name, so perhaps we could derive the names for these items, with some simple prefix convention - IM_ND_safepower_float ?

@ashwinbhat - I think I'm a little more on the fence about the target specific implementations. It's nice to be able to only install the data library for the code generators that you need. Concretely we have products that don't need, and thus don't include, the MDL or OSL source. Defining it all together makes that harder, though not impossible with some sort of build-time filtering of the data library (something else I think has a lot of potential).

I'm also a lot less clear how the target specific implementations would map to USD - I think that would be something that could be useful to map out ideas for.

[kwokcb]

Would this be a reasonable compromise ?

• Just embed the nodegraph as a child of a nodedef
• This would mean we can reuse all the existing GraphElement class interfaces and the real change is improve locality.
• All the current validation logic would be the same except we look within the scope of the nodedef and hence do not require any nodedef association meta-data.

Basically a path of "minimum disturbance". <implementation> could be used but would need to check the class hierarchy.

  <nodedef name="ND_safepower_float" node="safepower" nodegroup="math">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <output name="out" type="float"  />

   <nodegraph name="NG_safe_power_float"> <!-- back reference to nodedef removed -->
      <sign name="sign_in1" type="float">
        <input name="in" type="float" interfacename="in1" />
      </sign>
      <absval name="abs_in1" type="float">
        <input name="in" type="float" interfacename="in1" />
      </absval>
      <power name="power" type="float">
        <input name="in1" type="float" nodename="abs_in1" />
        <input name="in2" type="float" interfacename="in2" />
      </power>
      <multiply name="safepower" type="float">
        <input name="in1" type="float" nodename="sign_in1" />
        <input name="in2" type="float" nodename="power" />
      </multiply>
      <output name="out" type="float" nodename="safepower" defaultinput="in1" />
   </nodegraph>
</nodedef>

To allow for future additions, we could consider packaging <implementation> elements also inside as you propose @ashwinbhat, as a packaging option.

<nodedef name="ND_safesqrt_float" node="safesqrt" nodegroup="math">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <output name="out" type="float" />
  
   <implementation name="IM_safesqrt_float_glsl" file="mx_sqrt_float.glsl" function="mx_sqrt_float" target="genglsl">
	<input name="default" type="float" implname="default_value" />
   </implementation>
   <implementation name="IM_safesqrt_float_osl" file="mx_sqrt_float.osl" function="mx_sqrt_float" target="osl">
	<input name="default" type="float" implname="default_value" />
   </implementation>
   <implementation name="IM_safesqrt_float_msl" target="genmsl" sourcecode="{sqrt({value})}" />
</nodedef>

[dbsmythe]

This is a very interesting proposal. I will have to think more about any ramifications and possible side-effects of these ideas, but at first glance, I think that the best and simplest approach might be to allow placing <nodegraph> and/or <implementation> elements within <nodedef>s as a shorthand for the nodedef="..." attribute in those latter two existing elements, and with no other semantic or syntactic changes to any of those elements.

The one thing that I'm not quite sure about yet is the declaration of <output>s: currently, both the <nodedef> and the <nodegraph> require an <output> element which serve different purposes and have different scope: in effect, the <output> of the nodegraph tells what the output connects to within the nodegraph, while the <output> in the nodedef defines the name, type and default value or passthrough connection, and effectively the nodegraph's output connects to the nodedef's output. This makes sense when looked at it that way, but it feels odd to require two <output> elements in this proposed nested structure.

Another thing to consider: should we allow nodedefs that contain embedded nodegraphs or implementations to also be referenced by other external <nodegraph name="NG_xx" nodedef="ND_xx"> or <implementation name="IM_xx_targ" nodedef="ND_xx"> elements? If so, and there's a conflict, which wins: the external one or the internal one? If the internal one, there's no way to alter or override a definition; if the external one, there could be confusion caused by the nodegraph definition in the nodedef not actually being "the definition".

[ld-kerley]

@dbsmythe - I would imagine the conflict resolution rules would be the same as if we currently had two separate node graphs that implemented the same node definition. Any provided target attribute would infer a higher strength, but otherwise first one wins (which I hope is currently the case).

regarding the duplicated <output> - I would propose that we just allow the node definition level output to refer directly to a nodename inside the provided nodegraph implementation, which is what happened in @kwokcb original example (though there we didn't have the nodegraph container). That would feel the most robust and least prone to output mismatches. Effectively the contained nodegraph wouldn't actually need an output, so perhaps it does suggest that it isn't really a <nodegraph> type after all?

[ashwinbhat]

@ld-kerley regarding the implementation nodes for target these are just references, so the actual mdl, osl, glsl fragments would still be kept separate and as you mention filtering during build based on which code generators are enabled is possible.

regarding USD, I think USDShadeShader has a similar concept where you can embed source so I'm thinking long term this might be a step towards USD files that are self contained and the need to reconstruct mtlx for codegen will be reduced? i.e. a mx::nodedef could directly map to USDShaderNodeDef

[kwokcb]

@ld-kerley. @dbsmythe : for the duplicate outputs.

The original example tries to collapse both purposes into one port since they are at the same scope. You can specify a default value / input routing if no evaluation occurs, otherwise it's a connection to a graph node used for evaluation.

If we move the connection to the nodedef level this breaks encapsulation of the graph since the parent knows what's inside the child graph. i.e. if the implementation changes, the interface also changes -- which I assume we don't want to do. (Lee you also had this when you add an <interface> container.)

So I'd suggest we just keep the status-quo which keeps roles / responsibilities separate:

• only allow the nodedef output to specify defaults
• only allow the nodegraph output connect for evaluation.

This also allows for implementation children to be specified with the nodedef. e.g. where we may have source code, so a node lookup is meaningless.

---

UsdShade has output to output "routing" but which could map to the nodedef output "routing" data from the nodegraph output. As this is quite a different approach than MaterialX, I suggest we think on it for a USDShaderNodeDef but not for MaterialX. This "transposition" of logic is already done as part of UsdMtlx so would not be something "new".

[jstone-lucasfilm]

@kwokcb I like the idea of encapsulating the interface and implementation of a node in a single MaterialX element, but I'm struck by how similar this proposal is to our existing compound nodegraphs, which already support both interfaces and graph implementations:

https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/resources/Materials/Examples/StandardSurface/standard_surface_brick_procedural.mtlx

Since this functionality is already so close to what you're aiming to achieve, would it perhaps be more straightforward to go in the other direction, adding an optional node attribute to our compound nodegraphs?

This would give us a syntax like the following:

  <nodegraph name="NG_safepower_float" nodedef="ND_safepower_float" node="safepower">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <sign name="sign_in1" type="float">
      <input name="in" type="float" interfacename="in1" />
    </sign>
    <absval name="abs_in1" type="float">
      <input name="in" type="float" interfacename="in1" />
    </absval>
    <power name="power" type="float">
      <input name="in1" type="float" nodename="abs_in1" />
      <input name="in2" type="float" interfacename="in2" />
    </power>
    <multiply name="safepower" type="float">
      <input name="in1" type="float" nodename="sign_in1" />
      <input name="in2" type="float" nodename="power" />
    </multiply>
    <output name="out" type="float" nodename="safepower" defaultinput="in1" />
  </nodedef>

[kwokcb]

Hi @jstone-lucasfilm .

This is definitely a possible option, and is listed as an alternative approach above , but I think we'd want to make nodedef the one and only type of construct for a definition. As such this is very close to the first "flat" implementation I listed.

However, I believe the key current "stakeholder" scope / goals is the following (including Slack items)

• Have a way to encapsulate a definition as an atomic entity.

  • Allow for better "locality" to simplify creation, maintenance and reuse in libraries but also working documents.
  • Avoiding overloading the interpretation of a nodegraph based on meta-data tags: (*)
    • An editable "compound" graph, (current)
    • A non-editable "functional graph implementation". (current)
    • And with this variant that I'll call "functional graph definitions".

• Allow for easier "packing" of definitions into libraries.

• Allow for the ability to support all implementation types including those which use source code.

• Have a consistent representation which to drive USDShade and glTF representation. One underlying desire is to allow for an independent data driven representation of definitions for each data model without the explicit requirement of MaterialX.

(*) We'd need another rule as to "who wins" if you have both a (nodedef, function nodegraph) pair and function graph definition. For Doug's point on precedence, I'd hope that "functional graphs" could be retired / upgraded in later releases leaving only one meaning for nodegraphs as "compound graphs".

My vote is thus currently for Doug's approach.

[jstone-lucasfilm]

@kwokcb I see Doug's notes and questions above, but would you mind writing out a concrete example of the syntax that you're referring to as Doug's approach?

[kwokcb]

This would be the layout for nodegraph embedding:

  <nodedef name="ND_safepower_float" node="safepower" nodegroup="math">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <output name="out" type="float"  defaultinput="in1" /> <!-- defaults stay here -->

   <nodegraph name="NG_safe_power_float"> <!-- back reference to nodedef removed -->
      <sign name="sign_in1" type="float">
        <input name="in" type="float" interfacename="in1" />
      </sign>
      <absval name="abs_in1" type="float">
        <input name="in" type="float" interfacename="in1" />
      </absval>
      <power name="power" type="float">
        <input name="in1" type="float" nodename="abs_in1" />
        <input name="in2" type="float" interfacename="in2" />
      </power>
      <multiply name="safepower" type="float">
        <input name="in1" type="float" nodename="sign_in1" />
        <input name="in2" type="float" nodename="power" />
      </multiply>
      <output name="out" type="float" nodename="safepower"  /> <!-- output routing stay's here -->
   </nodegraph>
</nodedef>

and for source implementation embedding (repeat of Ashwin's example):

<nodedef name="ND_safesqrt_float" node="safesqrt" nodegroup="math">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <output name="out" type="float" />
  
  <!-- Source implementations -->
   <implementation name="IM_safesqrt_float_glsl" file="mx_sqrt_float.glsl" function="mx_sqrt_float" target="genglsl">
	<input name="default" type="float" implname="default_value" />
   </implementation>
   <implementation name="IM_safesqrt_float_osl" file="mx_sqrt_float.osl" function="mx_sqrt_float" target="osl">
	<input name="default" type="float" implname="default_value" />
   </implementation>
   <implementation name="IM_safesqrt_float_msl" target="genmsl" sourcecode="{sqrt({value})}" />
</nodedef>

[jstone-lucasfilm]

Got it, thanks @kwokcb, and I'll clean up the examples for each proposed path forward, making it easier to compare them (for me, at least!).

Proposal 1, extending our existing nodegraph element:

  <nodegraph name="NG_safepower_float" nodedef="ND_safepower_float" node="safepower">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <sign name="sign_in1" type="float">
      <input name="in" type="float" interfacename="in1" />
    </sign>
    <absval name="abs_in1" type="float">
      <input name="in" type="float" interfacename="in1" />
    </absval>
    <power name="power" type="float">
      <input name="in1" type="float" nodename="abs_in1" />
      <input name="in2" type="float" interfacename="in2" />
    </power>
    <multiply name="safepower" type="float">
      <input name="in1" type="float" nodename="sign_in1" />
      <input name="in2" type="float" nodename="power" />
    </multiply>
    <output name="out" type="float" nodename="safepower" defaultinput="in1" />
  </nodedef>

Proposal 2, nesting nodegraph elements within nodedef elements:

  <nodedef name="ND_safepower_float" node="safepower" nodegroup="math">
    <input name="in1" type="float" value="0.0" />
    <input name="in2" type="float" value="1.0" />
    <output name="out" type="float" defaultinput="in1" />
    <nodegraph name="NG_safe_power_float">
      <sign name="sign_in1" type="float">
        <input name="in" type="float" interfacename="in1" />
      </sign>
      <absval name="abs_in1" type="float">
        <input name="in" type="float" interfacename="in1" />
      </absval>
      <power name="power" type="float">
        <input name="in1" type="float" nodename="abs_in1" />
        <input name="in2" type="float" interfacename="in2" />
      </power>
      <multiply name="safepower" type="float">
        <input name="in1" type="float" nodename="sign_in1" />
        <input name="in2" type="float" nodename="power" />
      </multiply>
      <output name="out" type="float" nodename="safepower" />
    </nodegraph>
  </nodedef>

Is that an accurate comparison of the two proposed paths forward? If so, what are the arguments for choosing option 2 over option 1?

[dbsmythe]

The syntax in proposal 1 appears to nearly exactly match the syntax for a compound nodegraph, e.g. the kind where a bunch of nodes are collapsed into a "group". The only difference I see between proposal 1 and a compound nodegraph is the presence of the node="safepower" attribute (I'm assuming that the inclusion of the "nodedef" attribute was a copy/paste error and is not intended to be there), which seems a bit fragile.

To me, Proposal 2 is more aligned with sticking to established MaterialX conventions:

• Nodedefs define the existence and interfaces of nodes
• Nodegraphs are a wrapping construct that contain collections of other nodes

Having another different mechanism to define the existence of a node (e.g. Proposal 1) complicates this clarity.

Proposal 2 clearly says "This is defining a new node (the nodedef), and inside the nodedef there is the thing (or things) that define the node's implementation. Moreover, if there are multiple target implementations for a node, it would be trivial to extend the syntax for Proposal 2 to include multiple <nodegraph> elements with different target attributes, and/or add <implementation> elements within the nodedef to connect compiled implementations for various targets. I don't see how one could add an alternate target implementation if we used Proposal 1's syntax.

Additionally, by keeping the nodedef around (Proposal 2), it would be possible to add additional externally-defined implementations for a node, by creating another <nodegraph> or <implementation> element and using an nodedef="ND_xx" attribute in them to connect them to this nodedef.

[jstone-lucasfilm]

Got it, @dbsmythe, and those motivations for Proposal 2 definitely resonate with me.

Just to make sure I'm understanding the proposed syntactic change, is it fair to say that nodegraph elements would now be able to live within nodedef elements, but virtually all of the other rules for these two elements would remain as before? The one subtle difference that I see is that a nested nodegraph would no longer need to state which nodedef it is associated with, as this would be implied by the nesting relationship.

And in terms of the benefits of this proposed change, would it purely be an improvement to encapsulation, where an implementation nodegraph can live within the nodedef of the interface that it implements? Or are there other benefits as well?

[dbsmythe]

Correct- the nested nodegraph would no longer need to state which nodedef it's defining, as its nesting inside the nodedef would declare that implicitly. Additionally, implementation elements could also be nested inside a nodedef, with the implementation's nodedef attribute no longer being required due to implicit definition by the nesting. Multiple nodegraph and/or multiple implementation elements could be nested within a single nodedef as long as their targets are all different.

[dbsmythe]

Also, encapsulation and simplification of a common situation is the primary benefit, but I don't think this addition would allow any of the other existing methods of associating a nodedef and a nodegraph/compiled implementation to be deprecated, as the situations that led to those other methods of association aren't entirely solved by this new approach, just the one most common case of defining a nodegraph implementation of a node and providing the interface definition for it. Would be interested to hear @kwokcb 's and @ashwinbhat 's thoughts on this.

[kwokcb]

Hi @dbsmythe, I am curious what cases can not be covered and what do are your thoughts on addressing these ?
It would be useful to enumerate these but I will defer to @ashwinbhat on implementation variants as I have not invested this area much.

[dbsmythe]

The main case that is not covered by the new proposal is if there's an existing node definition (nodedef plus one or more nodegraph or compiled implementations embedded within it), and you need to add another implementation, e.g. for a different target than was originally defined. For that, you'd need to use a standalone nodegraph or implementation element referencing the nodedef via a "nodedef=" attribute as one must do now.

[ashwinbhat]

Hi @dbsmythe since the data library is considered readyonly, to support the case you highlighted we would still have to offer the existing system that allows for custom implementation in <libname>_<target>_impl.mtlx that applications can add. If we want to keep implementation out of nodedef then we need to define a specification for data library implementation <libname>_<target>_impl.mtlx that supports in-lining of source fragments instead of keeping them on disk as .glsl/.osl. One idea that we were thinking about is adding a source element.

[ld-kerley]

I would guess that the standard data library could use this new encapsulated node definition approach, but there may be other targets outside of the standard library that would still want to externally append an implementation or nodegraph to an existing node definition. So we would want to retain this concept in both the syntax and the C++ library support.

My 2cents - I like proposal 2 above too - it feels a lot clearer as to the intention.

[kwokcb]

This is the detailed examination for implementation. Summary in main description.

Functional NodeDef Implementation Changes

Logic To Examine / Update

Existing NodeDef association back search

• NodeGraph::getNodeDef() override: CHANGE

  • Look for parent nodedef first, then
  • resolveNameReference() for root search by name, then
  • rest of code which looks for <impl> with nodedef / nodegraph match can still work

• Implementation::getNodeDef() CHANGE

  • Look for parent nodedef first, then
  • resolveNameReference() for root search by name.
  • dos not have a <impl> search ?

Nodedef to Implementation

• NodeDef::getImplementation(target)
  • Get child implementations
  • Document::getMatchingImplementations()
    • Looks in cached map which uses the NodeDef's name as the key. Need to update this.
  • Document::refresh() : Cache refresh. CHANGE
    • Document::_cache->implementationMap is the cache
      • unordered map of "nodedef name" -> list of implementation references.
      • Does not check for implementation name, or target so can register 2 (nodegraphs / impls with same target) for the same nodedef.
    • Currently looks for nodedef strings
      • If it's a nodegraph when it adds [nodedef string, NodeGraph] to map
      • Else check for an <impl> elements which specify the nodedef and nodegraph pair or nodedef and impl pair.
    • Add in scan for all nodedef elements to see if they have child nodegraph / impl and add them in.
      • Add these first, and duplicates will be ignored ? Currently they are appended which is okay since the search will return all impl elements in order with first match being used.

Node/NodeGraph to Implementation (convenience method)

• Node::getImplementation(target)
  • Calls Node level getNodeDef(target) then NodeDef::getImplementation(target)
• NodeGraph::getImplementation(target)
  • Same as for Node.

Node to NodeDef

• Node::getNodeDef(target, allowRoughMatch)
  • Check for nodedef string on node -> looks for nodedef at root using resolveNameReference()
  • Then does the matching nodedef logic.
  • Should all be the same.
• Node:;getNodeDefOutput()
  • No change

Tests

• Update Node.cpp check to create embedded implementations

Creator

• Document::addNodeDefFromGraph() : CHANGE
  • Modify logic to allow creation of embedded implementations
  • Q: Should the option default to embed by default ?

[kwokcb]

Runnable code is now available in the Draft PR #2401.

[kwokcb]

One case which cannot be handled is referencing of functional nodegraphs by different definitions. This was added in for standard_surface to handle definitions with different default values between versions:

<implementation name="IMPL_standard_surface_surfaceshader_101" nodedef="ND_standard_surface_surfaceshader" nodegraph="NG_standard_surface_surfaceshader_100" />
<implementation name="IMPL_standard_surface_surfaceshader_100" nodedef="ND_standard_surface_surfaceshader_100" nodegraph="NG_standard_surface_surfaceshader_100" />

This would need to be supported for this proposal to provide a complete replacement that does not duplicate referenced functional graphs.

[ld-kerley]

In this concrete case for standard surface - I wonder if it's also worth considering the alternate idea of leveraging the inherit attribute defined, to not only inherit the interface, but also any encapsulated implementation/nodegraph elements.

[kwokcb]

Hi @ld-kerley, This is an intriguing idea and I think fits nicely into what is already there.

I remember now that I added in support for { nodedef : nodegraph } implementation dictionary support to allow explicit associations. This allowed the same functional nodegraph to be referenced from v1 and v1.0.1 of standard surface. ( The functional nodegraph for std surface already has no "back-pointer" since this would be a 1:N reference. )

If we package the functional nodegraph into v1:

• v1.0.1 already inherits from v1 so this already works for looking up interfaces (inputs, outputs).
• This could be easily extended to also look up for implementation elements (functional nodegraph, source impls etc).

<nodedef name="ND_standard_surface_surfaceshader" node="standard_surface" nodegroup="pbr" version="1.0.1" isdefaultversion="true" inherit="ND_standard_surface_surfaceshader_100"
           doc="Autodesk standard surface shader">
  <!-- interface overrides -->
  <!-- No embedded functional nodegraph -->
  </nodedef>

<nodedef name="ND_standard_surface_surfaceshader_100" node="standard_surface" nodegroup="pbr" version="1.0.0"
           doc="Autodesk standard surface shader">
    <!-- interface -->
    <!-- Child functional nodegraph -->
  </nodedef>
>

The key specification change is that you can inherit implementations in addition to interface elements. I posit that this is the most "elegant" approach

BTW, I have never seen any other case of shared functional references except for std surface -- partly I assume since nobody knows that this mechanism exists.

---

An alterative approach is to allow extend the implementation search to take into account inheritance. I don't particularly like these as it moves away from encapsulation.

There are 2 options I see:

1. Keep the syntax exactly the same. The nodegraph reference search will take into account inheritance to find the node graph embedded in ancestor definitions

<implementation name="IMPL_standard_surface_surfaceshader_101" nodedef="ND_standard_surface_surfaceshader" nodegraph="NG_standard_surface_surfaceshader_100" />
<implementation name="IMPL_standard_surface_surfaceshader_100" nodedef="ND_standard_surface_surfaceshader_100" nodegraph="NG_standard_surface_surfaceshader_100" />

2. Take a more USD style approach and start adding in path references.

<implementation name="IMPL_standard_surface_surfaceshader_101" nodedef="ND_standard_surface_surfaceshader" nodegraph="IMPL_standard_surface_surfaceshader_100/NG_standard_surface_surfaceshader_100" />
<implementation name="IMPL_standard_surface_surfaceshader_100" nodedef="ND_standard_surface_surfaceshader_100" nodegraph="NG_standard_surface_surfaceshader_100" />