fix(water): TAA water black edge line#2489
Conversation
📝 WalkthroughWalkthroughThis PR introduces height-aware wetness effects for water simulation by adding a new HLSL shader header with displacement-based cavity detection, gradient normal computation, and pool mask shaping logic, alongside C++ allocation of TAA water history render targets to store the computed effects. ChangesHeight-Aware Wetness Effects and TAA Water Rendering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.22.0)OpenGrep fatal error (exit code 2): [00.12][ERROR]: Error: exception Unix_error: No such file or directory stat src/Deferred.cpp 🔧 Infer (1.2.0)src/Deferred.cppUsage Error: Failed to execute compilation command: Error message: *** Infer needs a working compilation command to run. 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. Comment |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Deferred.cpp (1)
114-116: ⚡ Quick winAdd an issue-closing keyword in the PR description (if tracked).
Since this PR is a bug fix, please add a reference like
Fixes #<id>/Closes #<id>when there is a linked issue.As per coding guidelines, bug-fix PRs should include issue references using GitHub keywords.
🤖 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 `@src/Deferred.cpp` around lines 114 - 116, Update the PR description to include a GitHub issue-closing keyword referencing the tracked bug (e.g., add "Fixes #<id>" or "Closes #<id>") so the fix in Deferred.cpp (see SetupRenderTarget calls for RE::RENDER_TARGETS::kWATER_1 and kWATER_2) will automatically close the linked issue; if there are multiple related issues list each with its own keyword and ensure the PR description is saved/updated before merge.Source: Coding guidelines
🤖 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.
Inline comments:
In `@features/Wetness` Effects/Shaders/WetnessEffects/WetnessEffectsHeight.hlsli:
- Around line 57-68: GetTerrainHeightGradientNormal currently performs an unused
center height sample (hCenter) causing an extra texture fetch; remove the
hCenter declaration and its SampleTerrainDisplacementHeight call from
GetTerrainHeightGradientNormal so only the neighbor samples (used to compute hX
and hY) remain, leaving the ddx/ddy, hX/hY computation and final normalization
(mul(tbn, gradTS)) unchanged; verify no other references to hCenter in that
function.
---
Nitpick comments:
In `@src/Deferred.cpp`:
- Around line 114-116: Update the PR description to include a GitHub
issue-closing keyword referencing the tracked bug (e.g., add "Fixes #<id>" or
"Closes #<id>") so the fix in Deferred.cpp (see SetupRenderTarget calls for
RE::RENDER_TARGETS::kWATER_1 and kWATER_2) will automatically close the linked
issue; if there are multiple related issues list each with its own keyword and
ensure the PR description is saved/updated before merge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c2f3e29-58be-45a5-aab8-d8e9db44cb16
📒 Files selected for processing (2)
features/Wetness Effects/Shaders/WetnessEffects/WetnessEffectsHeight.hlslisrc/Deferred.cpp
|
✅ A pre-release build is available for this PR: |
No description provided.