Skip to content

Allow users to fix glTF coordinate system imports #19633

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

Merged
merged 40 commits into from
Jun 16, 2025

Conversation

janhohenheim
Copy link
Member

@janhohenheim janhohenheim commented Jun 13, 2025

Objective

Fixes #5670 as an opt-in for now

glTF uses the following coordinate system:

  • forward: Z
  • up: Y
  • right: -X

and Bevy uses:

  • forward: -Z
  • up: Y
  • right: X

For the longest time, Bevy has simply ignored this distinction. That caused issues when working across programs, as most software respects the
glTF coordinate system when importing and exporting glTFs. Your scene might have looked correct in Blender, Maya, TrenchBroom, etc. but everything would be flipped when importing it into Bevy!

Solution

Add an option to the glTF loader to perform coordinate conversion. Note that this makes a distinction in the camera nodes, as glTF uses a different coordinate system for them.

Follow Ups

  • Add global glTF loader settings, similar to the image loader, so that users can make third-party crates also load their glTFs with corrected coordinates
  • Decide on a migration strategy to make this the future default
    • Create an issue
    • Get feedback from Patrick Walton and Cart (not pinging them here to not spam them)
    • Include this pic for reference of how Blender assumes -Y as forward:
      image

Testing

I ran all glTF animation examples with the new setting enabled to validate that they look the same, just flipped.

Also got a nice test scene from Chris that includes a camera inside the glTF. Thanks @ChristopherBiscardi!

Blender (-Y forward):
image

Bevy (-Z forward, but the model looks the wrong way):
image

Bevy with convert_coordinates enabled (-Z forward):
image

Validation that the axes are correct with F3D's glTF viewer (+Z forward):
image

@janhohenheim janhohenheim added M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-glTF Related to the glTF 3D scene/model format labels Jun 13, 2025
Copy link
Contributor

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

@janhohenheim
Copy link
Member Author

Will fix the CI and expand the PR description in a few minutes :)

@mockersf
Copy link
Member

  • Will wait until porting the examples until this PR is merged

I would prefer them to be fixed in this PR. Introducing temporary changes in examples makes it harder to sort actual regressions

@janhohenheim
Copy link
Member Author

janhohenheim commented Jun 13, 2025

Sure, can do :)
Edit: actually, this is not necessary. The flag is not enabled by default in this PR, so all examples look exactly the same.

@ChristopherBiscardi
Copy link
Contributor

you can simply

We should remove the term "simply" entirely and make sure that phrasing is not used in the migration guide.

@janhohenheim
Copy link
Member Author

janhohenheim commented Jun 14, 2025

@ChristopherBiscardi done.

Also, I fixed the CI and expanded the PR description. Right now I'm porting all examples, but that will take quite a while. This PR can be merged in the meantime without any issues, as the flag is not enabled by default yet

@ChristopherBiscardi
Copy link
Contributor

Seems to do what it says on the tin re: rotation. I checked a few static assets from kenney and kaykit.

with camera:

    commands.spawn((
        Camera3d::default(),
        Transform::from_xyz(4., 5., 7.0)
            .looking_at(Vec3::new(0.0, 0.3, 0.0), Vec3::Y),
    ));

before:

before

after (with feature flag):

after

@janhohenheim janhohenheim removed X-Controversial There is active debate or serious implications around merging this PR M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jun 14, 2025
@alice-i-cecile alice-i-cecile added the X-Uncontroversial This work is generally agreed upon label Jun 14, 2025
@alice-i-cecile alice-i-cecile changed the title Fix glTF coordinate system imports Allow users to fix glTF coordinate system imports Jun 14, 2025
@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jun 14, 2025
@alice-i-cecile alice-i-cecile added the M-Needs-Release-Note Work that should be called out in the blog due to impact label Jun 14, 2025
Copy link
Contributor

It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note.

Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes.

@alice-i-cecile
Copy link
Member

Now that this is opt-in and not breaking, I want a brief release note to gather feedback please.

@janhohenheim
Copy link
Member Author

janhohenheim commented Jun 14, 2025

@alice-i-cecile done 👍
Will additionally expand the release notes when I'm adding global default glTF import settings in a followup PR

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 15, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 16, 2025
Merged via the queue into bevyengine:main with commit 9b743d2 Jun 16, 2025
34 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Jun 24, 2025
# Objective

- Followup to #19633
- As discussed, it's a bit cumbersome to specify that you want the
correct orientation every single time
- Also, glTFs loaded from third parties will still be loaded incorrectly

## Solution

- Allow opting into the new behavior globally or per-asset
- Also improved some docs while on it :)

## Testing

- Ran the animation examples
- Ran the test scene from the last PR with all configuration
combinations
Trashtalk217 pushed a commit to Trashtalk217/bevy that referenced this pull request Jul 10, 2025
…e#19685)

# Objective

- Followup to bevyengine#19633
- As discussed, it's a bit cumbersome to specify that you want the
correct orientation every single time
- Also, glTFs loaded from third parties will still be loaded incorrectly

## Solution

- Allow opting into the new behavior globally or per-asset
- Also improved some docs while on it :)

## Testing

- Ran the animation examples
- Ran the test scene from the last PR with all configuration
combinations
github-merge-queue bot pushed a commit that referenced this pull request Aug 14, 2025
# Objective

Fix glTF coordinate conversion not converting tangents.

<img width="995" height="565" alt="image"
src="https://github.com/user-attachments/assets/20aada7a-39fe-4527-b257-c5efb4555aaf"
/>

Report:
https://discord.com/channels/691052431525675048/692572690833473578/1405362252617355335
Thread:
https://discord.com/channels/691052431525675048/1405451520836898848

## Solution

Fixed by removing a redundant copy of the tangent attributes.

In #5370 attribute copying was moved to a more generic function (see
`convert_attribute`). But one code path was left behind, so the tangents
are actually copied twice. This is technically a bug, but a harmless one
since both copies are the same.

Later on, #19633 added the option to convert attribute coordinates. But
only the first tangent copy (in `convert_attribute`) applies the
conversion - the second copy will overwrite them with unconverted
values. This PR removes the second copy.

There's also some minor code quality tweaks - prefer
`contains_attribute()` to `attribute().is_some()`, and fixed some bad
indentation in log macros. I can revert these if we want to play it
safe.

## Testing

Tested a modified version of example `load_gltf` with and without
feature `gltf_convert_coordinates_default`. Also tested after hacking
the loader to use the code path where tangents are recalculated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-glTF Related to the glTF 3D scene/model format D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes M-Needs-Release-Note Work that should be called out in the blog due to impact S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coordinate system mismatch between Bevy and glTF
6 participants