Skip to content

Rebrand glTF coordinate conversion to an alternative strategy that is biased towards glTF models #20131

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 0 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -574,11 +574,6 @@ web = ["bevy_internal/web"]
# Enable hotpatching of Bevy systems
hotpatching = ["bevy_internal/hotpatching"]

# Enable converting glTF coordinates to Bevy's coordinate system by default. This will be Bevy's default behavior starting in 0.18.
gltf_convert_coordinates_default = [
"bevy_internal/gltf_convert_coordinates_default",
]

# Enable collecting debug information about systems and components to help with diagnostics
debug = ["bevy_internal/debug"]

Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_gltf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ pbr_multi_layer_material_textures = [
]
pbr_anisotropy_texture = ["bevy_pbr/pbr_anisotropy_texture"]
pbr_specular_textures = ["bevy_pbr/pbr_specular_textures"]
gltf_convert_coordinates_default = []

[dependencies]
# bevy
Expand Down Expand Up @@ -64,6 +63,8 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0.140"
smallvec = { version = "1", default-features = false }
tracing = { version = "0.1", default-features = false, features = ["std"] }

[dev-dependencies]
bevy_log = { path = "../bevy_log", version = "0.17.0-dev" }

[lints]
Expand Down
27 changes: 12 additions & 15 deletions crates/bevy_gltf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,20 +159,17 @@ pub struct GltfPlugin {
/// Can be modified with the [`DefaultGltfImageSampler`] resource.
pub default_sampler: ImageSamplerDescriptor,

/// Whether to convert glTF coordinates to Bevy's coordinate system by default.
/// If set to `true`, the loader will convert the coordinate system of loaded glTF assets to Bevy's coordinate system
/// such that objects looking forward in glTF will also look forward in Bevy.
/// 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 exact coordinate system conversion is as follows:
/// - glTF:
/// - forward: Z
/// - up: Y
/// - right: -X
/// - Bevy:
/// - forward: -Z
/// - up: Y
/// - right: X
pub convert_coordinates: bool,
/// The default is `false`.
pub use_model_forward_direction: bool,

/// Registry for custom vertex attributes.
///
Expand All @@ -185,7 +182,7 @@ impl Default for GltfPlugin {
GltfPlugin {
default_sampler: ImageSamplerDescriptor::linear(),
custom_vertex_attributes: HashMap::default(),
convert_coordinates: cfg!(feature = "gltf_convert_coordinates_default"),
use_model_forward_direction: false,
}
}
}
Expand Down Expand Up @@ -241,7 +238,7 @@ impl Plugin for GltfPlugin {
supported_compressed_formats,
custom_vertex_attributes: self.custom_vertex_attributes.clone(),
default_sampler,
default_convert_coordinates: self.convert_coordinates,
default_use_model_forward_direction: self.use_model_forward_direction,
});
}
}
67 changes: 23 additions & 44 deletions crates/bevy_gltf/src/loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ mod extensions;
mod gltf_ext;

use alloc::sync::Arc;
use bevy_log::warn_once;
use std::{
io::Error,
path::{Path, PathBuf},
Expand Down Expand Up @@ -152,20 +151,17 @@ pub struct GltfLoader {
pub custom_vertex_attributes: HashMap<Box<str>, MeshVertexAttribute>,
/// Arc to default [`ImageSamplerDescriptor`].
pub default_sampler: Arc<Mutex<ImageSamplerDescriptor>>,
/// Whether to convert glTF coordinates to Bevy's coordinate system by default.
/// If set to `true`, the loader will convert the coordinate system of loaded glTF assets to Bevy's coordinate system
/// such that objects looking forward in glTF will also look forward in Bevy.
/// 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 exact coordinate system conversion is as follows:
/// - glTF:
/// - forward: Z
/// - up: Y
/// - right: -X
/// - Bevy:
/// - forward: -Z
/// - up: Y
/// - right: X
pub default_convert_coordinates: bool,
/// The default is `false`.
pub default_use_model_forward_direction: bool,
}

/// Specifies optional settings for processing gltfs at load time. By default, all recognized contents of
Expand Down Expand Up @@ -207,23 +203,17 @@ pub struct GltfLoaderSettings {
pub default_sampler: Option<ImageSamplerDescriptor>,
/// If true, the loader will ignore sampler data from gltf and use the default sampler.
pub override_sampler: bool,
/// Overrides the default glTF coordinate conversion setting.
/// How to convert glTF coordinates on import. Assuming glTF cameras, glTF lights, and glTF meshes had global unit 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.
///
/// If set to `Some(true)`, the loader will convert the coordinate system of loaded glTF assets to Bevy's coordinate system
/// such that objects looking forward in glTF will also look forward in Bevy.
///
/// The exact coordinate system conversion is as follows:
/// - glTF:
/// - forward: Z
/// - up: Y
/// - right: -X
/// - Bevy:
/// - forward: -Z
/// - up: Y
/// - right: X
///
/// If `None`, uses the global default set by [`GltfPlugin::convert_coordinates`](crate::GltfPlugin::convert_coordinates).
pub convert_coordinates: Option<bool>,
/// If `None`, uses the global default set by [`GltfPlugin::use_model_forward_direction`](crate::GltfPlugin::use_model_forward_direction).
pub use_model_forward_direction: Option<bool>,
}

impl Default for GltfLoaderSettings {
Expand All @@ -236,7 +226,7 @@ impl Default for GltfLoaderSettings {
include_source: false,
default_sampler: None,
override_sampler: false,
convert_coordinates: None,
use_model_forward_direction: None,
}
}
}
Expand Down Expand Up @@ -296,20 +286,9 @@ async fn load_gltf<'a, 'b, 'c>(
paths
};

