fix(shadows): repair collapsed cascade fit; tame SSAO over dense grass#85
Conversation
The CSM cascade fit reconstructed each frustum slice by inverting the perspective VP and unprojecting the NDC clip-cube corners. But mat4_perspective uses the OpenGL [-1,1] NDC-z convention while wgpu presents [0,1] depth on every backend, so the near plane came back with a negative homogeneous w -- all 8 corners divided down onto the z=-1 plane and the frustum bounding sphere collapsed to ~0.12 m. Terrain a few metres from the camera then projected outside its own cascade and self-shadowed, producing a large dark region that swung disproportionately with the camera. Compute the frustum corners directly in view space from the FOV instead (no projection inversion, convention-proof): radius 0.12 -> ~22 m, ground back inside its cascades. Also fixes two SSAO problems that stacked on top of it: - Disable the screen-space contact-shadow march (ao.rs). It is half-res, unblurred, has no temporal accumulation, and is applied after TAA, so it flickered every frame over the dense wind-animated grass. - Cap the GTAO screen radius 0.25 -> 0.05 and default ssao_strength 1.0 -> 0.7. The radius default (2.0) is world-space metres and the quarter-screen clamp painted a broad blotchy dark wash over the grass. Corrected the stale "UV units" docs on ssao_radius. Verified on Windows (DX12, Radeon 760M). All changes are in native/shared, so they apply to every platform. Write-up: shooter docs/shadow-cascade-and-ssao-fixes.md
📝 WalkthroughWalkthroughAdjusted SSAO default strength and radius documentation in the renderer, updated the SSAO WGSL shader to add a contact-shadows enable flag (disabled by default) and reduced the horizon-scan clamp radius, and rewrote cascade frustum world-corner derivation in shadow mapping to compute view-space corners directly rather than via sub-projection inversion. ChangesSSAO Tuning and Documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Shadow Cascade Corner Derivation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
native/shared/src/shadows.rs (1)
649-663: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHoist cascade-invariant setup out of the loop and avoid shadowing
d.Two small cleanups on the rewritten corner block:
inv_view,half_w_per_d, andhalf_h_per_ddon't depend onc, so they're recomputed (including a full 4×4 inversion) on every cascade iteration. Move them above thefor c in 0..NUM_CASCADESloop.- The inner
for &d in [c_near, c_far]reuses the named, shadowing the normalized light-directiondthat the pancaking/eye math below relies on (Lines 729-731, 768-771). It's scope-safe today, but renaming avoids a subtle future-refactor trap.♻️ Proposed refactor
Add before Line 629 (
for c in 0..NUM_CASCADES {):let inv_view = crate::renderer::mat4_invert(camera_view); let half_w_per_d = 1.0 / camera_proj[0][0]; let half_h_per_d = 1.0 / camera_proj[1][1];Then within the loop:
- let inv_view = crate::renderer::mat4_invert(camera_view); - let half_w_per_d = 1.0 / camera_proj[0][0]; - let half_h_per_d = 1.0 / camera_proj[1][1]; let mut world_corners = [[0.0f32; 3]; 8]; let mut ci = 0usize; - for &d in [c_near, c_far].iter() { - let hw = d * half_w_per_d; - let hh = d * half_h_per_d; + for &slice_d in [c_near, c_far].iter() { + let hw = slice_d * half_w_per_d; + let hh = slice_d * half_h_per_d; for &(sx, sy) in [(-1.0f32, -1.0f32), (1.0, -1.0), (-1.0, 1.0), (1.0, 1.0)].iter() { // View space looks down -Z, so this slice sits at z = -d. - let vp = [sx * hw, sy * hh, -d, 1.0]; + let vp = [sx * hw, sy * hh, -slice_d, 1.0]; let wc = crate::renderer::mat4_mul_vec4(&inv_view, &vp); world_corners[ci] = [wc[0], wc[1], wc[2]]; ci += 1; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@native/shared/src/shadows.rs` around lines 649 - 663, Move the cascade-invariant setup out of the main shadow-cascade loop in shadows.rs: compute inv_view, half_w_per_d, and half_h_per_d once before iterating over cascades instead of recalculating them for each c. Also rename the inner loop variable in the world_corners block (the current d in the c_near/c_far iteration) so it does not shadow the normalized light-direction d used later in the pancaking and eye math; keep the existing logic in the relevant world_corners and cascade setup code paths unchanged apart from this cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@native/shared/src/shadows.rs`:
- Around line 649-663: Move the cascade-invariant setup out of the main
shadow-cascade loop in shadows.rs: compute inv_view, half_w_per_d, and
half_h_per_d once before iterating over cascades instead of recalculating them
for each c. Also rename the inner loop variable in the world_corners block (the
current d in the c_near/c_far iteration) so it does not shadow the normalized
light-direction d used later in the pancaking and eye math; keep the existing
logic in the relevant world_corners and cascade setup code paths unchanged apart
from this cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c7632639-d4a4-47e6-94de-33f80397198e
📒 Files selected for processing (3)
native/shared/src/renderer/mod.rsnative/shared/src/renderer/shaders/ao.rsnative/shared/src/shadows.rs
What
Fixes three stacked bugs behind a reported "flickering dark pixelated thing that moves when the camera moves." All changes are in
native/shared/, so they apply to every platform (verified on Windows / DX12 / Radeon 760M).1. Collapsed shadow-cascade fit (the "moving dark patch")
compute_cascade_vpsreconstructed each frustum slice by invertingsub_proj * camera_viewand unprojecting the NDC clip-cube corners.mat4_perspectiveuses the OpenGL [-1,1] NDC-z convention, but wgpu presents [0,1] depth on every backend — so the near plane came back with a negative homogeneous w, all 8 corners divided onto thez=-1plane, and the frustum bounding sphere collapsed to ~0.12 m. Terrain a few metres from the camera then fell outside its own cascade and self-shadowed; because the cascade follows the camera, that false shadow swung disproportionately as you moved.Fix: compute the frustum corners directly in view space from the FOV (
half = 1/proj[0][0],1/proj[1][1]), then transform to world with the affine view inverse. No projection inversion → convention-proof.radius 0.12 → ~22 m.2. SSAO contact-shadow flicker
The contact-shadow march (
Gchannel) is half-res, unblurred, has no temporal accumulation, and runs post-TAA, so it flickered every frame over the dense wind-animated grass. Gated off (CONTACT_SHADOWS_ENABLED = false).3. SSAO over-darkening grass
ssao_radiusdefault2.0is world metres (docs wrongly said "UV ~0.006") and the screen radius was clamped to ¼ of the viewport. Capped0.25 → 0.05, defaultssao_strength 1.0 → 0.7, fixed the docs.Notes
mat4_perspectiveuses OpenGL [-1,1] into wgpu [0,1], wasting half the depth range. The cascade fit no longer depends on it; converting to a true [0,1] perspective is a future cleanup that must match everywhereinv_projreconstructs.Full post-mortem:
shooter/docs/shadow-cascade-and-ssao-fixes.md.Summary by CodeRabbit
Bug Fixes
Documentation