-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Update with modelMatrix and correctly handle rotation #12706
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
Conversation
- All tiles including root get transformed - Tracks original position, scale, and rotation values. transformations only occur using these values to prevent transformations stacking GaussianSplatPrimitive - transformTile uses the root transform and tileset modelMatrix rather than ENU-based transform. - buildGSplatDrawCommand follows this now as well for creating the drawCommand modelMatrix - checks for tileset modelMatrix changes and marks itself dirty
|
Thank you for the pull request, @keyboardspecialist! ✅ We can confirm we have a CLA on file for you. |
weegeekps
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a good enough fix. @jjspace needs to be the final reviewer since he is handling the release tomorrow.
|
Thanks @keyboardspecialist! The code changes here look good to me (and are isolated enough that risk for today's release is low). However, I just tested the sandcastle from the original issue(#12705) in this branch, and I don't see any change in behavior. Should there be? I want to confirm this fixes the reported issue. |
|
Things look pretty good from a quick test. |
I disabled them as the scope was for translations and rotations. Scaling is a bit more involved since the splats themselves have to be scaled accordingly. eta: updated the sandcastle for clarity (and added the missing ui elements) |
That is expected. The problem was two fold: we weren't accounting for the tileset modelMatrix when transforming child tiles and when building the draw command, and we need to apply the rotation in local space to ensure we rotate about the correct origin. |
The tileset is rotated by 180 degrees, still ending up in an obscure place on earth. But one difference should be visible: Without the updates from this branch, the splats have all been messed up after the rotation. (I also tried that "unit cube" splat from #12682 , and the orange "edge" is still rotated, so I do have some concerns about whether "everything" is "fixed" here, but ... it's probably safe to say that something is fixed) |
|
Cool, sounds like this is behaving as expected then. @jjspace Please merge if you're comfortable with this going into today's release. |
|
It seems like everything is working as expected for now so I'm ok to merge this and get it in the release.
@weegeekps If this is something we still want to address please open an issue or include it in #12682
@javagl Same for you, I think what's here is fine but if you think something else should be done as a follow up please make note of it in an issue so it's tracked. |
The follow-up issue at #12682 already mentioned the model matrix (and I added a note there that this was partially addressed). I think that any new, "standalone" issues could/should be pulled out of that only as necessary (so no further action for now) |

Description
Tileset modelMatrix transforms were not being applied correctly to the Gaussian splat tilesets. Previously it was using ENU-based when transforming child tiles and building the draw command. This update changes it to use the root tileset transform and tileset modelMatrix in both cases.
A visual of the problem compounding, spiraling the asset into space

With these updates, we see proper handling of rotations
mmfix.mp4
GaussianSplat3DTileContent
GaussianSplatPrimitive
Issue number and link
Fixes issue #12705
Testing plan
A sample asset with a known geolocation was loaded into sandcastle. Rotation and translation transforms were then applied to the tileset modelMatrix.
Test Sandcastle
Author checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my change