Skip to content

fix(VR): interior water submersion masks#2075

Merged
davo0411 merged 4 commits into
community-shaders:devfrom
alandtse:int_underwater_masks
Apr 10, 2026
Merged

fix(VR): interior water submersion masks#2075
davo0411 merged 4 commits into
community-shaders:devfrom
alandtse:int_underwater_masks

Conversation

@alandtse
Copy link
Copy Markdown
Collaborator

@alandtse alandtse commented Apr 8, 2026

Summary by CodeRabbit

  • Bug Fixes
    • VR underwater rendering now falls back to a system-derived water height when tile data is missing, improving mask correctness.
    • Per-eye camera-relative adjustments make underwater masks more stable and consistent across stereo views.
    • Engine water-query failures no longer produce incorrect default water values, yielding more reliable underwater visuals.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 8, 2026

📝 Walkthrough

Walkthrough

Adds a VR fallback water-height source: shaders use SharedData::WaterSystemHeight when tile lookups return the "no water" sentinel, applying per-eye camera-relative Z offsets. CPU populates this field from player/TES water height when available and TryGetWaterData prefers cell->GetWaterHeight(...) if successful.

Changes

Cohort / File(s) Summary
Shared Constant Buffer
package/Shaders/Common/SharedData.hlsli, src/State.h
Replaced dummy padding with float WaterSystemHeight in the shared cbuffer/struct to expose a system-level water height (camera-relative Z, sentinel when absent).
CPU-side water computation
src/State.cpp, src/Utils/Game.cpp
State::UpdateSharedData initializes and conditionally sets WaterSystemHeight from TES/player water height (converted to eye‑0 camera-relative Z) for VR fallback. Util::TryGetWaterData now uses cell->GetWaterHeight(...) when available and leaves w as sentinel on failure.
VR underwater mask shader
features/Upscaling/Shaders/Upscaling/UnderwaterMaskUpscalePS.hlsl
When tile lookup equals NO_TILE sentinel, shader reads SharedData::WaterSystemHeight and recomputes per‑eye waterHeight using FrameBuffer::CameraPosAdjust.z offsets (eye 0 as reference) before proceeding with per‑eye mask logic; comments updated.

Sequence Diagram

sequenceDiagram
    participant CPU as CPU (State.cpp)
    participant GameUtil as Game Utils (TryGetWaterData)
    participant ConstBuf as SharedData ConstBuffer
    participant Shader as UnderwaterMaskUpscalePS

    CPU->>GameUtil: TryGetWaterData(position)
    alt GetWaterHeight succeeds
        GameUtil-->>CPU: cell water height (camera/world)
    else
        GameUtil-->>CPU: no-tile sentinel (-FLT_MAX)
    end

    CPU->>CPU: If VR & player underwaterCount>0\ncompute worldWater = player->GetWaterHeight()
    alt worldWater valid
        CPU->>CPU: WaterSystemHeight = worldWater - eye0.z
    else
        CPU->>CPU: WaterSystemHeight = sentinel
    end
    CPU->>ConstBuf: write WaterSystemHeight

    Shader->>ConstBuf: read tile water value (data.w)
    alt tile != NO_TILE
        Shader->>Shader: use tile-based per-eye waterHeight
    else
        Shader->>ConstBuf: read WaterSystemHeight
        Shader->>Shader: apply per-eye CameraPosAdjust.z (eye0 reference)
        Shader->>Shader: reconstruct per-eye underwater mask
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • davo0411

Poem

🐰
When tiles are gone and waters hide,
I nibble offsets, hop inside.
Eye-zero leads, the others follow,
Masks rebuilt beyond the hollow.
Splash—fallback saves our gentle waddle 💧

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: fixing VR interior water submersion masks by implementing a fallback mechanism for water height detection.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 8, 2026

No actionable suggestions for changed features.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 8, 2026

✅ A pre-release build is available for this PR:
Download

@alandtse alandtse force-pushed the int_underwater_masks branch from 647e77f to 5f328c6 Compare April 9, 2026 02:23
Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
Comment thread src/State.cpp
Comment thread src/State.cpp Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
package/Shaders/Common/SharedData.hlsli (1)

27-27: Keep sentinel terminology consistent across CPU and shader comments.

This comment uses -FLT_MAX, while the C++ side documents -NI_INFINITY. Consider using one term consistently (or explicitly noting they map to the same sentinel) to reduce ambiguity during debugging.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package/Shaders/Common/SharedData.hlsli` at line 27, The shader comment for
the WaterSystemHeight field uses the sentinel value "-FLT_MAX" while the C++/CPU
side documents "-NI_INFINITY"; update the comment for WaterSystemHeight (and any
nearby shader comments) to use the same sentinel term as the CPU side (use
"-NI_INFINITY") or explicitly state they are equivalent (e.g., "-NI_INFINITY
(-FLT_MAX on shaders)") so TES::GetWaterHeight mapping is unambiguous across CPU
and shader code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@package/Shaders/Common/SharedData.hlsli`:
- Line 27: The shader comment for the WaterSystemHeight field uses the sentinel
value "-FLT_MAX" while the C++/CPU side documents "-NI_INFINITY"; update the
comment for WaterSystemHeight (and any nearby shader comments) to use the same
sentinel term as the CPU side (use "-NI_INFINITY") or explicitly state they are
equivalent (e.g., "-NI_INFINITY (-FLT_MAX on shaders)") so TES::GetWaterHeight
mapping is unambiguous across CPU and shader code.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0706e6b9-88e3-4d20-af7e-b01e7fe8debd

📥 Commits

Reviewing files that changed from the base of the PR and between df73cbb and 37686c2.

📒 Files selected for processing (3)
  • package/Shaders/Common/SharedData.hlsli
  • src/State.cpp
  • src/State.h

@davo0411 davo0411 merged commit 6e4cd11 into community-shaders:dev Apr 10, 2026
17 checks passed
ParticleTroned pushed a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request May 2, 2026
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
(cherry picked from commit 6e4cd11)
ParticleTroned added a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request May 5, 2026
Adds a VR diagnostics switch to bypass the community-shaders#2062/community-shaders#2075 underwater mask changes for in-game A/B testing.

The toggle disables the analytical underwater mask shader path, the TESWaterSystem fallback height, and the shared VR water-height eye correction while preserving SRV unbind safety.
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.

3 participants