Open kwokcb opened 2 years ago
Slack note from @dbsmythe :
Regarding the suggestion of nodegraph inheritance, is the idea that if nodegraph B inherits from nodegraph A, then B would be treated as if it already contained all the nodes and outputs defined in A, and then with any nodes and outputs defined in B "added" to the nodegraph definition (and replacing the definitions of any nodes or outputs defined in A with the exact same name)? This is an interesting idea, and I am curious if there are any hurdles for various existing applications beyond MaterialX code generation to support this. E.g. perhaps a UI for interactively editing an inherited nodegraph could show B with "normal looking" nodes, outputs and connections for things that it defines, and nodes/outputs/connections that are inherited from A's definition could be shown in a dimmed or other alternate visualization.
Slack note from @dbsmythe :
Regarding the suggestion of nodegraph inheritance, is the idea that if nodegraph B inherits from nodegraph A, then B would be treated as if it already contained all the nodes and outputs defined in A, and then with any nodes and outputs defined in B "added" to the nodegraph definition (and replacing the definitions of any nodes or outputs defined in A with the exact same name)? This is an interesting idea, and I am curious if there are any hurdles for various existing applications beyond MaterialX code generation to support this. E.g. perhaps a UI for interactively editing an inherited nodegraph could show B with "normal looking" nodes, outputs and connections for things that it defines, and nodes/outputs/connections that are inherited from A's definition could be shown in a dimmed or other alternate visualization.
Slack reply:
I think there are two routes for this: Where only the interface (of A) is inherited (to match a nodedef inheritance). The "internals" of the nodegraph are not. The other is as you say to add all nodes from A to B, but as said can add new interfaces / inputs and outputs. For you interaction scenario, there could be API to get all nodes just in B, or get all nodes from A and B. Is more akin to the current source code implementations. This however does not allow for "subtractive" inheritance which is another point which came up when looking at USDUvTexture and nodedefs. In this case B removed an output found in A. I think this is generally not a good pattern for backwards compatibility but I ended up creating an inheritance of from A to B for the nodedef but copied the nodegraph. A possible different approach could be if nodegraph within nodegraph as an alternative to support reuse. But alas this functionality (though in the spec) is not supported and believe is more disruptive in terms of support integration.
Allowing the derived class B to access internal nodes of A creates tight coupling: if the internal network of A changes sufficiently, it will break B.
Ie, In my view, the internal network of A is an "implementation of certain logic" that is abstracted and shielded byu the interface of the network. This allows changing A (adding new outputs, improving the efficiency, etc) without affecting B.
So perhaps we should favor composition over inheritance, to achieve the reuse: 1) Build a functional node graph IMP_UsdUVTexture_Core (which would look the same IMP_UsdUVTexture_23 above) 2) create IMP_UsdUVTexture_23 and feed its inputs to the inputs of the IMP_UsdUVTexture_Core instance node inside, and extract outputs r, g, b, & rgb 3) create IMP_UsdUVTexture_22 and feed its inputs to the inputs of the IMP_UsdUVTexture_Core instance node inside, and extract outputs r, g, b, rgb, & rgba
It's true that now we would have an unintended IMP_UsdUVTexture_Core shader available to users, in addition to the intended IMP_UsdUVTexture_23 and IMP_UsdUVTexture_22. So, maybe we could consider extending MaterialX to allow named compound graphs, if it does not allow them already. Ie, graphs that don't have nodedef, but that do have a name which can be used inside sibling node graphs. These named compound graphs would not leak into the open, though (outside of the .mtlx file they are defined in).
<nodegraph name="UsdUVTexture_CORE">
...
</nodegraph>
<nodegraph name="IMP_UsdUVTexture_22" nodedef="ND_UsdUVTexture_22">
<UsdUVTexture_CORE>
...
</UsdUVTexture_CORE>
</nodegraph>
<nodegraph name="IMP_UsdUVTexture_23" nodedef="ND_UsdUVTexture_23">
<UsdUVTexture_CORE>
...
</UsdUVTexture_CORE>
</nodegraph>
Hi @rafalSFX, Sorry for the delay in reply.
Yes I had originally tried a composition approach for an earlier case of Autodesk std surface version update but instead went with inheritance as this mechanism required the least amount of change.
Nodegraphs do have a name and can be referenced as an implementation via
Going up a level,I think it just happens to be that in this USD texture case there is a subgraph which can be reused but I'm mostly proposing making a nodegraph implementation (functional) support inheritance the same way code based implementations support inheritance.
@dbsmythe , @jstone-lucasfilm , @rafalSFX : I had a nodegraph referencing proposal from a while back. If you think it's worthwhile, I will put it up as a issue or bring up in Slack. What are your thoughts ?
For sure, I can see the appeal of inheritance producing minimal XML code for the derived shaders (like name="ND_UsdUVTexture" and its name="IMP_UsdUVTexture_22"). It's just that it can come at the expense of abstraction and implementation hiding. Ie, it all rests on the assumption that the shader "image_bias" will continue to exist forever (even though it's an implementation detail), so that the derived shader's "rgba" output can reference it as its input.
But maybe that trade-off can be left to the users.
Indeed inheritance could be uses in a safer way (by avoiding referencing inherited child nodes directly), to extend the base shader by adding new inputs and outputs that are fairly independent of the base class implementation.
As for referencing nodegraphs, it could be worthwhile. It could be very useful for delegate-approach (composition of nodegraphs) of shader implementation. To me, it's akin to private class or file-local (static) helper functions.
I've created a larger issue with basics of what I know of node/nodegraph connectivity and added on referencing at the end. https://github.com/AcademySoftwareFoundation/MaterialX/issues/997
Nodegraph Inheritance Proposal
This proposal is to allow for one nodegraph to inherit from another nodegraph. This is for the same purpose of any existing support for inheritance including per node, and per nodedef.
There possible interpretations are as follows, given a nodegraph B which inherits from nodegraph A:
1, and 2 are "additive" model in which the interface cannot change.
Properties:
Examples
USDUvTexture
definition. (Note that all outputs must be defined due to an issue which exists at time of writing with signature resolution).Implementation with inheritance with additional output.