From 0a881ab37fc286a85c4e50dbb0a31708d43b8eaf Mon Sep 17 00:00:00 2001 From: Daniel Chia Date: Wed, 21 Jun 2023 15:00:19 -0700 Subject: [PATCH] Cascaded shadow maps: Fix prepass ortho depth clamping (#8877) # Objective - Fixes #8645 ## Solution Cascaded shadow maps use a technique commonly called shadow pancaking to enhance shadow map resolution by restricting the orthographic projection used in creating the shadow maps to the frustum slice for the cascade. The implication of this restriction is that shadow casters can be closer than the near plane of the projection volume. Prior to this PR, we address clamp the depth of the prepass vertex output to ensure that these shadow casters do not get clipped, resulting in shadow loss. However, a flaw / bug of the prior approach is that the depth that gets written to the shadow map isn't quite correct - the depth was previously derived by interpolated the clamped clip position, resulting in depths that are further than they should be. This creates artifacts that are particularly noticeable when a very 'long' object intersects the near plane close to perpendicularly. The fix in this PR is to propagate the unclamped depth to the prepass fragment shader and use that depth value directly. A complementary solution would be to use [DEPTH_CLIP_CONTROL](https://docs.rs/wgpu/latest/wgpu/struct.Features.html#associatedconstant.DEPTH_CLIP_CONTROL) to request `unclipped_depth`. However due to the relatively low support of the feature on Vulkan (I believe it's ~38%), I went with this solution for now to get the broadest fix out first. --- ## Changelog - Fixed: Shadows from directional lights were sometimes incorrectly omitted when the shadow caster was partially out of view. --------- Co-authored-by: Carter Anderson --- crates/bevy_pbr/src/prepass/mod.rs | 10 +++++++++- crates/bevy_pbr/src/prepass/prepass.wgsl | 19 ++++++++++++++++++- crates/bevy_pbr/src/render/pbr_prepass.wgsl | 12 ++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 2dd45e2c6e..d704c5a1c6 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -385,6 +385,13 @@ where )); if key.mesh_key.contains(MeshPipelineKey::DEPTH_CLAMP_ORTHO) { shader_defs.push("DEPTH_CLAMP_ORTHO".into()); + // PERF: This line forces the "prepass fragment shader" to always run in + // common scenarios like "directional light calculation". Doing so resolves + // a pretty nasty depth clamping bug, but it also feels a bit excessive. + // We should try to find a way to resolve this without forcing the fragment + // shader to run. + // https://github.com/bevyengine/bevy/pull/8877 + shader_defs.push("PREPASS_FRAGMENT".into()); } if layout.contains(Mesh::ATTRIBUTE_UV_0) { @@ -457,8 +464,9 @@ where // The fragment shader is only used when the normal prepass or motion vectors prepass // is enabled or the material uses alpha cutoff values and doesn't rely on the standard - // prepass shader + // prepass shader or we are clamping the orthographic depth. let fragment_required = !targets.is_empty() + || key.mesh_key.contains(MeshPipelineKey::DEPTH_CLAMP_ORTHO) || (key.mesh_key.contains(MeshPipelineKey::MAY_DISCARD) && self.material_fragment_shader.is_some()); diff --git a/crates/bevy_pbr/src/prepass/prepass.wgsl b/crates/bevy_pbr/src/prepass/prepass.wgsl index 9aba5afb6c..03ddee145a 100644 --- a/crates/bevy_pbr/src/prepass/prepass.wgsl +++ b/crates/bevy_pbr/src/prepass/prepass.wgsl @@ -41,6 +41,10 @@ struct VertexOutput { @location(3) world_position: vec4, @location(4) previous_world_position: vec4, #endif // MOTION_VECTOR_PREPASS + +#ifdef DEPTH_CLAMP_ORTHO + @location(5) clip_position_unclamped: vec4, +#endif // DEPTH_CLAMP_ORTHO } @vertex @@ -55,7 +59,8 @@ fn vertex(vertex: Vertex) -> VertexOutput { out.clip_position = mesh_position_local_to_clip(model, vec4(vertex.position, 1.0)); #ifdef DEPTH_CLAMP_ORTHO - out.clip_position.z = min(out.clip_position.z, 1.0); + out.clip_position_unclamped = out.clip_position; + out.clip_position.z = min(out.clip_position.z, 1.0); #endif // DEPTH_CLAMP_ORTHO #ifdef VERTEX_UVS @@ -96,6 +101,10 @@ struct FragmentInput { @location(3) world_position: vec4, @location(4) previous_world_position: vec4, #endif // MOTION_VECTOR_PREPASS + +#ifdef DEPTH_CLAMP_ORTHO + @location(5) clip_position_unclamped: vec4, +#endif // DEPTH_CLAMP_ORTHO } struct FragmentOutput { @@ -106,6 +115,10 @@ struct FragmentOutput { #ifdef MOTION_VECTOR_PREPASS @location(1) motion_vector: vec2, #endif // MOTION_VECTOR_PREPASS + +#ifdef DEPTH_CLAMP_ORTHO + @builtin(frag_depth) frag_depth: f32, +#endif // DEPTH_CLAMP_ORTHO } @fragment @@ -116,6 +129,10 @@ fn fragment(in: FragmentInput) -> FragmentOutput { out.normal = vec4(in.world_normal * 0.5 + vec3(0.5), 1.0); #endif +#ifdef DEPTH_CLAMP_ORTHO + out.frag_depth = in.clip_position_unclamped.z; +#endif // DEPTH_CLAMP_ORTHO + #ifdef MOTION_VECTOR_PREPASS let clip_position_t = view.unjittered_view_proj * in.world_position; let clip_position = clip_position_t.xy / clip_position_t.w; diff --git a/crates/bevy_pbr/src/render/pbr_prepass.wgsl b/crates/bevy_pbr/src/render/pbr_prepass.wgsl index 1a90c45709..bb2d1dfdd9 100644 --- a/crates/bevy_pbr/src/render/pbr_prepass.wgsl +++ b/crates/bevy_pbr/src/render/pbr_prepass.wgsl @@ -22,6 +22,10 @@ struct FragmentInput { @location(3) world_position: vec4, @location(4) previous_world_position: vec4, #endif // MOTION_VECTOR_PREPASS + +#ifdef DEPTH_CLAMP_ORTHO + @location(5) clip_position_unclamped: vec4, +#endif // DEPTH_CLAMP_ORTHO }; // Cutoff used for the premultiplied alpha modes BLEND and ADD. @@ -66,6 +70,10 @@ struct FragmentOutput { #ifdef MOTION_VECTOR_PREPASS @location(1) motion_vector: vec2, #endif // MOTION_VECTOR_PREPASS + +#ifdef DEPTH_CLAMP_ORTHO + @builtin(frag_depth) frag_depth: f32, +#endif // DEPTH_CLAMP_ORTHO } @fragment @@ -74,6 +82,10 @@ fn fragment(in: FragmentInput) -> FragmentOutput { var out: FragmentOutput; +#ifdef DEPTH_CLAMP_ORTHO + out.frag_depth = in.clip_position_unclamped.z; +#endif // DEPTH_CLAMP_ORTHO + #ifdef NORMAL_PREPASS // NOTE: Unlit bit not set means == 0 is true, so the true case is if lit if (material.flags & STANDARD_MATERIAL_FLAGS_UNLIT_BIT) == 0u {