From 5a36a616d5e71dca9178683845aab8b267d0c754 Mon Sep 17 00:00:00 2001 From: Dusty DeWeese Date: Mon, 5 Sep 2022 16:50:59 -0700 Subject: [PATCH] Revert "bevy_pbr: Fix tangent and normal normalization (#5666)" This reverts commit 681c9c6dc859be52565609e56ce9dc7f6f1c1f12. --- crates/bevy_pbr/src/render/mesh.rs | 10 +--- .../bevy_pbr/src/render/mesh_functions.wgsl | 49 +++++-------------- crates/bevy_pbr/src/render/mesh_types.wgsl | 2 - crates/bevy_pbr/src/render/pbr_functions.wgsl | 8 +-- 4 files changed, 14 insertions(+), 55 deletions(-) diff --git a/crates/bevy_pbr/src/render/mesh.rs b/crates/bevy_pbr/src/render/mesh.rs index 72bc2a5a6e13c..1ede0789564ec 100644 --- a/crates/bevy_pbr/src/render/mesh.rs +++ b/crates/bevy_pbr/src/render/mesh.rs @@ -18,7 +18,7 @@ use bevy_ecs::{ query::ROQueryItem, system::{lifetimeless::*, SystemParamItem, SystemState}, }; -use bevy_math::{Mat3A, Mat4, Vec2}; +use bevy_math::{Mat4, Vec2}; use bevy_reflect::TypeUuid; use bevy_render::{ extract_component::{ComponentUniforms, DynamicUniformIndex, UniformComponentPlugin}, @@ -134,9 +134,6 @@ 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; } @@ -167,14 +164,11 @@ pub fn extract_meshes( { let transform = transform.compute_matrix(); let previous_transform = previous_transform.map(|t| t.0).unwrap_or(transform); - let mut flags = if not_receiver.is_some() { + let flags = if not_receiver.is_some() { MeshFlags::empty() } else { MeshFlags::SHADOW_RECEIVER }; - if Mat3A::from_mat4(transform).determinant().is_sign_positive() { - flags |= MeshFlags::SIGN_DETERMINANT_MODEL_3X3; - } let uniform = MeshUniform { flags: flags.bits, transform, diff --git a/crates/bevy_pbr/src/render/mesh_functions.wgsl b/crates/bevy_pbr/src/render/mesh_functions.wgsl index 40779b28d5c6e..20c763bd22d79 100644 --- a/crates/bevy_pbr/src/render/mesh_functions.wgsl +++ b/crates/bevy_pbr/src/render/mesh_functions.wgsl @@ -17,47 +17,20 @@ fn mesh_position_local_to_clip(model: mat4x4, vertex_position: vec4) - } fn mesh_normal_local_to_world(vertex_normal: vec3) -> vec3 { - // 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( - 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; + return mat3x3( + 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, vertex_tangent: vec4) -> vec4 { - // 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( - normalize( - mat3x3( - 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() + mat3x3( + model[0].xyz, + model[1].xyz, + model[2].xyz + ) * vertex_tangent.xyz, + vertex_tangent.w ); } diff --git a/crates/bevy_pbr/src/render/mesh_types.wgsl b/crates/bevy_pbr/src/render/mesh_types.wgsl index d2e3276fe696a..466a27c646b73 100644 --- a/crates/bevy_pbr/src/render/mesh_types.wgsl +++ b/crates/bevy_pbr/src/render/mesh_types.wgsl @@ -15,5 +15,3 @@ struct SkinnedMesh { #endif const MESH_FLAGS_SHADOW_RECEIVER_BIT: u32 = 1u; -// 2^31 - if the flag is set, the sign is positive, else it is negative -const MESH_FLAGS_SIGN_DETERMINANT_MODEL_3X3_BIT: u32 = 2147483648u; diff --git a/crates/bevy_pbr/src/render/pbr_functions.wgsl b/crates/bevy_pbr/src/render/pbr_functions.wgsl index 1bc782fc19f3d..3b75d18024bd7 100644 --- a/crates/bevy_pbr/src/render/pbr_functions.wgsl +++ b/crates/bevy_pbr/src/render/pbr_functions.wgsl @@ -55,13 +55,7 @@ fn apply_normal_mapping( uv: vec2, #endif ) -> vec3 { - // 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 = world_normal; + var N: vec3 = normalize(world_normal); #ifdef VERTEX_TANGENTS #ifdef STANDARDMATERIAL_NORMAL_MAP