From 8025c0f2296b65c8d1c821ec636ceadecc87a920 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Mon, 22 Aug 2022 17:45:15 +0200 Subject: [PATCH 1/3] bevy_pbr: Remove incorrect and unnecessary normal-mapping code The FlightHelmet.gltf model's hose uses a double-sided material. Loading the model with a uniform scale of -1.0, and comparing against Blender, it was identified that negating the world-space tangent, bitangent, and interpolated normal produces incorrect lighting. Discussion with Morten Mikkelsen clarified that this is both incorrect and unnecessary. --- assets/shaders/array_texture.wgsl | 2 -- crates/bevy_pbr/src/render/mesh.wgsl | 1 - crates/bevy_pbr/src/render/pbr.wgsl | 2 -- crates/bevy_pbr/src/render/pbr_functions.wgsl | 13 ------------- crates/bevy_sprite/src/mesh2d/color_material.wgsl | 1 - crates/bevy_sprite/src/mesh2d/mesh2d.wgsl | 1 - 6 files changed, 20 deletions(-) diff --git a/assets/shaders/array_texture.wgsl b/assets/shaders/array_texture.wgsl index 91f4564e7fe27..4281da0b15308 100644 --- a/assets/shaders/array_texture.wgsl +++ b/assets/shaders/array_texture.wgsl @@ -14,7 +14,6 @@ var my_array_texture: texture_2d_array; var my_array_texture_sampler: sampler; struct FragmentInput { - @builtin(front_facing) is_front: bool, @builtin(position) frag_coord: vec4, #import bevy_pbr::mesh_vertex_output }; @@ -47,7 +46,6 @@ fn fragment(in: FragmentInput) -> @location(0) vec4 { #endif #endif in.uv, - in.is_front, ); pbr_input.V = calculate_view(in.world_position, pbr_input.is_orthographic); diff --git a/crates/bevy_pbr/src/render/mesh.wgsl b/crates/bevy_pbr/src/render/mesh.wgsl index 32cbe31c8d161..b60b13ee73363 100644 --- a/crates/bevy_pbr/src/render/mesh.wgsl +++ b/crates/bevy_pbr/src/render/mesh.wgsl @@ -53,7 +53,6 @@ fn vertex(vertex: Vertex) -> VertexOutput { } struct FragmentInput { - @builtin(front_facing) is_front: bool, #import bevy_pbr::mesh_vertex_output }; diff --git a/crates/bevy_pbr/src/render/pbr.wgsl b/crates/bevy_pbr/src/render/pbr.wgsl index b70b5b333943c..2cec91fcd9b5d 100644 --- a/crates/bevy_pbr/src/render/pbr.wgsl +++ b/crates/bevy_pbr/src/render/pbr.wgsl @@ -9,7 +9,6 @@ #import bevy_pbr::pbr_functions struct FragmentInput { - @builtin(front_facing) is_front: bool, @builtin(position) frag_coord: vec4, #import bevy_pbr::mesh_vertex_output }; @@ -84,7 +83,6 @@ fn fragment(in: FragmentInput) -> @location(0) vec4 { #ifdef VERTEX_UVS in.uv, #endif - in.is_front, ); pbr_input.V = calculate_view(in.world_position, pbr_input.is_orthographic); diff --git a/crates/bevy_pbr/src/render/pbr_functions.wgsl b/crates/bevy_pbr/src/render/pbr_functions.wgsl index a77c065fca221..2c5f08bc0a7bb 100644 --- a/crates/bevy_pbr/src/render/pbr_functions.wgsl +++ b/crates/bevy_pbr/src/render/pbr_functions.wgsl @@ -13,7 +13,6 @@ fn prepare_normal( #ifdef VERTEX_UVS uv: vec2, #endif - is_front: bool, ) -> 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 @@ -34,18 +33,6 @@ fn prepare_normal( #endif #endif - if ((standard_material_flags & STANDARD_MATERIAL_FLAGS_DOUBLE_SIDED_BIT) != 0u) { - if (!is_front) { - N = -N; -#ifdef VERTEX_TANGENTS -#ifdef STANDARDMATERIAL_NORMAL_MAP - T = -T; - B = -B; -#endif -#endif - } - } - #ifdef VERTEX_TANGENTS #ifdef VERTEX_UVS #ifdef STANDARDMATERIAL_NORMAL_MAP diff --git a/crates/bevy_sprite/src/mesh2d/color_material.wgsl b/crates/bevy_sprite/src/mesh2d/color_material.wgsl index 3b5893d4ce285..0e83d1487dcf2 100644 --- a/crates/bevy_sprite/src/mesh2d/color_material.wgsl +++ b/crates/bevy_sprite/src/mesh2d/color_material.wgsl @@ -19,7 +19,6 @@ var texture_sampler: sampler; var mesh: Mesh2d; struct FragmentInput { - @builtin(front_facing) is_front: bool, #import bevy_sprite::mesh2d_vertex_output }; diff --git a/crates/bevy_sprite/src/mesh2d/mesh2d.wgsl b/crates/bevy_sprite/src/mesh2d/mesh2d.wgsl index 00c3a9ab74416..b8204bae6d823 100644 --- a/crates/bevy_sprite/src/mesh2d/mesh2d.wgsl +++ b/crates/bevy_sprite/src/mesh2d/mesh2d.wgsl @@ -38,7 +38,6 @@ fn vertex(vertex: Vertex) -> VertexOutput { } struct FragmentInput { - @builtin(front_facing) is_front: bool, #import bevy_sprite::mesh2d_vertex_output }; From dbf909733b695cef11f726270b543c9daa06363f Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Mon, 22 Aug 2022 19:57:32 +0200 Subject: [PATCH 2/3] bevy_pbr: Fix negation of normal for back-side when no normal map --- assets/shaders/array_texture.wgsl | 13 ++++++++---- crates/bevy_pbr/src/render/pbr.wgsl | 11 +++++++--- crates/bevy_pbr/src/render/pbr_functions.wgsl | 20 ++++++++++++++++--- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/assets/shaders/array_texture.wgsl b/assets/shaders/array_texture.wgsl index 4281da0b15308..7e812b7e140e3 100644 --- a/assets/shaders/array_texture.wgsl +++ b/assets/shaders/array_texture.wgsl @@ -14,6 +14,7 @@ var my_array_texture: texture_2d_array; var my_array_texture_sampler: sampler; struct FragmentInput { + @builtin(front_facing) is_front: bool, @builtin(position) frag_coord: vec4, #import bevy_pbr::mesh_vertex_output }; @@ -33,13 +34,17 @@ fn fragment(in: FragmentInput) -> @location(0) vec4 { pbr_input.frag_coord = in.frag_coord; pbr_input.world_position = in.world_position; - pbr_input.world_normal = in.world_normal; + pbr_input.world_normal = prepare_world_normal( + in.world_normal, + (material.flags & STANDARD_MATERIAL_FLAGS_DOUBLE_SIDED_BIT) != 0u, + in.is_front, + ); pbr_input.is_orthographic = view.projection[3].w == 1.0; - pbr_input.N = prepare_normal( - pbr_input.material.flags, - in.world_normal, + pbr_input.N = apply_normal_mapping( + material.flags, + pbr_input.world_normal, #ifdef VERTEX_TANGENTS #ifdef STANDARDMATERIAL_NORMAL_MAP in.world_tangent, diff --git a/crates/bevy_pbr/src/render/pbr.wgsl b/crates/bevy_pbr/src/render/pbr.wgsl index 2cec91fcd9b5d..cac7dbf22cf86 100644 --- a/crates/bevy_pbr/src/render/pbr.wgsl +++ b/crates/bevy_pbr/src/render/pbr.wgsl @@ -9,6 +9,7 @@ #import bevy_pbr::pbr_functions struct FragmentInput { + @builtin(front_facing) is_front: bool, @builtin(position) frag_coord: vec4, #import bevy_pbr::mesh_vertex_output }; @@ -68,13 +69,17 @@ fn fragment(in: FragmentInput) -> @location(0) vec4 { pbr_input.frag_coord = in.frag_coord; pbr_input.world_position = in.world_position; - pbr_input.world_normal = in.world_normal; + pbr_input.world_normal = prepare_world_normal( + in.world_normal, + (material.flags & STANDARD_MATERIAL_FLAGS_DOUBLE_SIDED_BIT) != 0u, + in.is_front, + ); pbr_input.is_orthographic = view.projection[3].w == 1.0; - pbr_input.N = prepare_normal( + pbr_input.N = apply_normal_mapping( material.flags, - in.world_normal, + pbr_input.world_normal, #ifdef VERTEX_TANGENTS #ifdef STANDARDMATERIAL_NORMAL_MAP in.world_tangent, diff --git a/crates/bevy_pbr/src/render/pbr_functions.wgsl b/crates/bevy_pbr/src/render/pbr_functions.wgsl index 2c5f08bc0a7bb..3800753d07366 100644 --- a/crates/bevy_pbr/src/render/pbr_functions.wgsl +++ b/crates/bevy_pbr/src/render/pbr_functions.wgsl @@ -1,8 +1,22 @@ #define_import_path bevy_pbr::pbr_functions -// NOTE: This ensures that the world_normal is normalized and if -// vertex tangents and normal maps then normal mapping may be applied. -fn prepare_normal( +fn prepare_world_normal( + world_normal: vec3, + double_sided: bool, + is_front: bool, +) -> vec3 { + var output: vec3 = world_normal; +#ifndef VERTEX_TANGENTS +#ifndef STANDARDMATERIAL_NORMAL_MAP + // NOTE: When NOT using normal-mapping, if looking at the back face of a double-sided + // material, the normal needs to be inverted. This is a branchless version of that. + output = (f32(!double_sided || is_front) * 2.0 - 1.0) * output; +#endif +#endif + return output; +} + +fn apply_normal_mapping( standard_material_flags: u32, world_normal: vec3, #ifdef VERTEX_TANGENTS From 0c04b2a7862ddb2ea9b78f05723bfb840f0d5e00 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Mon, 22 Aug 2022 20:07:14 +0200 Subject: [PATCH 3/3] Minor shader fixes --- assets/shaders/array_texture.wgsl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assets/shaders/array_texture.wgsl b/assets/shaders/array_texture.wgsl index 7e812b7e140e3..8194a9dea17cb 100644 --- a/assets/shaders/array_texture.wgsl +++ b/assets/shaders/array_texture.wgsl @@ -36,14 +36,14 @@ fn fragment(in: FragmentInput) -> @location(0) vec4 { pbr_input.world_position = in.world_position; pbr_input.world_normal = prepare_world_normal( in.world_normal, - (material.flags & STANDARD_MATERIAL_FLAGS_DOUBLE_SIDED_BIT) != 0u, + (pbr_input.material.flags & STANDARD_MATERIAL_FLAGS_DOUBLE_SIDED_BIT) != 0u, in.is_front, ); pbr_input.is_orthographic = view.projection[3].w == 1.0; pbr_input.N = apply_normal_mapping( - material.flags, + pbr_input.material.flags, pbr_input.world_normal, #ifdef VERTEX_TANGENTS #ifdef STANDARDMATERIAL_NORMAL_MAP