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

Using get value producing attrs when checking texture assets in NormalMapTextureChecker #2885

Conversation

sirpalee
Copy link
Contributor

@sirpalee sirpalee commented Dec 22, 2023

Description of Change(s)

The change modifies NormalMapTextureChecker to use GetValueProducingAttributes instead of querying the value of the asset parameters on the texture. This way, the NormalMapTextureChecker works better with material networks that expose asset parameters on the Material and connect them to shader parameters.

Questions:

  • I'm checking the namespace of the producing attribute to see if it is an input parameter. Is there a better way to do this?
  • What should we do about asset attributes produced by output parameters? Is that something we should allow, or generally, that's not expected to work? Could we configure the normal map texture checker to turn this feature on/off?

@asluk

Fixes Issue(s)

No issue was created.

  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

@sirpalee sirpalee changed the title Using get value producing attrs when checking texture values in complianceChecker.py. Using get value producing attrs when checking texture assets in NormalMapTextureChecker Dec 22, 2023
@sirpalee sirpalee requested a review from asluk December 27, 2023 02:30
@sirpalee sirpalee force-pushed the pr/compliance-checker-producing-attr branch from bc33cf4 to 188fb42 Compare December 27, 2023 02:31
Copy link
Collaborator

@asluk asluk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me; thanks @sirpalee ! (I don't have approval perms though)

@tallytalwar
Copy link
Contributor

Filed as internal issue #USD-9116

@pixar-oss pixar-oss merged commit 1e2baf4 into PixarAnimationStudios:dev Jan 16, 2024
5 checks passed
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.

4 participants