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

Storm and Prman handle unnormalized quaternions on PointInstancer differently #1505

Closed
spitzak opened this issue Apr 30, 2021 · 4 comments
Closed

Comments

@spitzak
Copy link

spitzak commented Apr 30, 2021

Description of Issue

Storm uses the values in the orientations unchanged, producing odd scaling and distortion. Prman and Embree delegates appear to normalize the quaternions before use. I don't know which is correct behavior but it may be a good idea for them to match.

Steps to Reproduce

  1. Make a pointInstancer of some simple geometry with the orientation set to (1,1,1,1) and compare the Storm and Embree and Prman renderings.

System Information (OS, Hardware)

Linux

Package Versions

USD 20.8

@spiffmon
Copy link
Member

Orientations are documented as unit length quaternions, so Storm and all renderers should be able to use them as-is: https://graphics.pixar.com/usd/docs/api/class_usd_geom_point_instancer.html#a513e44e8c1c496f76077c242bb92a290

I realize that without validation this is a tenuous assumption to rely on, which probably explains the HdPrman and embree code.

@jilliene
Copy link

Filed as internal issue #USD-6676

@spiffmon
Copy link
Member

spiffmon commented May 3, 2021

@spitzak , we think what we'd like to do here is remove the normalization from Embree and hdPrman - do you object? To expand on the rationale a bit, there are two reasons for requiring the authored orientations to be unit length:

  1. For scalability in super-high particle count (even billions?) we decided to use half-precision quaternions, because it is only unit length quaternions that represent rotations, and half has its greatest precision in the 0-1 range, so it seemed like a good match, with the restriction that the quats be unit length as encoded.
  2. Normalizing isn't particularly cheap, and if we can allow renderers to rely on receiving unit-length quats, we gain some performance, especially nice for frame-changes.

@spitzak
Copy link
Author

spitzak commented May 3, 2021

That sounds like a good solution. It will reduce the chances that people will put un-normalized quaternions into their usd data, as it will render incorrectly in more renderers. Probably should get the DWA renderer to not do it as well.

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

No branches or pull requests

3 participants