Skip to content

fix(grass): no screen space shadow on back faces#2363

Merged
alandtse merged 1 commit into
devfrom
fix-grass-shadow
May 18, 2026
Merged

fix(grass): no screen space shadow on back faces#2363
alandtse merged 1 commit into
devfrom
fix-grass-shadow

Conversation

@jiayev
Copy link
Copy Markdown
Collaborator

@jiayev jiayev commented May 17, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved grass shadow rendering accuracy by refining how screen-space shadows are applied based on directional lighting angle and environmental location, enhancing visual realism in outdoor scenes.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 17, 2026

📝 Walkthrough

Walkthrough

The grass pixel shader's screen-space shadow calculation is now guarded by an additional check: the directional light angle must be non-negative. This prevents shadow application when the fragment is backlit, refining the existing interior-space check.

Changes

Grass Shader Screen-Space Shadow Guard

Layer / File(s) Summary
Screen-space shadow directional light angle check
package/Shaders/RunGrass.hlsl
The conditional gating screen-space shadow application now requires both !SharedData::InInterior and dirLightAngle >= 0.0, tightening when shadows are applied to the grass directional lighting calculation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • doodlum
  • alandtse

Poem

🐰 A shader tweak, so light and clean,
No inner glow from unseen gleam,
When angles wane below the shore,
The grass needs shadows less, not more! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: applying screen-space shadow only when the directional light angle is >= 0.0, which prevents shadows on back-facing surfaces.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-grass-shadow

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

No actionable suggestions for changed features.

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/RunGrass.hlsl (1)

609-609: PR follows conventional commit format correctly.

The title fix(grass): no screen space shadow on back faces properly follows the conventional commits specification. If this change addresses a specific reported issue or feature request, consider adding an issue reference (e.g., "Fixes #XXXX" or "Closes #XXXX") to the PR description for better traceability. As per coding guidelines, appropriate GitHub keywords help link PRs to issues.

🤖 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/RunGrass.hlsl` at line 609, The PR title is fine but the
PR/commit body should include an issue reference for traceability; update the PR
description (or amend the commit message that introduced the change around the
conditional in RunGrass.hlsl — the if statement using SharedData::InInterior and
dirLightAngle) to include a GitHub keyword like "Fixes #<issue-number>" or
"Closes #<issue-number>" that points to the related bug/feature so the change is
automatically linked and closed by the PR.
🤖 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.

Nitpick comments:
In `@package/Shaders/RunGrass.hlsl`:
- Line 609: The PR title is fine but the PR/commit body should include an issue
reference for traceability; update the PR description (or amend the commit
message that introduced the change around the conditional in RunGrass.hlsl — the
if statement using SharedData::InInterior and dirLightAngle) to include a GitHub
keyword like "Fixes #<issue-number>" or "Closes #<issue-number>" that points to
the related bug/feature so the change is automatically linked and closed by the
PR.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4f094cae-2cc3-4d89-b9bd-01de421bd3b2

📥 Commits

Reviewing files that changed from the base of the PR and between 8c9b18c and dd96ce9.

📒 Files selected for processing (1)
  • package/Shaders/RunGrass.hlsl

@alandtse alandtse merged commit 5d60c52 into dev May 18, 2026
14 checks passed
@jiayev jiayev deleted the fix-grass-shadow branch May 22, 2026 02:37
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