feat(height fog): separate inscattering color settings#2301
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReparameterizes exponential height fog: replaces directional exponent with Henyey‑Greenstein anisotropy, adds sunlight attenuation and inscattering color controls, exposes a “disable vanilla fog” toggle, and updates multiple shaders to layer and optionally bypass vanilla fog when composing exponential fog. ChangesExponential Height Fog Enhancement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Sequence Diagram(s)sequenceDiagram
participant Settings as Settings/UI
participant Weather as Weather Registry
participant Shader as Pixel Shader
participant Pixel as Pixel Output
Settings->>Weather: register new variables (inscatter, anisotropy, attenuation, disable flag)
Pixel->>Shader: sample scene color, vanilla fog
Shader->>Settings: query exponentialHeightFog settings
Shader->>Shader: compute exponential inscatter (HenyeyGreenstein, sunlight attenuation)
Shader->>Pixel: blend vanilla and exponential layers (maybe bypass vanilla)
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
…ment Henyey-Greenstein phase function
…or amount settings
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package/Shaders/Water.hlsl (1)
1282-1295: ⚡ Quick winExtract the new water-fog blend into a helper.
These two blocks are effectively the same implementation with different local variable names. Pulling them into a small helper would keep future
ShouldDisableVanillaFog()changes in one place and reduce the odds of the two water paths drifting apart again.Also applies to: 1333-1346
🤖 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 `@package/Shaders/Water.hlsl` around lines 1282 - 1295, Extract the repeated water-fog blending logic into a small helper function (e.g., BlendWaterFog) and replace the duplicated blocks in both branches (the block under ExponentialHeightFog::ShouldDisableVanillaFog() and the else block, and the similar occurrence at lines ~1333-1346) with calls to that helper; the helper should accept the inputs fogColor, finalColorPreFog, eyeIndex, fogDistanceFactor and exponentialHeightFog.w (or the full exponentialHeightFog.xyz and .w if you prefer) and perform the GetWaterFogFade(eyeIndex) multiplication and the two lerp blends currently done with fogDistanceFactor and exponentialHeightFog.w so both paths use the exact same implementation and names are unified.
🤖 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 `@package/Shaders/Water.hlsl`:
- Around line 1333-1346: The refraction path still forces fogFactor = 0 for
exponential fog, which breaks the new disableVanillaFog toggle; update the
refraction handling (the code that sets fogFactor and computes
refractionColor/finalColorPreFog) to honor
ExponentialHeightFog::ShouldDisableVanillaFog(): either gate the zeroing of
fogFactor with ExponentialHeightFog::ShouldDisableVanillaFog() or apply the
exponential fog blend (using exponentialHeightFog.xyz, exponentialHeightFog.w
and GetWaterFogFade(eyeIndex)) to refractionColor just like the non-refraction
path does; ensure you touch the logic that computes finalColorPreFog,
refractionColor and fogFactor so the new toggle is consistently respected.
---
Nitpick comments:
In `@package/Shaders/Water.hlsl`:
- Around line 1282-1295: Extract the repeated water-fog blending logic into a
small helper function (e.g., BlendWaterFog) and replace the duplicated blocks in
both branches (the block under ExponentialHeightFog::ShouldDisableVanillaFog()
and the else block, and the similar occurrence at lines ~1333-1346) with calls
to that helper; the helper should accept the inputs fogColor, finalColorPreFog,
eyeIndex, fogDistanceFactor and exponentialHeightFog.w (or the full
exponentialHeightFog.xyz and .w if you prefer) and perform the
GetWaterFogFade(eyeIndex) multiplication and the two lerp blends currently done
with fogDistanceFactor and exponentialHeightFog.w so both paths use the exact
same implementation and names are unified.
🪄 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: bc687227-9f8d-46de-9131-5827803dde13
📒 Files selected for processing (9)
features/Exponential Height Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlslifeatures/Exponential Height Fog/Shaders/Features/ExponentialHeightFog.inipackage/Shaders/Common/SharedData.hlslipackage/Shaders/Effect.hlslpackage/Shaders/ISSAOComposite.hlslpackage/Shaders/Lighting.hlslpackage/Shaders/Water.hlslsrc/Features/ExponentialHeightFog.cppsrc/Features/ExponentialHeightFog.h
|
✅ A pre-release build is available for this PR: |
This pull request introduces significant improvements and new features to the Exponential Height Fog system, focusing on enhanced physical realism, greater artistic control, and improved integration with vanilla fog. The most important changes include the addition of a physically-based inscattering model using the Henyey-Greenstein phase function, new settings for controlling fog color and sunlight attenuation, and options to blend or disable vanilla fog. The UI and serialization logic have also been updated to support these new parameters.
Physically-based fog and inscattering improvements:
Implemented the Henyey-Greenstein phase function for directional inscattering, allowing for anisotropic (directional) light scattering effects and more realistic fog appearance. The new parameter
directionalInscatteringAnisotropyreplaces the old exponent-based model. [1] [2] [3] [4] [5] [6] [7]Added new fog color blending controls:
fogInscatteringColor(custom color added to fog), andoriginalFogColorAmount(controls blending between the original and custom fog color). These are exposed in the UI and weather variable registry. [1] [2] [3] [4] [5]Sunlight and vanilla fog integration:
Introduced
sunlightAttenuationAmountto control how much direct sunlight is attenuated by the exponential height fog, improving control over sunlight/fog interactions. [1] [2] [3] [4] [5]Added
disableVanillaFogoption and blending logic, allowing users to disable vanilla fog entirely or blend it with exponential height fog. All affected shaders now respect this setting, and the UI exposes a checkbox with a tooltip. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]UI, serialization, and versioning updates:
These changes provide both improved physical accuracy and greater flexibility for artists and users configuring the fog system.
Summary by CodeRabbit
New Features
Improvements
Other