Add an option to toggle eliding constant nodes #2521
Conversation
Editing this node in a real time MaterialX editing software leads to constant shader recompilation because the value gets burned inside the shader text. On complex shading models, this has a noticeable perfomance impact. Add an option to keep these as regular nodes that do not require recompiling the shader.
| @@ -292,7 +292,7 @@ ShaderNodePtr ShaderNode::create(const ShaderGraph* parent, const string& name, | |||
| newNode->_classification = Classification::VDF | Classification::CLOSURE; | |||
| } | |||
| // Second, check for specific nodes types | |||
| else if (nodeDef.getNodeString() == CONSTANT) | |||
| else if (nodeDef.getNodeString() == CONSTANT && context.getOptions().elideConstantNodes) | |||
There was a problem hiding this comment.
Instead of checking the option here - I think it's clearer to apply the same logic inside ShaderGraph::optimize() when the constant nodes are actually being elided here.
I know we only use the CONSTANT classification for this at the moment, but if we ever used it for something else this would need to be refactored.
There was a problem hiding this comment.
I was going to say it was a matter of taste, but you are actually correct since non-elided ND_constant_filename nodes will become an issue in GLSL, which means we need to change the condition in ShaderGraph::optimize() from
else if (node->hasClassification(ShaderNode::Classification::DOT))to
else if (node->hasClassification(ShaderNode::Classification::DOT) ||
node->hasClassification(ShaderNode::Classification::CONSTANT))Mentioning this also opens another can of worms, because that is a vital elision only when the filename is replaced by a sampler, and only if the shading language does not allow passing samplers as function parameters. But that is a discussion for another time.
So my recommendation is to wait until I have completed @ld-kerley 's request and added proper elision at least for GLSL.
Editing this node in a real time MaterialX editing software leads to constant shader recompilation because the value gets burned inside the shader text. On complex shading models, this has a noticeable perfomance impact.
Add an option to keep these as regular nodes that do not require recompiling the shader.