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

Use glam for computing gLTF node transform #11361

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

richardhozak
Copy link
Contributor

Objective

gltf-rs does its own computations when accessing transform.matrix() which does not use glam types, rendering #11238 useless if people were to load gltf models and expecting the results to be deterministic across platforms.

Solution

Move the computation to bevy side which uses glam types, it was already used in one place, so I created one common function to handle the two cases.

The added benefit this has, is that some gltf files can have translation, rotation and scale directly instead of matrix which skips the transform computation completely, win-win.

@richardhozak
Copy link
Contributor Author

@Jondolf this might be of interest to you, since you've authored #11238

@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds A-Math Fundamental domain-agnostic mathematical operations C-Bug An unexpected or incorrect behavior labels Jan 15, 2024
@richardhozak
Copy link
Contributor Author

richardhozak commented Jan 16, 2024

I've addressed the comment on node_transform functions.

Also to share a little success story I've had with this:

Previously my game used bevy_obj to load levels and generate colliders from the mesh it loaded, as obj is simple format, it only has mesh information without any transforms, this worked perfectly when loaded into physics system.

Now, I've wanted to transition to gltf to add more features to the level, to use it as a scene format, I've had so many attempts at this which all failed, as the physics engine handled generating colliders just from meshes, now I realized I need to add all the transforms so the colliders can be properly sized and positioned. When implemented, I was overjoyed but all hope was lost when I found the physics simulation is no longer deterministic.

I've made this PR yesterday as a shot in the dark, hoping it would fix the determinism, but had not verified it. Today, I've included this change in my code and am happy to say that the determinism works again!

@alice-i-cecile alice-i-cecile changed the title Use glam for comuting gLTF node transform Use glam for computing gLTF node transform Jan 16, 2024
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 16, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 16, 2024
Merged via the queue into bevyengine:main with commit 184f233 Jan 16, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds A-Math Fundamental domain-agnostic mathematical operations C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants