fix(llf): harden inputs across particle lights#88
Conversation
|
Warning Review limit reached
More reviews will be available in 29 minutes and 18 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 |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Pull request overview
This PR hardens configuration and engine-input handling in LightLimitFix particle lights and Upscaling, addressing minor robustness issues (division-by-zero avoidance, deterministic config ordering, stricter hex validation, and sanitization of persisted settings).
Changes:
- Guarded particle-light fade interpolation against degenerate
lightFadeEnd <= lightFadeStartto prevent divide-by-zero/NaN propagation. - Sorted particle-light config file lists to make duplicate “first wins” behavior deterministic across filesystem iteration orders.
- Tightened gradient color parsing to require exactly 6 or 8 hex digits, and clamped
streamlineLogLevelprior to using it as a UI index.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Features/Upscaling.cpp |
Clamps persisted Streamline log level before UI use to avoid out-of-range indexing/persistence issues. |
src/Features/LightLimitFix/ParticleLights.cpp |
Sorts config lists for determinism and strengthens gradient hex-width validation. |
src/Features/LightLimitFix/Particle.cpp |
Adds a fade-range degeneracy guard to prevent divide-by-zero in dimmer interpolation. |
|
✅ A pre-release build is available for this PR: |
4a34adf to
8482cbb
Compare
8482cbb to
f45f94c
Compare
Addresses the minor hardening items in #87 (pre-existing, surfaced by CodeRabbit on #56): - Particle.cpp: guard the dimmer interpolation against lightFadeEnd <= lightFadeStart so equal engine fade globals can't divide by zero (NaN/Inf into GPU lighting). - ParticleLights.cpp: std::sort the get_configs results before the 'first wins' duplicate dedupe so the surviving config is deterministic across installs (both ParticleLights and Gradients loops). - ParticleLights.cpp: require exactly 6 or 8 hex digits for gradient colors so malformed widths (#1, #12345) fail closed instead of packing a garbage 32-bit color. - Upscaling.cpp: clamp the stored streamlineLogLevel (not just the UI index) so a stale/hand-edited out-of-range value can't persist through the save / restart-diff paths. Closes #87
- ParticleLights: validate hex digits with bounded find_first_not_of instead of strspn on string_view::data(), which is not guaranteed to be NUL-terminated. - Upscaling: clamp streamlineLogLevel in LoadSettings so a stale or out-of-range JSON value is sanitized on every load, not only when the Backend Diagnostics tree node is expanded. Drop the now-redundant in-tree clamp whose comment claimed coverage it did not provide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c9b2962 to
70348b2
Compare
Avoid degenerate particle-light fade interpolation, make duplicate config selection deterministic, and reject malformed gradient color widths with bounded string_view validation. The upscaling streamlineLogLevel part of alandtse#88 is intentionally skipped because this branch already sanitizes the stored setting during LoadSettings. Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
Summary
Addresses the four minor hardening items tracked in #87 — pre-existing issues surfaced by CodeRabbit during #56's review, in files outside that PR's scope, so split out here.
LightLimitFix/Particle.cpplightFadeEnd == lightFadeStart(non-zero) made the dimmer interpolation divide by zero → NaN/Inf into lighting. NowlightFadeEnd <= lightFadeStarttakes the full-brightness branch.LightLimitFix/ParticleLights.cppget_configsorder is filesystem-dependent; the "first wins" duplicate policy could pick different winners across installs.std::sortbefore both loops (ParticleLights + Gradients).LightLimitFix/ParticleLights.cpp#1/#12345packed as garbage 32-bit colors. Now requires exactly 6 (RRGGBB) or 8 (AARRGGBB) digits — fails closed.Upscaling.cppstreamlineLogLevelpersisted unclamped through save/restart-diff. Now the stored value itself is clamped.All four verified present on
devbefore fixing; each is the change CodeRabbit suggested, kept minimal.Validation
std::sortcalls with<algorithm>already included, asize()check, and astd::minclamp on auint), so I'm relying on CI's full build +cpp_tests+ shader validation here.Closes #87.
🤖 Generated with Claude Code