Skip to content

Commit

Permalink
Normalize only nonzero normals for mikktspace normal maps (#10905)
Browse files Browse the repository at this point in the history
# Objective

Fixes #5891.

For mikktspace normal maps, normals must be renormalized in vertex
shaders to match the way mikktspace bakes vertex tangents and normal
maps so that the exact inverse process is applied when shading.

However, for invalid normals like `vec3<f32>(0.0, 0.0, 0.0)`, this
normalization causes NaN values, and because it's in the vertex shader,
it affects the entire triangle and causes it to be shaded as black:

![incorrectly shaded
cone](https://github.com/bevyengine/bevy/assets/57632562/3334b3a9-f72a-4a08-853e-8077a346f5c9)

*A cone with a tip that has a vertex normal of [0, 0, 0], causing the
mesh to be shaded as black.*

In some cases, normals of zero are actually *useful*. For example, a
smoothly shaded cone without creases requires the apex vertex normal to
be zero, because there is no singular normal that works correctly, so
the apex shouldn't contribute to the overall shading. Duplicate vertices
for the apex fix some shading issues, but it causes visible creases and
is more expensive. See #5891 and #10298 for more details.

For correctly shaded cones and other similar low-density shapes with
sharp tips, vertex normals of zero can not be normalized in the vertex
shader.

## Solution

Only normalize the vertex normals and tangents in the vertex shader if
the normal isn't [0, 0, 0]. This way, mikktspace normal maps should
still work for everything except the zero normals, and the zero normals
will only be normalized in the fragment shader.

This allows us to render cones correctly:

![smooth cone with some
banding](https://github.com/bevyengine/bevy/assets/57632562/6b36e264-22c6-453b-a6de-c404b314ca1a)

Notice how there is still a weird shadow banding effect in one area. I
noticed that it can be fixed by normalizing
[here](https://github.com/bevyengine/bevy/blob/d2614f2d802d0fb8000821a81553b600cc85f734/crates/bevy_pbr/src/render/pbr_functions.wgsl#L51),
which produces a perfectly smooth cone without duplicate vertices:

![smooth
cone](https://github.com/bevyengine/bevy/assets/57632562/64f9ad5d-b249-4eae-880b-a1e61e07ae73)

I didn't add this change yet, because it seems a bit arbitrary. I can
add it here if that'd be useful or make another PR though.
  • Loading branch information
Jondolf authored Dec 10, 2023
1 parent 47fa620 commit 4b1865f
Showing 1 changed file with 32 additions and 22 deletions.
54 changes: 32 additions & 22 deletions crates/bevy_pbr/src/render/mesh_functions.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,20 @@ fn mesh_normal_local_to_world(vertex_normal: vec3<f32>, instance_index: u32) ->
// 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.
// Unreal Engine, Godot, and more all use the mikktspace method.
// We only skip normalization for invalid normals so that they don't become NaN.
// Do not change this code unless you really know what you are doing.
// http://www.mikktspace.com/
return normalize(
mat2x4_f32_to_mat3x3_unpack(
mesh[instance_index].inverse_transpose_model_a,
mesh[instance_index].inverse_transpose_model_b,
) * vertex_normal
);
if any(vertex_normal != vec3<f32>(0.0)) {
return normalize(
mat2x4_f32_to_mat3x3_unpack(
mesh[instance_index].inverse_transpose_model_a,
mesh[instance_index].inverse_transpose_model_b,
) * vertex_normal
);
} else {
return vertex_normal;
}
}

// Calculates the sign of the determinant of the 3x3 model matrix based on a
Expand All @@ -59,19 +64,24 @@ fn mesh_tangent_local_to_world(model: mat4x4<f32>, vertex_tangent: vec4<f32>, in
// 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.
// Unreal Engine, Godot, and more all use the mikktspace method.
// We only skip normalization for invalid tangents so that they don't become NaN.
// Do not change this code unless you really know what you are doing.
// http://www.mikktspace.com/
return vec4<f32>(
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_3x3m(instance_index)
);
if any(vertex_tangent != vec4<f32>(0.0)) {
return vec4<f32>(
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_3x3m(instance_index)
);
} else {
return vertex_tangent;
}
}

0 comments on commit 4b1865f

Please sign in to comment.