feat(grass lighting): add wrapped lighting toggle#1721
Conversation
toggles the use of a wrapped lighting model on grass from an older version of CS. Prevents certain grass mods from being too dark when sunlight is directly overhead.
Add toggle checkbox under lighting tree section of grass lighting UI
default value is disabled
adds toggleable wrapped light function if setting is enabled
📝 WalkthroughWalkthroughAdds a toggleable EnableWrappedLighting flag in shader shared data, engine settings/serialization, and UI, and conditions the grass shader to choose between wrapped-lighting math and the original diffuse lighting path. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant EngineSettings
participant Shader
participant GPU
User->>UI: Toggle "Enable Wrapped Lighting"
UI->>EngineSettings: Update EnableWrappedLighting
EngineSettings->>Shader: Supply SharedData (EnableWrappedLighting)
Shader->>GPU: Execute grass shader per-pixel
alt EnableWrappedLighting == true
Shader->>GPU: compute wrapAmount\ncompute wrappedDirLight\naccumulate wrapped diffuse
else
Shader->>GPU: compute saturate(dirLightAngle)\naccumulate standard diffuse
end
GPU-->>Shader: lighting result
Shader-->>EngineSettings: render output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@package/Shaders/RunGrass.hlsl`:
- Around line 632-643: The wrapped lighting branch in RunGrass.hlsl (guarded by
SharedData::grassLightingSettings.EnableWrappedLighting) double-applies
dirDetailShadow and omits the grass diffuse multiplier: adjust the
lightsDiffuseColor assignment so it does not multiply by dirDetailShadow again
if dirLightColor already includes it, and add Color::GrassDiffuseMult() just
like the standard branch; specifically, replace the current lightsDiffuseColor
+= dirLightColor * saturate(wrappedDirLight) * dirDetailShadow; with a single
multiplication that uses dirLightColor (no extra dirDetailShadow) times
saturate(wrappedDirLight) times Color::GrassDiffuseMult() to match the standard
model.
|
✅ A pre-release build is available for this PR: |
Listened to rabbit, replaced dirdetailshadow with grassdiffusemultiplier on line 637, farther away grass looks better now.
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
update tootltip description explanation
doodlum
left a comment
There was a problem hiding this comment.
Point lights do not get the new model
add wrapped model to point lights when enabled
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
doodlum
left a comment
There was a problem hiding this comment.
wrapAmount is duplicated
remove duplicate wrapamount float, moved first wrapamount float outside of the "if" function so it could be read further down
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
Adds the option to enable a wrapped lighting model on grass, a function taken from a CS v1.3. Prevents certain grass mods from looking too dark when sunlight is directly overhead.
Summary by CodeRabbit
New Features
UI
✏️ Tip: You can customize this high-level summary in your review settings.