Skip to content
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

Add color4 type to Sdr #1894

Conversation

seando-adsk
Copy link
Contributor

Description of Change(s)

As discussed in https://groups.google.com/g/usd-interest/c/G8BSo8jzfyY

Fixes Issue(s)

This allows differentiating between color4 and vector4 input types at the Sdr
level when inspecting MaterialX shader nodes.

  • I have submitted a signed Contributor License Agreement

As discussed in https://groups.google.com/g/usd-interest/c/G8BSo8jzfyY

This allows differentiating between color4 and vector4 input types at the Sdr
level when inspecting MaterialX shader nodes.
@jilliene
Copy link

jilliene commented Jun 7, 2022

Filed as internal issue #USD-7413

@spiffmon
Copy link
Member

@seando-adsk and @JGamache-autodesk , we're reviewing this, and I have a followup question, which is whether there is need to update any HdMtlx and/or Storm code to handle color4 for passing data back into MaterialX codegen?

That could be followup work, for sure, along with untangling the OSL component.

@JGamache-autodesk
Copy link
Contributor

Hi @spiffmon, the reconversion back to MaterialX was working before (and after) this fix. The hdMtlx code uses the type found in the MaterialX NodeDef and since the C++ representation of both float4 and color4 is GfVec4f, the values were converted correctly. I did not see any Hydra/Storm issues before, and none were introduced by this fix.

@spiffmon
Copy link
Member

spiffmon commented Aug 24, 2022 via email

@pixar-oss pixar-oss merged commit fed7569 into PixarAnimationStudios:dev Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants