Use native depth clip control instead of manual DEPTH_CLAMP_ORTHO where possible #16078
Labels
A-Rendering
Drawing game state to the screen
C-Feature
A new feature, making something new possible
C-Performance
A change motivated by improving speed, memory usage or compile times
D-Shaders
This code uses GPU shader languages
D-Straightforward
Simple bug fixes and API improvements, docs, test and examples
S-Ready-For-Implementation
This issue is ready for an implementation PR. Go for it!
Comments
JMS55
added
A-Rendering
BenjaminBrienen
added
S-Ready-For-Implementation
github-merge-queue bot
pushed a commit
that referenced
this issue
Dec 3, 2024
# Objective - Fixes #16078 ## Solution - Rename things to clarify that we _want_ unclipped depth for directional light shadow views, and need some way of disabling the GPU's builtin depth clipping - Use DEPTH_CLIP_CONTROL instead of the fragment shader emulation on supported platforms - Pass only the clip position depth instead of the whole clip position between vertex->fragment shader (no idea if this helps performance or not, compiler might optimize it anyways) - Meshlets - HW raster always uses DEPTH_CLIP_CONTROL since it targets a more limited set of platforms - SW raster was not handling DEPTH_CLAMP_ORTHO correctly, it ended up pretty much doing nothing. - This PR made me realize that SW raster technically should have depth clipping for all views that are not directional light shadows, but I decided not to bother writing it. I'm not sure that it ever matters in practice. If proven otherwise, I can add it. ## Testing - Did you test these changes? If so, how? - Lighting example. Both opaque (no fragment shader) and alpha masked geometry (fragment shader emulation) are working with depth_clip_control, and both work when it's turned off. Also tested meshlet example. - Are there any parts that need more testing? - Performance. I can't figure out a good test scene. - How can other people (reviewers) test your changes? Is there anything specific they need to know? - Toggle depth_clip_control_supported in prepass/mod.rs line 323 to turn this PR on or off. - If relevant, what platforms did you test these changes on, and are there any important ones you can't test? - Native --- ## Migration Guide - `MeshPipelineKey::DEPTH_CLAMP_ORTHO` is now `MeshPipelineKey::UNCLIPPED_DEPTH_ORTHO` - The `DEPTH_CLAMP_ORTHO` shaderdef has been renamed to `UNCLIPPED_DEPTH_ORTHO_EMULATION` - `clip_position_unclamped: vec4<f32>` is now `unclipped_depth: f32`
ecoskey
pushed a commit
to ecoskey/bevy
that referenced
this issue
Jan 6, 2025
# Objective - Fixes bevyengine#16078 ## Solution - Rename things to clarify that we _want_ unclipped depth for directional light shadow views, and need some way of disabling the GPU's builtin depth clipping - Use DEPTH_CLIP_CONTROL instead of the fragment shader emulation on supported platforms - Pass only the clip position depth instead of the whole clip position between vertex->fragment shader (no idea if this helps performance or not, compiler might optimize it anyways) - Meshlets - HW raster always uses DEPTH_CLIP_CONTROL since it targets a more limited set of platforms - SW raster was not handling DEPTH_CLAMP_ORTHO correctly, it ended up pretty much doing nothing. - This PR made me realize that SW raster technically should have depth clipping for all views that are not directional light shadows, but I decided not to bother writing it. I'm not sure that it ever matters in practice. If proven otherwise, I can add it. ## Testing - Did you test these changes? If so, how? - Lighting example. Both opaque (no fragment shader) and alpha masked geometry (fragment shader emulation) are working with depth_clip_control, and both work when it's turned off. Also tested meshlet example. - Are there any parts that need more testing? - Performance. I can't figure out a good test scene. - How can other people (reviewers) test your changes? Is there anything specific they need to know? - Toggle depth_clip_control_supported in prepass/mod.rs line 323 to turn this PR on or off. - If relevant, what platforms did you test these changes on, and are there any important ones you can't test? - Native --- ## Migration Guide - `MeshPipelineKey::DEPTH_CLAMP_ORTHO` is now `MeshPipelineKey::UNCLIPPED_DEPTH_ORTHO` - The `DEPTH_CLAMP_ORTHO` shaderdef has been renamed to `UNCLIPPED_DEPTH_ORTHO_EMULATION` - `clip_position_unclamped: vec4<f32>` is now `unclipped_depth: f32`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What problem does this solve or what need does it fill?
The DEPTH_CLAMP_ORTHO stuff we have for directional light shadow mapping uses a fragment shader. Having the fragment shader disables some really important hardware optimizations that exist for depth-only vertex-only passes.
What solution would you like?
For non-alpha masked draws, on supported platforms, use https://docs.rs/wgpu/latest/wgpu/struct.PrimitiveState.html#structfield.unclipped_depth instead of setting the DEPTH_CLAMP_ORTHO shader def.
Additionally when we do use DEPTH_CLAMP_ORTHO, we should not pass the entire
clip_position_unclamped: vec4<f32>from the vertex shader to the fragment shader, we only need to pass thezvalue.What alternative(s) have you considered?
We could remove DEPTH_CLAMP_ORTHO entirely, and instead expand the shadow view frustum either by some fixed amount, or based on the AABB's of entities in the scene.
Additional context
#8877
https://therealmjp.github.io/posts/shadow-maps/#disabling-z-clipping
The text was updated successfully, but these errors were encountered: