Fix normals during mesh scaling (#13380)

# Objective

- Fixes scaling normals and tangents of meshes

## Solution

- When scaling a mesh by `Vec3::new(1., 1., -1.)`, the normals should be
flipped along the Z-axis. For example a normal of `Vec3::new(0., 0.,
1.)` should become `Vec3::new(0., 0., -1.)` after scaling. This is
achieved by multiplying the normal by the reciprocal of the scale,
cheking for infinity and normalizing. Before, the normal was multiplied
by a covector of the scale, which is incorrect for normals.
- Tangents need to be multiplied by the `scale`, not its reciprocal as
before

---------

Co-authored-by: vero <11307157+atlv24@users.noreply.github.com>

crates/bevy_render/src/mesh/mesh/mod.rs

@@ -811,10 +811,9 @@ impl Mesh {
     /// Transforms the vertex positions, normals, and tangents of the mesh in place by the given [`Transform`].
     pub fn transform_by(&mut self, transform: Transform) {
         // Needed when transforming normals and tangents
-        let covector_scale = transform.scale.yzx() * transform.scale.zxy();
-
+        let scale_recip = 1. / transform.scale;
         debug_assert!(
-            covector_scale != Vec3::ZERO,
+            transform.scale.yzx() * transform.scale.zxy() != Vec3::ZERO,
             "mesh transform scale cannot be zero on more than one axis"
         );
 
@@ -840,8 +839,9 @@ impl Mesh {
         {
             // Transform normals, taking into account non-uniform scaling and rotation
             normals.iter_mut().for_each(|normal| {
-                let scaled_normal = Vec3::from_slice(normal) * covector_scale;
-                *normal = (transform.rotation * scaled_normal.normalize_or_zero()).to_array();
+                *normal = (transform.rotation
+                    * scale_normal(Vec3::from_array(*normal), scale_recip))
+                .to_array();
             });
         }
 
@@ -850,7 +850,7 @@ impl Mesh {
         {
             // Transform tangents, taking into account non-uniform scaling and rotation
             tangents.iter_mut().for_each(|tangent| {
-                let scaled_tangent = Vec3::from_slice(tangent) * covector_scale;
+                let scaled_tangent = Vec3::from_slice(tangent) * transform.scale;
                 *tangent = (transform.rotation * scaled_tangent.normalize_or_zero()).to_array();
             });
         }
@@ -928,10 +928,9 @@ impl Mesh {
     /// Scales the vertex positions, normals, and tangents of the mesh in place by the given [`Vec3`].
     pub fn scale_by(&mut self, scale: Vec3) {
         // Needed when transforming normals and tangents
-        let covector_scale = scale.yzx() * scale.zxy();
-
+        let scale_recip = 1. / scale;
         debug_assert!(
-            covector_scale != Vec3::ZERO,
+            scale.yzx() * scale.zxy() != Vec3::ZERO,
             "mesh transform scale cannot be zero on more than one axis"
         );
 
@@ -954,8 +953,7 @@ impl Mesh {
         {
             // Transform normals, taking into account non-uniform scaling
             normals.iter_mut().for_each(|normal| {
-                let scaled_normal = Vec3::from_slice(normal) * covector_scale;
-                *normal = scaled_normal.normalize_or_zero().to_array();
+                *normal = scale_normal(Vec3::from_array(*normal), scale_recip).to_array();
             });
         }
 
@@ -964,7 +962,7 @@ impl Mesh {
         {
             // Transform tangents, taking into account non-uniform scaling
             tangents.iter_mut().for_each(|tangent| {
-                let scaled_tangent = Vec3::from_slice(tangent) * covector_scale;
+                let scaled_tangent = Vec3::from_slice(tangent) * scale;
                 *tangent = scaled_tangent.normalize_or_zero().to_array();
             });
         }
@@ -1744,10 +1742,27 @@ fn generate_tangents_for_mesh(mesh: &Mesh) -> Result<Vec<[f32; 4]>, GenerateTang
     Ok(mikktspace_mesh.tangents)
 }
 
+/// Correctly scales and renormalizes an already normalized `normal` by the scale determined by its reciprocal `scale_recip`
+fn scale_normal(normal: Vec3, scale_recip: Vec3) -> Vec3 {
+    // This is basically just `normal * scale_recip` but with the added rule that `0. * anything == 0.`
+    // This is necessary because components of `scale_recip` may be infinities, which do not multiply to zero
+    let n = Vec3::select(normal.cmpeq(Vec3::ZERO), Vec3::ZERO, normal * scale_recip);
+
+    // If n is finite, no component of `scale_recip` was infinite or the normal was perpendicular to the scale
+    // else the scale had at least one zero-component and the normal needs to point along the direction of that component
+    if n.is_finite() {
+        n.normalize_or_zero()
+    } else {
+        Vec3::select(n.abs().cmpeq(Vec3::INFINITY), n.signum(), Vec3::ZERO).normalize()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::Mesh;
-    use crate::render_asset::RenderAssetUsages;
+    use crate::{mesh::VertexAttributeValues, render_asset::RenderAssetUsages};
+    use bevy_math::Vec3;
+    use bevy_transform::components::Transform;
     use wgpu::PrimitiveTopology;
 
     #[test]
@@ -1759,4 +1774,56 @@ mod tests {
         )
         .with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, vec![[0.0, 0.0, 0.0]]);
     }
+
+    #[test]
+    fn transform_mesh() {
+        let mesh = Mesh::new(
+            PrimitiveTopology::TriangleList,
+            RenderAssetUsages::default(),
+        )
+        .with_inserted_attribute(
+            Mesh::ATTRIBUTE_POSITION,
+            vec![[-1., -1., 2.], [1., -1., 2.], [0., 1., 2.]],
+        )
+        .with_inserted_attribute(
+            Mesh::ATTRIBUTE_NORMAL,
+            vec![
+                Vec3::new(-1., -1., 1.).normalize().to_array(),
+                Vec3::new(1., -1., 1.).normalize().to_array(),
+                [0., 0., 1.],
+            ],
+        )
+        .with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, vec![[0., 0.], [1., 0.], [0.5, 1.]]);
+
+        let mesh = mesh.transformed_by(
+            Transform::from_translation(Vec3::splat(-2.)).with_scale(Vec3::new(2., 0., -1.)),
+        );
+
+        if let Some(VertexAttributeValues::Float32x3(positions)) =
+            mesh.attribute(Mesh::ATTRIBUTE_POSITION)
+        {
+            // All positions are first scaled resulting in `vec![[-2, 0., -2.], [2., 0., -2.], [0., 0., -2.]]`
+            // and then shifted by `-2.` along each axis
+            assert_eq!(
+                positions,
+                &vec![[-4.0, -2.0, -4.0], [0.0, -2.0, -4.0], [-2.0, -2.0, -4.0]]
+            );
+        } else {
+            panic!("Mesh does not have a position attribute");
+        }
+
+        if let Some(VertexAttributeValues::Float32x3(normals)) =
+            mesh.attribute(Mesh::ATTRIBUTE_NORMAL)
+        {
+            assert_eq!(normals, &vec![[0., -1., 0.], [0., -1., 0.], [0., 0., -1.]]);
+        } else {
+            panic!("Mesh does not have a normal attribute");
+        }
+
+        if let Some(VertexAttributeValues::Float32x2(uvs)) = mesh.attribute(Mesh::ATTRIBUTE_UV_0) {
+            assert_eq!(uvs, &vec![[0., 0.], [1., 0.], [0.5, 1.]]);
+        } else {
+            panic!("Mesh does not have a uv attribute");
+        }
+    }
 }