Skip to content

Commit

Permalink
bevy_pbr: Fix incorrect and unnecessary normal-mapping code (bevyengi…
Browse files Browse the repository at this point in the history
…ne#5766)

# Objective

- Fixes bevyengine#4019 
- Fix lighting of double-sided materials when using a negative scale
- 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.

## Solution

- Remove the code that negates the T, B, and N vectors (the interpolated world-space tangent, calculated world-space bitangent, and interpolated world-space normal) when seeing the back face of a double-sided material with negative scale.
- Negate the world normal for a double-sided back face only when not using normal mapping

### Before, on `main`, flipping T, B, and N

<img width="932" alt="Screenshot 2022-08-22 at 15 11 53" src="https://user-images.githubusercontent.com/302146/185965366-f776ff2c-cfa1-46d1-9c84-fdcb399c273c.png">

### After, on this PR

<img width="932" alt="Screenshot 2022-08-22 at 15 12 11" src="https://user-images.githubusercontent.com/302146/185965420-8be493e2-3b1a-4188-bd13-fd6b17a76fe7.png">

### Double-sided material without normal maps

https://user-images.githubusercontent.com/302146/185988113-44a384e7-0b55-4946-9b99-20f8c803ab7e.mp4

---

## Changelog

- Fixed: Lighting of normal-mapped, double-sided materials applied to models with negative scale
- Fixed: Lighting and shadowing of back faces with no normal-mapping and a double-sided material

## Migration Guide

`prepare_normal` from the `bevy_pbr::pbr_functions` shader import has been reworked.

Before:
```rust
    pbr_input.world_normal = in.world_normal;

    pbr_input.N = prepare_normal(
        pbr_input.material.flags,
        in.world_normal,
#ifdef VERTEX_TANGENTS
#ifdef STANDARDMATERIAL_NORMAL_MAP
        in.world_tangent,
#endif
#endif
        in.uv,
        in.is_front,
    );
```

After:
```rust
    pbr_input.world_normal = prepare_world_normal(
        in.world_normal,
        (material.flags & STANDARD_MATERIAL_FLAGS_DOUBLE_SIDED_BIT) != 0u,
        in.is_front,
    );

    pbr_input.N = apply_normal_mapping(
        pbr_input.material.flags,
        pbr_input.world_normal,
#ifdef VERTEX_TANGENTS
#ifdef STANDARDMATERIAL_NORMAL_MAP
        in.world_tangent,
#endif
#endif
        in.uv,
    );
```
  • Loading branch information
superdump authored and ItsDoot committed Feb 1, 2023
1 parent af5962c commit 9b6b22a
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 27 deletions.
11 changes: 7 additions & 4 deletions assets/shaders/array_texture.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,23 @@ fn fragment(in: FragmentInput) -> @location(0) vec4<f32> {

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,
(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 = prepare_normal(
pbr_input.N = apply_normal_mapping(
pbr_input.material.flags,
in.world_normal,
pbr_input.world_normal,
#ifdef VERTEX_TANGENTS
#ifdef STANDARDMATERIAL_NORMAL_MAP
in.world_tangent,
#endif
#endif
in.uv,
in.is_front,
);
pbr_input.V = calculate_view(in.world_position, pbr_input.is_orthographic);

Expand Down
1 change: 0 additions & 1 deletion crates/bevy_pbr/src/render/mesh.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ fn vertex(vertex: Vertex) -> VertexOutput {
}

struct FragmentInput {
@builtin(front_facing) is_front: bool,
#import bevy_pbr::mesh_vertex_output
};

Expand Down
11 changes: 7 additions & 4 deletions crates/bevy_pbr/src/render/pbr.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,17 @@ fn fragment(in: FragmentInput) -> @location(0) vec4<f32> {

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,
Expand All @@ -84,7 +88,6 @@ fn fragment(in: FragmentInput) -> @location(0) vec4<f32> {
#ifdef VERTEX_UVS
in.uv,
#endif
in.is_front,
);
pbr_input.V = calculate_view(in.world_position, pbr_input.is_orthographic);
output_color = pbr(pbr_input);
Expand Down
33 changes: 17 additions & 16 deletions crates/bevy_pbr/src/render/pbr_functions.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,23 @@ fn alpha_discard(material: StandardMaterial, output_color: vec4<f32>) -> vec4<f3
return color;
}

// 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<f32>,
double_sided: bool,
is_front: bool,
) -> vec3<f32> {
var output: vec3<f32> = 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<f32>,
#ifdef VERTEX_TANGENTS
Expand All @@ -36,7 +50,6 @@ fn prepare_normal(
#ifdef VERTEX_UVS
uv: vec2<f32>,
#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
Expand All @@ -57,18 +70,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
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_sprite/src/mesh2d/color_material.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ var texture_sampler: sampler;
var<uniform> mesh: Mesh2d;

struct FragmentInput {
@builtin(front_facing) is_front: bool,
#import bevy_sprite::mesh2d_vertex_output
};

Expand Down
1 change: 0 additions & 1 deletion crates/bevy_sprite/src/mesh2d/mesh2d.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ fn vertex(vertex: Vertex) -> VertexOutput {
}

struct FragmentInput {
@builtin(front_facing) is_front: bool,
#import bevy_sprite::mesh2d_vertex_output
};

Expand Down

0 comments on commit 9b6b22a

Please sign in to comment.