Skip to content

fix(sss): prevent divide 0 albedo when blend decals#1489

Closed
jiayev wants to merge 1 commit into
community-shaders:devfrom
jiayev:burley-fix
Closed

fix(sss): prevent divide 0 albedo when blend decals#1489
jiayev wants to merge 1 commit into
community-shaders:devfrom
jiayev:burley-fix

Conversation

@jiayev
Copy link
Copy Markdown
Collaborator

@jiayev jiayev commented Sep 18, 2025

This pull request updates the subsurface scattering shader logic in Burley.hlsli to improve color normalization and mask handling. The changes make the subsurface scattering effect more robust by using mask-driven blending for albedo normalization and sample inclusion, leading to more accurate rendering results.

Subsurface Scattering Improvements

  • Albedo normalization now uses a blend between 1.0 and the albedo value based on sssAmount, rather than always normalizing by the albedo, which improves color accuracy in regions with low subsurface scattering.
  • Sample inclusion in the scattering computation is now gated by a mask value, ensuring only relevant pixels contribute to the effect. The mask is checked before processing each sample.
  • Sample color normalization uses mask-driven blending, similar to the center pixel, for consistency and better handling of masked regions.
  • The final output color is now blended between 1.0 and the albedo based on sssAmount, rather than always multiplying by the albedo, which helps maintain correct color in non-scattering regions.

Code Clean-up

  • Removed redundant mask logic from the sample weight calculation, since samples are now filtered by the mask earlier in the loop.

Summary by CodeRabbit

  • New Features
    • Subsurface Scattering now supports per-pixel masking, skipping contributions in fully masked areas for cleaner results.
    • Intensity smoothly scales with the SSS amount, providing more predictable control.
    • Color contribution blends with albedo based on SSS settings and mask, improving realism and reducing over-tinting.
    • Sampling weights are more consistent, enhancing visual stability across samples.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 18, 2025

Walkthrough

The Burley subsurface scattering shader adjusts albedo scaling to interpolate with sssAmount, applies per-sample masking with early skips, scales diffuse mean free path by sssAmount, removes redundant mask checks, and makes sample weight unconditional. Final color composition now lerps with sssAmount rather than multiplying directly by AlbedoTexture.

Changes

Cohort / File(s) Summary of Changes
Burley SSS shader logic updates
features/Subsurface Scattering/Shaders/SubsurfaceScattering/Burley.hlsli
- OriginalColor divisor: use lerp(1.0, max(surfaceAlbedo, EPSILON), sssAmount)
- Scale diffuseMeanFreePath by sssAmount
- Per-sample: add MaskTexture gating with early skip; albedo in sampleColor lerps with sampleMask; remove redundant mask re-check
- Make sampleWeight unconditional: (diffusionProfile / pdf) * normalWeight
- Final color: replace direct AlbedoTexture multiply with lerp(1.0, AlbedoTexture, sssAmount)

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant R as Renderer
  participant S as Burley.hlsli (Shader)
  participant T as Textures (Albedo, Mask)

  R->>S: Invoke SSS pass
  Note over S: Compute centerColor/weight<br/>Adjust albedo divisor via sssAmount
  S->>S: diffuseMeanFreePath *= sssAmount

  loop For each sample
    S->>T: Sample MaskTexture
    alt mask <= epsilon
      S-->>S: Skip sample
    else mask > epsilon
      S->>T: Sample AlbedoTexture
      Note over S: sampleColor uses lerp(1.0, max(Albedo, EPS), sampleMask)
      S->>S: Compute pdf, diffusionProfile, normalWeight
      S-->>S: sampleWeight = (diffusionProfile / pdf) * normalWeight
      S-->>S: Accumulate color and weight
    end
  end

  Note over S: Final color scales via lerp(1.0, AlbedoTexture, sssAmount)
  S-->>R: Output SSS color
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • Pentalimbed
  • doodlum

