Cleanup old unused code for conditional nodes #1344
Conversation
| // TODO: Enable calculateScopes() again when support for | ||
| // conditional nodes are improved. | ||
| // | ||
| // calculateScopes(); |
There was a problem hiding this comment.
This is where the feature where disabled a long time ago by not performing the scope calculations.
| @@ -92,7 +92,7 @@ class MX_GENSHADER_API ShaderGenerator | |||
| virtual void emitFunctionDefinitions(const ShaderGraph& graph, GenContext& context, ShaderStage& stage) const; | |||
|
|
|||
| /// Add the function call for a single node. | |||
| virtual void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage, bool checkScope = true) const; | |||
| virtual void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage) const; | |||
There was a problem hiding this comment.
This parameter has no effect since the feature is not in use. But note that this is a change to shader generation API.
Since there is a default value on the parameter calls to the API are not effected, and it's only derived shader generators that my be affected by this change, if they happen to override this function. I suspect that's very rare. But let me know if this API breakage needs to be handled with more care, and we should keep the parameter in for now.
There was a problem hiding this comment.
@niklasharrysson The approach I've been taking for backwards compatibility is to add a deprecated wrapper for the original function interface, so that existing clients can continue to build their code with the latest version of MaterialX.
Now that we've set C++14 as the baseline for MaterialX, we can additionally mark these wrappers with the [[deprecated]] keyword, so that clients receive a warning at compile time. Here's an example of this pattern in the current codebase:
There was a problem hiding this comment.
That's a great pattern to follow. I have added a wrapper now.
jstone-lucasfilm
changed the title
…ion#1344) This change list removes some old unused code paths related to conditional node handling in shader generation. In the early days of ShaderGen we added a feature to optimize how code was generated for conditional nodes. In particular the goal was to identify sub-graphs that were only used by certain branches of conditionals, and restrict codegen of these nodes to be only emitted inside the scope of the corresponding branches. However the feature as implemented were never made 100% safe and has been disabled for many years. Since the code is unused, unsafe, and since shader compilers will handle this later anyway on the generated shader, I think it's about time to clean this up.
This change list removes some old unused code paths related to conditional node handling in shader generation.
In the early days of ShaderGen we added a feature to optimize how code was generated for conditional nodes. In particular the goal was to identify sub-graphs that were only used by certain branches of conditionals, and restrict codegen of these nodes to be only emitted inside the scope of the corresponding branches. However the feature as implemented were never made 100% safe and has been disabled for many years.
Since the code is unused, unsafe, and since shader compilers will handle this later anyway on the generated shader, I think it's about time to clean this up.