Allow setting correct glTF coordinate conversions globally

crates/bevy_gltf/src/lib.rs

@@ -99,6 +99,7 @@ mod vertex_attributes;
 extern crate alloc;
 
 use alloc::sync::Arc;
+use bevy_platform::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Mutex;
 use tracing::warn;
 
@@ -152,12 +153,68 @@ impl DefaultGltfImageSampler {
     }
 }
 
+// Has to store an Arc<...> as there is no other way to mutate fields of asset loaders.
+/// Stores the default value for whether to convert the coordinates of loaded glTF assets to Bevy's coordinate system.
+/// If set to `true`, the loader will convert the coordinate system of loaded glTF assets to Bevy's coordinate system.
+/// - glTF:
+///   - forward: Z
+///   - up: Y
+///   - right: -X
+/// - Bevy:
+///   - forward: -Z
+///   - up: Y
+///   - right: X
+#[derive(Resource)]
+pub struct DefaultGltfConvertCoordinates(Arc<AtomicBool>);
+
+impl DefaultGltfConvertCoordinates {
+    /// Creates a new [`DefaultGltfConvertCoordinates`].
+    pub fn new(convert_coordinates: bool) -> Self {
+        Self(Arc::new(AtomicBool::new(convert_coordinates)))
+    }
+
+    /// Returns the current default [`bool`].
+    pub fn get(&self) -> bool {
+        self.0.load(Ordering::SeqCst)
+    }
+
+    /// Makes a clone of internal [`Arc`] pointer.
+    ///
+    /// Intended only to be used by code with no access to ECS.
+    pub fn get_internal(&self) -> Arc<AtomicBool> {
+        self.0.clone()
+    }
+
+    /// Replaces default glTF coordinate conversion setting.
+    ///
+    /// Doesn't apply to glTF assets already loaded, i.e. `GltfLoader`'s output.
+    /// Assets need to manually be reloaded.
+    pub fn set(&self, convert_coordinates: bool) {
+        self.0.store(convert_coordinates, Ordering::SeqCst);
+    }
+}
+
 /// Adds support for glTF file loading to the app.
 pub struct GltfPlugin {
     /// The default image sampler to lay glTF sampler data on top of.
     ///
-    /// Can be modified with [`DefaultGltfImageSampler`] resource.
+    /// 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.
+    /// - glTF:
+    ///   - forward: Z
+    ///   - up: Y
+    ///   - right: -X
+    /// - Bevy:
+    ///   - forward: -Z
+    ///   - up: Y
+    ///   - right: X
+    ///
+    /// Can be modified with the [`DefaultGltfConvertCoordinates`] resource.
+    pub convert_coordinates: bool,
+
     /// Registry for custom vertex attributes.
     ///
     /// To specify, use [`GltfPlugin::add_custom_vertex_attribute`].
@@ -169,6 +226,7 @@ impl Default for GltfPlugin {
         GltfPlugin {
             default_sampler: ImageSamplerDescriptor::linear(),
             custom_vertex_attributes: HashMap::default(),
+            convert_coordinates: false,
         }
     }
 }
@@ -219,10 +277,17 @@ impl Plugin for GltfPlugin {
         let default_sampler_resource = DefaultGltfImageSampler::new(&self.default_sampler);
         let default_sampler = default_sampler_resource.get_internal();
         app.insert_resource(default_sampler_resource);
+
+        let default_convert_coordinates_resource =
+            DefaultGltfConvertCoordinates::new(self.convert_coordinates);
+        let default_convert_coordinates = default_convert_coordinates_resource.get_internal();
+        app.insert_resource(default_convert_coordinates_resource);
+
         app.register_asset_loader(GltfLoader {
             supported_compressed_formats,
             custom_vertex_attributes: self.custom_vertex_attributes.clone(),
             default_sampler,
+            default_convert_coordinates,
         });
     }
 }

crates/bevy_gltf/src/loader/mod.rs

@@ -38,6 +38,7 @@ use bevy_pbr::{
     DirectionalLight, MeshMaterial3d, PointLight, SpotLight, StandardMaterial, MAX_JOINTS,
 };
 use bevy_platform::collections::{HashMap, HashSet};
