Skip to content

fix(linear lighting): Fix PBR specular with DALC#1696

Merged
doodlum merged 3 commits into
community-shaders:devfrom
Dlizzio:fix(LL)-fix-PBR-specular-with-DALC
Jan 13, 2026
Merged

fix(linear lighting): Fix PBR specular with DALC#1696
doodlum merged 3 commits into
community-shaders:devfrom
Dlizzio:fix(LL)-fix-PBR-specular-with-DALC

Conversation

@Dlizzio
Copy link
Copy Markdown
Contributor

@Dlizzio Dlizzio commented Jan 12, 2026

Made sure DALC feeding into reflections goes through the same "dimming filter" that other parts of the lighting system use.

Summary by CodeRabbit

  • Bug Fixes
    • Improved ambient luminance handling across deferred rendering and dynamic cubemap paths, yielding more consistent and accurate directional ambient/specular lighting in scenes.

✏️ Tip: You can customize this high-level summary in your review settings.

Made sure DALC feeding into reflections goes through the same "dimming filter" that other parts of the lighting system use.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 12, 2026

📝 Walkthrough

Walkthrough

A minor shader change: the directional ambient input is now wrapped with Color::Ambient before converting to luminance in two shader files, altering the intermediate luminance used in specular/lighting calculations.

Changes

Cohort / File(s) Summary
Directional Ambient Luminance Computation
package/Shaders/DeferredCompositeCS.hlsl, features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli
Replaced RGBToLuminance(max(0, mul(SharedData::DirectionalAmbient, float4(R, 1.0)))) with RGBToLuminance(Color::Ambient(max(0, mul(SharedData::DirectionalAmbient, float4(R, 1.0))))) in directional ambient/specular luminance calculations (two locations total).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • jiayev
  • doodlum
  • alandtse

Poem

🐰 I nibbled code in morning light,
Wrapped ambient snug and made it right,
A tiny hop, a subtle tune,
Luminance gleams beneath the moon 🥕

🚥 Pre-merge checks | ✅ 3
✅ 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 'fix(linear lighting): Fix PBR specular with DALC' clearly identifies the main change: a fix to PBR specular lighting in relation to DALC (detail ambient lighting contribution), which aligns with the actual code changes that wrap directional ambient calculations with an Ambient() call.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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


📜 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 adeac45 and 45cec43.

📒 Files selected for processing (1)
  • features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.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/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli
**/*

⚙️ CodeRabbit configuration file

**/*: When reviewing PRs, please provide suggestions for:

  1. Conventional Commit Titles (if not following https://www.conventionalcommits.org/ or
    if the existing title does not describe the code changes):
    Format: type(scope): description
    Length: 50 characters limit for title, 72 for body
    Style: lowercase description, no ending period
    Examples:

    • feat(vr): add cross-eye sampling
    • fix(water): resolve flowmap bug
    • docs: update shader documentation
  2. Issue References (if PR fixes bugs or implements features):
    Suggest adding appropriate GitHub keywords:

Otherwise, use your standard review approach focusing on code quality.

Files:

  • features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli
🧠 Learnings (3)
📓 Common learnings
Learnt from: jiayev
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Learnt from: ThePagi
Repo: doodlum/skyrim-community-shaders PR: 1369
File: src/Features/SnowCover.cpp:515-515
Timestamp: 2025-10-02T14:20:33.454Z
Learning: In the Community Shaders codebase (skyrim-community-shaders repository), hardcoded shader resource slot numbers are used directly in code rather than being defined as named constants. This is the established convention and should not be flagged as an issue.
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 577
File: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli:57-61
Timestamp: 2025-06-17T05:40:22.785Z
Learning: Default parameter values are supported in the HLSL compiler used by the skyrim-community-shaders project, contrary to standard HLSL (FXC/DXC) limitations.
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.

Applied to files:

  • features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli
📚 Learning: 2025-07-18T15:21:03.641Z
Learnt from: jiayev
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-07-18T15:21:03.641Z
Learning: In the skyrim-community-shaders rendering pipeline, materials with alpha < 1 or alpha blending enabled are rendered in non-deferred mode rather than deferred mode. This means issues with dynamic cubemaps on transparent materials are actually non-deferred rendering issues.

Applied to files:

  • features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.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). (4)
  • GitHub Check: Build plugin and addons
  • GitHub Check: Run Shader Unit Tests
  • GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
  • GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
🔇 Additional comments (1)
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (1)

167-167: LGTM! Consistent DALC handling for reflections.

This change correctly aligns the GetDynamicCubemap function's DALC processing with the pattern already used in GetDynamicCubemapSpecularIrradiance (lines 41-43). Both functions now apply Color::Ambient() to the directional ambient before converting to luminance, ensuring consistent behavior through the lighting pipeline.


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

github-actions Bot commented Jan 12, 2026

Using provided base ref: 467ca14
Using base ref: 467ca14
Base commit date: 2026-01-12T21:51:00+09:00 (Monday, January 12, 2026 09:51 PM)
No actionable suggestions for changed features.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 12, 2026

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

@doodlum doodlum changed the title fix(LL): Fix PBR specular with DALC fix(linear lighting): Fix PBR specular with DALC Jan 12, 2026
Copy link
Copy Markdown
Collaborator

@doodlum doodlum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jiayev jiayev requested a review from doodlum January 13, 2026 01:33
@doodlum doodlum merged commit 8d300bb into community-shaders:dev Jan 13, 2026
14 checks passed
@Dlizzio Dlizzio deleted the fix(LL)-fix-PBR-specular-with-DALC branch March 27, 2026 14:10
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