From a18a818e8b708da7271ecba9a640186a3af32a4d Mon Sep 17 00:00:00 2001 From: OmikronApex <45161725+OmikronApex@users.noreply.github.com> Date: Wed, 8 Jul 2026 00:00:58 +0200 Subject: [PATCH] fix(playback): throttle legacy bridge-hit recording; emit loop-set for manual A/B (#811) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(playback): throttle legacy bridge-hit recording; emit loop-set for manual A/B window.feedBack.getLoop() is a read surface plugins legitimately poll (note_detect HUD ticked it at ~30 Hz), but every call recorded a playback.loop-api bridge hit: compat-shim bookkeeping, a playback:bridge-hit event, and a diagnostics snapshot rebuild + stringify per call — real main-thread cost and a saturated hitCount in the capability inspector, even with no song playing. - _recordPlaybackBridge now throttles per bridgeId|surface (5 s window). Bridge hits are a 'surface still in use' signal, not a call counter. - setLoopEnd() (manual A/B buttons) now emits the same loop-set transport event as setLoop(), so event-driven consumers no longer need to poll getLoop() to see button-armed loops. Co-Authored-By: Claude Fable 5 * docs(changelog): note loop-api bridge throttle fix Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- CHANGELOG.md | 1 + static/app.js | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62cd916..0b260ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Player frame-time hotspots removed (trace-backed) + weak-hardware hardening.** A Chrome performance trace of a 3D-highway session surfaced two core per-frame layout-thrash sources, now fixed: the highway's visibility check read `canvas.offsetParent` every rAF frame (forces style/layout recalc — now sampled every 10th frame with a cached value, force-refreshed on init/canvas-replace/resize/override-clear), and the v3 player chrome loop called `matches(':hover')` per frame and unconditionally rewrote the Up-Next pill's `textContent`/bar width at 6 Hz (now hover-tracked via mouseenter/mouseleave, DOM writes only on value change, progress bar moved from `width` to compositor-only `scaleX`). The 3D highway pre-warms shader programs (`ren.compile`) and deterministic label textures at init — and chart-dependent chord/section label textures on first draw — so first-appearance shader-compile/texture-upload frame spikes move into the load spinner. For weaker hardware: the per-frame renderer bundle is now a single reused object instead of a fresh ~35-field allocation per frame (object identity is stable and meaningless; array fields still swap reference on chart changes), custom viz get `bundle.lowerBoundT`/`bundle.lowerBoundTime` binary-search helpers for visible-window culling, the default 2D highway's beat lines no longer scan every beat in the song per frame, and the 3D highway stops reading `localStorage` per frame (1 Hz poll) and caches its lyrics text-measurement layout per displayed line instead of re-measuring every syllable every frame. A second, throttled-CPU trace pass additionally removed: shader-program re-resolution churn from label texture swaps (`material.needsUpdate` is now only set on a null↔texture transition — swapping between two cached label textures never changes the compiled program), the 3D highway's per-frame `getBoundingClientRect` layout read in its canvas-size self-check (now every 10th frame, still immediate on backing-store change), and the core 60 Hz HUD clock rewriting `textContent` on every tick (now write-on-change, ~1/s). The dominant residual — steady `getParameters` shader-program re-resolution (~4% of throttled main thread) — turned out to be Three r158+'s transparent-DoubleSide two-pass rendering, which sets `material.needsUpdate` twice per object per frame; all 18 of the 3D highway's transparent DoubleSide materials are flat unlit quads (labels, rails, chord frames, lanes), so they now declare `forceSinglePass: true`, eliminating the recompile churn and halving those objects' draw calls. ### Fixed +- **`playback.loop-api` bridge no longer fires dozens of times per second.** Every `window.feedBack.getLoop()` call recorded a full bridge hit — compat-shim bookkeeping, a `playback:bridge-hit` event, and a diagnostics snapshot rebuild + stringify — so a plugin polling loop state from a HUD tick (note_detect at ~30 Hz) flooded the capability inspector and burned main-thread time even with no song playing. `_recordPlaybackBridge` now throttles per bridge/surface (5 s window): bridge hits are a "surface still in use" signal, not a call counter. The manual A/B loop buttons (`setLoopEnd`) also now emit the same `loop-set` transport event as `setLoop()`, so plugins can react to loop changes via `playback:loop-set` / `playback:loop-cleared` events instead of polling `getLoop()`. - **3D Highway: recover from a WebGL context loss instead of crashing on alt-tab.** Switching the active window / alt-tabbing away from the app (most often on Windows) can trigger a GPU context reset; the 3D highway's WebGL renderer had **no `webglcontextlost` handler**, so a lost context was left to escalate into a render-process crash — matching the intermittent "randomly crashes when I change windows" desktop reports. The renderer now binds `webglcontextlost`/`webglcontextrestored` on its own WebGL canvas (`ren.domElement`): the loss is `preventDefault()`'d so the browser keeps the context restorable, `draw()` bails while the context is down so no GL work runs on a dead context, and on restore the viewport is re-applied and rendering resumes (Three re-uploads scene resources on the next frame). Listeners are torn down with the renderer. `plugins/highway_3d` → 3.31.3. Tests: `tests/js/highway_3d_context_loss.test.js`. (The sibling `keys_highway_3d` / `drum_highway_3d` renderers share the same gap — tracked as a follow-up in their repos.) - **Guitar Pro 6 (`.gpx`) import no longer fails on every real file.** The GPX BCFS container reader (`lib/gp2rs_gpx.py`) rejected any file whose final sector wasn't a full `0x1000` block — but a real `.gpx`'s BCFZ-declared decompressed size isn't sector-aligned, so the last (small) container file always lands in a partial trailing sector. The bounds check *raised* `GPX BCFS sector pointer out of range (malformed file)` instead of clamping the tail read, so `_load_gpif` threw before `score.gpif` could be extracted and **no GP6 file could be imported into the song editor** (both real test files failed identically — this wasn't file-specific). GP7/GP8 `.gp` files were unaffected — they take the ZIP path, not BCFS, which is why prior GP-import work didn't surface it. The reader now **clamps the final sector read to the buffer end** (the per-file size field trims the padding anyway), matching canonical GPX readers (alphaTab / PyGuitarPro); a sector whose *start* is past the end still raises, preserving the malformed-file guard. Verified against two real GP6 files — both now unpack to valid GPIF with all tracks. Tests: `tests/test_gp2rs_gpx.py` (partial-final-sector round-trip, multi-file container, sector-aligned baseline, and the preserved out-of-range guard). - **v3 Songs grid: fixed the scroll stutter that "skips every so many scrolls," up or down.** The virtualized grid rebuilt its **entire** visible window (`grid.innerHTML = …` + a full `wireCards` pass) every time it slid by one row, so each row-boundary crossing was a heavy synchronous frame that stalled the main thread and buffered held-arrow key-repeats into a visible lurch (a tester's "super fast for a second then slowed down") at fixed scroll offsets — in **both directions and regardless of whether the page was already loaded** (the cost was DOM teardown, not fetching, which is why scrolling back up over cached songs hitched too). `renderWindow()` now **reconciles the window in place**: it reuses the card nodes that stay on-screen and builds only the row that enters/leaves (~6 nodes per slide instead of ~60), keyed by absolute index with a real-vs-skeleton + select-mode signature so hole-fills (after a page fetch) and select-mode toggles still rebuild exactly the nodes that changed. `wireCards`'s `data-wired` guard then wires only the freshly-built nodes, so per-slide listener churn drops with it. Follow-up to the stage-2 virtualized grid (got-feedback/feedBack#636 item 3). Frontend-only: `static/v3/songs.js`. Tests: `tests/js/v3_songs_window_recycle.test.js` (window stays `[start,end)` contiguous + in-window node identity reused across a down-then-up scroll; select-mode toggle and rail-seek jump rebuild correctly). diff --git a/static/app.js b/static/app.js index dc48c15..0e98b90 100644 --- a/static/app.js +++ b/static/app.js @@ -5568,9 +5568,24 @@ function _playbackApi() { : null; } +// Bridge hits are a "this legacy surface is still in use" signal, not a call +// counter — but recordBridgeHit is not cheap (compat-shim bookkeeping, a +// playback:bridge-hit event, and a diagnostics snapshot rebuild per call). +// Plugins legitimately poll read surfaces like window.feedBack.getLoop() from +// HUD ticks (note_detect polled at ~30 Hz), which turned every tick into a +// snapshot serialization on the main thread and saturated the inspector's +// hitCount. Throttle per surface: the first call records immediately, repeats +// within the window are dropped. +const _bridgeRecordLast = new Map(); +const _BRIDGE_RECORD_MIN_MS = 5000; function _recordPlaybackBridge(bridgeId, legacySurface, reason) { const playback = _playbackApi(); if (!playback || typeof playback.recordBridgeHit !== 'function') return; + const key = `${bridgeId}|${legacySurface}`; + const now = Date.now(); + const last = _bridgeRecordLast.get(key); + if (last != null && now - last < _BRIDGE_RECORD_MIN_MS) return; + _bridgeRecordLast.set(key, now); playback.recordBridgeHit({ bridgeId, legacySurface, @@ -7944,6 +7959,10 @@ function setLoopEnd() { if (loopB <= loopA) { loopB = null; return; } document.getElementById('btn-loop-b').className = 'px-3 py-1.5 bg-green-900/50 rounded-lg text-xs text-green-300 transition'; updateLoopUI(); + // Manual A/B arming is a loop mutation like setLoop()'s — emit the same + // transport event so event-driven consumers (note_detect drill sync) see + // button-armed loops without having to poll getLoop(). + window.feedBack?.playback?.transportEvent?.('loop-set', { requesterId: 'core.loop', loopA, loopB, loop: { startTime: loopA, endTime: loopB, enabled: true, state: 'active' } }); } function clearLoop(options) {