Skip to content

Commit

Permalink
bevy_pbr: Fix tangent and normal normalization (#5666)
Browse files Browse the repository at this point in the history
# Objective

- 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. This is in order to match the mikktspace approach exactly.
- Fixes #5514 by ensuring the tangent basis matrix (TBN) is orthonormal

## Solution

- Normalize the world normal in the vertex stage and not the fragment stage
- Normalize the world tangent xyz in the vertex stage
- Take into account the sign of the determinant of the local to world matrix when calculating the bitangent

---

## Changelog

- Fixed - scaling a model that uses normal mapping now has correct lighting again
  • Loading branch information
superdump authored and cart committed Aug 18, 2022
1 parent b3ab4fc commit a7ece4c
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 18 deletions.
16 changes: 11 additions & 5 deletions crates/bevy_pbr/src/render/mesh.rs
Expand Up @@ -9,7 +9,7 @@ use bevy_ecs::{
prelude::*,
system::{lifetimeless::*, SystemParamItem, SystemState},
};
use bevy_math::{Mat4, Vec2};
use bevy_math::{Mat3A, Mat4, Vec2};
use bevy_reflect::TypeUuid;
use bevy_render::{
extract_component::{ComponentUniforms, DynamicUniformIndex, UniformComponentPlugin},
Expand Down Expand Up @@ -117,6 +117,9 @@ bitflags::bitflags! {
#[repr(transparent)]
struct MeshFlags: u32 {
const SHADOW_RECEIVER = (1 << 0);
// 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;
}
Expand All @@ -143,13 +146,16 @@ pub fn extract_meshes(

for (entity, _, transform, handle, not_receiver, not_caster) in visible_meshes {
let transform = transform.compute_matrix();
let shadow_receiver_flags = if not_receiver.is_some() {
MeshFlags::empty().bits
let mut flags = if not_receiver.is_some() {
MeshFlags::empty()
} else {
MeshFlags::SHADOW_RECEIVER.bits
MeshFlags::SHADOW_RECEIVER
};
if Mat3A::from_mat4(transform).determinant().is_sign_positive() {
flags |= MeshFlags::SIGN_DETERMINANT_MODEL_3X3;
}
let uniform = MeshUniform {
flags: shadow_receiver_flags,
flags: flags.bits,
transform,
inverse_transpose_model: transform.inverse().transpose(),
};
Expand Down
49 changes: 38 additions & 11 deletions crates/bevy_pbr/src/render/mesh_functions.wgsl
Expand Up @@ -17,20 +17,47 @@ 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;
// 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,
mesh.inverse_transpose_model[1].xyz,
mesh.inverse_transpose_model[2].xyz
) * vertex_normal
);
}

// Calculates the sign of the determinant of the 3x3 model matrix based on a
// mesh flag
fn sign_determinant_model_3x3() -> 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;
}

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>(
mat3x3<f32>(
model[0].xyz,
model[1].xyz,
model[2].xyz
) * vertex_tangent.xyz,
vertex_tangent.w
normalize(
mat3x3<f32>(
model[0].xyz,
model[1].xyz,
model[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_3x3()
);
}
2 changes: 2 additions & 0 deletions crates/bevy_pbr/src/render/mesh_types.wgsl
Expand Up @@ -14,3 +14,5 @@ struct SkinnedMesh {
#endif

let MESH_FLAGS_SHADOW_RECEIVER_BIT: u32 = 1u;
// 2^31 - if the flag is set, the sign is positive, else it is negative
let MESH_FLAGS_SIGN_DETERMINANT_MODEL_3X3_BIT: u32 = 2147483648u;
10 changes: 8 additions & 2 deletions crates/bevy_pbr/src/render/pbr_functions.wgsl
Expand Up @@ -15,7 +15,13 @@ fn prepare_normal(
#endif
is_front: bool,
) -> vec3<f32> {
var N: vec3<f32> = normalize(world_normal);
// 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
#ifdef STANDARDMATERIAL_NORMAL_MAP
Expand Down Expand Up @@ -236,7 +242,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));
Expand Down

0 comments on commit a7ece4c

Please sign in to comment.