Skip to content

fix(foliage): flip foliage normal and fix sss#2344

Merged
alandtse merged 1 commit into
devfrom
fixfoliage
May 15, 2026
Merged

fix(foliage): flip foliage normal and fix sss#2344
alandtse merged 1 commit into
devfrom
fixfoliage

Conversation

@jiayev
Copy link
Copy Markdown
Collaborator

@jiayev jiayev commented May 14, 2026

This pull request introduces improvements to subsurface lighting and normal handling in tree animation shaders, as well as a small fix for screen space shadowing. The changes enhance the realism of foliage rendering and prevent potential shadow artifacts.

Tree Animation and Subsurface Lighting:

  • package/Shaders/Common/PBR.hlsli: Added a special-case branch for subsurface lighting when TREE_ANIM is defined, using a simplified calculation tailored for animated foliage.
  • package/Shaders/Lighting.hlsl: When TREE_ANIM is defined, transforms the surface normal to ensure consistent lighting for animated tree geometry by adjusting the normal in view space and converting it back to world space.

Screen Space Shadows:

  • package/Shaders/Lighting.hlsl: Adds a check to only apply screen space shadows if the directional light angle is non-negative, preventing shadow artifacts in certain interior or edge cases.

Summary by CodeRabbit

  • Visual Improvements

    • Improved foliage and vegetation lighting in direct light conditions
    • Enhanced shadow rendering with directional lighting awareness in outdoor environments
  • Performance

    • Optimized shader compilation for specific rendering scenarios

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

📝 Walkthrough

Walkthrough

Two shader files receive conditional logic and calculations for tree animation rendering. PBR transmission uses foliage-specific attenuation when TREE_ANIM is active. Lighting applies normal-space transformation and enforces a sun-facing constraint on Screen Space Shadows during vegetation shading.

Changes

Tree Animation Rendering

Layer / File(s) Summary
Subsurface transmission for vegetation
package/Shaders/Common/PBR.hlsli
GetDirectLightInput branches Flags::Subsurface transmission: non-TREE_ANIM uses forward/back scatter with softLightColor; TREE_ANIM uses foliage attenuation from -NdotL with detailedLightColor.
Normal transformation and shadow conditions for vegetation
package/Shaders/Lighting.hlsl
MODELSPACENORMALS path adds TREE_ANIM conditional to adjust worldNormal via view-space Z flip; Screen Space Shadows now check both interior and sun-facing (dirLightAngle >= 0.0).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • alandtse

Poem

🐰 Leaves now scatter light with care,
Normals flip to face the air,
Shadows skip when sun's not there—
Trees stand tall and glow so fair,
Animation's magic in the shader pair! ✨🌳

🚥 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 directly addresses the main changes: 'flip foliage normal' corresponds to the TREE_ANIM normal adjustment in Lighting.hlsl, and 'fix sss' refers to the subsurface scattering fix in PBR.hlsli. It is concise and clearly summarizes the primary objectives.
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 fixfoliage

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/Common/PBR.hlsli (1)

175-188: ⚡ Quick win

Add a closing issue reference in the PR description.

Since this is a bug-fix PR, consider adding Fixes #<id> or Closes #<id> for traceability.

As per coding guidelines "Issue References (if PR fixes bugs or implements features): Suggest adding appropriate GitHub keywords: 'Fixes #123' or 'Closes #123' for bug fixes."

🤖 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/Common/PBR.hlsli` around lines 175 - 188, The PR is a bug-fix
but lacks a closing issue reference; update the PR description to include a
GitHub keyword like "Fixes #<id>" or "Closes #<id>" referencing the tracked bug
so the issue is auto-closed on merge—add that line (e.g., Fixes `#123`) to the PR
description and ensure the issue ID corresponds to the bug being fixed in the
changes around material.SubsurfaceColor / subsurface foliage handling in
PBR.hlsli.
🤖 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/Common/PBR.hlsli`:
- Around line 175-188: The PR is a bug-fix but lacks a closing issue reference;
update the PR description to include a GitHub keyword like "Fixes #<id>" or
"Closes #<id>" referencing the tracked bug so the issue is auto-closed on
merge—add that line (e.g., Fixes `#123`) to the PR description and ensure the
issue ID corresponds to the bug being fixed in the changes around
material.SubsurfaceColor / subsurface foliage handling in PBR.hlsli.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5058781a-7257-437a-9e9d-764b44b295f0

📥 Commits

Reviewing files that changed from the base of the PR and between c2c2cd5 and d719974.

📒 Files selected for processing (2)
  • package/Shaders/Common/PBR.hlsli
  • package/Shaders/Lighting.hlsl

@alandtse alandtse merged commit 557ffc9 into dev May 15, 2026
22 checks passed
ParticleTroned pushed a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request May 16, 2026
@jiayev jiayev deleted the fixfoliage 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.

2 participants