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

Autodesk: Support float type matrix (GfMatrix4f) for instanceTransform #2724

Merged

Conversation

erikaharrison-adsk
Copy link
Contributor

Description of Change(s)

Support VtArray as the value type for token instanceTransform

A instancer may contain huge number of matrices and occupy lots of memory. Some data models only need float precision
matrices for instancers, however USD only accepts VtArray, doubling the memory size needed.
To save the memory, this fix allows USD to accept instancer matrices of type VtArray directly.

Fixes Issue(s)

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

@jesschimein
Copy link
Contributor

Filed as internal issue #USD-8762

@pixar-oss pixar-oss merged commit 9b544c3 into PixarAnimationStudios:dev Dec 15, 2023
3 of 5 checks passed
@jesschimein
Copy link
Contributor

jesschimein commented Dec 18, 2023 via email

@wingfiring
Copy link
Contributor

Hi @jesschimein,

I noticed that the following statements are removed from the merge commit

image

The original conditional expression in if and the else branch are

if ((primvar.name == HdInstancerTokens->instanceTransform ||
                 primvar.name == HdInstancerTokens->instanceTransforms) &&
                TF_VERIFY(value.IsHolding<VtArray<GfMatrix4d> >())) {
...
}
else {
                source.reset(new HdVtBufferSource(primvar.name, value));

if TF_VERIFY(value.IsHolding<VtArray<GfMatrix4d> >()) is false, the control flow will jump to else branch. In my PR, I moved the value.IsHolding into if branch, so I added the else branch as above to do the same thing as original. It doesn’t affect our current work, but it might affect extensibility (for example, set a non-matrix data and would be consumed by customized render delegate).
Could you please confirm that the change is intentional? Thanks a lot!

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