EN-021: SSR + IBL specular exclusive ownership (env-miss fallback)#78
Closed
proggeramlug wants to merge 1 commit into
Closed
EN-021: SSR + IBL specular exclusive ownership (env-miss fallback)#78proggeramlug wants to merge 1 commit into
proggeramlug wants to merge 1 commit into
Conversation
Round-2 audit F10: compose is `hdr + ssr` and fs_main_scene already adds IBL specular into hdr, so SSR-hit pixels double-counted specular for roughness ~0.05-0.85 (worst on metals at r~0.55-0.75). The fix is ownership, not subtraction: - fs_main_scene scales ibl_spec by the complement of SSR's OWN roughness fade x its strength. The share rides the free dir_light_count.z lane (written per frame; 0 when SSR is disabled, so the full IBL term returns automatically). - The SSR march no longer returns black on a miss: camera-facing rays and off-screen/no-hit marches fall back to the env panorama sampled by the world-space reflection direction at the material path's roughness x 6 mip ramp, x env intensity (camera_pos.w), x the same fresnel/fade/strength weighting as a hit. Without this, reducing ibl_spec would darken every off-screen reflection - the reason the ticket deferred the one-line version. - SsrParams gains the view->world rotation (transpose of the view 3x3) + env LOD/intensity; SSR bind group gains the env panorama at bindings 9/10 and its cache invalidates wherever the env source swaps (HDR upload, panorama/procedural lighting-bg swaps). Suite green (99 lib + 7 golden; goldens unchanged within tolerance). Boot-smoked on the shooter at 4K - no WGSL errors, clean frames. Closes EN-021.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
Landed in main via the round-2 consolidation merge (#83). All commits from this branch are now on main; closing as merged-through-integration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #75. Implements the ticket filed during round-2 (audit F10) — the double-count fix paired with the env-miss fallback that made a one-liner unshippable.
Design: ownership, not subtraction
Within SSR's active roughness range, SSR now owns specular outright:
ibl_spec ×= 1 − ssr_share, wheressr_share = dir_light_count.z × (1 − smoothstep(0.5, 0.85, roughness))— the exact complement of the SSR shader's own fade. The.zlane is written every frame inclear_additional_lights(SSR strength, or 0 when SSR is off — full IBL returns automatically).sample_env,roughness × 6mip ramp, × env intensity (camera_pos.w), × the identical fresnel/fade/strength weighting as a hit. Previously: black, which is why scaling IBL down alone would have darkened every off-screen reflection.SsrParamsgains the view→world rotation + env LOD/intensity; the SSR bind group binds the panorama at 9/10 and its cache invalidates on every env-source swap (HDR upload, panorama/procedural swaps).Expected visual delta
Metals (in the shooter: alien carapaces, weapon) stop popping brighter when their reflected content scrolls on-screen; hit vs miss luma converges. Dielectrics gain coherent hit-or-env behaviour at grazing angles.
Validation