let convert_coordinates = match settings.convert_coordinates {
let convert_coordinates = match settings.use_model_forward_direction {
Some(convert_coordinates) => convert_coordinates,
None => {
let convert_by_default = loader.default_convert_coordinates;
if !convert_by_default && !cfg!(feature = "gltf_convert_coordinates_default") {
warn_once!(
"Starting from Bevy 0.18, by default all imported glTF models will be rotated by 180 degrees around the Y axis to align with Bevy's coordinate system. \
You are currently importing glTF files using the old behavior. Consider opting-in to the new import behavior by enabling the `gltf_convert_coordinates_default` feature. \
If you encounter any issues please file a bug! \
If you want to continue using the old behavior going forward (even when the default changes in 0.18), manually set the corresponding option in the `GltfPlugin` or `GltfLoaderSettings`. See the migration guide for more details."
);
}
convert_by_default
}
None => loader.default_use_model_forward_direction,
};

#[cfg(feature = "bevy_animation")]
Expand Down
4 changes: 0 additions & 4 deletions crates/bevy_internal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,6 @@ web = [

hotpatching = ["bevy_app/hotpatching", "bevy_ecs/hotpatching"]

gltf_convert_coordinates_default = [
"bevy_gltf?/gltf_convert_coordinates_default",
]

debug = ["bevy_utils/debug"]

[dependencies]
Expand Down
1 change: 0 additions & 1 deletion docs/cargo_features.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ The default feature set enables most of the expected features of a game engine,
|ghost_nodes|Experimental support for nodes that are ignored for UI layouting|
|gif|GIF image format support|
|glam_assert|Enable assertions to check the validity of parameters passed to glam|
|gltf_convert_coordinates_default|Enable converting glTF coordinates to Bevy's coordinate system by default. This will be Bevy's default behavior starting in 0.18.|
|hotpatching|Enable hotpatching of Bevy systems|
|ico|ICO image format support|
|jpeg|JPEG image format support|
Expand Down
90 changes: 0 additions & 90 deletions release-content/migration-guides/convert-coordinates.md

This file was deleted.

58 changes: 58 additions & 0 deletions release-content/release-notes/convert-coordinates.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
---
title: Allow importing glTFs with corrected model forward semantics
authors: ["@janhohenheim"]
pull_requests: [19633, 19685, 19816, 20131, 20122]
---

Bevy uses the following coordinate system for all worldspace entities that have a `Transform`:

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

But glTF is a bit more complicated. Models in glTF scenes use the following coordinate system:

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

but cameras and lights in glTF scenes use the following coordinate system:

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

As you can see, this clashes with Bevy assumption that everything in the world uses the same coordinate system.
In the past, we only imported glTFs using the camera / light coordinate system for everything, as that is already aligned with Bevy.
In other words, the glTF importer simply assumed that glTF models used -Z as their forward direction, even though they use +Z.

But that meant that on the Bevy side, a glTF model's `Transform::forward()` would actually point backwards from the point of view of the model,
which is counterintuitive and very annoying when working across different art pipelines.

To remedy this, users can now change the import behavior to instead favor correct `Transform::forward()` semantics for models.
The downside is that glTF cameras and lights that have a global identity transform in glTF will now look to +Z instead of -Z in Bevy.
This should not be a problem in many cases, as the whole scene is rotated so that the end result on your screen will be rendered the exact same way.

To globally opt into the behavior that favors glTF models over glTF cameras, you can set `GltfPlugin::use_model_forward_direction`:

```rust
App::new()
.add_plugins(DefaultPlugins.set(GltfPlugin {
use_model_forward_direction: true,
..default()
}))
.run();
```

You can also control this on a per-asset-level:

```rust
let handle = asset_server.load_with_settings(
"fox.gltf#Scene0",
|settings: &mut GltfLoaderSettings| {
settings.use_model_forward_direction = Some(true);
},
);
```

Setting the above to `None` will fall back to the global setting taken from `GltfPlugin::use_model_forward_direction`.
Loading