Skip to content

Commit 595b0a7

Browse files
authored
Fix spotlight basis (#20272)
# Objective - Fix a visual regression introduced by #20191 where theres a slight green line in lighting example between cube and plane bestRanar: > I know some minor shadow differences were expected after > #20191 > But the new green line below the front face of the cube in lighting seems interesting. Is that expected? > https://pixel-eagle.com/project/b25a040a-a980-4602-b90c-d480ab84076d/run/11500/compare/11494?screenshot=3D+Rendering/lighting.png ## Solution It turns out, i accidentally unflipped the x and y axis in the basis construction of spotlights to actually match the JCGT paper. This wasn't something i realized at the time, but its just a handedness flip. This means that the handedness flip Aceeri was asking about in #20191 which I just extracted from the orthonormalize implementation was just there to correct for a mistranslation in the original implementation, probably. So this means we can just yeet it, because two handedness flips means none at all. And indeed, removing the extra flip fixes the regression. So now the code is more straightforward and understandable and it works :D Most of the diff is updating the comments to reflect this new knowledge. ## Testing - lighting example
1 parent f808216 commit 595b0a7

File tree

3 files changed

+5
-22
lines changed

3 files changed

+5
-22
lines changed

crates/bevy_light/src/spot_light.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -171,19 +171,15 @@ pub fn orthonormalize(z_basis: Dir3) -> Mat3 {
171171
let y_basis = Vec3::new(b, sign + z_basis.y * z_basis.y * a, -z_basis.y);
172172
Mat3::from_cols(x_basis, y_basis, z_basis.into())
173173
}
174-
/// Constructs a left-handed orthonormal basis with translation, using only the forward direction and translation of a given [`GlobalTransform`].
174+
/// Constructs a right-handed orthonormal basis with translation, using only the forward direction and translation of a given [`GlobalTransform`].
175175
///
176-
/// This is a handedness-inverted version of [`orthonormalize`] which also includes translation.
177-
/// we mirror this construction in the fragment shader and need our implementations to match exactly.
178-
// See bevy_pbr/shadows.wgsl:spot_light_world_from_view
176+
/// This is a version of [`orthonormalize`] which also includes translation.
179177
pub fn spot_light_world_from_view(transform: &GlobalTransform) -> Mat4 {
180178
// the matrix z_local (opposite of transform.forward())
181179
let fwd_dir = transform.back();
182180

183181
let basis = orthonormalize(fwd_dir);
184182
let mut mat = Mat4::from_mat3(basis);
185-
// handedness flip
186-
mat.x_axis = -mat.x_axis;
187183
mat.w_axis = transform.translation().extend(1.0);
188184
mat
189185
}

crates/bevy_pbr/src/render/shadows.wgsl

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,21 +62,6 @@ fn fetch_point_shadow(light_id: u32, frag_position: vec4<f32>, surface_normal: v
6262
return sample_shadow_cubemap(frag_ls * flip_z, distance_to_light, depth, light_id);
6363
}
6464

65-
// Constructs a left-handed orthonormal basis from a given unit Z vector.
66-
//
67-
// NOTE: requires unit-length (normalized) input to function properly.
68-
//
69-
// this method of constructing a basis from a vec3 is used by glam::Vec3::any_orthonormal_pair
70-
// the construction of the orthonormal basis up and right vectors here needs to precisely mirror the code
71-
// in bevy_light/spot_light.rs:spot_light_world_from_view
72-
// so we use `bevy_math::orthonormalize` which matches the rust impl, but we also invert the handedness
73-
fn spot_light_world_from_view(z_basis: vec3<f32>) -> mat3x3<f32> {
74-
var basis = orthonormalize(z_basis);
75-
// handedness flip
76-
basis[0] = -basis[0];
77-
return basis;
78-
}
79-
8065
fn fetch_spot_shadow(
8166
light_id: u32,
8267
frag_position: vec4<f32>,
@@ -103,7 +88,7 @@ fn fetch_spot_shadow(
10388
+ ((*light).shadow_depth_bias * normalize(surface_to_light))
10489
+ (surface_normal.xyz * (*light).shadow_normal_bias) * distance_to_light;
10590

106-
let light_inv_rot = spot_light_world_from_view(fwd);
91+
let light_inv_rot = orthonormalize(fwd);
10792

10893
// because the matrix is a pure rotation matrix, the inverse is just the transpose, and to calculate
10994
// the product of the transpose with a vector we can just post-multiply instead of pre-multiplying.

crates/bevy_render/src/maths.wgsl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ fn copysign(a: f32, b: f32) -> f32 {
7575
//
7676
// https://jcgt.org/published/0006/01/01/paper.pdf
7777
// this method of constructing a basis from a vec3 is also used by `glam::Vec3::any_orthonormal_pair`
78+
// the construction of the orthonormal basis up and right vectors here needs to precisely match the rust
79+
// implementation in bevy_light/spot_light.rs:spot_light_world_from_view
7880
fn orthonormalize(z_basis: vec3<f32>) -> mat3x3<f32> {
7981
let sign = copysign(1.0, z_basis.z);
8082
let a = -1.0 / (sign + z_basis.z);

0 commit comments

Comments
 (0)