Skip to content

fix(pbr): fix inconsistencies#1526

Merged
doodlum merged 56 commits into
devfrom
pbr-fixup
Sep 26, 2025
Merged

fix(pbr): fix inconsistencies#1526
doodlum merged 56 commits into
devfrom
pbr-fixup

Conversation

@doodlum
Copy link
Copy Markdown
Collaborator

@doodlum doodlum commented Sep 25, 2025

Summary by CodeRabbit

  • New Features

    • Roughness-aware specular AO and multi-bounce ambient occlusion for richer, more realistic lighting.
  • Improvements

    • More consistent specular luminance across interiors, skylight, reflections, and VR; specular response normalized by luminance.
    • Skylighting now factors surface albedo for more accurate ambient response.
    • Adjusted wetness/reflection blending and dynamic-cubemap specular handling.
    • Increased albedo render-target precision to reduce banding.
  • Defaults

    • Screen Space GI defaults: saturation 0.8, strength 1.0.
  • Chores

    • Skylighting and feature metadata version bumps.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 25, 2025

Walkthrough

Adds Color-based AO helpers and gamma tweaks, migrates PBR AO/specular to Color:: APIs, introduces luminance-normalized specular irradiance in DynamicCubemaps and DeferredCompositeCS (with roughness propagation), updates Skylighting to accept albedo (and its callers), changes ALBEDO RT format, and tweaks SSGI defaults.

Changes

Cohort / File(s) Summary of edits
Dynamic cubemaps luminance normalization
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli
Adds directionalAmbientColorSpecular, computes specularIrradianceLuminance (EnvTexture level) and applies luminance-based normalization/modulation to specular irradiance across interior, skylighting, and non-skylighting branches; minor brace reformatting.
SSGI / Deferred composite (roughness-aware specular AO)
package/Shaders/DeferredCompositeCS.hlsl
Extends SampleSSGISpecular signature to accept roughness; replaces specular AO with Color::SpecularAOLagarde; adds MultiBounceAO usage, luminance-normalized specular/reflection adjustments, and updates call sites and final irradiance composition.
Color utilities (AO helpers & gamma tweaks)
package/Shaders/Common/Color.hlsli
Adds Color::MultiBounceAO, Color::SpecularAOLagarde, and ReflectionNormalisationScale; changes GammaToLinear/LinearToGamma exponents (1.8 → 1.6 / inverse) and minor comment fix.
PBR helper migration
package/Shaders/Common/PBR.hlsli
Removes local PBR::MultiBounceAO and PBR::SpecularAOLagarde; replaces calls with Color:: equivalents and adjusts diffuse lobe weighting.
Skylighting interface & call sites
features/Skylighting/Shaders/Skylighting/Skylighting.hlsli, package/Shaders/Lighting.hlsl, package/Shaders/RunGrass.hlsl
Skylighting::applySkylighting gains an albedo parameter; ambient modulation now uses Color::MultiBounceAO(GammaToLinear(albedo / PBRLightingScale), skylightingDiffuse); callers updated to pass albedo/outputAlbedo; related wetness/outputAlbedo blends adjusted.
Render target format change
src/Deferred.cpp
Changes ALBEDO RT format from DXGI_FORMAT_R8G8B8A8_UNORMDXGI_FORMAT_R10G10B10A2_UNORM in SetupResources.
SSGI defaults
src/Features/ScreenSpaceGI.h
Updates Settings defaults: GISaturation 0.9 → 0.8; GIStrength 1.5 → 1.0.
Feature metadata bumps
features/Dynamic Cubemaps/Shaders/Features/DynamicCubemaps.ini, features/Skylighting/Shaders/Features/Skylighting.ini
Version string updates (2-2-0 → 2-2-1 and 1-2-0 → 1-2-1).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant DCS as DeferredCompositeCS
  participant C as Color (helpers)
  participant DC as DynamicCubemaps
  participant S as Skylighting
  participant G as G-Buffer/Albedo

  Note over DCS,C: Roughness-aware SSGI + luminance-normalized specular
  DCS->>DCS: SampleSSGISpecular(pix, lobe, out ao, out il, normal, view, roughness)
  DCS->>C: SpecularAOLagarde(NdotV, ao, roughness)
  DCS->>C: MultiBounceAO(linAlbedo, ssgiAo)
  DCS->>DC: Fetch env irradiance (EnvTexture / EnvReflectionsTexture)
  DC-->>DCS: Luminance-normalized irradiance
  DCS->>S: applySkylighting(diffuseColor, directionalAmbientColor, albedo, skylightingDiffuse)
  S->>C: MultiBounceAO(GammaToLinear(albedo / PBRLightingScale), skylightingDiffuse)
  S-->>DCS: Updated ambient contribution
  DCS->>G: Compose final irradiance with AO, specular IL, and luminance adjustments
