feat: terrain blending ready for release#1405
Conversation
WalkthroughUpdated terrain blending in the pixel shader to compute a per-pixel stochastic blend (used for NormalGlossiness.w and Albedo.w); removed the Changes
Sequence Diagram(s)sequenceDiagram
participant PS as Pixel Shader
participant SN as screenNoise
participant BF as blendFactorTerrain
participant OUT as psout
PS->>SN: sample screenNoise
PS->>BF: read blendFactorTerrain
PS->>PS: compute stochasticBlend = lerp(screenNoise, blendFactorTerrain, 0.1)
PS->>OUT: set NormalGlossiness.w = stochasticBlend
PS->>OUT: set Albedo.w = stochasticBlend
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
package/Shaders/Lighting.hlsl (2)
3380-3385: Stochastic terrain alpha looks good; verify alpha consistency across all GBuffer targetsNice improvement switching to a noise-thresholded alpha and lightly biasing toward the analytic factor. One concern: only NormalGlossiness.w and Albedo.w are switched to the new stochastic value, while psout.Diffuse.w remains the analytic blendFactorTerrain. Earlier in the deferred path, several targets’ .w channels (e.g., MotionVectors.zw pack psout.Diffuse.w; Specular.w, Reflectance.w, Masks.w, and SNOW Parameters.w) are set from psout.Diffuse.w before this block.
If downstream passes assume the terrain-blend alpha is consistent across GBuffer attachments, these divergent alphas can produce subtle edge artifacts or halos. Please verify the consumer expectations and, if necessary, unify:
- Option A (preferred): Compute stochasticBlend immediately after blendFactorTerrain is computed (near Lines 1020–1033) and set psout.Diffuse.w to it before any target writes; keep NormalGlossiness/Albedo referencing psout.Diffuse.w as before; remove this late override.
- Option B: If you intend different alphas per target, add a brief comment explaining the rationale to prevent future regressions.
Also, since screenNoise includes FrameCount, the dither pattern is animated. That’s usually fine with TAA, but can shimmer when TAA is off. Consider gating the temporal component or using a blue-noise sequence if you see visible flicker.
3381-3384: Minor: prefer step() and name the smoothing constantPure nit, but step() reads a bit clearer and avoids a conditional; also, name the smoothing to make it tunable.
Apply this diff within this block:
- float stochasticBlend = screenNoise < blendFactorTerrain ? 1.0 : 0.0; - stochasticBlend = lerp(stochasticBlend, blendFactorTerrain, 0.1); + const float kTerrainBlendSmoothing = 0.1; + float stochasticBlend = step(screenNoise, blendFactorTerrain); + stochasticBlend = lerp(stochasticBlend, blendFactorTerrain, kTerrainBlendSmoothing);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
package/Shaders/Lighting.hlsl(1 hunks)src/FeatureIssues.cpp(0 hunks)
💤 Files with no reviewable changes (1)
- src/FeatureIssues.cpp
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.
Applied to files:
package/Shaders/Lighting.hlsl
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Build plugin and addons
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
…kyrim-community-shaders into terrain-blending-ready
|
✅ A pre-release build is available for this PR: |
1 participant
Summary by CodeRabbit