fix: fix bright character lighting#1448
Conversation
WalkthroughIntroduces per-pixel modulation of character light intensity by multiplying CharacterLightParams.z with a sampled noise texture value using baseShadowUV in package/Shaders/Lighting.hlsl. The clamped range using CharacterLightParams.w and zero remains unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
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/Lighting.hlsl (1)
2709-2713: Use dynamic-res-adjusted UV for noise; avoid screen-space mismatch and VR driftSampling the noise with baseShadowUV ignores VPOSOffset and dynamic-resolution adjustments applied to shadowUV. In VR or when dynamic resolution changes, this can cause pattern drift/flicker. Sample the noise with the same adjusted UV path for consistency. Also clamp the sampled value to [0,1] to be safe.
@@ if (Permutation::PixelShaderDescriptor & Permutation::LightingFlags::CharacterLight) { float charLightMul = saturate(dot(viewDirection, worldNormal.xyz)) * CharacterLightParams.x + CharacterLightParams.y * saturate(dot(float2(0.164398998, -0.986393988), worldNormal.yz)); - float charLightColor = min(CharacterLightParams.w, max(0, CharacterLightParams.z * TexCharacterLightProjNoiseSampler.Sample(SampCharacterLightProjNoiseSampler, baseShadowUV).x)); + // Use the same dynamic-res + VPOS adjusted path as shadow sampling to keep the pattern stable. + float2 charNoiseUV = FrameBuffer::GetDynamicResolutionAdjustedScreenPosition(baseShadowUV * VPOSOffset.xy + VPOSOffset.zw); + float charNoise = saturate(TexCharacterLightProjNoiseSampler.Sample(SampCharacterLightProjNoiseSampler, charNoiseUV).x); + float charLightColor = min(CharacterLightParams.w, max(0, CharacterLightParams.z * charNoise)); diffuseColor += (charLightMul * charLightColor).xxx; }Verification:
- Test with dynamic resolution on/off and in VR; look for stability of the character-light pattern during camera pans.
- Optional: if you want a crisper dither, consider SampleLevel(..., 0) for charNoise to avoid mip smoothing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
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
🧠 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:
package/Shaders/Lighting.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
|
✅ A pre-release build is available for this PR: |
Summary by CodeRabbit