Loading
sequenceDiagram
  autonumber
  participant R as Renderer
  participant RT as Deferred.cpp
  participant P as Pixel Shaders

  Note over RT: ALBEDO RT format change
  R->>RT: SetupResources()
  RT->>RT: Create ALBEDO RT (`DXGI_FORMAT_R10G10B10A2_UNORM`)
  RT-->>P: New format bound for albedo targets
  Note over P: Shaders consume new RT format (no API change)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • jiayev
  • alandtse

Poem

A rabbit tweaks shaders by moonlit code,
Gamma, roughness, cubemaps on the road.
AO hops twice — reflections hum bright,
Albedo greets skylight, pixels take flight.
Hop, compile, render — the meadow glows tonight. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title indicates a PBR-related fix but is overly broad and does not clearly convey which inconsistencies were addressed among the extensive shading and lighting updates. Consider updating the title to specify the key inconsistencies resolved, for example “fix(pbr): normalize specular irradiance and unify AO workflows,” so it clearly reflects the primary changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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
  • Commit unit tests in branch pbr-fixup

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.

Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
@github-actions
Copy link
Copy Markdown

Using provided base ref: 48266de
Using base ref: 48266de
Base commit date: 2025-09-25T16:02:06+01:00 (Thursday, September 25, 2025 04:02 PM)
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.

Actionable comments posted: 9

📜 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 3b1bc95 and cb5997c.

