Handle Topological Value Changes

[kwokcb]

Proposal

Detect value changes which actually cause topological changes in the shader graph produced via code generation.
See issue #1394.

• Currently add in 1 check for conditional nodes which currently are treated as only value changes and hence
  skips code generation resulting in no updates to be visible.

This routine can be incrementally improved over time and used for a topological value change check is required
to determine if a shader needs to be rebuilt. For example:

• Another possibility would be colorspace or unit changes on inputs which can require code insertion changes.
• Currently even though nodes are registered a being pragmatically inserting new code the only
  correspondence is via the implementation source name which is a bit fragile. A larger change to allow generators to "publish" this information seems more appropriate but beyond the scope of this initial proposal to just add the interface.

Adding @ashwinbhat for comments.

Changes

• Add a new code gen API utility called inputValueAffectsShaderCode which takes an input and a target.
• Detect on inputs directly on nodes as well as upstream interface inputs for now.
• It would be useful to use the target to check against code generators to see if they return that a value
  change requires rebuild.
• Update the Graph Editor to use this routine. The update mechanism is tweaked to perform delayed update as with other changes requiring material rebuilds.

Example

This test graph has a switch inside a nodegraph where the inputs to the switch are interface inputs.

Example Document

The following includes both a switch and a ifgreater node both of which are in the conditional node group. Both can require code regeneration on input value change.

<?xml version="1.0"?>
<materialx version="1.38">
  <switch name="switch_color3" type="color3" xpos="11.057971" ypos="-2.775862">
    <input name="in1" type="color3" value="0.816626, 0.0898488, 0.0898488" />
    <input name="in2" type="color3" value="0.0364417, 0.616739, 0.621027" />
    <input name="in3" type="color3" value="0.568383, 0.789731, 0.0289632" />
    <input name="in4" type="color3" value="0.0109218, 0.638142, 0.109069" />
    <input name="in5" type="color3" value="0.699266, 0.00683878, 0.270943" />
    <input name="which" type="float" value="1" />
  </switch>
  <surface_unlit name="surface_unlit" type="surfaceshader" xpos="14.231884" ypos="-2.620690">
    <input name="emission_color" type="color3" nodename="switch_color3" />
  </surface_unlit>
  <ifgreater name="ifgreater_color3" type="color3" xpos="11.217391" ypos="0.318966">
    <input name="in1" type="color3" value="0.863081, 0.0253227, 0.0253227" />
    <input name="in2" type="color3" value="0.0457673, 0.209991, 0.645477" />
    <input name="value2" type="float" value="0.3" />
  </ifgreater>
  <surface_unlit name="surface_unlit2" type="surfaceshader" xpos="14.231884" ypos="0.405172">
    <input name="emission_color" type="color3" nodename="ifgreater_color3" />
  </surface_unlit>
</materialx>

Modification on the graph, or directly on the inputs (if detached) will cause shader rebuilds.

[jstone-lucasfilm]

@kwokcb I've done an initial, naïve integration from main to this pull request, but I'll need your assistance to resolve a handful of merge errors that prevent compilation.

[kwokcb]

@jstone-lucasfilm, This should be cleaned up now.
I don't understand the Python wheel build failures. Seems to be missing files ?

[JGamache-autodesk]

@kwokcb Other candidates for the topo change category include all nodes that have a C++ implementation instead of Native or NodeGraph implementations.
This is dependent on the shadergen though. Something that is topological for GLSL won't necessary be topological in OSL.
The list for GLSL can be found in the MayaUSD codebase.

[JGamache-autodesk]

It is possible that you will only need to look at the list of implementations registered in ShaderGenerator::_implFactory and cover all cases for all generators. That would be the simplest solution if it works.

[kwokcb]

Hi @JGamache-autodesk ,

Yes, I was thinking the list could be expanded as needed going forward. There is a target argument which can be checked so if its specific to glsl then a call which passes genglsl could be used. The specifics of target specific logic can be worked out I think after this initial PR.

[kwokcb]

Hi @JGamache-autodesk

I did a quick check on your suggestion but the call to register registerImplementation() covers far more nodes than is required, and also does not cover all the nodes that are "optimized" out which is what is in this PR which is for all targets.

Maybe I think it's better to have an explicit interface for this.
( BTW, the Maya USD list is missing ifgreateq which is optimized away, so might want to patch that. )

[JGamache-autodesk]

Are you sure the ifgreatereq node can be optimized away? You could do that in ShaderGraph::optimize() if you know for sure that both value1 and value2 inputs are pinned down and return true when asked for isUniform() (see uniform attribute on NodeDef inputs). Then you could do the comparison once and for all in the optimizer and select the branch. You could do something similar for mix nodes if the input is pinned down to constant 1.0 or 0.0. Could be useful to prune unused lobes of a costly surface shader to streamline it a bit.

