-
Notifications
You must be signed in to change notification settings - Fork 353
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
Remove swizzle node for 1.39 #1793
Remove swizzle node for 1.39 #1793
Conversation
This changelist removes the swizzle node from the MaterialX data libraries for v1.39, adding an upgrade pathway for instances of this node in legacy documents. Additionally, it addresses a minor inconsistency in the specification for the extract and extractrowvector nodes, which take zero-based indices rather than positive-valued selections.
Attaching updated comparison renders in GLSL and OSL: |
a05410a
into
AcademySoftwareFoundation:dev_1.39
can we also completely remove |
@ld-kerley Absolutely, and there's a dedicated file in the MDL shader generation system that can now be removed as well, but I'm taking this change in stages! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upgrade function is really cleanly written :) - I was really worried it would be a lot more complex than that. Nice work!
{ | ||
InputPtr inInput = node->getInput("in"); | ||
InputPtr channelsInput = node->getInput("channels"); | ||
if (inInput && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to handle the case where inInput
doesn't exist? All the node defs appear to return 0
, but it's possible the swizzle
node was wired in to a port that doesn't have 0
as its default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point, and perhaps we should handle this case by converting the swizzle
node to a constant
node containing the appropriate values. I haven't encountered this case in our test assets, but it's theoretically possible.
This changelist removes the swizzle node from the MaterialX data libraries for v1.39, adding an upgrade pathway for instances of this node in legacy documents.
Additionally, it addresses a minor inconsistency in the specification for the extract and extractrowvector nodes, which take zero-based indices rather than positive-valued selections.