AcademySoftwareFoundation / MaterialX

MaterialX is an open standard for the exchange of rich material and look-development content across applications and renderers.
http://www.materialx.org/
Apache License 2.0
1.78k stars 324 forks source link

Remove Switch C++ node #1862

Closed ld-kerley closed 3 weeks ago

ld-kerley commented 3 weeks ago

Replace with nodegraph implementation.

jstone-lucasfilm commented 3 weeks ago

@ld-kerley This looks like a good proposal to me, and I'm curious as to whether it would additionally allow us to remove the switch-related helper methods found here:

https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/source/MaterialXGenMdl/mdl/materialx/stdlib_1_6.mdl#L3908

As you noted in a related thread, the repetition of graph definitions for each type is not ideal, and further in the future this could make a good test case for a polymorphic/generic approach to graph definitions.

ld-kerley commented 3 weeks ago

+1 to some sort of mechanism to remove the code duplication.

I did see the MDL functions - but I didn't want to cut too deep and cause other issues I didn't understand, I don't have a way to run MDL tests :). I'm happy to remove them if you or someone else wants to give the idea the thumbs up? maybe @krohmerNV has insight if these are still needed for some reason?