Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
04ec20a
Removed `gltf_convert_coordinates_default` feature and Bevy 0.18 warn…
greeble-dev Aug 1, 2025
a67c1e9
Changed `convert_coordinates` from a bool to a `GltfConvertCoordinate…
greeble-dev Aug 1, 2025
391741f
Removed node conversion.
greeble-dev Aug 1, 2025
1cf0d3f
Removed camera and rotation conversions that were only used by node c…
greeble-dev Aug 1, 2025
3d2bf55
Added scene conversion.
greeble-dev Aug 1, 2025
6625359
Fixed skinned mesh inverse bindposes. The Mat4 conversion assumed tha…
greeble-dev Aug 2, 2025
13d13b5
Mesh conversion now rotates the mesh primitive entities. This negates…
greeble-dev Aug 2, 2025
080c3bc
Added TODOs.
greeble-dev Aug 2, 2025
25d8992
Added utility functions for conversion transforms.
greeble-dev Aug 2, 2025
592e076
More TODOs.
greeble-dev Aug 3, 2025
20b3c3a
Merge branch 'main' into gltf-coordinate-conversion
greeble-dev Aug 3, 2025
0042c67
Merge branch 'main' into gltf-coordinate-conversion
greeble-dev Aug 11, 2025
63b16ac
Updated documentation. Fixed `coordinate_conversion` not being a publ…
greeble-dev Aug 11, 2025
f24c988
Renamed `GltfConvertCoordinates` variable from `scene` to `scenes`. T…
greeble-dev Aug 11, 2025
7984aa6
Removed the awkwardly special case `ConvertInverseCoordinates` in fav…
greeble-dev Aug 11, 2025
5cde975
Updated docs. Clarified that the mesh entity actually gets the invers…
greeble-dev Aug 11, 2025
0b4235e
Merge branch 'main' into gltf-coordinate-conversion
greeble-dev Aug 16, 2025
9248860
Merge branch 'main' into gltf-coordinate-conversion
greeble-dev Aug 16, 2025
415b989
Fixed cases that were still using `use_model_forward_direction`.
greeble-dev Aug 16, 2025
e0e777f
Updated release notes.
greeble-dev Aug 16, 2025
b24b258
Fixed typo
greeble-dev Aug 17, 2025
fb77cfb
Merge branch 'main' into gltf-coordinate-conversion
greeble-dev Aug 21, 2025
62f4018
Updated morph and bounds conversions.
greeble-dev Aug 21, 2025
b2567a5
Updated examples.
greeble-dev Aug 21, 2025
3369ef0
Reverted all changes to the 0.17 release note.
greeble-dev Aug 21, 2025
292dad0
Added new migration guide for 0.18.
greeble-dev Aug 22, 2025
f02a9e2
Fixed markdown lint.
greeble-dev Aug 22, 2025
b82be31
Improved comment.
greeble-dev Aug 22, 2025
9e9dea5
Fixed markdown lint for real this time... maybe?
greeble-dev Aug 22, 2025
252f1cc
Merge branch 'main' into gltf-coordinate-conversion
greeble-dev Oct 4, 2025
9eed9ce
Improved migration guide.
greeble-dev Oct 5, 2025
988746f
Fixed markdown lint.
greeble-dev Oct 5, 2025
9b74297
Tweaked migration guide to be more explicit about which options the u…
greeble-dev Oct 23, 2025
acd10e8
`rotate_scene_entity` and `rotate_meshes`
cart Dec 11, 2025
05fadb7
Changed variable names in migration guide.
greeble-dev Dec 12, 2025
1123386
Reverted accidental commit.
greeble-dev Dec 12, 2025
f545dab
Merge branch 'main' into gltf-coordinate-conversion
greeble-dev Dec 12, 2025
f83a7ae
Merge branch 'main' into gltf-coordinate-conversion
greeble-dev Dec 12, 2025
4b565e2
Merge branch 'main' into gltf-coordinate-conversion
greeble-dev Dec 15, 2025
f9552c1
Delete stray temp.txt
alice-i-cecile Dec 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 81 additions & 46 deletions crates/bevy_gltf/src/convert_coordinates.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,16 @@
use core::f32::consts::PI;
//! Utilities for converting from glTF's [standard coordinate system](https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#coordinate-system-and-units)
//! to Bevy's.
use serde::{Deserialize, Serialize};

use bevy_math::{Mat4, Quat, Vec3};
use bevy_transform::components::Transform;

pub(crate) trait ConvertCoordinates {
/// Converts the glTF coordinates to Bevy's coordinate system.
/// - glTF:
/// - forward: Z
/// - up: Y
/// - right: -X
/// - Bevy:
/// - forward: -Z
/// - up: Y
/// - right: X
///
/// See <https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#coordinate-system-and-units>
/// Converts from glTF coordinates to Bevy's coordinate system. See
/// [`GltfConvertCoordinates`] for an explanation of the conversion.
fn convert_coordinates(self) -> Self;
}

pub(crate) trait ConvertCameraCoordinates {
/// Like `convert_coordinates`, but uses the following for the lens rotation:
/// - forward: -Z
/// - up: Y
/// - right: X
///
/// The same convention is used for lights.
/// See <https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#view-matrix>
fn convert_camera_coordinates(self) -> Self;
}

impl ConvertCoordinates for Vec3 {
fn convert_coordinates(self) -> Self {
Vec3::new(-self.x, self.y, -self.z)
Expand All @@ -48,34 +30,87 @@ impl ConvertCoordinates for [f32; 4] {
}
}

impl ConvertCoordinates for Quat {
fn convert_coordinates(self) -> Self {
// Solution of q' = r q r*
Quat::from_array([-self.x, self.y, -self.z, self.w])
}
/// Options for converting scenes and assets from glTF's [standard coordinate system](https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#coordinate-system-and-units)
/// (+Z forward) to Bevy's coordinate system (-Z forward).
///
/// The exact coordinate system conversion is as follows:
/// - glTF:
/// - forward: +Z
/// - up: +Y
/// - right: -X
/// - Bevy:
/// - forward: -Z
/// - up: +Y
/// - right: +X
///
/// Note that some glTF files may not follow the glTF standard.
///
/// If your glTF scene is +Z forward and you want it converted to match Bevy's
/// `Transform::forward`, enable the `rotate_scene_entity` option. If you also want `Mesh`
/// assets to be converted, enable the `rotate_meshes` option.
///
/// Cameras and lights in glTF files are an exception - they already use Bevy's
/// coordinate system. This means cameras and lights will match
/// `Transform::forward` even if conversion is disabled.
#[derive(Copy, Clone, Default, Debug, Serialize, Deserialize)]
pub struct GltfConvertCoordinates {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me like the big downside of this new approach is that local transforms within a scene (that correspond to nodes) will now no longer have "correct" forward directions. This relies on nesting the scene underneath a top level entity that is "uncorrected", which we use to call "forward" on. Trying to do that within the scene will not produce the expected "forward" behavior.

This is likely to be problematic in the world of BSN "scene inheritance", where rather than spawning the gltf scene root underneath a higher level "bevy scene root", we would instead treat the gltf scene root as the root, and write our changes directly on top. I believe this is the better model when compared to the current "add another root on top" approach, as it allows developers to think about and interact with GLTF scenes exactly as if they were bevy scenes (ex: if the root node has a property, then when we inherit from it, we can directly read/write that property on the root).

I believe this will also come into play if developers try to animate the root transform of the scene, which will snap our corrections back to whatever the animation thinks is correct.

The benefit of course is that the scene is "locally consistent". But within that space, everything is still "uncorrected". If a "player" node exists inside a larger GLTF "world" scene, that player scene's forward will be wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got one thing to check before I reply in full - does BSN require a single scene root entity or does it support multiple roots? Asking because glTF supports multiple roots, so if BSN doesn't then that could spoil things.

Copy link
Contributor Author

@greeble-dev greeble-dev Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, gonna reply now so we can keep moving forward - I'll assume that BSN does support multiple roots.

There's something I should have clarified in the PR: dropping support for node conversion in the short-term doesn't stop us adding it back in later. There are some awkward questions around cameras (which the 0.17 conversion has to address too), but if they're solved then it's feasible to support "scene conversion" and "node conversion" as mutually exclusive options. I've edited the PR to add more detail on this - see the "Future" section at the bottom.

Having two options might seem more complex than necessary. But I think it reflects that different users will have different ideas of what the correct semantics are, and which parts of the scene they prefer to modify.

So right now the 0.17 conversion could be framed as "support one use case, but with bugs". While this PR is "support a different use case, without bugs". Both choices have paths to supporting the other's use case. But I feel this PR is the least worst choice in the short-term, and solves the more common use case of current users.

Another path forward is to try solving the camera problems first, then come back to scene conversion versus node conversion later. I can expand on this solution if desired.

This is likely to be problematic in the world of BSN "scene inheritance", where rather than spawning the gltf scene root underneath a higher level "bevy scene root", we would instead treat the gltf scene root as the root, and write our changes directly on top.

That behaviour can be supported by this PR if the correction is baked into the glTF root nodes instead of the current scene root.

I believe this will also come into play if developers try to animate the root transform of the scene, which will snap our corrections back to whatever the animation thinks is correct.

That issue can be solved by this PR if the animation of root nodes is corrected. Note that 0.17 and node conversion in general already requires correcting animations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll assume that BSN does support multiple roots.

Yes and no. BSN currently has two forms: bsn! and bsn_list!. These are separate things because bsn! can then be a single root / directly inheritable / directly writeable on top and bsn_list! can be spawned flat as many roots or as a relationship. We're also discussing "BSN sets", which would be collections of multiple named BSN entries.

I think we'll want to latch on to one of the two models for glTF assets (or perhaps load it as a BSN set and force developers to select individual scenes from it).

Having two options might seem more complex than necessary. But I think it reflects that different users will have different ideas of what the correct semantics are, and which parts of the scene they prefer to modify.
Another path forward is to try solving the camera problems first, then come back to scene conversion versus node conversion later. I can expand on this solution if desired.

I don't object to supporting both paths, provided they are clearly labeled. I think node conversion is likely the "endgame" and I think I actually prefer leaving the current node conversion code in, but putting it under an experimental flag with docs explaining the situation and limitations. No need to throw it out entirely / start from scratch. It seems like the limitations are a result of not doing some things, rather than doing things incorrectly.

That behaviour can be supported by this PR if the correction is baked into the glTF root nodes instead of the current scene root.

I don't think thats true in all cases. The current corrective behavior only works because we have a root node that is "uncorrected / identity" that we call forward on. Doing the correction at the root removes that, so forward would still be mismatched.

Copy link
Contributor Author

@greeble-dev greeble-dev Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think thats true in all cases. The current corrective behavior only works because we have a root node that is "uncorrected / identity" that we call forward on. Doing the correction at the root removes that, so forward would still be mismatched.

I think I disagree, but I might be just misunderstanding how BSN works. The "scene conversion" option in this PR is basically "do whatever is necessary to make it visually correct but without changing node semantics". I think that can work however BSN organizes the hierarchy, even if some cases are more complex (like needing animation fixup). But maybe this discussion should wait until BSN lands and I can understand it better.

I don't object to supporting both paths, provided they are clearly labeled. I think node conversion is likely the "endgame" and I think I actually prefer leaving the current node conversion code in, but putting it under an experimental flag with docs explaining the situation and limitations. No need to throw it out entirely / start from scratch. It seems like the limitations are a result of not doing some things, rather than doing things incorrectly.

On node conversion being the endgame, I don't agree. It's a useful option, but I think some users will also want the option of whole scene conversion that doesn't change node semantics. Maybe that's how their assets ended up, or maybe they prefer those semantics, or maybe they feel more comfortable with a more minimal option.

Moving forward, I've sketched out a PR that might more agreeable: greeble-dev#1. This keeps the current node conversion and adds a mutually exclusive scene conversion option. I don't like this, because I think it's risky to continue shipping a quirky node conversion when users might come to depend on the quirks, and the path to reliable node conversion remains unclear. But I can follow it through to a real PR if desired.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "scene conversion" option in this PR is basically "do whatever is necessary to make it visually correct but without changing node semantics". I think that can work however BSN organizes the hierarchy

The point is that "do whatever" currently relies on adding an additional layer of hierarchy when spawning in Bevy to provide the "correct" forward direction. If we remove that, then we can't "do whatever" to fix it without changing the node semantics. This isn't a problem currently, but it will surface the moment we remove that added layer of hierarchy (and I strongly think that we should).

Just want to note that I fully agree with what @greeble-dev says here

I'm cool with "no node conversion, reliable scene conversion" path for 0.18. I just expect us to very quickly feel the need to reopen this conversation (and reintroduce essentially all of the existing node conversion code) when BSN lands and we drop the extra hierarchy layer.

Semantics: "scene conversion"

I don't love this framing for the current implementation. I think a more functional description like rotate_scene_root would help people conceptualize this better than something like "scene conversion" which could be interpreted to be a more holistic conversion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'm still confused about the hierarchy issues, but I'll assume we don't have to resolve that now.

I'm happy to develop greeble-dev#1 into an alternative PR if a maintainer thinks that might help move things forward or make decisions clearer.

Semantics: "scene conversion"

I don't love this framing for the current implementation. I think a more functional description like rotate_scene_root would help people conceptualize this better than something like "scene conversion" which could be interpreted to be a more holistic conversion.

I'm reluctant to use the word "root" as (pre-BSN at least) that could be misinterpreted as the glTF root node(s) or the entity with the SceneRoot component.

I could call it rotate_scenes. But the catch is that the separate mesh and (future) node conversions are more complex, and arguably a functional description won't fit in their names - they've got more going on than a single rotation. So we'd probably end up with an awkward struct GltfCoordinateConversion { rotate_scenes, meshes, nodes }.

Or we add rotate to all of them, even if it's a bit approximate. So I favor the current state, but I'm fine to change it to rotate_scenes, rotate_meshes if you prefer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pushed a proposed rotate_scene_entity (this matches the terminology in the docs and solves the "root ambiguity" issue) and rotate_meshes rename. I really don't think high level / general purpose names are appropriate here. Open to alternatives though!

I also added some additional clarity to the rotate_scene_entity docs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm down to merge this for 0.18 if y'all are.

/// If true, convert scenes by rotating the top-level transform of the scene entity.
///
/// This will ensure that [`Transform::forward`] of the "root" entity (the one with [`SceneInstance`](bevy_scene::SceneInstance))
/// aligns with the "forward" of the glTF scene.
///
/// The glTF loader creates an entity for each glTF scene. Entities are
/// then created for each node within the glTF scene. Nodes without a
/// parent in the glTF scene become children of the scene entity.
///
/// This option only changes the transform of the scene entity. It does not
/// directly change the transforms of node entities - it only changes them
/// indirectly through transform inheritance.
pub rotate_scene_entity: bool,

/// If true, convert mesh assets. This includes skinned mesh bind poses.
///
/// This option only changes mesh assets and the transforms of entities that
/// instance meshes. It does not change the transforms of entities that
/// correspond to glTF nodes.
pub rotate_meshes: bool,
}

impl ConvertCoordinates for Mat4 {
fn convert_coordinates(self) -> Self {
let m: Mat4 = Mat4::from_scale(Vec3::new(-1.0, 1.0, -1.0));
// Same as the original matrix
let m_inv = m;
m_inv * self * m
impl GltfConvertCoordinates {
const CONVERSION_TRANSFORM: Transform =
Transform::from_rotation(Quat::from_xyzw(0.0, 1.0, 0.0, 0.0));

fn conversion_mat4() -> Mat4 {
Mat4::from_scale(Vec3::new(-1.0, 1.0, -1.0))
}
}

impl ConvertCoordinates for Transform {
fn convert_coordinates(mut self) -> Self {
self.translation = self.translation.convert_coordinates();
self.rotation = self.rotation.convert_coordinates();
self
pub(crate) fn scene_conversion_transform(&self) -> Transform {
if self.rotate_scene_entity {
Self::CONVERSION_TRANSFORM
} else {
Transform::IDENTITY
}
}

pub(crate) fn mesh_conversion_transform(&self) -> Transform {
if self.rotate_meshes {
Self::CONVERSION_TRANSFORM
} else {
Transform::IDENTITY
}
}

pub(crate) fn mesh_conversion_transform_inverse(&self) -> Transform {
// We magically know that the transform is its own inverse. We still
// make a distinction at the interface level in case that changes.
self.mesh_conversion_transform()
}
}

impl ConvertCameraCoordinates for Transform {
fn convert_camera_coordinates(mut self) -> Self {
self.translation = self.translation.convert_coordinates();
self.rotate_y(PI);
self
pub(crate) fn mesh_conversion_mat4(&self) -> Mat4 {
if self.rotate_meshes {
Self::conversion_mat4()
} else {
Mat4::IDENTITY
}
}
}
24 changes: 7 additions & 17 deletions crates/bevy_gltf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@
//! See the [glTF Extension Registry](https://github.com/KhronosGroup/glTF/blob/main/extensions/README.md) for more information on extensions.

mod assets;
mod convert_coordinates;
pub mod convert_coordinates;
mod label;
mod loader;
mod vertex_attributes;
Expand All @@ -155,7 +155,7 @@ pub mod prelude {
pub use crate::{assets::Gltf, assets::GltfExtras, label::GltfAssetLabel};
}

use crate::extensions::GltfExtensionHandlers;
use crate::{convert_coordinates::GltfConvertCoordinates, extensions::GltfExtensionHandlers};

pub use {assets::*, label::GltfAssetLabel, loader::*};

Expand Down Expand Up @@ -198,19 +198,9 @@ pub struct GltfPlugin {
/// Can be modified with the [`DefaultGltfImageSampler`] resource.
pub default_sampler: ImageSamplerDescriptor,

/// _CAUTION: This is an experimental feature with [known issues](https://github.com/bevyengine/bevy/issues/20621). Behavior may change in future versions._
///
/// How to convert glTF coordinates on import. Assuming glTF cameras, glTF lights, and glTF meshes had global identity transforms,
/// their Bevy [`Transform::forward`](bevy_transform::components::Transform::forward) will be pointing in the following global directions:
/// - When set to `false`
/// - glTF cameras and glTF lights: global -Z,
/// - glTF models: global +Z.
/// - When set to `true`
/// - glTF cameras and glTF lights: global +Z,
/// - glTF models: global -Z.
///
/// The default is `false`.
pub use_model_forward_direction: bool,
/// The default glTF coordinate conversion setting. This can be overridden
/// per-load by [`GltfLoaderSettings::convert_coordinates`].
pub convert_coordinates: GltfConvertCoordinates,

/// Registry for custom vertex attributes.
///
Expand All @@ -223,7 +213,7 @@ impl Default for GltfPlugin {
GltfPlugin {
default_sampler: ImageSamplerDescriptor::linear(),
custom_vertex_attributes: HashMap::default(),
use_model_forward_direction: false,
convert_coordinates: GltfConvertCoordinates::default(),
}
}
}
Expand Down Expand Up @@ -276,7 +266,7 @@ impl Plugin for GltfPlugin {
supported_compressed_formats,
custom_vertex_attributes: self.custom_vertex_attributes.clone(),
default_sampler,
default_use_model_forward_direction: self.use_model_forward_direction,
default_convert_coordinates: self.convert_coordinates,
extensions: extensions.0.clone(),
});
}
Expand Down
18 changes: 3 additions & 15 deletions crates/bevy_gltf/src/loader/gltf_ext/scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ use itertools::Itertools;
#[cfg(feature = "bevy_animation")]
use bevy_platform::collections::{HashMap, HashSet};

use crate::{
convert_coordinates::{ConvertCameraCoordinates as _, ConvertCoordinates as _},
GltfError,
};
use crate::GltfError;

pub(crate) fn node_name(node: &Node) -> Name {
let name = node
Expand All @@ -29,8 +26,8 @@ pub(crate) fn node_name(node: &Node) -> Name {
/// on [`Node::transform()`](gltf::Node::transform) directly because it uses optimized glam types and
/// if `libm` feature of `bevy_math` crate is enabled also handles cross
/// platform determinism properly.
pub(crate) fn node_transform(node: &Node, convert_coordinates: bool) -> Transform {
let transform = match node.transform() {
pub(crate) fn node_transform(node: &Node) -> Transform {
match node.transform() {
gltf::scene::Transform::Matrix { matrix } => {
Transform::from_matrix(Mat4::from_cols_array_2d(&matrix))
}
Expand All @@ -43,15 +40,6 @@ pub(crate) fn node_transform(node: &Node, convert_coordinates: bool) -> Transfor
rotation: bevy_math::Quat::from_array(rotation),
scale: Vec3::from(scale),
},
};
if convert_coordinates {
if node.camera().is_some() || node.light().is_some() {
transform.convert_camera_coordinates()
} else {
transform.convert_coordinates()
}
} else {
transform
}
}

Expand Down
Loading