But as soon as one of the two inputs is varying (connected, or with non-uniform value), then it should be impossible to elide because that value turns into an editable shader uniform parameter that needs to be computed in the shader. The term uniform is very misleading here, because a uniform NodeDef input never results in creating a uniform shader parameter.

In fact, looking at the optimization code, I see recent changes that are very dangerous. If I create a color3 dot node and connect it to multiple locations, I expect to end up with only one shader uniform parameter to tweak and have it affect all connected locations. The code introduced in #1152 should have affected only ND_dot_filename and not any of the other dot nodes. Makes no sense that a dot node is now topological, and makes them undistinguishable from constant nodes, which are very topological by definition.

Can I suggest we either revert the dot optimization, or spend time to properly check for uniform inputs? The flag ShaderPort::isUniform() is exactly what we should be looking for. Which means:

1- Eliding an unconnected constant node should add the uniform flag to the input where the value is propagated.
2- Eliding any other node should propagate the uniform state of the propagated branch input, but should not make it uniform if it is not.
3- All optimizations should first check that the optimizable input(s) is marked as uniform.

This would allow optimizing safely all comparison nodes, as well as the mix nodes. Note that this does not make these nodes topological. The elision is coming from a propagated constant node, which is the one and only topological node in the generic ShaderGraph code.

Those that have a C++ implementation, like the swizzle node are topological, but for a different reason.

[JGamache-autodesk]

For an initial implementation of constant propagation code, see JGamache-autodesk@fe99fbc?w=1
Using this modified gold shader:

<?xml version="1.0"?>
<materialx version="1.38" colorspace="lin_rec709">
  <constant name="c1" type="float">
    <input name="value" type="float" value="0.0" />
  </constant>
  <constant name="c2" type="float">
    <input name="value" type="float" value="0.0" />
  </constant>
  <constant name="c3" type="float">
    <input name="value" type="float" value="0.0" />
  </constant>
  <mix name="m1" type="float">
    <input name="fg" type="float" nodename="c1" />
    <input name="bg" type="float" nodename="c2" />
    <input name="mix" type="float" nodename="c3" />
  </mix>
  <mix name="lobePruner" type="float">
    <input name="fg" type="float" value="0.4" />
    <input name="bg" type="float" nodename="c2" />
    <input name="mix" type="float" nodename="m1" />
  </mix>
  <standard_surface name="SR_gold" type="surfaceshader">
    <input name="base" type="float" value="1" />
    <input name="base_color" type="color3" value="0.944, 0.776, 0.373" />
    <input name="specular" type="float" value="1" />
    <input name="specular_color" type="color3" value="0.998, 0.981, 0.751" />
    <input name="specular_roughness" type="float" value="0.02" />
    <input name="metalness" type="float" value="1" />
    <input name="transmission" type="float" nodename="lobePruner" />
    <input name="subsurface" type="float" nodename="lobePruner" />
    <input name="sheen" type="float" nodename="lobePruner" />
    <input name="coat" type="float" nodename="lobePruner" />
    <input name="emission" type="float" nodename="lobePruner" />
  </standard_surface>
  <surfacematerial name="Gold" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="SR_gold" />
  </surfacematerial>
</materialx>

Will print the following debugging output:

Bypassing c2_value
       Setting uniform: lobePruner_bg
       Will revisit: lobePruner
       Setting uniform: m1_bg
       Will revisit: m1
Bypassing c1_value
       Setting uniform: m1_fg
Bypassing c3_value
       Setting uniform: m1_mix
Bypassing m1_bg
       Setting uniform: lobePruner_mix
       Will revisit: lobePruner
Bypassing lobePruner_bg
       Setting uniform: SR_gold_transmission
       Will revisit: SR_gold
       Setting uniform: SR_gold_subsurface
       Setting uniform: SR_gold_sheen
       Setting uniform: SR_gold_coat
       Setting uniform: SR_gold_emission

Which is a good start, but is missing the crucial part where all those newly uniform flags set on various SR_gold inputs result in pruning lobes in the standard_surface NodeGraph.

[kwokcb]

Hi @JGamache-autodesk,
Perhaps we can move this discussion to a new issue on how to refine optimization logic ?

[jstone-lucasfilm]

@kwokcb Thinking more about how conditional nodes ought to be handled in shader generation, I wonder if this PR is putting the cart before the horse, where the first step ought to be designing the right set of rules for constant folding optimizations in shader generation. At the very least, I'm imagining that this new helper function will need to take a GenOptions structure as an input, so that the interface reduction setting and other optimizations requested by the client will be taken into account.