From c78412a2ca8a6ca2a49429df4290519b75280bed Mon Sep 17 00:00:00 2001
From: Robert Swain <robert.swain@gmail.com>
Date: Sat, 13 Aug 2022 09:27:00 +0200
Subject: [PATCH 1/2] bevy_pbr: Fix tangent and normal normalization

Morten Mikkelsen clarified that the world normal and tangent must be normalized
in the vertex stage and the interpolated values must not be normalized in the
fragment stage.
---
 .../bevy_pbr/src/render/mesh_functions.wgsl   | 24 +++++++++++--------
 crates/bevy_pbr/src/render/pbr_functions.wgsl |  4 ++--
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/crates/bevy_pbr/src/render/mesh_functions.wgsl b/crates/bevy_pbr/src/render/mesh_functions.wgsl
index 20c763bd22d79..ee5f160549501 100644
--- a/crates/bevy_pbr/src/render/mesh_functions.wgsl
+++ b/crates/bevy_pbr/src/render/mesh_functions.wgsl
@@ -17,20 +17,24 @@ fn mesh_position_local_to_clip(model: mat4x4<f32>, vertex_position: vec4<f32>) -
 }
 
 fn mesh_normal_local_to_world(vertex_normal: vec3<f32>) -> vec3<f32> {
-    return mat3x3<f32>(
-        mesh.inverse_transpose_model[0].xyz,
-        mesh.inverse_transpose_model[1].xyz,
-        mesh.inverse_transpose_model[2].xyz
-    ) * vertex_normal;
+    return normalize(
+        mat3x3<f32>(
+            mesh.inverse_transpose_model[0].xyz,
+            mesh.inverse_transpose_model[1].xyz,
+            mesh.inverse_transpose_model[2].xyz
+        ) * vertex_normal
+    );
 }
 
 fn mesh_tangent_local_to_world(model: mat4x4<f32>, vertex_tangent: vec4<f32>) -> vec4<f32> {
     return vec4<f32>(
-        mat3x3<f32>(
-            model[0].xyz,
-            model[1].xyz,
-            model[2].xyz
-        ) * vertex_tangent.xyz,
+        normalize(
+            mat3x3<f32>(
+                model[0].xyz,
+                model[1].xyz,
+                model[2].xyz
+            ) * vertex_tangent.xyz
+        ),
         vertex_tangent.w
     );
 }
diff --git a/crates/bevy_pbr/src/render/pbr_functions.wgsl b/crates/bevy_pbr/src/render/pbr_functions.wgsl
index cd922bf74412c..5db9b61140ddc 100644
--- a/crates/bevy_pbr/src/render/pbr_functions.wgsl
+++ b/crates/bevy_pbr/src/render/pbr_functions.wgsl
@@ -15,7 +15,7 @@ fn prepare_normal(
 #endif
     is_front: bool,
 ) -> vec3<f32> {
-    var N: vec3<f32> = normalize(world_normal);
+    var N: vec3<f32> = world_normal;
 
 #ifdef VERTEX_TANGENTS
 #ifdef STANDARDMATERIAL_NORMAL_MAP
@@ -236,7 +236,7 @@ fn pbr(
 fn tone_mapping(in: vec4<f32>) -> vec4<f32> {
     // tone_mapping
     return vec4<f32>(reinhard_luminance(in.rgb), in.a);
-    
+
     // Gamma correction.
     // Not needed with sRGB buffer
     // output_color.rgb = pow(output_color.rgb, vec3(1.0 / 2.2));

From d3f69f46e4e94a62a926006d3dbaa1af510da54c Mon Sep 17 00:00:00 2001
From: Robert Swain <robert.swain@gmail.com>
Date: Sat, 13 Aug 2022 12:40:15 +0200
Subject: [PATCH 2/2] bevy_pbr: Add comments clarifying why tangent/normals are
 processed this way

---
 crates/bevy_pbr/src/render/mesh_functions.wgsl | 12 ++++++++++++
 crates/bevy_pbr/src/render/pbr_functions.wgsl  |  6 ++++++
 2 files changed, 18 insertions(+)

diff --git a/crates/bevy_pbr/src/render/mesh_functions.wgsl b/crates/bevy_pbr/src/render/mesh_functions.wgsl
index ee5f160549501..6c50c7369a4de 100644
--- a/crates/bevy_pbr/src/render/mesh_functions.wgsl
+++ b/crates/bevy_pbr/src/render/mesh_functions.wgsl
@@ -17,6 +17,12 @@ fn mesh_position_local_to_clip(model: mat4x4<f32>, vertex_position: vec4<f32>) -
 }
 
 fn mesh_normal_local_to_world(vertex_normal: vec3<f32>) -> vec3<f32> {
+    // 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. Do not change this code
+    // unless you really know what you are doing.
+    // http://www.mikktspace.com/
     return normalize(
         mat3x3<f32>(
             mesh.inverse_transpose_model[0].xyz,
@@ -27,6 +33,12 @@ fn mesh_normal_local_to_world(vertex_normal: vec3<f32>) -> vec3<f32> {
 }
 
 fn mesh_tangent_local_to_world(model: mat4x4<f32>, vertex_tangent: vec4<f32>) -> 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. Do not change this code
+    // unless you really know what you are doing.
+    // http://www.mikktspace.com/
     return vec4<f32>(
         normalize(
             mat3x3<f32>(
diff --git a/crates/bevy_pbr/src/render/pbr_functions.wgsl b/crates/bevy_pbr/src/render/pbr_functions.wgsl
index 5db9b61140ddc..a77c065fca221 100644
--- a/crates/bevy_pbr/src/render/pbr_functions.wgsl
+++ b/crates/bevy_pbr/src/render/pbr_functions.wgsl
@@ -15,6 +15,12 @@ fn prepare_normal(
 #endif
     is_front: bool,
 ) -> vec3<f32> {
+    // NOTE: The mikktspace method of normal mapping explicitly requires that the world normal NOT
+    // be re-normalized in the fragment shader. This is primarily to match the way mikktspace
+    // bakes vertex tangents and normal maps so that this is the exact inverse. Blender, Unity,
+    // Unreal Engine, Godot, and more all use the mikktspace method. Do not change this code
+    // unless you really know what you are doing.
+    // http://www.mikktspace.com/
     var N: vec3<f32> = world_normal;
 
 #ifdef VERTEX_TANGENTS