Conversation
WalkthroughRefactors LightLimitFix to use FrameBuffer for camera/projection data, removes per-eye view-space positions and per-frame inverse-projection storage, switches cluster Z indexing to a log-based formula, uses squared-radius cluster culling, disables deferred contact shadows, adds VR-aware per-frame CB binding in prepass, and introduces UpdateStructure and ClearShaderCache. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CPU as CPU:LightLimitFix
participant GFX as D3D Context
participant FB as FrameBuffer (CB)
participant CSB as ClusterBuildingCS
participant CSC as ClusterCullingCS
rect rgba(230,240,255,0.6)
note over CPU,GFX: UpdateStructure pipeline (new)
CPU->>GFX: Compute clusterSize, renderSize
CPU->>GFX: Bind FB / per-frame CBs (bind VR CB if active)
CPU->>GFX: Bind UAVs/SRVs (Lights, Clusters, Counters)
GFX->>CSB: Dispatch ClusterBuildingCS
CSB-->>GFX: Write cluster bounds
GFX->>CSC: Dispatch ClusterCullingCS
CSC-->>GFX: Write light indices per cluster
GFX->>GFX: Unbind UAVs (cleanup)
end
sequenceDiagram
autonumber
participant Pre as PrepassPasses
participant Ctx as globals::d3d::context
participant VR as VR Relocation (0x3180688)
rect rgba(240,255,240,0.6)
note over Pre,Ctx: Prepass CB binding (changed)
Pre->>Ctx: Bind per-frame CB to CS slot (12)
Pre->>VR: If REL::Module::IsVR() fetch VR CB
alt VR CB available
Pre->>Ctx: Bind VR CB to CS slot (13)
else No VR
Pre-->>Ctx: Continue with per-frame only
end
end
note over VR: *Relocation read when REL::Module::IsVR()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Poem
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 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)
Files:
features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-08-03T18:37:19.690ZApplied to files:
📚 Learning: 2025-08-17T18:37:35.839ZApplied to files:
⏰ 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)
🔇 Additional comments (1)
✨ 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 |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (2)
src/Features/LightLimitFix.cpp (2)
140-145: Add failure handling for shader compilation and respect global defines.Util::CompileShader(...) return is unchecked. If compilation fails, later Dispatch() will crash. Also consider passing current global define set (e.g., LLFDEBUG) to keep CS in sync with the rest of the pipeline.
Proposed guard (pattern):
- clusterBuildingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterBuildingCS.hlsl", {}, "cs_5_0"); - clusterCullingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterCullingCS.hlsl", {}, "cs_5_0"); + clusterBuildingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterBuildingCS.hlsl", globals::state->GetDefines(), "cs_5_0"); + clusterCullingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterCullingCS.hlsl", globals::state->GetDefines(), "cs_5_0"); + if (!clusterBuildingCS || !clusterCullingCS) { + logger::error("[LLF] Failed to compile compute shaders"); + return; + }
971-1040: Potential UAV/SRV bounds overrun when resolution changes; add live reallocation.UpdateStructure recomputes clusterSize each frame, but clusters/lightIndexList/lightGrid were sized once in SetupResources. If clusterSize grows, ClusterBuildingCS/ClusterCullingCS will index past UAV bounds.
Apply this diff to add an in-place guard and reallocation:
void LightLimitFix::UpdateStructure() { auto context = globals::d3d::context; lightsNear = *globals::game::cameraNear; lightsFar = *globals::game::cameraFar; auto renderSize = Util::ConvertToDynamic(globals::state->screenSize); if (REL::Module::IsVR()) renderSize.x *= .5; clusterSize[0] = ((uint)renderSize.x + 63) / 64; clusterSize[1] = ((uint)renderSize.y + 63) / 64; clusterSize[2] = 32; + + // Reallocate resources if clusterSize changed + if (clusterSize[0] != allocatedClusterSize[0] || + clusterSize[1] != allocatedClusterSize[1] || + clusterSize[2] != allocatedClusterSize[2]) { + RecreateClusterResources(clusterSize[0], clusterSize[1], clusterSize[2]); + allocatedClusterSize[0] = clusterSize[0]; + allocatedClusterSize[1] = clusterSize[1]; + allocatedClusterSize[2] = clusterSize[2]; + }And add the helper (implementation example):
// new method (outside selected range; add to .h and .cpp) void LightLimitFix::RecreateClusterResources(uint newX, uint newY, uint newZ) { uint clusterCount = newX * newY * newZ; D3D11_BUFFER_DESC sbDesc{}; sbDesc.Usage = D3D11_USAGE_DEFAULT; sbDesc.BindFlags = D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_UNORDERED_ACCESS; sbDesc.MiscFlags = D3D11_RESOURCE_MISC_BUFFER_STRUCTURED; D3D11_SHADER_RESOURCE_VIEW_DESC srvDesc{}; srvDesc.Format = DXGI_FORMAT_UNKNOWN; srvDesc.ViewDimension = D3D11_SRV_DIMENSION_BUFFER; srvDesc.Buffer.FirstElement= 0; D3D11_UNORDERED_ACCESS_VIEW_DESC uavDesc{}; uavDesc.Format = DXGI_FORMAT_UNKNOWN; uavDesc.ViewDimension = D3D11_UAV_DIMENSION_BUFFER; uavDesc.Buffer.FirstElement= 0; uavDesc.Buffer.Flags = 0; // clusters sbDesc.StructureByteStride = sizeof(ClusterAABB); sbDesc.ByteWidth = sizeof(ClusterAABB) * clusterCount; clusters.reset(new Buffer(sbDesc)); srvDesc.Buffer.NumElements = clusterCount; clusters->CreateSRV(srvDesc); uavDesc.Buffer.NumElements = clusterCount; clusters->CreateUAV(uavDesc); // counter (unchanged size = 1) sbDesc.StructureByteStride = sizeof(uint32_t); sbDesc.ByteWidth = sizeof(uint32_t) * 1; lightIndexCounter.reset(new Buffer(sbDesc)); srvDesc.Buffer.NumElements = 1; lightIndexCounter->CreateSRV(srvDesc); uavDesc.Buffer.NumElements = 1; lightIndexCounter->CreateUAV(uavDesc); // light index list uint numList = clusterCount * CLUSTER_MAX_LIGHTS; sbDesc.StructureByteStride = sizeof(uint32_t); sbDesc.ByteWidth = sizeof(uint32_t) * numList; lightIndexList.reset(new Buffer(sbDesc)); srvDesc.Buffer.NumElements = numList; lightIndexList->CreateSRV(srvDesc); uavDesc.Buffer.NumElements = numList; lightIndexList->CreateUAV(uavDesc); // light grid sbDesc.StructureByteStride = sizeof(LightGrid); sbDesc.ByteWidth = sizeof(LightGrid) * clusterCount; lightGrid.reset(new Buffer(sbDesc)); srvDesc.Buffer.NumElements = clusterCount; lightGrid->CreateSRV(srvDesc); uavDesc.Buffer.NumElements = clusterCount; lightGrid->CreateUAV(uavDesc); }
🧹 Nitpick comments (6)
src/Features/LightLimitFix.h (1)
330-335: Formatter OK; consider trimming per-eye position spam in logs.Dumping both eyes’ posWS each time can be noisy. Optional: print only eye 0 or behind a verbose flag.
src/Features/LightLimitFix.cpp (2)
675-687: Unbind CS before releasing shaders; then recompile.Small safety to avoid releasing a bound shader.
void LightLimitFix::ClearShaderCache() { + globals::d3d::context->CSSetShader(nullptr, nullptr, 0); if (clusterBuildingCS) { clusterBuildingCS->Release(); clusterBuildingCS = nullptr; } if (clusterCullingCS) { clusterCullingCS->Release(); clusterCullingCS = nullptr; } clusterBuildingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterBuildingCS.hlsl", {}, "cs_5_0"); clusterCullingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterCullingCS.hlsl", {}, "cs_5_0"); }
960-969: Map/Unmap path is fine; consider early-out when lightCount == 0.Avoid mapping on empty frames; also set SRVs to null to prevent stale reads in PS.
- lightCount = std::min((uint)lightsData.size(), MAX_LIGHTS); + lightCount = std::min((uint)lightsData.size(), MAX_LIGHTS); + if (lightCount == 0) { + UpdateStructure(); // still rebuild empty grids/counters + return; + }package/Shaders/Lighting.hlsl (1)
2593-2604: Deferred LLF contact shadows disabled — confirm intent or gate by setting.The DEFERRED branch’s contact-shadow block is fully commented. If unintentional, shadows silently disappear in deferred with LLF. Prefer a runtime toggle or a compile-time define tied to settings.EnableContactShadows to avoid regressions.
-// # if defined(DEFERRED) -// [branch] if (SharedData::lightLimitFixSettings.EnableContactShadows && !(light.lightFlags & LightLimitFix::LightFlags::Simple) && shadowComponent != 0.0 && lightAngle > 0.0) { -// float3 normalizedLightDirectionVS = normalize(light.positionVS[eyeIndex].xyz - viewPosition.xyz); -// contactShadow = LightLimitFix::ContactShadows(viewPosition, screenNoise, normalizedLightDirectionVS, contactShadowSteps, eyeIndex); -// } -// # endif +# if defined(DEFERRED) + [branch] if (SharedData::lightLimitFixSettings.EnableContactShadows && + !(light.lightFlags & LightLimitFix::LightFlags::Simple) && + shadowComponent != 0.0 && + lightAngle > 0.0) { + // Reconstruct VS light dir from WS if positionVS is removed: + float3 normalizedLightDirectionVS = normalize(FrameBuffer::WorldToView(light.positionWS[eyeIndex].xyz, false, eyeIndex) - viewPosition.xyz); + contactShadow = LightLimitFix::ContactShadows(viewPosition, screenNoise, normalizedLightDirectionVS, contactShadowSteps, eyeIndex); + } +# endiffeatures/Light Limit Fix/Shaders/LightLimitFix/ClusterCullingCS.hlsl (2)
49-56: Shared-memory prefetch is dead code; remove it (and the barrier) or tile properly.
sharedLights[...]is written but never read; the subsequent loop fetches fromlights[i]again. The sync adds overhead for no benefit.Minimal cleanup:
- if (groupIndex < LightCount) { - uint lightIndex = groupIndex; - Light light = lights[lightIndex]; - sharedLights[groupIndex] = light; - } - - GroupMemoryBarrierWithGroupSync(); + // Shared prefetch removed; per-thread SRV fetches are used below.Or switch to tiled loading to actually leverage
groupshared(can provide a patch if desired).
81-82: Unnecessary group barrier.No group-shared data is consumed after the light loop; this barrier is redundant.
- GroupMemoryBarrierWithGroupSync(); - + // No group-shared data after this point; barrier removed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
features/Light Limit Fix/Shaders/LightLimitFix/ClusterBuildingCS.hlsl(2 hunks)features/Light Limit Fix/Shaders/LightLimitFix/ClusterCullingCS.hlsl(3 hunks)features/Light Limit Fix/Shaders/LightLimitFix/Common.hlsli(0 hunks)features/Light Limit Fix/Shaders/LightLimitFix/LightLimitFix.hlsli(1 hunks)package/Shaders/Lighting.hlsl(1 hunks)src/Deferred.cpp(1 hunks)src/Features/LightLimitFix.cpp(4 hunks)src/Features/LightLimitFix.h(2 hunks)
💤 Files with no reviewable changes (1)
- features/Light Limit Fix/Shaders/LightLimitFix/Common.hlsli
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
features/Light Limit Fix/Shaders/LightLimitFix/LightLimitFix.hlslipackage/Shaders/Lighting.hlslsrc/Deferred.cppfeatures/Light Limit Fix/Shaders/LightLimitFix/ClusterCullingCS.hlslfeatures/Light Limit Fix/Shaders/LightLimitFix/ClusterBuildingCS.hlslsrc/Features/LightLimitFix.cppsrc/Features/LightLimitFix.h
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/Light Limit Fix/Shaders/LightLimitFix/LightLimitFix.hlslifeatures/Light Limit Fix/Shaders/LightLimitFix/ClusterCullingCS.hlslfeatures/Light Limit Fix/Shaders/LightLimitFix/ClusterBuildingCS.hlsl
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Deferred.cppsrc/Features/LightLimitFix.cppsrc/Features/LightLimitFix.h
🧠 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:
package/Shaders/Lighting.hlslfeatures/Light Limit Fix/Shaders/LightLimitFix/ClusterBuildingCS.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:
features/Light Limit Fix/Shaders/LightLimitFix/ClusterBuildingCS.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 (10)
src/Deferred.cpp (2)
289-289: LGTM: using globals::d3d::context here matches the rest of the pipeline.
Consistent with StartDeferred/DeferredPasses and avoids accidental renderer-state drift.
294-310: Clear CS slot 13 when not VR; normalize perFrame deref
- Unbind CS slot 13 in the non‑VR branch to avoid leaving a stale VR constant buffer bound; use a single dereference style for globals::game::perFrame.
Minimal change:
{ - ID3D11Buffer* buffers[1] = { *globals::game::perFrame.get() }; + ID3D11Buffer* buffers[1] = { *globals::game::perFrame }; + ID3D11Buffer* nullCB[1] = { nullptr }; ID3D11Buffer* vrBuffer = nullptr; if (REL::Module::IsVR()) { static REL::Relocation<ID3D11Buffer**> VRValues{ REL::Offset(0x3180688) }; vrBuffer = *VRValues.get(); } if (vrBuffer) { context->CSSetConstantBuffers(12, 1, buffers); context->CSSetConstantBuffers(13, 1, &vrBuffer); } else { context->CSSetConstantBuffers(12, 1, buffers); + context->CSSetConstantBuffers(13, 1, nullCB); } }Repo-wide checks (run from repo root):
rg -n -C2 "CSSetConstantBuffers\s*(\s13\s,\s1\s,\s*" --type=cpp
rg -n -C2 "CSSetConstantBuffers\s*(\s13\s,\s1\s,\s*&?nullptr|{ ?nullptr ?}" --type=cppsrc/Features/LightLimitFix.cpp (1)
430-442: Perf markers: nice touch.Begin/EndPerfEvent around Prepass keeps PIX/NSight traces readable.
features/Light Limit Fix/Shaders/LightLimitFix/ClusterBuildingCS.hlsl (2)
1-2: Good move to FrameBuffer::CameraProjInverse; matches the rest of the pipeline.
20-21: Mul order change is correct; confirm VR parity.Using mul(CameraProjInverse, clip) is the right convention. For VR, you already take max/min of both eyes below; looks consistent.
Would you like me to run a quick register scan to ensure CS SRV/UAV slots (u0/t0..t2) don’t collide with other features? I can provide a script.
features/Light Limit Fix/Shaders/LightLimitFix/ClusterCullingCS.hlsl (5)
23-29: Good switch to squared-distance AABB test.Avoids sqrt, preserves correctness. No issues spotted.
60-60: Precomputing radiusSquared is correct and cheaper.
69-72: Confirm WorldToView signature / eye‑jitter args for non‑VRAutomated search couldn't locate the WorldToView definition (search output: "Common: No such file or directory"). Confirm whether the 2nd and 3rd parameters select eye/index or jitter; if so, use the overload without eye args for non‑VR or ensure the constants passed (true, 0) are correct for a non‑VR path.
File: features/Light Limit Fix/Shaders/LightLimitFix/ClusterCullingCS.hlsl lines 69–72.
63-67: Fix VR eye selection in ClusterCullingCS.hlslBoth FrameBuffer::WorldToView calls use the left-eye args — the right-eye call should pass the right-eye flags/index.
File: features/Light Limit Fix/Shaders/LightLimitFix/ClusterCullingCS.hlsl (around lines 63–64)
Apply:
- float3 positionVSLeft = FrameBuffer::WorldToView(light.positionWS[0].xyz, true, 0); - float3 positionVSRight = FrameBuffer::WorldToView(light.positionWS[1].xyz, true, 0); + float3 positionVSLeft = FrameBuffer::WorldToView(light.positionWS[0].xyz, /*isLeftEye=*/true, /*eyeIndex=*/0); + float3 positionVSRight = FrameBuffer::WorldToView(light.positionWS[1].xyz, /*isLeftEye=*/false, /*eyeIndex=*/1);I couldn't find the FrameBuffer::WorldToView definition in the repo to confirm parameter semantics — verify the bool/index map to eye selection before applying. Union-of-eyes (left || right) increases per-cluster light counts; ensure MAX_CLUSTER_LIGHTS is sufficient.
1-1: PerFrame uses register(b0) — verify no cbuffer collision with Common/FrameBuffer.hlslifeatures/Light Limit Fix/Shaders/LightLimitFix/ClusterCullingCS.hlsl:4 declares
cbuffer PerFrame : register(b0); repo-wide search shows 24 occurrences ofregister(b0)and many shaders includeCommon/FrameBuffer.hlsli, butFrameBuffer.hlsliwas not found in the tree — verify whether FrameBuffer defines any b0 cbuffers or relocatePerFrameto an unusedb#.
|
✅ 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: 1
♻️ Duplicate comments (2)
features/Light Limit Fix/Shaders/LightLimitFix/LightLimitFix.hlsli (1)
24-33: Cluster Z index: log-domain hazards (NaN/Inf), negative/zero z, out-of-range.log(z / near) explodes for z <= 0 or z < near; casting NaN/Inf to uint is undefined. Clamp z to valid positive range, guard denominator, and clamp the result.
Apply:
- if (!FrameBuffer::FrameParams.y) // Fix first person lights - uv = 0.5; - - uint clusterZ = log(z / SharedData::CameraData.y) * clusterSize.z / log(SharedData::CameraData.x / SharedData::CameraData.y); + if (!FrameBuffer::FrameParams.y) // Fix first person lights + uv = 0.5; + + // Robust log mapping in [near, far] + float nearZ = max(1e-5, SharedData::CameraData.y); + float farZ = max(nearZ + 1e-5, SharedData::CameraData.x); + float zAbs = max(nearZ, abs(z)); // handle sign + float denom = max(1e-5, log(farZ / nearZ)); // avoid div by 0 + float z01 = saturate(log(zAbs / nearZ) / denom); + uint clusterZ = (uint)floor(z01 * (clusterSize.z - 1) + 0.5); @@ - // Bounds validation to prevent out-of-range cluster indices + // Bounds validation to prevent out-of-range cluster indices if (any(cluster >= clusterSize)) return false;src/Features/LightLimitFix.h (1)
185-192: Realloc on ClusterSize change to avoid UAV OOB writes.UpdateStructure recomputes ClusterSize each frame, but buffers are allocated once in SetupResources. If resolution/eyeCount changes, compute dispatch will write past UAV bounds.
Apply header additions:
@@ uint clusterSize[3] = { 16 }; + uint allocatedClusterSize[3] = { 0, 0, 0 }; // tracks current GPU allocation + void RecreateClusterResources(uint newX, uint newY, uint newZ);And call RecreateClusterResources from UpdateStructure when any dimension differs (see cpp suggestion).
🧹 Nitpick comments (3)
src/Features/LightLimitFix.h (2)
199-213: Remove stale EnableContactShadows setting.Shader-side contact shadows were removed. Keeping this field is confusing and unused.
Apply:
- bool EnableContactShadows = false;
329-334: Formatter: label eyes to aid debugging.Minor: clarify which posWS is left/right to avoid misreads when comparing VR eyes.
Apply:
- return fmt::format_to(ctx.out(), "{{address {:x} color {} radius {} posWS {} {}}}", + return fmt::format_to(ctx.out(), "{{address {:x} color {} radius {} posWS_L {} posWS_R {}}}",src/Features/LightLimitFix.cpp (1)
127-129: Shader compile on startup: OK. Add minimal failure guard.If CompileShader fails, CSSetShader(null) + Dispatch is wasted. Early-out on null to skip dispatch work.
Apply:
- clusterBuildingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterBuildingCS.hlsl", {}, "cs_5_0"); - clusterCullingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterCullingCS.hlsl", {}, "cs_5_0"); + clusterBuildingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterBuildingCS.hlsl", {}, L"cs_5_0"); + clusterCullingCS = (ID3D11ComputeShader*)Util::CompileShader(L"Data\\Shaders\\LightLimitFix\\ClusterCullingCS.hlsl", {}, L"cs_5_0");And in UpdateStructure (see below) early-return if either CS is null.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
features/Light Limit Fix/Shaders/LightLimitFix/LightLimitFix.hlsli(1 hunks)package/Shaders/Common/SharedData.hlsli(1 hunks)package/Shaders/Lighting.hlsl(4 hunks)src/Features/LightLimitFix.cpp(4 hunks)src/Features/LightLimitFix.h(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
package/Shaders/Common/SharedData.hlslisrc/Features/LightLimitFix.cppfeatures/Light Limit Fix/Shaders/LightLimitFix/LightLimitFix.hlslipackage/Shaders/Lighting.hlslsrc/Features/LightLimitFix.h
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Features/LightLimitFix.cppsrc/Features/LightLimitFix.h
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/Light Limit Fix/Shaders/LightLimitFix/LightLimitFix.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/Light Limit Fix/Shaders/LightLimitFix/LightLimitFix.hlslipackage/Shaders/Lighting.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:
features/Light Limit Fix/Shaders/LightLimitFix/LightLimitFix.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: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Build plugin and addons
🔇 Additional comments (7)
package/Shaders/Common/SharedData.hlsli (1)
71-71: Pad size change: confirm cross-CPU/GP layout parity.pad0 became float2 here. Verify every matching CPU-side struct packs the same (e.g., LightLimitFix::PerFrame.pad0[2]) and that no other code still assumes a single float.
package/Shaders/Lighting.hlsl (2)
2605-2618: Gating parallax shadow work on shadowComponent looks good.Avoids wasted work when a light is fully shadowed.
2630-2637: PBR path: passing parallaxShadow into LightProperties is correct.Keeps POM shadowing consistent across lobes.
src/Features/LightLimitFix.h (1)
97-99: CB layout parity with HLSL.PerFrame.pad0[2] matches SharedData.LightLimitFixSettings.pad0(float2). Good.
src/Features/LightLimitFix.cpp (3)
662-674: ClearShaderCache correctness.Releasing and recompiling both CS variants is correct; no lingering UAV/CB bindings.
947-956: Lights buffer update: OK.WRITE_DISCARD + tight memcpy of used elements only is efficient.
958-1026: Recreate cluster GPU buffers when ClusterSize changes; ensure CPU light-limit matches shader MAX_CLUSTER_LIGHTS
- Verified: shaders reference MAX_CLUSTER_LIGHTS (features/Light Limit Fix/Shaders/LightLimitFix/ClusterCullingCS.hlsl — visibleLightIndices / check at the MAX_CLUSTER_LIGHTS bounds; package/Shaders/* also use MAX_CLUSTER_LIGHTS).
- Not found in repository: a CPU-side CLUSTER_MAX_LIGHTS definition or any allocatedClusterSize / RecreateClusterResources / SetupResources reallocation logic. Current UpdateStructure-like dispatch will risk writing past UAV bounds when render/VR size changes.
- Fix (high priority): add tracking of allocatedClusterSize and (re)create clusters, lightIndexCounter, lightIndexList, lightGrid when clusterSize changes (before CS Dispatch). Also ensure the CPU constant name/value used for max cluster lights equals the shader MAX_CLUSTER_LIGHTS.
1 participant
Uses exact correct camera information.
Always updates, uses jitter.
Cleanup.
Summary by CodeRabbit
Bug Fixes
New Features
Refactor
Revert