-
Notifications
You must be signed in to change notification settings - Fork 360
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
Switch nodes no longer cull unused branches in shader generation #1526
Comments
This is a good area for discussion, though we may want to consider this category of optimization as a future part of our "interface reduction" rules for shader generation, rather than making a special case for the switch node. Culling generated code based on a constant input isn’t always what the user will want, as this requires shader recompilation when inputs are modified in an editor, and it seems worthwhile to make this behavior controllable by the client application. |
The culling behavior of the switch node was a bug introduced way back in 1.38.0 that was fixed in 1.38.8. If you look at the node definition of the switch node: <nodedef name="ND_switch_float" node="switch" nodegroup="conditional">
<input name="in1" type="float" value="0.0" />
<input name="in2" type="float" value="0.0" />
<input name="in3" type="float" value="0.0" />
<input name="in4" type="float" value="0.0" />
<input name="in5" type="float" value="0.0" />
<input name="which" type="float" value="0.0" />
<output name="out" type="float" defaultinput="in1" />
</nodedef> You will see no hint that leaving the I expect, given this node definition, that my shader will have a shader uniform parameter for the This would make it consistent with the behavior of 99% of the nodes. The Now, if I were to connect a My vote is to close this issue as "by design" and move on to future optimization code in the shadergen to properly handle constant propagation. |
@JGamache-autodesk This sounds like a fair take to me, and I'd be interested in thoughts from @kwokcb. |
Sorry, if this is long winded... Some additional context to consider:
I am not so sure about "guessing" what the user wants as there are different scenarios for code generation and not just reuse for an edit workflow. I'm not sure you want to force a constant to be added by the user. This forces artists to consider shader optimization which could be the case but depends on how technical they are -- also as noted this may not apply to all backend generators. I agree that performing branch pruning "under the covers" is also not a good solution but I was mostly worried that behaviour has changed. It was always intentional to prune (even before 1.38) as optimization was the only factor considered. Reversing this without an new option I don't see as any better, so agree that explicit optimization options should be added. BTW, in addition to optimization for reuse I think that the optimization option used to produce a shader should be recorded on the |
These are good thoughts for the future, @kwokcb, and I'd recommend that we focus on improvements to the client-side controls for generated shader interfaces in MaterialX, removing the need for heuristics that apply a single set of rules to all users. I'll close out this issue for now, and am looking forward to future discussions in this space. |
With this PR,
switch
nodes no longer optimize to use only 1 branch.The previous logic checks for an unconnected constant value which is used to perform the optimization but was removed.
The resulting generated code is now larger since this was removed.
The text was updated successfully, but these errors were encountered: