chore: gamma space normalisation#1535
Conversation
|
Warning Rate limit exceeded@doodlum has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughUpdates cubemap and composite shaders: changes Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Pixel as Pixel Shader
participant DC as DynamicCubemaps.hlsli
participant ENV as EnvTexture / EnvReflectionsTexture
participant DCmp as DeferredCompositeCS.hlsl
Note over DC,ENV: Cubemap resources now TextureCube<float3>
Pixel->>DC: Request specular (view, N, roughness)
DC->>ENV: Sample environment (float3)
DC->>DC: compute NoV/LOD → luminance scaling → GammaToLinear as needed
DC-->>Pixel: specularIrradiance / finalIrradiance
Pixel->>DCmp: Compose lighting (IBL / SKYLIGHTING / dynamic cubemap)
DCmp->>ENV: Sample env/skylight (float3)
DCmp->>DCmp: saturate skylightingSpecular → compute luminance norms → assign finalIrradiance
DCmp-->>Pixel: Final color
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package/Shaders/DeferredCompositeCS.hlsl (1)
233-258: White fallback on skipped branchesSame 1.0 defaults as in hlsli. Causes unintended brightening when skylightingSpecular hits 0 or 1. Use 0.0 defaults and saturate blend factor.
- float3 specularIrradiance = 1; + float3 specularIrradiance = 0; @@ - float3 specularIrradianceReflections = 1.0; + float3 specularIrradianceReflections = 0.0; @@ - finalIrradiance = lerp(specularIrradiance, specularIrradianceReflections, skylightingSpecular); + finalIrradiance = lerp(specularIrradiance, specularIrradianceReflections, saturate(skylightingSpecular));
🧹 Nitpick comments (3)
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (2)
35-39: Minor: Remove unused NoVNoV is computed but unused in this function. Drop it to avoid confusion.
- float NoV = saturate(dot(N, V)); - float level = roughness * 7.0;
47-49: Deduplicate env sampling + normalizationThe sample/normalize/GammaToLinear sequence repeats in 6+ places. Consider a helper like SampleNormalizedEnv(cube, dir, level, dalcLuma) to reduce drift.
Also applies to: 55-63, 98-106, 148-156, 170-176, 181-187, 191-199
package/Shaders/DeferredCompositeCS.hlsl (1)
193-210: Factor out repeated normalizationSame repeated normalize-by-MIP15-then-GammaToLinear sequence as hlsli; a small inline helper will de-duplicate and reduce divergence in future edits.
Also applies to: 235-255, 268-275
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli(4 hunks)features/Dynamic Cubemaps/Shaders/Features/DynamicCubemaps.ini(1 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:
package/Shaders/DeferredCompositeCS.hlslfeatures/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
🧠 Learnings (2)
📓 Common learnings
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.hlslfeatures/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). (3)
- GitHub Check: Build plugin and addons
- 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 (8)
features/Dynamic Cubemaps/Shaders/Features/DynamicCubemaps.ini (1)
2-2: Version bump looks goodMatches shader interface changes to Env/Reflections textures. Ensure any release notes mention required cache rebuild for cubemaps, if applicable.
Do you want me to add a short changelog note entry for 2-2-2?
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (3)
53-63: Confirm gamma-space normalization intentYou normalize by RGBToLuminance of MIP 15 in gamma, then convert to linear. This matches the PR title; just confirm Color::RGBToLuminance expects gamma here (vs linear), or wrap the luminance input with Color::LinearToGamma if required by your conventions.
Based on learnings
133-135: Minor: retain consistency with earlier fixThis path correctly applies Color::ReflectionNormalisationScale; once the earlier function is fixed, both will match. No change needed here.
13-14: TextureCube to float3: alpha‐channel swizzles verified
No.aaccesses onEnvTextureorEnvReflectionsTexturefound in any.hlsl/.hlslishader—safe to bind 3-channel formats.package/Shaders/DeferredCompositeCS.hlsl (4)
191-193: Directional ambient specular luminance computationThis aligns with the DynamicCubemaps GetDynamicCubemap path (includes ReflectionNormalisationScale). Keep consistent with fixes in hlsli.
268-275: Good: reflections path normalization is consistentReflections path uses matching luminance source and converts to linear once. Keep.
308-312: Final color transforms look consistentAdding reflectance * finalIrradiance in linear then LinearToGamma at end is correct.
21-26: Verify t6/t7 register usage within DeferredCompositeCS and its includes
EnvTexture (t6) and EnvReflectionsTexture (t7) share registers—confirm no other resource in this CS or in any included header (Color.hlsli, FrameBuffer.hlsli, GBuffer.hlsli, MotionBlur.hlsli, SharedData.hlsli, SphericalHarmonics.hlsli, VR.hlsli) binds t6 or t7.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
package/Shaders/Common/Color.hlsli (1)
69-72: Retuned normalization scales: confirm pipeline-wide consistency.Changing PBRLightingScale and ReflectionNormalisationScale to 0.65 affects diffuse compensation and DALC‑matched reflection brightness. Please verify:
- DeferredCompositeCS and DynamicCubemaps paths use these exact factors (no duplicated literals).
- Visual baselines (interior vs exterior) remain matched after the gamma-space changes in DeferredCompositeCS.
If you plan further tuning, consider routing both scales through SharedData so feature toggles and presets can adjust them without recompiles.
package/Shaders/DeferredCompositeCS.hlsl (6)
191-210: Unify luminance space: DALC luma (linear) vs cubemap/IBL luma (gamma).directionalAmbientColorSpecular is computed from a linear value, but cubemap and IBL luminance below are computed in gamma space (see iblColorLuminance using LinearToGamma). Mixing spaces can skew normalization. Two consistent options:
- Option A (keep gamma‑space luma everywhere): convert DALC to gamma before RGBToLuminance.
- float directionalAmbientColorSpecular = Color::RGBToLuminance(max(0, mul(SharedData::DirectionalAmbient, float4(R, 1.0)))) * Color::ReflectionNormalisationScale; + float directionalAmbientColorSpecular = + Color::RGBToLuminance( + Color::LinearToGamma(max(0, mul(SharedData::DirectionalAmbient, float4(R, 1.0))))) + * Color::ReflectionNormalisationScale;
- Option B (move everything to linear luma): linearize cubemap/IBL samples before RGBToLuminance (requires corresponding edits below). This is physically cleaner but changes look.
Given the current iblColor path already uses gamma‑space luma, Option A is minimal and keeps behavior consistent.
Based on learnings
194-209: Cubemap sample color space: avoid unintended double conversion.You treat EnvTexture samples as gamma and convert to linear only at the end:
- specularIrradiance = EnvTexture.SampleLevel(...)
- specularIrradianceLuminance = RGBToLuminance(EnvTexture.SampleLevel(..., 15))
- finalIrradiance = GammaToLinear(specularIrradiance)
If the SRV is created with an sRGB format, hardware already returns linear, and GammaToLinear will double‑convert, darkening results. Please verify SRV formats for dynamic cubemaps and reflections. If they’re sRGB, remove GammaToLinear here and compute luma in linear; if they’re linear, keep your current conversion or switch to Option A in the previous comment to match spaces.
As a safety/robustness tweak, also avoid hardcoding mip 15; query the last mip:
- float3 specularIrradiance = EnvTexture.SampleLevel(LinearSampler, R, level); - float specularIrradianceLuminance = Color::RGBToLuminance(EnvTexture.SampleLevel(LinearSampler, R, 15)); + uint w, h, mipCount; + EnvTexture.GetDimensions(0, w, h, mipCount); + float lastMip = max(0.0, (float)(mipCount - 1)); + float clampedLevel = saturate(roughness) * lastMip; + float3 specularIrradiance = EnvTexture.SampleLevel(LinearSampler, R, clampedLevel); + float specularIrradianceLuminance = + Color::RGBToLuminance(EnvTexture.SampleLevel(LinearSampler, R, lastMip)); - finalIrradiance = Color::GammaToLinear(specularIrradiance); + // If SRV is sRGB -> linear already, skip GammaToLinear below. + finalIrradiance = Color::GammaToLinear(specularIrradiance);(Keep the comment as a reminder until SRV format is confirmed.)
233-244: SKYLIGHTING env specular: same SRV/space and mip robustness concerns.Mirror of the interior path:
- Verify SRV format to avoid double GammaToLinear.
- Use queried last mip, not 15.
- Compute luma and scaling in a single consistent space.
Suggested changes:
- specularIrradiance = EnvTexture.SampleLevel(LinearSampler, R, level); - float specularIrradianceLuminance = Color::RGBToLuminance(EnvTexture.SampleLevel(LinearSampler, R, 15)); + uint w, h, mipCount; + EnvTexture.GetDimensions(0, w, h, mipCount); + float lastMip = max(0.0, (float)(mipCount - 1)); + float clampedLevel = saturate(roughness) * lastMip; + specularIrradiance = EnvTexture.SampleLevel(LinearSampler, R, clampedLevel); + float specularIrradianceLuminance = + Color::RGBToLuminance(EnvTexture.SampleLevel(LinearSampler, R, lastMip)); - specularIrradiance = Color::GammaToLinear(specularIrradiance); + specularIrradiance = Color::GammaToLinear(specularIrradiance); // remove if SRV is sRGB
245-256: Reflections path: apply the same fixes as specular env path.EnvReflectionsTexture likely shares the same SRV format and mip layout; apply the same SRV format check and lastMip query. Example:
- specularIrradianceReflections = EnvReflectionsTexture.SampleLevel(LinearSampler, R, level); - float specularIrradianceLuminance = Color::RGBToLuminance(EnvReflectionsTexture.SampleLevel(LinearSampler, R, 15)); + uint wR, hR, mipCountR; + EnvReflectionsTexture.GetDimensions(0, wR, hR, mipCountR); + float lastMipR = max(0.0, (float)(mipCountR - 1)); + float clampedLevelR = saturate(roughness) * lastMipR; + specularIrradianceReflections = EnvReflectionsTexture.SampleLevel(LinearSampler, R, clampedLevelR); + float specularIrradianceLuminance = Color::RGBToLuminance( + EnvReflectionsTexture.SampleLevel(LinearSampler, R, lastMipR)); - specularIrradianceReflections = Color::GammaToLinear(specularIrradianceReflections); + specularIrradianceReflections = Color::GammaToLinear(specularIrradianceReflections); // remove if SRV is sRGB
268-275: Non‑skylighting reflections: SRV format and lastMip again.Apply the SRV format verification and lastMip query here as well:
- float3 specularIrradianceReflections = EnvReflectionsTexture.SampleLevel(LinearSampler, R, level); - float specularIrradianceReflectionsLuminance = Color::RGBToLuminance(EnvReflectionsTexture.SampleLevel(LinearSampler, R, 15)); + uint wR, hR, mipCountR; + EnvReflectionsTexture.GetDimensions(0, wR, hR, mipCountR); + float lastMipR = max(0.0, (float)(mipCountR - 1)); + float clampedLevelR = saturate(roughness) * lastMipR; + float3 specularIrradianceReflections = EnvReflectionsTexture.SampleLevel(LinearSampler, R, clampedLevelR); + float specularIrradianceReflectionsLuminance = + Color::RGBToLuminance(EnvReflectionsTexture.SampleLevel(LinearSampler, R, lastMipR)); - finalIrradiance = Color::GammaToLinear(specularIrradianceReflections); + finalIrradiance = Color::GammaToLinear(specularIrradianceReflections); // remove if SRV is sRGB
191-196: Optional: factor repeated “normalize by luminance then linearize” into a helper.To de‑duplicate and keep space handling consistent, extract a small inline helper:
float3 NormalizeCubemapToDALC(TextureCube<float3> tex, SamplerState s, float3 R, float roughness, float dalcLuma /*already in chosen space*/, bool samplesAreGamma) { uint w, h, mipCount; tex.GetDimensions(0, w, h, mipCount); float lastMip = max(0.0, (float)(mipCount - 1)); float clamped = saturate(roughness) * lastMip; float3 c = tex.SampleLevel(s, R, clamped); float l = Color::RGBToLuminance(tex.SampleLevel(s, R, lastMip)); float3 n = (c / max(l, 0.001)) * dalcLuma; return samplesAreGamma ? Color::GammaToLinear(n) : n; }Then call it for EnvTexture and EnvReflectionsTexture with a single flag once SRV formats are known.
Also applies to: 223-231, 233-244, 245-256, 268-275
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package/Shaders/Common/Color.hlsli(1 hunks)package/Shaders/DeferredCompositeCS.hlsl(3 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.hlslpackage/Shaders/Common/Color.hlsli
🧠 Learnings (3)
📓 Common learnings
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.
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#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.
📚 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.hlslpackage/Shaders/Common/Color.hlsli
📚 Learning: 2025-07-18T15:21:03.641Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#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:
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). (2)
- GitHub Check: Feature Version Audit (PR)
- GitHub Check: Check for changes in PRs
🔇 Additional comments (4)
package/Shaders/DeferredCompositeCS.hlsl (4)
219-223: Clamp added for skylightingSpecular is good.
saturate avoids SH ringing/overshoot before mixing.
223-231: IBL luminance path matches gamma‑space normalization.iblColorLuminance computed as RGBToLuminance(LinearToGamma(iblColor)) aligns with your gamma‑space normalization choice. Ensure DALC luma (Line 191) matches this choice (see earlier comment).
260-267: IBL non‑skylighting path: same luminance‑space consistency.iblColorLuminance uses LinearToGamma; ensure DALC luma matches (Line 191), otherwise normalization differs between skylighting ON/OFF.
20-26: Manual verification of SRV formats for dynamic cubemaps/reflections
I was unable to find any calls to CreateTexture2D or CreateShaderResourceView specifying an sRGB DXGI_FORMAT for EnvTexture, EnvReflectionsTexture, or ReflectanceTexture in the repository. Please confirm in your D3D initialization code whether these cubemap render targets and their SRVs use an sRGB format (e.g. DXGI_FORMAT_R8G8B8A8_UNORM_SRGB) or a linear format, and update the shader’s gamma ↔ linear conversion paths accordingly.
|
✅ A pre-release build is available for this PR: |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (2)
83-96: Question: gamma-domain scaling of luminance scalarYou convert directionalAmbientColorSpecular to linear, scale by skylightingSpecular, then convert back to gamma before applying as a multiplier to gamma-space specular samples. That’s consistent if RGBToLuminance expects gamma inputs, but please confirm the convention for directional ambient here to avoid mixed-domain math. I’d prefer to carry the scalar in linear entirely to reduce round-trips.
31-39: Optional: consolidate repeated normalization into a helperMultiple branches repeat “sample → luminance at high mip → divide → scale → GammaToLinear”. Consider a small inline function to centralize this. Reduces risk of future inconsistencies.
Also applies to: 98-111, 118-136
package/Shaders/DeferredCompositeCS.hlsl (1)
223-231: Gamma/linear convention check for luminance inputsYou compute iblColor in linear, convert to gamma for RGBToLuminance — consistent if RGBToLuminance expects gamma space. Please confirm this convention is uniform across Color.hlsli to avoid mixed-domain luminance.
Also applies to: 269-270
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli(4 hunks)package/Shaders/DeferredCompositeCS.hlsl(3 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.hlslipackage/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: 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.hlslipackage/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 (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Build plugin and addons
🔇 Additional comments (8)
features/Dynamic Cubemaps/Shaders/DynamicCubemaps/DynamicCubemaps.hlsli (5)
53-63: Interior path looks correct post-normalizationSample EnvTexture, normalize by its own luminance, then convert to linear. This aligns with the updated gamma/linear workflow.
41-41: Normalization scale inconsistency (apply ReflectionNormalisationScale)directionalAmbientColorSpecular here omits Color::ReflectionNormalisationScale, unlike GetDynamicCubemap (Line 136) and DeferredCompositeCS (Line 191). Apply the same scale to keep energy consistent across paths.
- float directionalAmbientColorSpecular = Color::RGBToLuminance(max(0, mul(SharedData::DirectionalAmbient, float4(R, 1.0)))); + float directionalAmbientColorSpecular = Color::RGBToLuminance(max(0, mul(SharedData::DirectionalAmbient, float4(R, 1.0)))) * Color::ReflectionNormalisationScale;
100-106: Luminance source must match sampled cube (reflections vs env)You sample EnvReflectionsTexture for color but compute luminance from EnvTexture. This skews normalization. Use the same source as the color sample.
- float3 specularIrradiance = EnvReflectionsTexture.SampleLevel(SampColorSampler, R, level); - - float specularIrradianceLuminance = Color::RGBToLuminance(EnvTexture.SampleLevel(SampColorSampler, R, 15)); + float3 specularIrradiance = EnvReflectionsTexture.SampleLevel(SampColorSampler, R, level); + float specularIrradianceLuminance = Color::RGBToLuminance(EnvReflectionsTexture.SampleLevel(SampColorSampler, R, 15));
169-192: Same white default in GetDynamicCubemap() skylighting pathZero-init the specular terms to avoid adding white when branches are bypassed; clamp weight already handled.
- float3 specularIrradiance = 1.0; + float3 specularIrradiance = 0.0; @@ - float3 specularIrradianceReflections = 1.0; + float3 specularIrradianceReflections = 0.0; @@ - finalIrradiance = lerp(specularIrradiance, specularIrradianceReflections, skylightingSpecular); + finalIrradiance = lerp(specularIrradiance, specularIrradianceReflections, skylightingSpecular);
72-99: Defaulting specular terms to 1.0 injects white energy when branches skipSpecular defaults of 1.0 for specularIrradiance/specularIrradianceReflections cause brightening if a branch is skipped. Zero-init to avoid bias. Lerp weight is already saturated above.
- float3 specularIrradianceReflections = 1.0; + float3 specularIrradianceReflections = 0.0; @@ - float3 specularIrradiance = 1.0; + float3 specularIrradiance = 0.0; @@ - finalIrradiance = lerp(specularIrradiance, specularIrradianceReflections, skylightingSpecular); + finalIrradiance = lerp(specularIrradiance, specularIrradianceReflections, skylightingSpecular);package/Shaders/DeferredCompositeCS.hlsl (3)
191-191: Good: applies ReflectionNormalisationScale consistentlyDirectional ambient specular uses ReflectionNormalisationScale here, aligning with DynamicCubemaps.GetDynamicCubemap.
273-280: Non-skylighting reflections path looks consistentColor and luminance both come from EnvReflectionsTexture, then GammaToLinear — consistent normalization.
20-26: EnvTexture/EnvReflectionsTexture registers (t6/t7) have no conflicts. Search across all shader files found no other bindings of EnvTexture or EnvReflectionsTexture at t6/t7, nor any bindings at t30/t31.
…m/skyrim-community-shaders into gamma-space-normalisation
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
…m/skyrim-community-shaders into gamma-space-normalisation
…aps.hlsli Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…m/skyrim-community-shaders into gamma-space-normalisation
* 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>
1 participant
Summary by CodeRabbit
Bug Fixes
Chores