Poem

I burrow through pixels, soft and bright,
Nudging masks to guide the light.
With sssAmount I gently blend,
Paths of cream where colors wend.
A hop, a lerp—then home I glide,
Through scattering fields, diffuse and wide. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely identifies the primary fix—preventing a divide-by-zero on albedo when blending decals—and aligns with the PR objectives and the Burley.hlsli changes to albedo normalization and mask handling, so it accurately summarizes the main change for reviewers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3f462fa and 57101d2.

📒 Files selected for processing (1)
  • features/Subsurface Scattering/Shaders/SubsurfaceScattering/Burley.hlsli (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not include TODO/FIXME placeholders; provide complete, working solutions

Files:

  • features/Subsurface Scattering/Shaders/SubsurfaceScattering/Burley.hlsli
features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}: Place all feature shaders under features/YourFeature/Shaders/
Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)

Files:

  • features/Subsurface Scattering/Shaders/SubsurfaceScattering/Burley.hlsli
⏰ 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: Build plugin and addons
  • GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
  • GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
🔇 Additional comments (6)
features/Subsurface Scattering/Shaders/SubsurfaceScattering/Burley.hlsli (6)

62-62: Good fix for preventing divide-by-zero with decals.

The change from dividing by surfaceAlbedo to using lerp(1.0, max(surfaceAlbedo.xyz, EPSILON_SSS_ALBEDO), sssAmount) effectively prevents divide-by-zero errors when albedo is near zero (which can occur with blend decals), while also properly scaling the albedo influence by the SSS amount.


111-115: Effective early-exit optimization for masked samples.

Good optimization to check the mask before processing each sample. This avoids unnecessary computation for pixels that won't contribute to the final result.


116-116: Consistent mask-based albedo normalization.

The sample color normalization correctly uses the sample's mask value with the same lerp pattern as the center pixel, maintaining consistency in how albedo influences colors based on the SSS mask strength.


134-134: Proper final color blending with SSS amount.

The change to lerp(1.0f, AlbedoTexture[DTid.xy].xyz, sssAmount) for the final color multiplication is consistent with the overall approach of scaling albedo influence by SSS amount, preventing color artifacts when SSS is low or zero.


65-66: Cannot verify diffuseMeanFreePath scaling — Burley.hlsli missing

features/Subsurface Scattering/Shaders/SubsurfaceScattering/Burley.hlsli is not present in the repo. SeparableSSSCS.hlsl defines MeanFreePathBase / MeanFreePathHuman (lines 19–20) and reads sssAmount (lines 44–47) and calls BurleyNormalizedSS; confirm whether the line that does "diffuseMeanFreePath *= sssAmount" exists in a different file or attach the missing Burley.hlsli. If the scaling is intentional, verify it does not produce visible discontinuities at low sssAmount and consider clamping or a non-linear mapping instead.


126-126: Verify removal of mask check in sampleWeight calculation (features/Subsurface Scattering/Shaders/SubsurfaceScattering/Burley.hlsli:126).
Repo searches for 'Burley.hlsli', 'sampleWeight', and 'diffusionProfile.*pdf' returned no matches; cannot confirm that lines 112–115 filter samples by mask. Provide the file or paste lines ~100–130 (incl. 112–126) or run: rg -n -C3 'sampleWeight|diffusionProfile.*pdf|Burley' to verify.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

Using provided base ref: 3f462fa
Using base ref: 3f462fa
Base commit date: 2025-09-18T00:36:38+01:00 (Thursday, September 18, 2025 12:36 AM)
No actionable suggestions for changed features.

@jiayev jiayev marked this pull request as draft September 18, 2025 05:33
@github-actions
Copy link
Copy Markdown

✅ A pre-release build is available for this PR:
Download

@jiayev jiayev closed this Sep 18, 2025
@jiayev jiayev deleted the burley-fix branch September 18, 2025 05:54
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.

1 participant