+use bevy_platform::sync::atomic::{AtomicBool, Ordering};
 use bevy_render::{
     camera::{Camera, OrthographicProjection, PerspectiveProjection, Projection, ScalingMode},
     mesh::Mesh3d,
@@ -151,6 +152,17 @@ pub struct GltfLoader {
     pub custom_vertex_attributes: HashMap<Box<str>, MeshVertexAttribute>,
     /// Arc to default [`ImageSamplerDescriptor`].
     pub default_sampler: Arc<Mutex<ImageSamplerDescriptor>>,
+    /// Arc to default [`AtomicBool`] for whether to convert glTF coordinates to Bevy's coordinate system.
+    /// If set to `true`, the loader will convert the coordinate system of loaded glTF assets to Bevy's coordinate system.
+    /// - glTF:
+    ///   - forward: Z
+    ///   - up: Y
+    ///   - right: -X
+    /// - Bevy:
+    ///   - forward: -Z
+    ///   - up: Y
+    ///   - right: X
+    pub default_convert_coordinates: Arc<AtomicBool>,
 }
 
 /// Specifies optional settings for processing gltfs at load time. By default, all recognized contents of
@@ -188,11 +200,14 @@ pub struct GltfLoaderSettings {
     pub include_source: bool,
     /// Overrides the default sampler. Data from sampler node is added on top of that.
     ///
-    /// If None, uses global default which is stored in `DefaultGltfImageSampler` resource.
+    /// If None, uses global default which is stored in the [`DefaultGltfImageSampler`](crate::DefaultGltfImageSampler) resource.
     pub default_sampler: Option<ImageSamplerDescriptor>,
     /// If true, the loader will ignore sampler data from gltf and use the default sampler.
     pub override_sampler: bool,
-    /// If true, the loader will convert glTF coordinates to Bevy's coordinate system.
+    /// Overrides the default glTF coordinate conversion setting.
+    ///
+    /// If None, uses global default which is stored in the [`DefaultGltfConvertCoordinates`](crate::DefaultGltfConvertCoordinates) resource.
+    /// If set to `true`, the loader will convert the coordinate system of loaded glTF assets to Bevy's coordinate system.
     /// - glTF:
     ///   - forward: Z
     ///   - up: Y
@@ -201,7 +216,7 @@ pub struct GltfLoaderSettings {
     ///   - forward: -Z
     ///   - up: Y
     ///   - right: X
-    pub convert_coordinates: bool,
+    pub convert_coordinates: Option<bool>,
 }
 
 impl Default for GltfLoaderSettings {
@@ -214,7 +229,7 @@ impl Default for GltfLoaderSettings {
             include_source: false,
             default_sampler: None,
             override_sampler: false,
-            convert_coordinates: false,
+            convert_coordinates: None,
         }
     }
 }
@@ -274,6 +289,11 @@ async fn load_gltf<'a, 'b, 'c>(
         paths
     };
 