📒 Files selected for processing (2)
  • features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (3 hunks)
  • package/Shaders/DeferredCompositeCS.hlsl (5 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/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli
  • package/Shaders/DeferredCompositeCS.hlsl
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
🧠 Learnings (2)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#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.
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#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
  • package/Shaders/DeferredCompositeCS.hlsl
⏰ 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: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
  • GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
  • GitHub Check: Build plugin and addons
🔇 Additional comments (6)
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (2)

40-41: Directional ambient specular calculation looks correct

The computation of directional ambient color contribution for specular reflections using the reflection direction is appropriate for PBR.


70-73: Verify gamma space conversions are intentional

The directional ambient color specular is converted from linear to gamma at line 72, then back to linear at line 73. This seems unusual - typically we'd stay in one color space for calculations.

Please verify this pattern is intentional. If the goal is to apply the modulation in gamma space, consider adding a comment explaining why.

package/Shaders/DeferredCompositeCS.hlsl (4)

44-50: Excellent multi-bounce AO implementation

The MultiBounceAO function correctly implements the Jimenez et al. 2016 technique with appropriate coefficients. This will improve the realism of ambient occlusion by accounting for multiple light bounces.


67-69: Improved SSGI specular AO blending

The new quadratic blending based on N·V is more physically accurate than the previous power/bias approach, providing smoother transitions.


162-167: Good integration of multi-bounce AO

Using sqrt for diffuse and full value for ambient provides appropriate weighting. The conversion to linear space for albedo ensures correct color math.


216-218: Check gamma/linear conversion pattern

Similar to the other file, the directional ambient color is converted to linear, multiplied, then back to gamma. Verify this is intentional for your lighting pipeline.

Comment thread features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli Outdated
Comment thread features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli Outdated
Comment thread features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli Outdated
Comment thread package/Shaders/DeferredCompositeCS.hlsl Outdated
Comment thread package/Shaders/DeferredCompositeCS.hlsl Outdated
Comment thread package/Shaders/DeferredCompositeCS.hlsl Outdated
Comment thread package/Shaders/DeferredCompositeCS.hlsl Outdated
Comment thread package/Shaders/DeferredCompositeCS.hlsl Outdated
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.

Actionable comments posted: 2

📜 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 cb5997c and 472006f.

📒 Files selected for processing (2)
  • package/Shaders/Common/Color.hlsli (2 hunks)
  • package/Shaders/Lighting.hlsl (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:

  • package/Shaders/Lighting.hlsl
  • package/Shaders/Common/Color.hlsli
🧠 Learnings (2)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#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: jiayev
PR: doodlum/skyrim-community-shaders#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.
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#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:

  • package/Shaders/Lighting.hlsl
  • package/Shaders/Common/Color.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: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
  • GitHub Check: Build plugin and addons
  • GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)

Comment thread package/Shaders/Common/Color.hlsli
Comment thread package/Shaders/Lighting.hlsl Outdated
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.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
package/Shaders/Common/PBR.hlsli (1)

469-476: Fix type/semantic issues in AO application (SpecularAOLagarde is float; MultiBounceAO is float3)

  • Color::SpecularAOLagarde returns float, but it’s assigned to float3.
  • Using the .y component from MultiBounceAO (float3) is arbitrary; use the full RGB for diffuse and per-channel for specular.

Apply:

-        float3 diffuseAO = surfaceProperties.AO;
-        float3 specularAO = Color::SpecularAOLagarde(NdotV, surfaceProperties.AO, surfaceProperties.Roughness);
+        float3 diffuseAO = Color::MultiBounceAO(diffuseColor, surfaceProperties.AO);
+        float specularAO = Color::SpecularAOLagarde(NdotV, surfaceProperties.AO, surfaceProperties.Roughness);
@@
-        diffuseAO = Color::MultiBounceAO(diffuseColor, diffuseAO.x).y;
-        specularAO = Color::MultiBounceAO(surfaceProperties.F0, specularAO.x).y;
+        float3 specularAOColor = Color::MultiBounceAO(surfaceProperties.F0, specularAO);
@@
-        diffuseLobeWeight *= diffuseAO * Color::PBRLightingScale;
-        specularLobeWeight *= specularAO;
+        diffuseLobeWeight *= diffuseAO * Color::PBRLightingScale;
+        specularLobeWeight *= specularAOColor;
📜 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 6c3115b and bc24c28.

📒 Files selected for processing (6)
  • features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (3 hunks)
  • package/Shaders/Common/Color.hlsli (2 hunks)
  • package/Shaders/Common/PBR.hlsli (1 hunks)
  • package/Shaders/DeferredCompositeCS.hlsl (5 hunks)
  • package/Shaders/Lighting.hlsl (2 hunks)
  • package/Shaders/RunGrass.hlsl (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • package/Shaders/Common/Color.hlsli
🧰 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:

  • package/Shaders/RunGrass.hlsl
  • package/Shaders/Common/PBR.hlsli
  • features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli
  • package/Shaders/Lighting.hlsl
  • package/Shaders/DeferredCompositeCS.hlsl
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
🧠 Learnings (3)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#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.
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#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
  • package/Shaders/Lighting.hlsl
  • package/Shaders/DeferredCompositeCS.hlsl
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
PR: doodlum/skyrim-community-shaders#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)

Applied to files:

  • package/Shaders/DeferredCompositeCS.hlsl
🔇 Additional comments (12)
package/Shaders/RunGrass.hlsl (2)

760-761: API update: Skylighting call looks correct

Passing albedo as the new third parameter matches the updated Skylighting::applySkylighting signature.


968-969: API update: Second Skylighting call also correct

Callsite properly supplies albedo with the new parameter order.

package/Shaders/Lighting.hlsl (2)

3116-3117: API update: Skylighting callsite matches new signature

Passing outputAlbedo before skylightingDiffuse is consistent with the updated API.


3363-3364: Restrict Reflectance remap; avoid corrupting env/wetness radiance

This unconditional remap alters env/wetness/dynamic‑cubemap paths where Reflectance carries radiance, not F0/lobes. Guard it to the PBR lobe-weight case only.

-    psout.Reflectance.xyz = lerp(0.0, psout.Reflectance.xyz, saturate(psout.Reflectance.xyz / 0.04));
+    #if defined(TRUE_PBR) && !defined(HAIR) && !defined(WETNESS_EFFECTS) && !(defined(ENVMAP) || defined(MULTI_LAYER_PARALLAX) || defined(EYE))
+    psout.Reflectance.xyz = lerp(0.0.xxx, psout.Reflectance.xyz, saturate(psout.Reflectance.xyz / 0.04.xxx));
+    #endif
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (3)

40-41: Directional ambient luminance term: OK

Computing directionalAmbientColorSpecular via luminance of directional ambient along R is reasonable.


70-73: Color space handling: reasonable

Linearize, modulate, and re-gamma the scalar modulation. Fine.


76-82: Interior skylighting branch: OK

Luminance normalization and gamma/linear conversions look consistent here.

package/Shaders/DeferredCompositeCS.hlsl (5)

53-58: SSGI specular integration: OK

New roughness parameter and Lagarde AO are wired correctly.


150-159: Diffuse multi-bounce AO: OK

Using linAlbedo and MultiBounceAO to modulate diffuse and ambient is correct.


162-163: Direct irradiance add uses linear albedo: OK

Adds SSGI indirect specular il with linear albedo appropriately.


204-207: Skylighting scalar modulation: OK

Linearize → scale by skylightingSpecular → gamma is consistent with nearby usage.


220-226: Fix reflections luminance type and add epsilon

-        if (skylightingSpecular > 0.0){
+        if (skylightingSpecular > 0.0) {
             specularIrradianceReflections = Color::GammaToLinear(EnvReflectionsTexture.SampleLevel(LinearSampler, R, level));
-
-            float3 specularIrradianceReflectionsLuminance = Color::RGBToLuminance(Color::GammaToLinear(EnvReflectionsTexture.SampleLevel(LinearSampler, R, 15)));
-            specularIrradianceReflections /= specularIrradianceReflectionsLuminance;
+            float specularIrradianceReflectionsLuminance = Color::RGBToLuminance(Color::GammaToLinear(EnvReflectionsTexture.SampleLevel(LinearSampler, R, 15)));
+            specularIrradianceReflections /= max(specularIrradianceReflectionsLuminance, 0.001);
             specularIrradianceReflections = Color::GammaToLinear(Color::LinearToGamma(specularIrradianceReflections) * directionalAmbientColorSpecular);
         }

Comment thread features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli Outdated
Comment thread features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli Outdated
Comment thread package/Shaders/DeferredCompositeCS.hlsl Outdated
Comment thread package/Shaders/DeferredCompositeCS.hlsl Outdated
Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
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.

Actionable comments posted: 0

🧹 Nitpick comments (3)
package/Shaders/DeferredCompositeCS.hlsl (3)

53-58: Roughness-aware spec AO: clamp NdotV for stability

Recommend saturating NdotV to [0,1] before feeding the Lagarde AO to avoid surprises from backfacing views.

-	ao = Color::SpecularAOLagarde(NdotV, ao, roughness);
+	ao = Color::SpecularAOLagarde(saturate(NdotV), ao, roughness);

158-162: Avoid gamma round‑trip; accumulate in linear space

This encodes to gamma, adds, then decodes back to linear, introducing unnecessary precision loss. Add the directional ambient contribution directly in linear.

-	diffuseColor += Color::LinearToGamma(Color::GammaToLinear(directionalAmbientColor) * multiBounceAO);
-
-	linDiffuseColor = Color::GammaToLinear(diffuseColor);
+	linDiffuseColor += Color::GammaToLinear(directionalAmbientColor) * multiBounceAO;

187-191: Interior luminance normalization: LGTM; consider a named epsilon

Safe divisor via max(luminance, 0.001) is correct. Prefer a named const (e.g., kLuminanceEpsilon) to avoid magic numbers and keep it consistent across branches.

📜 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 7d6a73b and 2090d73.

📒 Files selected for processing (3)
  • features/Dynamic Cubemaps/Shaders/Features/DynamicCubemaps.ini (1 hunks)
  • features/Skylighting/Shaders/Features/Skylighting.ini (1 hunks)
  • package/Shaders/DeferredCompositeCS.hlsl (5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:

  • package/Shaders/DeferredCompositeCS.hlsl
🧠 Learnings (3)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#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: jiayev
PR: doodlum/skyrim-community-shaders#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.
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#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:

  • package/Shaders/DeferredCompositeCS.hlsl
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
PR: doodlum/skyrim-community-shaders#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)

Applied to files:

  • package/Shaders/DeferredCompositeCS.hlsl
⏰ 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: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
  • GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
  • GitHub Check: Build plugin and addons
🔇 Additional comments (10)
features/Dynamic Cubemaps/Shaders/Features/DynamicCubemaps.ini (1)

2-2: Version bump aligns with shader updates
Metadata version tracks the new DynamicCubemaps changes. Looks good.

features/Skylighting/Shaders/Features/Skylighting.ini (1)

2-2: Skylighting metadata update looks good
Version increment matches the accompanying shader adjustments. No issues spotted.

package/Shaders/DeferredCompositeCS.hlsl (8)

149-155: Multi-bounce AO integration looks good

Using albedo-driven multi-bounce AO and applying sqrt to diffuse makes sense and should reduce over-darkening.


204-205: Skylighting scale propagation: LGTM

Multiplying the scalar normalization by skylightingSpecular is correct.


208-214: Skylighting env normalization: LGTM

Safe luminance division and conditional fetch look correct.


218-224: Skylighting reflections normalization: LGTM

Same normalization pattern correctly applied.


230-231: Non‑skylighting reflections normalization: LGTM

Correct linear/gamma ordering with safe denominator.


257-262: SSGI specular YCoCg mixing: LGTM

Pre‑scaling env by AO, then blending chroma in YCoCg before adding back is reasonable and guarded with max(0).


182-183: Compute luminance in linear space (fix function order/type mismatch risk)

Luminance must be derived from linear RGB. Current code computes luminance in gamma then linearizes a scalar, which is incorrect and may rely on a float overload unintentionally.

-		float directionalAmbientColorSpecular = Color::GammaToLinear(Color::RGBToLuminance(max(0, mul(SharedData::DirectionalAmbient, float4(R, 1.0))) * Color::ReflectionNormalisationScale));
+		float directionalAmbientColorSpecular = Color::RGBToLuminance(
+			Color::GammaToLinear(max(0, mul(SharedData::DirectionalAmbient, float4(R, 1.0))))
+			* Color::ReflectionNormalisationScale
+		);

246-252: All SampleSSGISpecular calls updated; no other usages detected

@doodlum doodlum changed the title chore(pbr): improvements fix(pbr): fix inconsistencies Sep 26, 2025
doodlum and others added 3 commits September 26, 2025 20:21
Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
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.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
features/Skylighting/Shaders/Skylighting/Skylighting.hlsli (1)

39-55: Clamp and clarify albedo space in MultiBounceAO

applySkylighting now consumes “albedo” for MultiBounceAO via GammaToLinear(albedo / Color::PBRLightingScale). If callers may pass linear weights (e.g., lobe weights), this conversion misinterprets them. Either:

  • Document that “albedo” must be gamma-space albedo (recommended), or
  • Accept linear albedo/weights and skip GammaToLinear.

Also clamp to [0,1] to avoid overshooting:

- directionalAmbientColor = Color::LinearToGamma(Color::GammaToLinear(directionalAmbientColor) * Color::MultiBounceAO(Color::GammaToLinear(albedo / Color::PBRLightingScale), skylightingDiffuse));
+ float3 albedoLin = saturate(Color::GammaToLinear(albedo / Color::PBRLightingScale));
+ directionalAmbientColor = Color::LinearToGamma(Color::GammaToLinear(directionalAmbientColor) * Color::MultiBounceAO(albedoLin, skylightingDiffuse));
🧹 Nitpick comments (1)
package/Shaders/Lighting.hlsl (1)

2886-2891: Wetness reflectance scaling may be too weak (sqrt removal)

Using 0.02 * wetnessGlossinessSpecular (vs sqrt) will noticeably dull wet reflections. If this was not intentional, consider restoring sqrt for perceptual scaling:

- float3 wetnessReflectance = DynamicCubemaps::GetDynamicCubemap(wetnessNormal, vertexNormal, viewDirection, waterRoughnessSpecular, 0.02 * wetnessGlossinessSpecular, skylightingSH);
+ float3 wetnessReflectance = DynamicCubemaps::GetDynamicCubemap(wetnessNormal, vertexNormal, viewDirection, waterRoughnessSpecular, 0.02 * sqrt(wetnessGlossinessSpecular), skylightingSH);

And the non-Skylighting branch likewise.

📜 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 2090d73 and fc61e74.

📒 Files selected for processing (5)
  • features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (3 hunks)
  • features/Skylighting/Shaders/Skylighting/Skylighting.hlsli (3 hunks)
  • package/Shaders/Common/Color.hlsli (3 hunks)
  • package/Shaders/DeferredCompositeCS.hlsl (5 hunks)
  • package/Shaders/Lighting.hlsl (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • package/Shaders/Common/Color.hlsli
🧰 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/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli
  • package/Shaders/Lighting.hlsl
  • features/Skylighting/Shaders/Skylighting/Skylighting.hlsli
  • package/Shaders/DeferredCompositeCS.hlsl
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
  • features/Skylighting/Shaders/Skylighting/Skylighting.hlsli
🧠 Learnings (3)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#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: jiayev
PR: doodlum/skyrim-community-shaders#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.
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#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
  • package/Shaders/Lighting.hlsl
  • package/Shaders/DeferredCompositeCS.hlsl
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
PR: doodlum/skyrim-community-shaders#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)

Applied to files:

  • package/Shaders/Lighting.hlsl
  • package/Shaders/DeferredCompositeCS.hlsl
⏰ 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: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
  • GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
  • GitHub Check: Build plugin and addons
🔇 Additional comments (18)
package/Shaders/Lighting.hlsl (6)

2114-2114: Confirm F0 color‑space and intent

Switch to Color::GammaToTrueLinear for baseColor in F0 mix looks intentional. Please confirm baseColor here is gamma-encoded and that TRUE linear (not display linear) is expected for F0 computation to avoid subtle energy errors.


2969-2969: LGTM: Wetness indirect specular weight

Capping indirectSpecularLobeWeight with wetness lobe is reasonable to preserve wet highlights.


3089-3096: LGTM: PBR lighting scale application

Using Color::PBRLightingScale on both color and lobe weight keeps energy consistent across pipelines.


3118-3119: Skylighting callsite updated

Signature change propagated correctly.

Please verify all other call sites were updated (e.g., RunGrass).


3301-3326: LGTM: Roughness write under wetness

Using lerp with wetnessGlossinessSpecular to bias glossiness toward wetness path is consistent with visual intent.


3345-3349: LGTM: Dynamic cubemap roughness write

NormalGlossiness.z encoding for dynamic cubemap path correctly aligns with envRoughness and wetness.

features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (4)

56-62: LGTM: Luminance normalization with epsilon (interior path)

Good use of RGBToLuminance(GammaToLinear(...)) and max(eps) guard.


69-79: LGTM: Skylighting modulation of directional ambient specular

Scaling directionalAmbientColorSpecular by skylightingSpecular before use is correct.


73-89: LGTM: Luminance-normalized EnvTexture and EnvReflections paths

Consistent normalization and epsilon usage across both branches.


95-99: LGTM: Non-skylighting path normalization

Matches the same approach; consistent across pipelines.

package/Shaders/DeferredCompositeCS.hlsl (8)

53-58: LGTM: Roughness‑aware specular AO

Switching to Color::SpecularAOLagarde with NdotV and roughness is correct and improves plausibility.


150-159: LGTM: Multi-bounce AO for diffuse (linear workflow)

Converting albedo to linear, computing MultiBounceAO, and applying in linear space is correct.


162-163: LGTM: Indirect lighting accumulation uses linear albedo

Adding ssgiIl scaled by linear albedo is consistent with energy conservation.


182-186: LGTM: Directional ambient luminance computed in linear

This matches best practices and aligns with the normalization math that follows.


189-194: LGTM: Interior luminance normalization with epsilon

Safe divisor and linear-space modulation are correct.


206-226: LGTM: Skylighting branch normalization and modulation

Consistent handling for EnvTexture/EnvReflections with safeguards; modulation after skylighting mix is correct.


246-254: LGTM: SampleSSGISpecular signature propagation (VR path too)

All call sites updated with roughness; matches the new function contract.


259-265: LGTM: Specular SSGI merge with YCoCg dampening

Combining via YCoCg with partial chroma replacement avoids color shifts; looks good.

Comment thread package/Shaders/Lighting.hlsl
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.

Actionable comments posted: 0

🧹 Nitpick comments (4)
package/Shaders/DeferredCompositeCS.hlsl (4)

150-163: Color-space sanity: scale albedo in the intended space; guard sqrt.

  • If PBRLightingScale is defined in linear space, scale after GammaToLinear to avoid non-linear bias.
  • Guard sqrt with saturate for safety.

Proposed change (adjust if PBRLightingScale is known to be in gamma space):

-	float3 linAlbedo = Color::GammaToLinear(albedo / Color::PBRLightingScale);
+	float3 linAlbedo = Color::GammaToLinear(albedo) / Color::PBRLightingScale;

-	linDiffuseColor *= sqrt(multiBounceAO);
+	linDiffuseColor *= sqrt(saturate(multiBounceAO));

This keeps all math in linear as per the linear-lighting pipeline. Based on learnings


189-195: Consistent epsilon and minor hygiene.

  • Replace magic 0.001 with a named constant for clarity and consistency across branches.

Apply:

-        specularIrradiance = (specularIrradiance / max(specularIrradianceLuminance, 0.001)) * directionalAmbientColorSpecular;
+        specularIrradiance = (specularIrradiance / max(specularIrradianceLuminance, kLuminanceEpsilon)) * directionalAmbientColorSpecular;
@@
-            specularIrradiance = (specularIrradiance / max(specularIrradianceLuminance, 0.001)) * directionalAmbientColorSpecular;
+            specularIrradiance = (specularIrradiance / max(specularIrradianceLuminance, kLuminanceEpsilon)) * directionalAmbientColorSpecular;
@@
-            specularIrradianceReflections = (specularIrradianceReflections / max(specularIrradianceLuminance, 0.001)) * directionalAmbientColorSpecular;
+            specularIrradianceReflections = (specularIrradianceReflections / max(specularIrradianceLuminance, kLuminanceEpsilon)) * directionalAmbientColorSpecular;
@@
-	    specularIrradianceReflections = (specularIrradianceReflections / max(specularIrradianceReflectionsLuminance, 0.001)) * directionalAmbientColorSpecular;
+	    specularIrradianceReflections = (specularIrradianceReflections / max(specularIrradianceReflectionsLuminance, kLuminanceEpsilon)) * directionalAmbientColorSpecular;

Add once near the top (outside selected lines):

static const float kLuminanceEpsilon = 1e-3f;

Also applies to: 213-216, 223-226, 232-235


206-227: Clamp skylightingSpecular to [0,1] before branching/weighting.

Preempt any out-of-range values to avoid surprising lerp/branch behavior.

-		directionalAmbientColorSpecular *= skylightingSpecular;
+		skylightingSpecular = saturate(skylightingSpecular);
+		directionalAmbientColorSpecular *= skylightingSpecular;

261-267: SSGI specular merge: confirm occlusion intent.

You multiply environment irradiance by ssgiAo but not ssgiIlSpecular. If the intent is to occlude SSGI specular too, multiply it by ssgiAo.

-		finalIrradiance = (finalIrradiance * ssgiAo);
+		finalIrradiance = (finalIrradiance * ssgiAo);
+		ssgiIlSpecular *= ssgiAo; // optional: if you want SSGI specular occluded as well
📜 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 fc61e74 and d85c719.

📒 Files selected for processing (1)
  • package/Shaders/DeferredCompositeCS.hlsl (5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:

  • package/Shaders/DeferredCompositeCS.hlsl
🧠 Learnings (3)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#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: jiayev
PR: doodlum/skyrim-community-shaders#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.
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#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:

  • package/Shaders/DeferredCompositeCS.hlsl
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
PR: doodlum/skyrim-community-shaders#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)

Applied to files:

  • package/Shaders/DeferredCompositeCS.hlsl
⏰ 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: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
  • GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
  • GitHub Check: Build plugin and addons
🔇 Additional comments (3)
package/Shaders/DeferredCompositeCS.hlsl (3)

182-185: Luminance seed ordering is correct now.

Gamma-to-linear then RGBToLuminance and normalization scale application look good.


250-256: Call-site propagation LGTM.

Both VR and non‑VR paths pass roughness; matches the new signature.


53-58: Verify SampleSSGISpecular call sites

  • Confirm every invocation passes seven arguments—including the new roughness parameter
  • Ensure the roughness value is correctly normalized to [0,1]

@doodlum doodlum merged commit 2ea3b3a into dev Sep 26, 2025
17 checks passed
Pentalimbed pushed a commit to Pentalimbed/skyrim-community-shaders that referenced this pull request Dec 16, 2025
* chore(ui): update discord banner (community-shaders#1493)

* fix: use proper filename settingsuser.json (community-shaders#1491)

* chore(upscaling): increase fsr sharpness

* chore: rename d3d12interop to d3d12SwapChainActive (community-shaders#1494)

* feat(llf): remove particle lights (community-shaders#1495)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* feat(llf): move llf to core (community-shaders#1496)

* fix: remove water clamp (community-shaders#1497)

* fix(upscaling): more upscaling fixes (community-shaders#1498)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix: fix some internal errors when debugging (community-shaders#1500)

* fix(ui): fix save settings conflicts & welcome screen (community-shaders#1501)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(ui): add constraints for discord banner size (community-shaders#1463)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fix(VR): fix exiting menu using controllers (community-shaders#1502)

* build: fix warnings (community-shaders#1505)

* feat(UI): allow tooltips for disabled elements (community-shaders#1503)

* feat(upscaling): add downscale percentages (community-shaders#1506)

* perf(ssgi): optimize  (community-shaders#1499)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* feat(ui): font size and perf overlay improvements (community-shaders#1511)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* chore: remove unused hooks (community-shaders#1510)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix: adjust IsInterior to consider kNoSky or kFixedDimensions flags (community-shaders#1512)

* fix(hair): correct hair indirect normal, marschner by default (community-shaders#1515)

Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: mostly revert ISHDR to 1.3.6 (community-shaders#1516)

* chore(upscaling): simplify interop and upscale methods (community-shaders#1514)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(hair): typo in code (community-shaders#1517)

* feat(ibl): lerp sky ibl using skylighting (community-shaders#1519)

* fix(sss): burley artifacts with effect blend (community-shaders#1518)

* fix(upscaling): fix screenshots when upscaling enabled (community-shaders#1520)

* fix(upscaling): fix mipbias sometimes being wrong (community-shaders#1521)

* fix: fix compile error if snow shader on (community-shaders#1522)

* chore(upscaling): revert fsr to typical settings (community-shaders#1523)

* fix: fix minor ui issues (community-shaders#1524)

* chore(grass collision): simpler grass collision (community-shaders#1525)

* fix: update skylighting and version

* fix(pbr): fix inconsistencies (community-shaders#1526)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: jiayev <l936249247@hotmail.com>

* feat(upscaling): sharpening slider (community-shaders#1527)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* chore: bump versions

* fix(ibl): add ibl to reflection normalization (community-shaders#1528)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(hair): remove pbr lighting mult for hair (community-shaders#1531)

* chore(upscaling): add back upscale multiplier (community-shaders#1532)

* fix(upscaling): fix minor upscaling issues (community-shaders#1536)

* chore: gamma space normalisation (community-shaders#1535)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* feat(grass collision): implement with texture and history (community-shaders#1539)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore(grass collision): less aggressive (community-shaders#1546)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(skylighting): fix cell id casting (community-shaders#1544)

* chore(emat): auto detect terrain parallax (community-shaders#1545)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* chore: update versions

* feat(VR): enable upscaling (community-shaders#1507)

* fix(terrain shadows): fix brightened lods (community-shaders#1547)

* chore(upscaling): reduce ghosting near camera (community-shaders#1548)

* fix: fix grass not animating (community-shaders#1549)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fix(grass collision): fix non-standard timescales (community-shaders#1550)

* build: deploy only updated files (community-shaders#1556)

* feat: add Clear Shader Cache to Advanced (community-shaders#1555)

* chore(featureissues): default collapse testing menu (community-shaders#1554)

* fix(VR): use only supported shaders from cache (community-shaders#1553)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* build: use gersemi cmake formatter (community-shaders#1557)

* fix(terrain): vanilla diffuse in pbr terrain cell too bright due to wrong color space (community-shaders#1558)

* docs: add new feature development template guide (community-shaders#1529)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* docs(UI): remove duplicate GPL license statement (community-shaders#1561)

* feat: add renderdoc for debugging (community-shaders#1560)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
Co-authored-by: Alan Tse <alandtse@gmail.com>

* fix(ui): welcome popup size issues (community-shaders#1573)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* chore(grass collision): minor tweaks (community-shaders#1568)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(terrain helper): fix conflicting bit (community-shaders#1566)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* feat(UI): separate theme settings, UI refactor, font support (community-shaders#1571)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* chore: bump versions

* build: fix zipping aio (community-shaders#1579)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(grass collision): clamp maximum depth of grass (community-shaders#1578)

* feat(UI): enhance shader blocking (community-shaders#1564)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Alan Tse <alandtse@gmail.com>

* fix: remove duplicate buffer setup (community-shaders#1586)

* feat: update shader compile elapsed time every second (community-shaders#1587)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* build: add cmake install commands (community-shaders#1372)

* feat(perf-overlay): add size controls (community-shaders#1591)

* fix(perf-overlay): fix infinite draw calls table height (community-shaders#1590)

* refactor(perf-overlay): remove collapsible headers (community-shaders#1572)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(perf-overlay): removed ImGuiTableFlags_ScrollX/Y for scroll bar issues (community-shaders#1594)

* build: fix shader copying to relative paths (community-shaders#1603)

* fix(ibl): apply ibl to cubemap normalisation for non deferred (community-shaders#1604)

* fix(grass): use correct light direction (community-shaders#1602)

* fix(welcome-popup): adjust font size & window spacing (community-shaders#1592)

* feat(lod): add gamma sliders (community-shaders#1588)

* build: correct CodeRabbit schema syntax (community-shaders#1608)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>

* build: add compile-time validation of GPU buffers (community-shaders#1427)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>

* ci: run shader validation on CMake and CI config changes (community-shaders#1606)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>

* feat: procedural sun

* limb darkening

* another darkening

* build(deps): remove orphaned Intel XeSS dependency (community-shaders#1611)

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix: accumulate sunlight color in pixel shader output

* fix(ui): enter key now behaves properly when first time popup is open (community-shaders#1615)

* feat(ui): add tabs to advanced settings & PBR search (community-shaders#1599)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* build: add HLSL intellisense (community-shaders#1614)

* refactor(UI): move light limit visualization into debug (community-shaders#1619)

* refactor(ui): add settings for shader block hotkeys (community-shaders#1624)

Co-authored-by: Bruce <44987693+brucenguyen@users.noreply.github.com>

* fix(ui): anchor reset settings button position  (community-shaders#1621)

Co-authored-by: Giovanni Correia <Gistix@users.noreply.github.com>

* fix(hair): use indirect normal for deferred marschner hair (community-shaders#1626)

* build: fix Package-AIO-Manual for fresh pulls (community-shaders#1625)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(snow): use world space vectors (community-shaders#1618)

* feat(UI): add gaussian blur shader core files (community-shaders#1595)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* feat(ui): add test conditions button (community-shaders#1637)

* fix(ui): blocked shader info overflow in Shader Debug tab (community-shaders#1632)

* fix(upscaling): replace NIS with RCAS for DLSS (community-shaders#1620)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(dynamic cubemaps): add a check for timeskip (community-shaders#1639)

* refactor: restructure lighting (community-shaders#1633)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* feat(ui): add themes & fonts (community-shaders#1596)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* feat(water): add flowmap parallax (community-shaders#1636)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix cloud shadow setting saving

---------

Co-authored-by: zxcvbn <66063766+zndxcvbn@users.noreply.github.com>
Co-authored-by: davo0411 <davidkehoe0411@outlook.com>
Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Alan Tse <alandtse@users.noreply.github.com>
Co-authored-by: soda <130315225+soda3000@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: ThePagi <32794457+ThePagi@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
Co-authored-by: Alan Tse <alandtse@gmail.com>
Co-authored-by: Yupeng Zhang <ArcEarth@outlook.com>
Co-authored-by: kuplion <kuplion@hotmail.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: Giovanni Correia <Gistix@users.noreply.github.com>
Co-authored-by: Bruce <44987693+brucenguyen@users.noreply.github.com>
Co-authored-by: Midona <106106405+midona-rhel@users.noreply.github.com>
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