Fix incorrect Mesh2d world normal and tangent computation

_release-content/migration-guides/mesh2d_world_normal_and_tangent.md

@@ -0,0 +1,38 @@
+---
+title: "`Mesh2d` world normal and tangent computation"
+pull_requests: [25369]
+---
+
+`mesh2d_tangent_local_to_world` in `bevy_sprite_render::mesh2d::functions` now takes the
+instance index as a third argument, so that it can read the mesh flags to correct the
+tangent's sign for mirrored transforms.
+
+It also now normalizes the world tangent, and `mesh2d_normal_local_to_world` now normalizes
+the world normal. Previously neither was normalized, so a scaled `Transform` produced a
+`world_normal` scaled by the inverse of the scale and a `world_tangent` scaled by the scale.
+This matches the 3D behavior in `bevy_pbr`.
+
+If you call `mesh2d_tangent_local_to_world` from a custom 2D vertex shader, pass the vertex's
+instance index:
+
+```wgsl
+// Before
+out.world_tangent = mesh_functions::mesh2d_tangent_local_to_world(
+    world_from_local,
+    vertex.tangent
+);
+
+// After
+out.world_tangent = mesh_functions::mesh2d_tangent_local_to_world(
+    world_from_local,
+    vertex.tangent,
+    vertex.instance_index
+);
+```
+
+If you were compensating for the missing normalization by normalizing `world_normal` or
+`world_tangent` yourself in a fragment shader, that is now redundant but harmless.
+
+`MeshFlags::SIGN_DETERMINANT_MODEL_3X3` is new, but you do not need to set it. It is derived
+from the transform in `Mesh2dUniform::from_components`, so custom extraction systems that
+build `Mesh2dTransforms` with `MeshFlags::empty()` keep working unchanged.

assets/shaders/mesh2d_tangents.wesl

@@ -0,0 +1,26 @@
+// Visual regression test for `Mesh2d` tangent space. Red means a vector is not unit length.
+import bevy_sprite_render::mesh2d::vertex_output::VertexOutput;
+
+fn tangent_space_color(v: vec3<f32>) -> vec3<f32> {
+    // Checked before remapping, because an over-long vector remaps to the same color as a unit one.
+    // Loose enough to absorb interpolation and f32 error.
+    if abs(length(v) - 1.0) > 0.01 {
+        return vec3<f32>(1.0, 0.0, 0.0);
+    }
+    return v * 0.5 + 0.5;
+}
+
+@fragment
+fn fragment(mesh: VertexOutput) -> @location(0) vec4<f32> {
+    if mesh.uv.y < 0.4 {
+        return vec4<f32>(tangent_space_color(mesh.world_normal), 1.0);
+    }
+    if mesh.uv.y < 0.8 {
+        return vec4<f32>(tangent_space_color(mesh.world_tangent.xyz), 1.0);
+    }
+    // Handedness.
+    if mesh.world_tangent.w > 0.0 {
+        return vec4<f32>(0.0, 0.4, 1.0, 1.0);
+    }
+    return vec4<f32>(1.0, 0.9, 0.0, 1.0);
+}

crates/bevy_sprite_render/src/mesh2d/functions.wesl

@@ -1,6 +1,7 @@
 import bevy_sprite_render::mesh2d::{
     view_bindings::view,
     bindings::{mesh, metadata},
+    types::MESH_FLAGS_SIGN_DETERMINANT_MODEL_3X3_BIT,
 };
 import bevy_render::mesh::metadata_types::MeshMetadata;
 import bevy_render::maths::{affine3_to_square, mat2x4_f32_to_mat3x3_unpack};
@@ -25,22 +26,62 @@ fn mesh2d_position_local_to_clip(world_from_local: mat4x4<f32>, vertex_position:
     return mesh2d_position_world_to_clip(world_position);
 }
 