+    let convert_coordinates = match settings.convert_coordinates {
+        Some(convert_coordinates) => convert_coordinates,
+        None => loader.default_convert_coordinates.load(Ordering::SeqCst),
+    };
+
     #[cfg(feature = "bevy_animation")]
     let (animations, named_animations, animation_roots) = {
         use bevy_animation::{animated_field, animation_curves::*, gltf_curves::*, VariableCurve};
@@ -318,7 +338,7 @@ async fn load_gltf<'a, 'b, 'c>(
                             let translations: Vec<Vec3> = tr
                                 .map(Vec3::from)
                                 .map(|verts| {
-                                    if settings.convert_coordinates {
+                                    if convert_coordinates {
                                         Vec3::convert_coordinates(verts)
                                     } else {
                                         verts
@@ -375,7 +395,7 @@ async fn load_gltf<'a, 'b, 'c>(
                                 .into_f32()
                                 .map(Quat::from_array)
                                 .map(|quat| {
-                                    if settings.convert_coordinates {
+                                    if convert_coordinates {
                                         Quat::convert_coordinates(quat)
                                     } else {
                                         quat
@@ -663,7 +683,7 @@ async fn load_gltf<'a, 'b, 'c>(
                     accessor,
                     &buffer_data,
                     &loader.custom_vertex_attributes,
-                    settings.convert_coordinates,
+                    convert_coordinates,
                 ) {
                     Ok((attribute, values)) => mesh.insert_attribute(attribute, values),
                     Err(err) => warn!("{}", err),
@@ -786,7 +806,7 @@ async fn load_gltf<'a, 'b, 'c>(
                 .map(|mats| {
                     mats.map(|mat| Mat4::from_cols_array_2d(&mat))
                         .map(|mat| {
-                            if settings.convert_coordinates {
+                            if convert_coordinates {
                                 mat.convert_coordinates()
                             } else {
                                 mat
@@ -875,7 +895,7 @@ async fn load_gltf<'a, 'b, 'c>(
             &node,
             children,
             mesh,
-            node_transform(&node, settings.convert_coordinates),
+            node_transform(&node, convert_coordinates),
             skin,
             node.extras().as_deref().map(GltfExtras::from),
         );
@@ -926,6 +946,7 @@ async fn load_gltf<'a, 'b, 'c>(
                         #[cfg(feature = "bevy_animation")]
                         None,
                         &gltf.document,
+                        convert_coordinates,
                     );
                     if result.is_err() {
                         err = Some(result);
@@ -1345,9 +1366,10 @@ fn load_node(
     #[cfg(feature = "bevy_animation")] animation_roots: &HashSet<usize>,
     #[cfg(feature = "bevy_animation")] mut animation_context: Option<AnimationContext>,
     document: &Document,
+    convert_coordinates: bool,
 ) -> Result<(), GltfError> {
     let mut gltf_error = None;
-    let transform = node_transform(gltf_node, settings.convert_coordinates);
+    let transform = node_transform(gltf_node, convert_coordinates);
     let world_transform = *parent_transform * transform;
     // according to https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#instantiation,
     // if the determinant of the transform is negative we must invert the winding order of
@@ -1616,6 +1638,7 @@ fn load_node(
                 #[cfg(feature = "bevy_animation")]
                 animation_context.clone(),
                 document,
+                convert_coordinates,
             ) {
                 gltf_error = Some(err);
                 return;

crates/bevy_gltf/src/vertex_attributes.rs

@@ -141,12 +141,21 @@ impl<'a> VertexAttributeIter<'a> {
             VertexAttributeIter::F32x2(it) => Ok(Values::Float32x2(it.collect())),
             VertexAttributeIter::U32x2(it) => Ok(Values::Uint32x2(it.collect())),
             VertexAttributeIter::F32x3(it) => Ok(if convert_coordinates {
+                // The following f32x3 values need to be converted to the correct coordinate system
+                // - Positions
+                // - Normals
+                //
+                // See <https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#meshes-overview>
                 Values::Float32x3(it.map(ConvertCoordinates::convert_coordinates).collect())
             } else {
                 Values::Float32x3(it.collect())
             }),
             VertexAttributeIter::U32x3(it) => Ok(Values::Uint32x3(it.collect())),
             VertexAttributeIter::F32x4(it) => Ok(if convert_coordinates {
+                // The following f32x4 values need to be converted to the correct coordinate system
+                // - Tangents
+                //
+                // See <https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#meshes-overview>
                 Values::Float32x4(it.map(ConvertCoordinates::convert_coordinates).collect())
             } else {
                 Values::Float32x4(it.collect())

release-content/release-notes/convert-coordinates.md

@@ -1,7 +1,7 @@
 ---
 title: Allow importing glTFs with a corrected coordinate system
 authors: ["@janhohenheim"]
-pull_requests: [19633]
+pull_requests: [19633, 19685]
 ---
 
 glTF uses the following coordinate system:
@@ -24,17 +24,34 @@ Long-term, we'd like to fix our glTF imports to use the correct coordinate syste
 But changing the import behavior would mean that *all* imported glTFs of *all* users would suddenly look different, breaking their scenes!
 Not to mention that any bugs in the conversion code would be incredibly frustating for users.
 
-This is why we are now gradually rolling out support for corrected glTF imports. Starting now you can opt into the new behavior by setting the `GltfLoaderSettings`:
+This is why we are now gradually rolling out support for corrected glTF imports. Starting now you can opt into the new behavior by setting `convert_coordinates` on `GltfPlugin`:
 
 ```rust
 // old behavior, ignores glTF's coordinate system
+App::new()
+    .add_plugins(DefaultPlugins)
+    .run();
+
+// new behavior, converts the coordinate system of all glTF assets into Bevy's coordinate system
+App::new()
+    .add_plugins(DefaultPlugins.set(GltfPlugin {
+        convert_coordinates: true,
+        ..default()
+    }))
+    .run();
+```
+
+You can also control this on a per-asset-level:
+
+```rust
+// Use the global default
 let handle = asset_server.load("fox.gltf#Scene0");
 
-// new behavior, converts glTF's coordinate system into Bevy's coordinate system
+// Manually opt in or out of coordinate conversion for an individual asset
 let handle = asset_server.load_with_settings(
     "fox.gltf#Scene0",
     |settings: &mut GltfLoaderSettings| {
-        settings.convert_coordinates = true;
+        settings.convert_coordinates = Some(true);
     },
 );
 ```