Improve translation of normals primvars in Hydra #1963
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sebastienblor
changed the title
cpichard
reviewed
Jul 10, 2024
libs/render_delegate/utils.cpp
Outdated
| @@ -681,30 +681,24 @@ void HdArnoldGetPrimvars( | |||
| // The number of motion keys has to be matched between points and normals, so if there are multiple | |||
| // position keys, so we are forcing the user to use the SamplePrimvars function. | |||
| if (multiplePositionKeys && primvarDesc.name == HdTokens->normals) { | |||
| HdArnoldInsertPrimvar(primvars, primvarDesc.name, primvarDesc.role, primvarDesc.interpolation, | |||
| #ifdef USD_HAS_SAMPLE_INDEXED_PRIMVAR | |||
There was a problem hiding this comment.
May be we could remove this ifdef as it was used for very old version ?
There was a problem hiding this comment.
it's needed for versions older than 21.05, so I think we still need it ?
cpichard
approved these changes
Jul 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
test_1483 was failing due to incorrect handling of normals in some cases. Debugging showed that we were skipping the authoring of normals in some cases, since _ConvertValueToArnoldParameter was actually doing nothing when the argument "requiredValues" was not set. Also, HdArnoldSampledPrimvarType is created with 3 samples so when we were pushing a value we ended with 4 of them (the first being empty). Fixing this solved these tests, but as a side effect it then caused other tests to fail because of other issues :
As a separate task, we should rework the ifdefs related to USD_HAS_SAMPLE_INDEXED_PRIMVAR. I might be missing something, but what I'm seeing seems overcomplicated, we should probably be able to remove the ifdef from HdArnoldSample which would then simplify a lot of code
Fixes #1959