Skip to content

Address review feedback on Phase 4 underwater medium tracking without expanding scope#209

Draft
Copilot wants to merge 1 commit into
feat/underwater-medium-trackingfrom
copilot/sub-pr-206
Draft

Address review feedback on Phase 4 underwater medium tracking without expanding scope#209
Copilot wants to merge 1 commit into
feat/underwater-medium-trackingfrom
copilot/sub-pr-206

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 19, 2026

This stacked PR resolves review feedback on the Phase 4 underwater-medium work while keeping the editor_params host/shader mismatch explicitly out of scope (it is pre-existing from PR #201). The branch focuses on correctness of water-state seeding/layout and clarifies shader behavior boundaries.

  • Host-side water-plane classification fix

    • Corrected camera-in-water detection to use the engine’s plane convention dot(n, p) + d = 0.
    • Replaced the previous comparison with signed-distance logic so non-zero-height water planes seed water_state.x correctly.
  • Push/cbuffer alignment intent preserved

    • Kept water_state at the intended offset for Phase 4 medium tracking.
    • Did not fold in unrelated editor_params host-struct expansion in this PR.
  • Shader documentation cleanup (behavioral scope)

    • Updated in_water comments to match implementation: Phase 4 toggles on MAT_WATER refract path only.
    • Removed misleading mention that dielectric path flips medium state.
// Engine-side camera-under-water check (plane: dot(n, p) + d = 0)
const float signed_dist =
    nx * cam.pos.x + ny * cam.pos.y + nz * cam.pos.z + d;
if (signed_dist <= 0.0f) { cam_in_water = true; }

Copilot AI changed the title [WIP] [WIP] Address feedback on editor_params bug in underwater medium tracking PR Address review feedback on Phase 4 underwater medium tracking without expanding scope May 19, 2026
Copilot AI requested a review from havokentity May 19, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants