diff --git a/crates/bevy_gltf/src/convert_coordinates.rs b/crates/bevy_gltf/src/convert_coordinates.rs new file mode 100644 index 0000000000000..4148cecd9ac72 --- /dev/null +++ b/crates/bevy_gltf/src/convert_coordinates.rs @@ -0,0 +1,80 @@ +use core::f32::consts::PI; + +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 + 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 + /// + /// See + fn convert_camera_coordinates(self) -> Self; +} + +impl ConvertCoordinates for Vec3 { + fn convert_coordinates(self) -> Self { + Vec3::new(-self.x, self.y, -self.z) + } +} + +impl ConvertCoordinates for [f32; 3] { + fn convert_coordinates(self) -> Self { + [-self[0], self[1], -self[2]] + } +} + +impl ConvertCoordinates for [f32; 4] { + fn convert_coordinates(self) -> Self { + // Solution of q' = r q r* + [-self[0], self[1], -self[2], self[3]] + } +} + +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]) + } +} + +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 ConvertCoordinates for Transform { + fn convert_coordinates(mut self) -> Self { + self.translation = self.translation.convert_coordinates(); + self.rotation = self.rotation.convert_coordinates(); + self + } +} + +impl ConvertCameraCoordinates for Transform { + fn convert_camera_coordinates(mut self) -> Self { + self.translation = self.translation.convert_coordinates(); + self.rotate_y(PI); + self + } +} diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index 97600f3ed064c..4262d43eb750d 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -91,6 +91,7 @@ //! You can use [`GltfAssetLabel`] to ensure you are using the correct label. mod assets; +mod convert_coordinates; mod label; mod loader; mod vertex_attributes; diff --git a/crates/bevy_gltf/src/loader/gltf_ext/scene.rs b/crates/bevy_gltf/src/loader/gltf_ext/scene.rs index 83e6778b99e37..3fce51d52706b 100644 --- a/crates/bevy_gltf/src/loader/gltf_ext/scene.rs +++ b/crates/bevy_gltf/src/loader/gltf_ext/scene.rs @@ -10,7 +10,10 @@ use itertools::Itertools; #[cfg(feature = "bevy_animation")] use bevy_platform::collections::{HashMap, HashSet}; -use crate::GltfError; +use crate::{ + convert_coordinates::{ConvertCameraCoordinates as _, ConvertCoordinates as _}, + GltfError, +}; pub(crate) fn node_name(node: &Node) -> Name { let name = node @@ -26,8 +29,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) -> Transform { - match node.transform() { +pub(crate) fn node_transform(node: &Node, convert_coordinates: bool) -> Transform { + let transform = match node.transform() { gltf::scene::Transform::Matrix { matrix } => { Transform::from_matrix(Mat4::from_cols_array_2d(&matrix)) } @@ -40,6 +43,15 @@ pub(crate) fn node_transform(node: &Node) -> Transform { rotation: bevy_math::Quat::from_array(rotation), scale: Vec3::from(scale), }, + }; + if convert_coordinates { + if node.camera().is_some() { + transform.convert_camera_coordinates() + } else { + transform.convert_coordinates() + } + } else { + transform } } diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index abc555002a6be..5e0f752e5017c 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -84,6 +84,7 @@ use self::{ texture::{texture_handle, texture_sampler, texture_transform_to_affine2}, }, }; +use crate::convert_coordinates::ConvertCoordinates as _; /// An error that occurs when loading a glTF file. #[derive(Error, Debug)] @@ -191,6 +192,16 @@ pub struct GltfLoaderSettings { pub default_sampler: Option, /// 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. + /// - glTF: + /// - forward: Z + /// - up: Y + /// - right: -X + /// - Bevy: + /// - forward: -Z + /// - up: Y + /// - right: X + pub convert_coordinates: bool, } impl Default for GltfLoaderSettings { @@ -203,6 +214,7 @@ impl Default for GltfLoaderSettings { include_source: false, default_sampler: None, override_sampler: false, + convert_coordinates: false, } } } @@ -303,7 +315,16 @@ async fn load_gltf<'a, 'b, 'c>( match outputs { ReadOutputs::Translations(tr) => { let translation_property = animated_field!(Transform::translation); - let translations: Vec = tr.map(Vec3::from).collect(); + let translations: Vec = tr + .map(Vec3::from) + .map(|verts| { + if settings.convert_coordinates { + Vec3::convert_coordinates(verts) + } else { + verts + } + }) + .collect(); if keyframe_timestamps.len() == 1 { Some(VariableCurve::new(AnimatableCurve::new( translation_property, @@ -350,8 +371,17 @@ async fn load_gltf<'a, 'b, 'c>( } ReadOutputs::Rotations(rots) => { let rotation_property = animated_field!(Transform::rotation); - let rotations: Vec = - rots.into_f32().map(Quat::from_array).collect(); + let rotations: Vec = rots + .into_f32() + .map(Quat::from_array) + .map(|quat| { + if settings.convert_coordinates { + Quat::convert_coordinates(quat) + } else { + quat + } + }) + .collect(); if keyframe_timestamps.len() == 1 { Some(VariableCurve::new(AnimatableCurve::new( rotation_property, @@ -633,6 +663,7 @@ async fn load_gltf<'a, 'b, 'c>( accessor, &buffer_data, &loader.custom_vertex_attributes, + settings.convert_coordinates, ) { Ok((attribute, values)) => mesh.insert_attribute(attribute, values), Err(err) => warn!("{}", err), @@ -752,7 +783,17 @@ async fn load_gltf<'a, 'b, 'c>( let reader = gltf_skin.reader(|buffer| Some(&buffer_data[buffer.index()])); let local_to_bone_bind_matrices: Vec = reader .read_inverse_bind_matrices() - .map(|mats| mats.map(|mat| Mat4::from_cols_array_2d(&mat)).collect()) + .map(|mats| { + mats.map(|mat| Mat4::from_cols_array_2d(&mat)) + .map(|mat| { + if settings.convert_coordinates { + mat.convert_coordinates() + } else { + mat + } + }) + .collect() + }) .unwrap_or_else(|| { core::iter::repeat_n(Mat4::IDENTITY, gltf_skin.joints().len()).collect() }); @@ -834,7 +875,7 @@ async fn load_gltf<'a, 'b, 'c>( &node, children, mesh, - node_transform(&node), + node_transform(&node, settings.convert_coordinates), skin, node.extras().as_deref().map(GltfExtras::from), ); @@ -1306,7 +1347,7 @@ fn load_node( document: &Document, ) -> Result<(), GltfError> { let mut gltf_error = None; - let transform = node_transform(gltf_node); + let transform = node_transform(gltf_node, settings.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 @@ -1359,7 +1400,6 @@ fn load_node( }, ..OrthographicProjection::default_3d() }; - Projection::Orthographic(orthographic_projection) } gltf::camera::Projection::Perspective(perspective) => { @@ -1377,6 +1417,7 @@ fn load_node( Projection::Perspective(perspective_projection) } }; + node.insert(( Camera3d::default(), projection, diff --git a/crates/bevy_gltf/src/vertex_attributes.rs b/crates/bevy_gltf/src/vertex_attributes.rs index d4ae811c906eb..2620d608a0c85 100644 --- a/crates/bevy_gltf/src/vertex_attributes.rs +++ b/crates/bevy_gltf/src/vertex_attributes.rs @@ -6,6 +6,8 @@ use gltf::{ }; use thiserror::Error; +use crate::convert_coordinates::ConvertCoordinates; + /// Represents whether integer data requires normalization #[derive(Copy, Clone)] struct Normalization(bool); @@ -132,15 +134,23 @@ impl<'a> VertexAttributeIter<'a> { } /// Materializes values for any supported format of vertex attribute - fn into_any_values(self) -> Result { + fn into_any_values(self, convert_coordinates: bool) -> Result { match self { VertexAttributeIter::F32(it) => Ok(Values::Float32(it.collect())), VertexAttributeIter::U32(it) => Ok(Values::Uint32(it.collect())), VertexAttributeIter::F32x2(it) => Ok(Values::Float32x2(it.collect())), VertexAttributeIter::U32x2(it) => Ok(Values::Uint32x2(it.collect())), - VertexAttributeIter::F32x3(it) => Ok(Values::Float32x3(it.collect())), + VertexAttributeIter::F32x3(it) => Ok(if convert_coordinates { + 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(Values::Float32x4(it.collect())), + VertexAttributeIter::F32x4(it) => Ok(if convert_coordinates { + Values::Float32x4(it.map(ConvertCoordinates::convert_coordinates).collect()) + } else { + Values::Float32x4(it.collect()) + }), VertexAttributeIter::U32x4(it) => Ok(Values::Uint32x4(it.collect())), VertexAttributeIter::S16x2(it, n) => { Ok(n.apply_either(it.collect(), Values::Snorm16x2, Values::Sint16x2)) @@ -188,7 +198,7 @@ impl<'a> VertexAttributeIter<'a> { VertexAttributeIter::U16x4(it, Normalization(true)) => Ok(Values::Float32x4( ReadColors::RgbaU16(it).into_rgba_f32().collect(), )), - s => s.into_any_values(), + s => s.into_any_values(false), } } @@ -198,7 +208,7 @@ impl<'a> VertexAttributeIter<'a> { VertexAttributeIter::U8x4(it, Normalization(false)) => { Ok(Values::Uint16x4(ReadJoints::U8(it).into_u16().collect())) } - s => s.into_any_values(), + s => s.into_any_values(false), } } @@ -211,7 +221,7 @@ impl<'a> VertexAttributeIter<'a> { VertexAttributeIter::U16x4(it, Normalization(true)) => { Ok(Values::Float32x4(ReadWeights::U16(it).into_f32().collect())) } - s => s.into_any_values(), + s => s.into_any_values(false), } } @@ -224,7 +234,7 @@ impl<'a> VertexAttributeIter<'a> { VertexAttributeIter::U16x2(it, Normalization(true)) => Ok(Values::Float32x2( ReadTexCoords::U16(it).into_f32().collect(), )), - s => s.into_any_values(), + s => s.into_any_values(false), } } } @@ -252,28 +262,49 @@ pub(crate) fn convert_attribute( accessor: gltf::Accessor, buffer_data: &Vec>, custom_vertex_attributes: &HashMap, MeshVertexAttribute>, + convert_coordinates: bool, ) -> Result<(MeshVertexAttribute, Values), ConvertAttributeError> { - if let Some((attribute, conversion)) = match &semantic { - gltf::Semantic::Positions => Some((Mesh::ATTRIBUTE_POSITION, ConversionMode::Any)), - gltf::Semantic::Normals => Some((Mesh::ATTRIBUTE_NORMAL, ConversionMode::Any)), - gltf::Semantic::Tangents => Some((Mesh::ATTRIBUTE_TANGENT, ConversionMode::Any)), - gltf::Semantic::Colors(0) => Some((Mesh::ATTRIBUTE_COLOR, ConversionMode::Rgba)), - gltf::Semantic::TexCoords(0) => Some((Mesh::ATTRIBUTE_UV_0, ConversionMode::TexCoord)), - gltf::Semantic::TexCoords(1) => Some((Mesh::ATTRIBUTE_UV_1, ConversionMode::TexCoord)), - gltf::Semantic::Joints(0) => { - Some((Mesh::ATTRIBUTE_JOINT_INDEX, ConversionMode::JointIndex)) + if let Some((attribute, conversion, convert_coordinates)) = match &semantic { + gltf::Semantic::Positions => Some(( + Mesh::ATTRIBUTE_POSITION, + ConversionMode::Any, + convert_coordinates, + )), + gltf::Semantic::Normals => Some(( + Mesh::ATTRIBUTE_NORMAL, + ConversionMode::Any, + convert_coordinates, + )), + gltf::Semantic::Tangents => Some(( + Mesh::ATTRIBUTE_TANGENT, + ConversionMode::Any, + convert_coordinates, + )), + gltf::Semantic::Colors(0) => Some((Mesh::ATTRIBUTE_COLOR, ConversionMode::Rgba, false)), + gltf::Semantic::TexCoords(0) => { + Some((Mesh::ATTRIBUTE_UV_0, ConversionMode::TexCoord, false)) } - gltf::Semantic::Weights(0) => { - Some((Mesh::ATTRIBUTE_JOINT_WEIGHT, ConversionMode::JointWeight)) + gltf::Semantic::TexCoords(1) => { + Some((Mesh::ATTRIBUTE_UV_1, ConversionMode::TexCoord, false)) } + gltf::Semantic::Joints(0) => Some(( + Mesh::ATTRIBUTE_JOINT_INDEX, + ConversionMode::JointIndex, + false, + )), + gltf::Semantic::Weights(0) => Some(( + Mesh::ATTRIBUTE_JOINT_WEIGHT, + ConversionMode::JointWeight, + false, + )), gltf::Semantic::Extras(name) => custom_vertex_attributes .get(name.as_str()) - .map(|attr| (*attr, ConversionMode::Any)), + .map(|attr| (*attr, ConversionMode::Any, false)), _ => None, } { let raw_iter = VertexAttributeIter::from_accessor(accessor.clone(), buffer_data); let converted_values = raw_iter.and_then(|iter| match conversion { - ConversionMode::Any => iter.into_any_values(), + ConversionMode::Any => iter.into_any_values(convert_coordinates), ConversionMode::Rgba => iter.into_rgba_values(), ConversionMode::TexCoord => iter.into_tex_coord_values(), ConversionMode::JointIndex => iter.into_joint_index_values(), diff --git a/release-content/release-notes/convert-coordinates.md b/release-content/release-notes/convert-coordinates.md new file mode 100644 index 0000000000000..22e69fd4efe78 --- /dev/null +++ b/release-content/release-notes/convert-coordinates.md @@ -0,0 +1,50 @@ +--- +title: Allow importing glTFs with a corrected coordinate system +authors: ["@janhohenheim"] +pull_requests: [19633] +--- + +glTF uses the following coordinate system: + +- forward: Z +- up: Y +- right: -X + +and Bevy uses: + +- forward: -Z +- up: Y +- right: X + +This means that to correctly import glTFs into Bevy, vertex data should be rotated by 180 degrees around the Y axis. +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! + +Long-term, we'd like to fix our glTF imports to use the correct coordinate system by default. +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`: + +```rust +// old behavior, ignores glTF's coordinate system +let handle = asset_server.load("fox.gltf#Scene0"); + +// new behavior, converts glTF's coordinate system into Bevy's coordinate system +let handle = asset_server.load_with_settings( + "fox.gltf#Scene0", + |settings: &mut GltfLoaderSettings| { + settings.convert_coordinates = true; + }, +); +``` + +Afterwards, your scene will be oriented such that your modeling software's forward direction correctly corresponds to Bevy's forward direction. + +For example, Blender assumes -Y to be forward, so exporting the following model to glTF and loading it in Bevy with the new settings will ensure everything is +oriented the right way across all programs in your pipeline: + + +![Blender Coordinate System](blender-coords.png) + +If you opt into this, please let us know how it's working out! Is your scene looking like you expected? Are the animations playing correctly? Is the camera at the right place? Are the lights shining from the right spots?