Add configurable rendering budgets, compact diagnostics and safer presets - #1
Add configurable rendering budgets, compact diagnostics and safer presets#1velrino wants to merge 7 commits into
Conversation
PR review — Linear Ability Casting Extended ThreeJS #1HOLD · 2 blockers PR #1 — configurable rendering budgets, diagnostics, and safer presetsOpen the pull request · reviewed 2026-09-10
TL;DRThe implementation is unusually thoughtful for a large first contribution: the frame budget, hidden-tab suspension, conditional passes, safer preset writes, tests, and documentation mostly agree with the PR claims. I would not merge this head yet because the new Compare export can silently aggregate or label metrics against rendering state that was not actually constant, and "Export all presets" drops the exact quarantined entries the recovery work promises to preserve. The remaining two issues are smaller but real: wheel zoom does not wake Economy mode from 15 FPS, and the adaptive-resolution heuristic mistakes a genuinely 30 Hz display for a GPU overrun. Findings🔴 BLOCKER 1 — Compare samples do not bind to the effective rendering stateEvidence: PerformancePanel.js lines 198–235 stores configured settings plus CPU/call/GPU arrays, but not adaptive scale. The scale passed to Economy enables adaptive resolution by default. A 10-second recording can therefore step through 1.0 → 0.85 → 0.7 → 0.6 while continuing to average metrics as if it were one controlled condition. The setting-comparison guard cannot detect this because A second instance of the same integrity problem occurs after switching profiles: the browser probe observed configured Risk: the feature intended to compare performance can produce an apparently precise report that blends multiple pixel budgets or labels the wrong light budget. Suggested fix: freeze adaptive resolution for a recording, cancel/restart when effective scale changes, or record per-frame scale and report its range/trajectory. Also include effective light-pool size or block recording until a required reload. Add tests that a completed sample cannot silently mix these states. 🔴 BLOCKER 2 — "Export all presets" omits quarantined presetsEvidence: failed stored entries are intentionally moved into The focused probe loaded one valid and one stale preset. The manager reported one quarantined entry, while the exported JSON contained only the valid preset. Risk: a user can choose "Export all presets" as a backup/migration step, clear the browser or move devices, and permanently lose the exact unreadable entry this PR says is being preserved. A wholly unreadable backup is even less discoverable: it is copied to a private storage key but has no visible export/recovery route. Suggested fix: include 🟠 SHOULD FIX — Wheel zoom stays at the 15 FPS idle budgetEvidence: wheel zoom is handled directly inside CameraRig, while App only marks InputManager pointer/action events active. Browser proof in Economy mode: target FPS was 15 before the wheel event; the desired camera distance changed from 11.5 to 12.97, but target FPS remained 15. A subsequent pointer-move immediately raised it to 30. Impact: scroll zoom and its damping animation are visibly sampled at the idle rate, contradicting the claim that the first input snaps the loop active. Suggested fix: give CameraRig an interaction callback (wheel and OrbitControls start/change) that calls 🟠 SHOULD FIX — A real 30 Hz display is classified as an overrunEvidence: AdaptiveResolution.sample() compares rendered intervals with a target capped at 60 FPS, but it does not know when the display itself is below 60 Hz. Feeding six seconds of perfect 30 Hz frames with a 60 FPS target produced The existing "display slower than the cap" test covers a 120 FPS setting on a 60 Hz panel, not a 60 FPS setting on a 30 Hz panel. Impact: a visible page on a 30 Hz display or comparable browser-level throttle loses 40% render scale without gaining a frame. Suggested fix: establish a conservative effective refresh ceiling independent of the workload, or use CPU/GPU evidence to distinguish refresh-limited cadence from render overrun. Add the 30 Hz negative control. Claims vs. repository vs. implementation
Local proof at exact head
Rendered receiptsOpen the contributor's live demo (public deployment linked by the PR).
(Both in DecisionRecommendation: request changes. Fix the two data-integrity blockers, add focused regressions, and then re-check the two input/refresh-rate advisories. The rendering optimizations themselves are directionally good and the branch is close, but the PR should not advertise trustworthy comparison and preset recovery while those two outward-facing paths can misstate or omit data. Founder calls: none. The PR asks generally for feedback but presents no explicit unresolved choice that must be decided before the author can fix these defects. No review, approval, merge, or comment was posted to the external repository. This is a read-only review artifact. |
|
Thank you, @vitorbarbosa19, for the thorough review and the concrete reproductions. The goal is to improve what we have, and this feedback helped identify gaps in both the comparison reports and preset recovery. I've addressed all four findings in 8defa2d, now pushed to this PR:
Validation after a fresh Thanks again for helping make this more reliable. The updated head is ready for another look. |
Hi @achrefelouafi! First of all, congratulations on this project — the procedural abilities look fantastic, and the care you put into the effects and the live VFX editor really shows. Thank you for sharing it!
While exploring the sandbox on my MacBook M4 Max, I noticed sustained rendering load and identified a few possible improvements. I wanted to contribute some options that could make the experience more comfortable on different devices while keeping the visual character of your work. These are suggestions, and I'd be happy to adjust the approach or narrow the scope based on what you feel fits the project.
TL;DR
Add configurable rendering budgets, Balanced/Economy profiles and a compact performance panel, reduce unnecessary rendering work, and improve preset validation and recovery. The latest commit includes 19 passing regression tests, a successful production build and a browser check of renderer initialization and adaptive resizing.
Live demo
@achrefelouafi, you can try the changes here: https://linear-ability-casting-threejs.vercel.app
This deployment contains commit
47fb55c, the current PR head.🎥 Video
This recording demonstrates the performance panel from an earlier iteration of this branch; it predates the latest adaptive-resolution and preset-recovery changes.
record.mp4
Motivation
The original setup renders at display refresh rate, with a pixel-ratio cap of 1.75 and 4096² shadows refreshed every frame. On my Mac, this prompted me to explore adjustable quality settings and ways to avoid work when effects are inactive.
Changes
PCFShadowMapdirectly.Validation
Latest commit (
47fb55c):npm test: 19 tests pass, covering timing, particle visibility, shadow cadence, adaptive resolution, profile selection, preset validation and persistence/recovery failures.npm run buildandgit diff --check: pass.Earlier commits also received browser smoke checks for all ten abilities, visibility suspension/resumption, performance-panel interactions, mobile layout and profile persistence. These were not all repeated after the latest changes and do not constitute full visual lifecycle verification.
Validation notes and measurement conditions · Latest test/build output and browser results
Tradeoffs and measurement limits
Earlier short idle comparisons recorded roughly 120 → 59–61 draw calls per frame, alongside a frame-rate change from about 120 to 30 FPS. Aggregate CPU work per second decreased, while GPU samples varied substantially. These observations do not establish a stable per-frame GPU speedup; power and temperature have not been measured.
Lower resolution, frame rate, bloom resolution and shadow refresh trade some sharpness and smoothness for less rendering work. Full render-on-demand was left out because the scene continues animating while idle. The controls make these tradeoffs adjustable, and I'd welcome your feedback on the defaults and overall scope.
Thank you again for making this project available — it has been a pleasure to explore, and I hope this contribution is useful!