+// Matches the `mesh_normal_local_to_world` function in `bevy_pbr/mesh_functions.wesl`.
 fn mesh2d_normal_local_to_world(vertex_normal: vec3<f32>, instance_index: u32) -> vec3<f32> {
-    return mat2x4_f32_to_mat3x3_unpack(
-        mesh[instance_index].local_from_world_transpose_a,
-        mesh[instance_index].local_from_world_transpose_b,
-    ) * vertex_normal;
-}
-
-fn mesh2d_tangent_local_to_world(world_from_local: mat4x4<f32>, vertex_tangent: vec4<f32>) -> vec4<f32> {
-    return vec4<f32>(
-        mat3x3<f32>(
-            world_from_local[0].xyz,
-            world_from_local[1].xyz,
-            world_from_local[2].xyz
-        ) * vertex_tangent.xyz,
-        vertex_tangent.w
-    );
+    // NOTE: The mikktspace method of normal mapping requires that the world normal is
+    // re-normalized in the vertex shader to match the way mikktspace bakes vertex tangents
+    // and normal maps so that the exact inverse process is applied when shading. Blender, Unity,
+    // Unreal Engine, Godot, and more all use the mikktspace method.
+    // We only skip normalization for invalid normals so that they don't become NaN.
+    // Do not change this code unless you really know what you are doing.
+    // http://www.mikktspace.com/
+    if any(vertex_normal != vec3<f32>(0.0)) {
+        return normalize(
+            mat2x4_f32_to_mat3x3_unpack(
+                mesh[instance_index].local_from_world_transpose_a,
+                mesh[instance_index].local_from_world_transpose_b,
+            ) * vertex_normal
+        );
+    } else {
+        return vertex_normal;
+    }
+}
+
+// Matches the `sign_determinant_model_3x3m` function in `bevy_pbr/mesh_functions.wesl`.
+// Calculates the sign of the determinant of the 3x3 model matrix based on a
+// mesh flag
+fn sign_determinant_model_3x3m(mesh_flags: u32) -> f32 {
+    // bool(u32) is false if 0u else true
+    // f32(bool) is 1.0 if true else 0.0
+    // * 2.0 - 1.0 remaps 0.0 or 1.0 to -1.0 or 1.0 respectively
+    return f32(bool(mesh_flags & MESH_FLAGS_SIGN_DETERMINANT_MODEL_3X3_BIT)) * 2.0 - 1.0;
+}
+
+// Matches the `mesh_tangent_local_to_world` function in `bevy_pbr/mesh_functions.wesl`.
+fn mesh2d_tangent_local_to_world(world_from_local: mat4x4<f32>, vertex_tangent: vec4<f32>, instance_index: u32) -> vec4<f32> {
+    // NOTE: The mikktspace method of normal mapping requires that the world tangent is
+    // re-normalized in the vertex shader to match the way mikktspace bakes vertex tangents
+    // and normal maps so that the exact inverse process is applied when shading. Blender, Unity,
+    // Unreal Engine, Godot, and more all use the mikktspace method.
+    // We only skip normalization for invalid tangents so that they don't become NaN.
+    // Do not change this code unless you really know what you are doing.
+    // http://www.mikktspace.com/
+    if any(vertex_tangent != vec4<f32>(0.0)) {
+        return vec4<f32>(
+            normalize(
+                mat3x3<f32>(
+                    world_from_local[0].xyz,
+                    world_from_local[1].xyz,
+                    world_from_local[2].xyz
+                ) * vertex_tangent.xyz
+            ),
+            // NOTE: Multiplying by the sign of the determinant of the 3x3 model matrix accounts for
+            // situations such as negative scaling.
+            vertex_tangent.w * sign_determinant_model_3x3m(mesh[instance_index].flags)
+        );
+    } else {
+        return vertex_tangent;
+    }
 }
 
 fn get_tag(instance_index: u32) -> u32 {

crates/bevy_sprite_render/src/mesh2d/mesh.rs

@@ -244,6 +244,9 @@ pub struct Mesh2dUniform {
 impl Mesh2dUniform {
     /// Creates a new [`Mesh2dUniform`] from the given transform, bind group
     /// slot, tag, and optional metadata index.
+    ///
+    /// [`MeshFlags::SIGN_DETERMINANT_MODEL_3X3`] is derived from the transform, so callers do not
+    /// need to set it in [`Mesh2dTransforms::flags`].
     pub fn from_components(
         mesh_transforms: &Mesh2dTransforms,
         material_bind_group_slot: MaterialBindGroupSlot,
@@ -252,22 +255,37 @@ impl Mesh2dUniform {
     ) -> Self {
         let (local_from_world_transpose_a, local_from_world_transpose_b) =
             mesh_transforms.world_from_local.inverse_transpose_3x3();
+
+        // A mirroring transform flips the sign of the tangent.
+        let mut flags = MeshFlags::from_bits_retain(mesh_transforms.flags);
+        if mesh_transforms
+            .world_from_local
+            .matrix3
+            .determinant()
+            .is_sign_positive()
+        {
+            flags |= MeshFlags::SIGN_DETERMINANT_MODEL_3X3;
+        }
+
         Self {
             world_from_local: mesh_transforms.world_from_local.to_transpose(),
             local_from_world_transpose_a,
             local_from_world_transpose_b,
             material_bind_group_slot: material_bind_group_slot.0,
-            flags: mesh_transforms.flags,
+            flags: flags.bits(),
             tag,
             metadata_index: metadata_index.unwrap_or(0),
         }
     }
 }
 
-// NOTE: These must match the bit flags in bevy_sprite_render/src/mesh2d/mesh2d.wesl!
+// NOTE: These must match the bit flags in bevy_sprite_render/src/mesh2d/types.wesl!
 bitflags::bitflags! {
     #[repr(transparent)]
     pub struct MeshFlags: u32 {
+        /// Indicates the sign of the determinant of the 3x3 model matrix. If the sign is positive,
+        /// then the flag should be set, else it should not be set.
+        const SIGN_DETERMINANT_MODEL_3X3 = 1 << 31;
         const NONE                       = 0;
         const UNINITIALIZED              = 0xFFFF;
     }

crates/bevy_sprite_render/src/mesh2d/mesh2d.wesl

@@ -35,7 +35,8 @@ fn vertex(vertex: Vertex) -> VertexOutput {
 @if(VERTEX_TANGENTS)
     out.world_tangent = mesh_functions::mesh2d_tangent_local_to_world(
         world_from_local,
-        uncompressed_vertex.tangent
+        uncompressed_vertex.tangent,
+        vertex.instance_index
     );
 
 @if(VERTEX_COLORS)

crates/bevy_sprite_render/src/mesh2d/types.wesl

@@ -17,3 +17,6 @@ struct Mesh2d {
     /// The index of the mesh metadata buffer.
     metadata_index: u32,
 };
+
+// if the flag is set, the sign is positive, else it is negative
+const MESH_FLAGS_SIGN_DETERMINANT_MODEL_3X3_BIT: u32 = 1u << 31u;

crates/bevy_sprite_render/src/sprite_mesh/sprite_material.wesl

@@ -56,7 +56,8 @@ fn vertex(vertex: Vertex) -> VertexOutput {
 @if(VERTEX_TANGENTS)
     out.world_tangent = mesh_functions::mesh2d_tangent_local_to_world(
         world_from_local,
-        uncompressed_vertex.tangent
+        uncompressed_vertex.tangent,
+        instance_index
     );
 
 @if(VERTEX_COLORS)

examples/testbed/2d.rs

@@ -5,7 +5,7 @@
 mod helpers;
 
 use argh::FromArgs;
-use bevy::prelude::*;
+use bevy::{prelude::*, sprite_render::Material2dPlugin};
 
 use helpers::Next;
 
@@ -24,6 +24,7 @@ fn main() {
 
     let mut app = App::new();
     app.add_plugins((DefaultPlugins,))
+        .add_plugins(Material2dPlugin::<tangents::TangentMaterial>::default())
         .add_systems(OnEnter(Scene::Shapes), shapes::setup)
         .add_systems(OnEnter(Scene::Bloom), bloom::setup)
         .add_systems(OnEnter(Scene::Text), text::setup)
@@ -36,6 +37,7 @@ fn main() {
         )
         .add_systems(OnEnter(Scene::ColorConsistency), color_consistency::setup)
         .add_systems(OnExit(Scene::ColorConsistency), color_consistency::teardown)
+        .add_systems(OnEnter(Scene::Tangents), tangents::setup)
         .add_systems(Update, switch_scene)
         .add_systems(Update, gizmos::draw_gizmos.run_if(in_state(Scene::Gizmos)));
 
@@ -61,6 +63,7 @@ enum Scene {
     Gizmos,
     TextureAtlasBuilder,
     ColorConsistency,
+    Tangents,
 }
 
 impl Scene {
@@ -73,6 +76,7 @@ impl Scene {
         Scene::Gizmos,
         Scene::TextureAtlasBuilder,
         Scene::ColorConsistency,
+        Scene::Tangents,
     ];
 }
 
@@ -631,3 +635,56 @@ mod color_consistency {
         commands.insert_resource(ClearColor::default());
     }
 }
+
+mod tangents {
+    use bevy::{
+        prelude::*, reflect::TypePath, render::render_resource::AsBindGroup, shader::ShaderRef,
+        sprite_render::Material2d,
+    };
+
+    const SHADER_ASSET_PATH: &str = "shaders/mesh2d_tangents.wesl";
+
+    /// Draws the mesh's tangent space, turning red where a vector is not unit length.
+    #[derive(Asset, TypePath, AsBindGroup, Debug, Clone, Default)]
+    pub struct TangentMaterial {}
+
+    impl Material2d for TangentMaterial {
+        fn fragment_shader() -> ShaderRef {
+            SHADER_ASSET_PATH.into()
+        }
+    }
+
+    pub fn setup(
+        mut commands: Commands,
+        mut meshes: ResMut<Assets<Mesh>>,
+        mut materials: ResMut<Assets<TangentMaterial>>,
+    ) {
+        commands.spawn((Camera2d, DespawnOnExit(super::Scene::Tangents)));
+
+        let mesh = meshes.add(
+            Rectangle::default()
+                .mesh()
+                .build()
+                .with_generated_tangents()
+                .unwrap(),
+        );
+        let material = materials.add(TangentMaterial {});
+
+        // Scale z uniformly with x and y: at a z scale of 1 the inverse transpose leaves the
+        // normal unit length anyway, and a missing normal normalization would go unnoticed.
+        commands.spawn((
+            Mesh2d(mesh.clone()),
+            MeshMaterial2d(material.clone()),
+            Transform::from_xyz(-160.0, 0.0, 0.0).with_scale(Vec3::splat(256.0)),
+            DespawnOnExit(super::Scene::Tangents),
+        ));
+
+        // Mirrored on X, which flips the handedness of the tangent.
+        commands.spawn((
+            Mesh2d(mesh),
+            MeshMaterial2d(material),
+            Transform::from_xyz(160.0, 0.0, 0.0).with_scale(Vec3::new(-256.0, 256.0, 1.0)),
+            DespawnOnExit(super::Scene::Tangents),
+        ));
+    }
+}