fix: fix bloom intensity#1458
Conversation
WalkthroughReplaces the bloom accumulation term in package/Shaders/ISHDR.hlsl to apply DisplayMapping::RangeCompress to the clamped difference (max(0, Param.x - hdrColor)) before multiplying by bloomColor. No other logic or control flow changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Poem
✨ Finishing Touches🧪 Generate unit tests
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package/Shaders/ISHDR.hlsl (1)
117-117: Consider luminance-based gate to avoid hue shifts.Per-channel gating can tint bloom. If desired, gate on luminance and apply a scalar weight.
- hdrColor += DisplayMapping::RangeCompress(max(0, Param.x - hdrColor)) * bloomColor; + const float l = Color::RGBToLuminance(hdrColor); + const float w = DisplayMapping::RangeCompress(max(0, l - Param.x)); + hdrColor += w * bloomColor;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package/Shaders/ISHDR.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/ISHDR.hlsl
🧠 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/ISHDR.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 (2)
package/Shaders/ISHDR.hlsl (2)
117-117: Good call on using RangeCompress to tame overbloom.Compressing the gate before scaling bloomColor is a sensible way to reduce runaway bloom intensity while preserving highlight roll-off.
117-117: Confirm bloom threshold polarity (invert subtraction)
IfParam.xis intended as the bloom threshold, the currentmax(0, Param.x - hdrColor)will boost darker regions rather than highlights. Swap the operands to extract bright areas:- hdrColor += DisplayMapping::RangeCompress(max(0, Param.x - hdrColor)) * bloomColor; + hdrColor += DisplayMapping::RangeCompress(max(0, hdrColor - Param.x)) * bloomColor;Verify that
Param.xindeed maps to the bloom threshold in your engine setup.
|
✅ A pre-release build is available for this PR: |
Summary by CodeRabbit