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

Regression: shader discovery returns NdrNodes instead of SdrShaderNodes #1131

Closed
JGamache-autodesk opened this issue Apr 20, 2022 · 5 comments · Fixed by #1182
Closed

Regression: shader discovery returns NdrNodes instead of SdrShaderNodes #1131

JGamache-autodesk opened this issue Apr 20, 2022 · 5 comments · Fixed by #1182
Assignees
Labels
bug Something isn't working

Comments

@JGamache-autodesk
Copy link

Describe the bug
Prior to PR #974 the shader discovery parser was returning SdrShaderNodes. It now returns NdrNodes, which are considered legacy by USD. See PixarAnimationStudios/OpenUSD#1836 for details.

To Reproduce

Expected behavior
Revert the NdrArnoldParserPlugin changes found in #974 while still making sure the results are sane.

Screenshots

Used Software Versions

  • Arnold: latest
  • USD: 21.11
  • Compiler: As shipping with MtoA
  • OS: Windows

Additional context

@JGamache-autodesk JGamache-autodesk added the bug Something isn't working label Apr 20, 2022
@compso compso added this to Arnold USD May 6, 2022
@compso compso moved this to Todo in Arnold USD May 6, 2022
@sebastienblor
Copy link
Collaborator

sebastienblor commented Jun 14, 2022

I'm seeing in the comment that the reason to switch to NdrProperty was because some attributes needed by Arnold were not supported through SdrShaderProperty. It says "some attributes like 4 component colors", does it mean that RGBA attribute won't be supported ?

@JGamache-autodesk
Copy link
Author

color4 from MaterialX is currently handled as if it was a vector4. Problem is known and a fix was submitted which introduces a Color4 type and fixes the MaterialX discovery.

@sebastienblor
Copy link
Collaborator

ok good to know. I'll try to make it work as a vector4.

@sebastienblor
Copy link
Collaborator

sebastienblor commented Jun 14, 2022

@JGamache-autodesk there's a PR in #1182 , but I cannot set you as a reviewer. Do you think it's ok ?
I'm not seeing any issue in my tests, even with shaders having RGBA parameters. The original PR had lots of changes,, but I suspect this specific change was meant for materialx. Meanwhile we decided to change our strategy for materialx, so maybe this won't be a problem anymore.

@JGamache-autodesk
Copy link
Author

Code look OK to me. Definitely fixes the issue. Thanks.

Repository owner moved this from Todo to Done in Arnold USD Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants