fix(llf,isl): guard degenerate inputs helpers#65
Conversation
Pre-existing robustness gaps in code that Tier-2 test extraction is about to move; fixing here first keeps that refactor behavior-preserving. - InverseSquareLighting::CalculateRadius clamped only NaN. An exact-zero sqrt argument yields radius 0 (callers divide by radius -> 0/0 fade) and a zero cutoffOverride yields +inf (sqrt(+inf) is not NaN, so isnan missed it). Clamp any non-finite or non-positive result to 1.0. - ForEachShadowLight rejected null / non-canonical / misaligned pointers but not the low 64 KiB. A near-null garbage slot (e.g. 0x8) passed and was dereferenced (AV/CTD). Reject raw below 0x10000. - ComputeFrameTimePercentile90 guarded only count == 0; a negative count would drive a negative n into std::copy / std::nth_element (UB). <= 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 14 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Pull request overview
This PR hardens LightLimitFix and InverseSquareLighting helper paths against degenerate runtime inputs that could otherwise produce invalid GPU lighting data or unsafe shadow-light traversal.
Changes:
- Rejects near-null shadow light pointers before dereferencing.
- Handles negative frame-time sample counts with the existing fallback path.
- Clamps non-finite or non-positive inverse-square radius results to
1.0f.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Features/LightLimitFix/ShadowCasterManager.h |
Adds a low-address guard to shadow light pointer plausibility checks. |
src/Features/LightLimitFix/ShadowCasterManager.cpp |
Extends frame-time percentile fallback handling to non-positive counts. |
src/Features/InverseSquareLighting.cpp |
Ensures calculated radii are finite and positive before use by attenuation callers. |
|
No actionable suggestions for changed features. |
|
✅ A pre-release build is available for this PR: |
Summary
Three pre-existing robustness gaps in
InverseSquareLightingandShadowCasterManager, fixed in their current inline locations. These guard degenerate / corrupt inputs that can reach the GPU lighting path or dereference garbage.Landing this before the Tier-2 test-extraction refactor (#56) keeps that PR behavior-preserving — #56 then extracts this already-fixed code into testable headers and adds the unit tests for these exact cases.
Fixes
InverseSquareLighting::CalculateRadiusclamped onlyNaN. An exact-zerosqrtargument yields radius0(callers divide byradius, so a0/0attenuation fade), and a zerocutoffOverrideyields+inf(sqrt(+inf)is notNaN, so theisnan-only check let it through). Now clamps any non-finite or non-positive result to1.0.ShadowCasterManager::ForEachShadowLightrejected null / non-canonical / misaligned pointers but not the low 64 KiB. A near-null garbage slot (e.g.0x8) passed the check and was dereferenced (AV/CTD). Now also rejectsraw < 0x10000(the Windows x64 null-guard region is never a valid user mapping).ComputeFrameTimePercentile90guarded onlycount == 0; a negative count would drive a negativenintostd::copy/std::nth_element(UB). Now guards<= 0.Test plan
ALL, SE/AE/VR universal) build succeeds; clean link, zero/W4 /WXwarnings🤖 Generated with Claude Code