fix(shaders): per-ray underwater medium tracking (Phase 4 #134)#206
Merged
havokentity merged 6 commits intoMay 19, 2026
Merged
Conversation
havokentity
added a commit
that referenced
this pull request
May 19, 2026
1. Engine.cpp:7227 -- the in-water camera test used the wrong half- space inequality. `prim_plane` and intersectPlane define planes as `dot(n, p) + d = 0` (documented in `prim_plane`'s help text and used by intersectPlane on both CPU and shader paths). The below-surface half-space is `dot(n, cam) + d <= 0`, not `dot(n, cam) <= d`. Worked for d == 0 by coincidence (water_pool golden), but a water plane at any non-zero altitude would have seeded `in_water` incorrectly. Rewrote the test as `signed_dist = dot(n, cam) + d; signed_dist <= 0`. 2. PathTrace.slang:3808 -- the `in_water` docstring referenced MAT_DIELECTRIC flipping the medium state, but the dielectric branch was intentionally left untouched (per the issue's "optional but symmetric" callout). Removed the MAT_DIELECTRIC reference and replaced with an explicit note that dielectric refraction is out of scope for Phase 4 -- documented the simplification (glass body underwater continues attenuating with water absorption) and pointed at Phase 5 SSS for the proper multi-medium stack. Verified: - Bit-identical render output on all three Phase 4 test scenes (water_pool.cfg + the two underwater fixtures) -- expected since the regressed water plane has d == 0. - 20/20 non-golden ctest pass.
havokentity
added a commit
that referenced
this pull request
May 19, 2026
1. Engine.cpp:7227 -- the in-water camera test used the wrong half- space inequality. `prim_plane` and intersectPlane define planes as `dot(n, p) + d = 0` (documented in `prim_plane`'s help text and used by intersectPlane on both CPU and shader paths). The below-surface half-space is `dot(n, cam) + d <= 0`, not `dot(n, cam) <= d`. Worked for d == 0 by coincidence (water_pool golden), but a water plane at any non-zero altitude would have seeded `in_water` incorrectly. Rewrote the test as `signed_dist = dot(n, cam) + d; signed_dist <= 0`. 2. PathTrace.slang:3808 -- the `in_water` docstring referenced MAT_DIELECTRIC flipping the medium state, but the dielectric branch was intentionally left untouched (per the issue's "optional but symmetric" callout). Removed the MAT_DIELECTRIC reference and replaced with an explicit note that dielectric refraction is out of scope for Phase 4 -- documented the simplification (glass body underwater continues attenuating with water absorption) and pointed at Phase 5 SSS for the proper multi-medium stack. Verified: - Bit-identical render output on all three Phase 4 test scenes (water_pool.cfg + the two underwater fixtures) -- expected since the regressed water plane has d == 0. - 20/20 non-golden ctest pass.
havokentity
force-pushed
the
feat/underwater-medium-tracking
branch
from
535ce20 to
756abb8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
shaders/PathTrace.slang:967
- The Vulkan
FrameUBO also needs to mirror the hostPtPushlayout. Since the host hassmoke_params3betweensmoke_params2andmotion_blur_params, thiswater_statefield is shifted by one vec4 in SPIR-V and will read the wrong values unless the missing field/padding is added beforemotion_blur_params.
float4 water_state;
havokentity
added a commit
that referenced
this pull request
May 19, 2026
Copilot flagged a host/shader cbuffer offset mismatch: the host PtPush inserts smoke_params3 (wave-6 "looks like smoke") between smoke_params2 and motion_blur_params, but the shader Push and Frame cbuffers in PathTrace.slang went directly from smoke_params2 to motion_blur_params. Result: the shader's motion_blur_params reads the host's smoke_params3 bytes, and water_state (this PR's new field) reads what should be motion_blur_params -- underwater state seeded from random shutter_speed-shaped bytes. This was a pre-existing layout drift on the integration branch (the host side of smoke wave-6 landed in PR #25 but the matching shader change is still on the unmerged feat/smoke-wave6 branch, commit 91c3bb4 -- the shader's smoke BRDF that reads smoke_params3 lives there). Surfaced now because adding water_state past the misaligned region makes the drift visible (throwing off the actual rendered output rather than silently ignoring a no-op slot). Fix: add `float4 smoke_params3` to both the Metal Push cbuffer and the Vulkan Frame cbuffer in PathTrace.slang, between smoke_params2 and motion_blur_params. Docs the slot semantics matching what Engine.cpp writes; the in-shader reader (the smoke turbulence BRDF) is still on the unmerged feat/smoke-wave6 branch and will pick up the slot once that PR lands. Net effect today: trailing fields (motion_blur_params, water_state, editor_params) now read from the correct host offsets on both backends. Verified: - 20/20 non-golden ctest pass. - Underwater render output matches the pre-rebase v3 baseline (water tracking + Snell's window + dark TIR pixels all read correctly post fix); regenerated water_pool golden.
The previous Beer's-law fix (b2690ac) tinted only the camera-primary segment when the camera was underwater. TIR-reflected underwater rays still escaped to the sky-color fallback, returning bright sky-blue through pixels that should be deep-water dark -- breaking the underwater illusion at any glancing-angle TIR pixel. Root cause: no per-ray "in_water" medium state. The MAT_WATER BRDF branch only knew about the immediate surface hit, not whether the ray had been travelling through water for several bounces. And the sky-miss fallback unconditionally returned skyColor(rd), regardless of medium. Fix: per-ray medium tracking through the bounce loop. 1. New int `in_water` state carried alongside throughput / ro / rd. Initialised at ray-gen from water_state.x -- the host scans analytic primitives for MAT_WATER planes and sets the bit when the camera position is on the below-the-surface side. 2. At each bounce, after traceScene() returns, apply per-segment Beer's-law attenuation: throughput *= exp(-absorb * h.t) when in_water == 1. This replaces the old nominal 1.5m entry-side estimate AND subsumes b2690ac's b == 0 exit-side special case with a single generic per-bounce attenuation. 3. At MAT_WATER hits, flip in_water on the refract path: - ndi_s < 0 (entering water from above) -> in_water = 1 - ndi_s > 0 (exiting water from below) -> in_water = 0 - reflect / TIR keep in_water unchanged (ray stays in medium). MAT_DIELECTRIC is left untouched -- the Phase 4 scope flags symmetric handling as optional, and the typical case (small glass body underwater) is fine with the conservative behaviour. 4. Sky-on-miss override: when a ray escapes to infinity AND in_water == 1, return throughput * water_state.yzw (the configurable deep-water tint, default near-black) instead of skyColor(rd). TIR rays from underwater no longer punch bright sky-blue holes through the water. Throughput still modulates the deep tint so segment absorption fades long-path rays toward true black. 5. New cvars: r_water_deep_color_r/g/b (default 0.0 = honest dark void). Plumbed through water_state.yzw on the new PtPush field. 6. New PtPush::water_state vec4. Layout-mirrored in PathTrace.slang's Push (Metal) and Frame (Vulkan / SPIR-V) cbuffers between motion_blur_params and editor_params. Adds 16 B to the push struct; static_assert and alignment guard updated. Verified on Mac M4 Max / Metal: * /tmp/agent24_under_v2.png (camera y=-1, pitch=75): dark navy surroundings, Snell's window cone visible in upper-right with sun-disc punch-through. TIR pixels (the rest of the frame) read as dark deep-water tint instead of bright sky. * /tmp/agent24_above_v2.png (water_pool.cfg standard): half- submerged sphere still refracts cleanly; underwater portion reads darker (post-fix segment absorption proportional to depth rather than nominal 1.5m). * /tmp/agent24_under_tir.png (cam y=-1, pitch=30, no objects): classic Snell's window centred, sun visible through the cone, TIR pixels outside the cone are dark navy not bright sky. * water_pool golden regenerated for the new look. * 20/20 non-golden ctest pass. * Metal goldens pass; sky-related software goldens (procedural_noon, bsc_night, bsc_night_clouds, smoke_emitter_basic, cornell_csg) all unchanged. Pre-existing Vulkan failures (cornell_csg vulkan, sdf_smin_row vulkan) unrelated -- per project memory Mac Vulkan was untested before 2026-05-16. PT_WATER_ENABLED gating preserved: when the megakernel is built without water support the new segment-absorption block, sky-miss override and in_water state are all compiled out (the existing PT_WATER_ENABLED guard pattern). The new cvars stay registered so saved configs don't error out on cvar-not-found.
1. Engine.cpp:7227 -- the in-water camera test used the wrong half- space inequality. `prim_plane` and intersectPlane define planes as `dot(n, p) + d = 0` (documented in `prim_plane`'s help text and used by intersectPlane on both CPU and shader paths). The below-surface half-space is `dot(n, cam) + d <= 0`, not `dot(n, cam) <= d`. Worked for d == 0 by coincidence (water_pool golden), but a water plane at any non-zero altitude would have seeded `in_water` incorrectly. Rewrote the test as `signed_dist = dot(n, cam) + d; signed_dist <= 0`. 2. PathTrace.slang:3808 -- the `in_water` docstring referenced MAT_DIELECTRIC flipping the medium state, but the dielectric branch was intentionally left untouched (per the issue's "optional but symmetric" callout). Removed the MAT_DIELECTRIC reference and replaced with an explicit note that dielectric refraction is out of scope for Phase 4 -- documented the simplification (glass body underwater continues attenuating with water absorption) and pointed at Phase 5 SSS for the proper multi-medium stack. Verified: - Bit-identical render output on all three Phase 4 test scenes (water_pool.cfg + the two underwater fixtures) -- expected since the regressed water plane has d == 0. - 20/20 non-golden ctest pass.
Copilot flagged a host/shader cbuffer offset mismatch: the host PtPush inserts smoke_params3 (wave-6 "looks like smoke") between smoke_params2 and motion_blur_params, but the shader Push and Frame cbuffers in PathTrace.slang went directly from smoke_params2 to motion_blur_params. Result: the shader's motion_blur_params reads the host's smoke_params3 bytes, and water_state (this PR's new field) reads what should be motion_blur_params -- underwater state seeded from random shutter_speed-shaped bytes. This was a pre-existing layout drift on the integration branch (the host side of smoke wave-6 landed in PR #25 but the matching shader change is still on the unmerged feat/smoke-wave6 branch, commit 91c3bb4 -- the shader's smoke BRDF that reads smoke_params3 lives there). Surfaced now because adding water_state past the misaligned region makes the drift visible (throwing off the actual rendered output rather than silently ignoring a no-op slot). Fix: add `float4 smoke_params3` to both the Metal Push cbuffer and the Vulkan Frame cbuffer in PathTrace.slang, between smoke_params2 and motion_blur_params. Docs the slot semantics matching what Engine.cpp writes; the in-shader reader (the smoke turbulence BRDF) is still on the unmerged feat/smoke-wave6 branch and will pick up the slot once that PR lands. Net effect today: trailing fields (motion_blur_params, water_state, editor_params) now read from the correct host offsets on both backends. Verified: - 20/20 non-golden ctest pass. - Underwater render output matches the pre-rebase v3 baseline (water tracking + Snell's window + dark TIR pixels all read correctly post fix); regenerated water_pool golden.
havokentity
force-pushed
the
feat/underwater-medium-tracking
branch
from
d750857 to
1f5aa9a
Compare
Copilot flagged a mismatch between the host's in-water scan and the shader's PRIM_PLANE intersection. PR #207 added a per-prim unit quaternion (AnalyticPrim::orient[4]) which the shader uses to rotate plane normals at intersect time via quatRotate (see PathTrace.slang PRIM_PLANE branch in testAnalyticPrim). My in-water test used the stored (unrotated) normal, so a rotated MAT_WATER plane could end up on one side in the shader and the other side in the host scan -- causing in_water to be seeded incorrectly. Fix: mirror the shader's rotation. Apply quatRotate(orient, pos_or_n) to the stored normal before the signed-distance test. Identity orient (0,0,0,1) collapses to n_raw so unrotated water planes are bit-identical to pre-fix; the new path activates only when the editor's rotate-mode gizmo (or a script) has set a non-identity orient on the water plane. Verified bit-identical render output on water_pool.cfg (water plane is identity orient there, so the new path is a no-op on this scene). 20/20 non-golden ctest pass.
Contributor
|
@havokentity I've opened a new pull request, #209, to work on those changes. Once the pull request is ready, I'll request review from you. |
…cs for primary-in-water)
Copilot flagged that the post-bounce atmospheric / volumetric march
fires on every dispatch with primary_t_recorded set, including
underwater frames. The march assumes an air medium (atmospheric
transmittance, Mie/Rayleigh in-scatter, cloud march -- all
sky-coloured) and would double-dim the camera-to-surface primary
segment AND add air-only single-scatter terms over a path that's
already attenuated by per-segment Beer's law in the bounce loop.
Same root issue as the pre-fix Beer's law: the renderer didn't
know the primary ray was underwater. Now that water_state.x +
in_water carries that signal through the bounce loop, gate the
two post-loop blocks behind a new `primary_in_water` flag latched
at ray-gen.
* `if (primary_hit && !primary_in_water) { ... }` -- skip surface
aerial perspective + HDRI haze additive on underwater frames.
* `(vol_outer_gate) && primary_t_recorded && !primary_in_water` --
skip atmospheric in-scatter march, cloud march, and smoke march
for the same reason.
A proper SSS volumetric in-scatter through a water medium is a
Phase 5 task; for Phase 4 we just don't paint air-only single
scatter on an underwater frame.
Verified:
- /tmp/agent24_under_tir_v4.png: the previously-faint blue tinge
in the lower-half "deep water" band is now a clean dark void.
- /tmp/agent24_under_v8.png: spheres + Snell's window still
render correctly.
- /tmp/agent24_above_v8.png: water_pool.cfg above-water render
is BIT-IDENTICAL to pre-fix -- the gates only activate when
cam_in_water at ray-gen is true.
- 20/20 non-golden ctest pass.
…ff path) Copilot flagged that `primary_in_water` was seeded from `water_state.x` even when `PT_WATER_ENABLED` was compiled off. In that build the water BRDF, in-loop segment absorption, sky-miss override, and the in_water medium tracking are ALL excised at compile time -- but the atmospheric / volumetric skip gates I added in d703a9b sit OUTSIDE the PT_WATER_ENABLED guard. Result: on a `-DPT_WATER_ENABLED=OFF` build, a scene with a MAT_WATER plane in its cfg (host still scans + sets water_state.x even though the BRDF is gone) and the camera below it would lose aerial perspective + volumetric in-scatter for no good reason -- the shader has no water medium to actually attenuate the primary ray, so the volumetric march is the only thing producing the correct in-air radiance. Fix: gate `primary_in_water` itself on PT_WATER_ENABLED. When the gate is off, force it to `false` so the atmospheric / volumetric skips never fire -- matching what the rest of the megakernel already does. Verified water_pool.cfg renders bit-identical to d703a9b (the gate is on in this build); 20/20 non-golden ctest pass.
8 tasks
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
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.
Summary
Fixes the last failure mode in #134's underwater rendering: TIR-reflected
rays escaping to the sky-color fallback returned bright sky-blue, which
punched holes through the deep-water illusion at glancing angles.
The previous fix (b2690ac) tinted only the camera-primary segment when
the camera was underwater. This PR replaces all the special-case Beer's-
law branches with proper per-ray medium tracking carried through the
bounce loop.
int in_waterstate initialised from the newwater_state.xpush field. Host scans analytic primitives forMAT_WATER planes and sets the bit when the camera is on the
below-the-surface side.
iteration when
in_water == 1. Subsumes the old 1.5m entry-sideestimate and b2690ac's b==0 exit-side hack into one generic block.
in_water; TIR / reflection keep it(the ray stays in the same medium). MAT_DIELECTRIC left untouched per
the issue's "optional but symmetric" callout.
throughput * water_state.yzw(configurable deep tint, defaultnear-black). Throughput still modulates so long-path rays fade to true
black via segment absorption along the way.
r_water_deep_color_r/g/b(defaults 0.0) plumbed throughthe new
water_state.yzw.PtPush::water_statevec4 (mirror in Push + Frame cbuffers);static_assert + alignment guard updated.
Test plan
/tmp/agent24_under_v2.png(cam y=-1, pitch=75): Snell's windowsun-disc visible, TIR pixels read DARK not sky-blue.
/tmp/agent24_above_v2.png(water_pool.cfg standard): half-submerged sphere refracts cleanly, deeper underwater portions read
darker (proper segment absorption replacing 1.5m nominal).
/tmp/agent24_under_tir.png(cam y=-1, pitch=30, geometry-free):Snell's window cone visible upper centre, TIR pixels outside the
cone are dark navy not bright sky.
bsc_night_clouds, smoke_emitter_basic, smoke_emitter_basic).
(cornell_csg vulkan, sdf_smin_row vulkan) -- per project memory
Mac Vulkan was untested before 2026-05-16.