From 73159a76a760661695652fd59644ad8c97a0b90e Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Wed, 15 Jul 2026 00:02:57 +0200 Subject: [PATCH] fix(highway): a SUPERSEDED renderer init is not a FAILED one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting a gig dropped the player onto the fallback 2D highway with no venue. startGig() calls setViz('venue'), which installs the 3D renderer — whose init is async — and then immediately starts its play queue. playSong() re-initialises that same renderer a tick later. A renderer mints a fresh readyPromise per init() and rejects the previous one with "superseded"; highway.js only checked that the RENDERER OBJECT was unchanged, which it is. So it treated a healthy, re-initialising renderer as a failed one, tore it down, and reverted to 2D: renderer async init failure: Error: superseded viz picker: reverted to default renderer (async-init-failure) The guard now also checks the PROMISE identity: a rejection from an init cycle the renderer has already moved on from is ignored. The renderer-identity guard stays (a rejection for a renderer since REPLACED is also not ours), and a genuine failure of the CURRENT cycle still reverts — both init() call sites go through _setRenderer, which re-wires the handler every time, so the new cycle is always watched. Reproduced and fixed against the real build: before: vizSelection=default viz-picker=default venue=inactive viz:reverted after: vizSelection=venue viz-picker=venue venue=ACTIVE (no revert) Also widens the paused-frame throttle's opt-out. The throttle fires whenever the CHART CLOCK is stalled — not only on a pause, but through a count-in and the credits/author overlay too. Its opt-out only asked "is a crowd video rolling", but the venue scene animates on a clock of its own with no pack at all (backdrop breathe, parallax, haze drift, warmth pulse — Math.sin(t) in the draw loop), so that motion was still being throttled. It now claims frames for both sources; a plain 3D highway with no venue reads motion mode 'off' and keeps the #654 GPU saving. HONEST CAVEAT on that second part: I could not get the throttle to fire in a reproduction. A control run on the shipped code showed 100 draws/sec while paused, not the ~10/sec a firing throttle would give — so the change is defensible on its own terms (a stalled clock is genuinely not a static picture) but it does NOT have a demonstrated symptom behind it. The viz fix above does. Tests: the superseded guard, and that the throttle opt-out covers both motion sources. All fail against the pre-fix source. eslint 0 errors; JS 1207/1207. --- plugins/highway_3d/screen.js | 22 +++++++++-- static/highway.js | 16 ++++++++ tests/js/highway_pause_throttle.test.js | 49 ++++++++++++++++++++++--- 3 files changed, 78 insertions(+), 9 deletions(-) diff --git a/plugins/highway_3d/screen.js b/plugins/highway_3d/screen.js index 577726f9..3bc2ff87 100644 --- a/plugins/highway_3d/screen.js +++ b/plugins/highway_3d/screen.js @@ -15398,15 +15398,29 @@ // highway throttled the whole room. Pausing the song dropped the // venue, the crowd and the stage to 10 fps. // - // Only claim continuous frames while a crowd video is actually - // rolling: with no venue pack (the common case) the paused scene IS - // static and the throttle should still save the GPU. + // Two independent sources of motion, and BOTH must keep their frames: + // + // • a crowd video rolling on its own clock (career venue pack), and + // • the venue scene's own fake-depth motion — the backdrop breathes, + // the haze drifts, warmth pulses, the shimmer moves. That is + // Math.sin(t) in the draw loop (see _venueApplyFakeDepthMotion), + // so it only moves while we are actually given frames, and it runs + // with NO pack at all. + // + // The throttle fires whenever the CHART CLOCK is stalled — which is + // not just a pause. A count-in and the credits/author overlay stall it + // exactly the same way, so the venue was stuttering there too. + // + // With no venue at all (plain 3D highway) the paused scene really is a + // still picture: motion mode reads 'off', we claim nothing, and the + // throttle still saves the GPU as #654 intended. needsContinuousFrames() { if (!_isReady || _ctxLost) return false; for (const v of _venueCrowdVideos) { if (v && !v.paused && !v.ended && v.readyState >= 2) return true; } - return false; + // 'off' also covers prefers-reduced-motion and "no venue scene". + try { return _venueEffectiveMotionMode() !== 'off'; } catch (_) { return false; } }, draw(bundle) { diff --git a/static/highway.js b/static/highway.js index cc74ae45..cb8811c1 100644 --- a/static/highway.js +++ b/static/highway.js @@ -986,6 +986,22 @@ function createHighway() { // inline arrow function. function _handleAsyncInitFailure(e) { if (hwState._renderer !== _installedRenderer) return; + // ...and ignore a rejection from a SUPERSEDED init cycle. + // + // A renderer mints a fresh readyPromise on every init(), and + // rejects the previous one ("superseded") when a newer init + // starts. The renderer object is unchanged, so the identity + // check above does not catch it — and we would tear down a + // perfectly healthy renderer that is merely re-initialising. + // + // This is exactly what starting a gig did: setViz('venue') + // installed the 3D renderer, then the queue's playSong() + // re-initialised it a tick later; init #1's promise rejected, + // and the gig dropped to the fallback 2D highway with the + // venue gone. A superseded init is not a failed init — the + // NEW cycle owns the outcome, and its own promise is what we + // must judge. + if (_installedRenderer.readyPromise !== rp) return; console.error('renderer async init failure:', e); _destroyCurrentIfInited(); hwState._renderer = _defaultRenderer; diff --git a/tests/js/highway_pause_throttle.test.js b/tests/js/highway_pause_throttle.test.js index bc5ffbc9..57baf518 100644 --- a/tests/js/highway_pause_throttle.test.js +++ b/tests/js/highway_pause_throttle.test.js @@ -114,13 +114,52 @@ test('the capability probe fails closed (absent / non-function / throwing)', () 'only an explicit true opts out — a truthy accident must not disable the throttle'); }); -test('3D highway claims continuous frames only while a crowd video is rolling', () => { +test('3D highway claims continuous frames for BOTH sources of venue motion', () => { const h3d = fs.readFileSync( path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'screen.js'), 'utf8'); const fn = extractBlock(h3d, 'needsContinuousFrames()'); + // (1) a crowd video rolling on its own clock (career venue pack) assert.match(fn, /_venueCrowdVideos/, 'must key off the actual crowd video elements'); - assert.match(fn, /\.paused/, 'a paused video is a still frame — throttle should still apply'); - // With no venue pack (the common case) the paused scene really is static and - // the GPU saving must survive: the method has to be able to return false. - assert.match(fn, /return false;/, 'must fall through to false with no live video'); + assert.match(fn, /\.paused/, 'a paused video is a still frame'); + // (2) the venue scene's OWN fake-depth motion — backdrop breathe, haze drift, + // warmth pulse, shimmer. Math.sin(t) in the draw loop, so it only moves while + // we get frames, and it runs with NO pack at all. Missing this meant the venue + // still stuttered on pause / count-in / credits whenever no video was rolling. + assert.match(fn, /_venueEffectiveMotionMode\s*\(\s*\)\s*!==\s*'off'/, + 'the venue scene animates without any video — it must claim frames too'); + // ...and with no venue at all the paused scene IS static: the #654 GPU saving + // must survive, so the method has to be able to return false. + assert.match(fn, /return false;/, 'must fall through to false on a plain 3D highway'); +}); + +// ── a SUPERSEDED init is not a FAILED init ────────────────────────────────── +// +// Starting a gig dropped the player onto the fallback 2D highway with no venue. +// +// setViz('venue') installs the 3D renderer, whose init is async; the gig then +// immediately starts its play queue, and playSong() re-initialises that same +// renderer a tick later. A renderer mints a fresh readyPromise per init() and +// rejects the previous one with "superseded" — but highway.js only checked that +// the RENDERER object was unchanged, which it is. So it treated a healthy +// re-initialising renderer as a failed one, tore it down, and reverted to 2D: +// +// renderer async init failure: Error: superseded +// viz picker: reverted to default renderer (async-init-failure) +// +// Reproduced and fixed against the real build (venue stays selected, scene +// active, no viz:reverted). + +test('a superseded readyPromise must not revert the viz to 2D', () => { + const src = highwaySources(); + const fn = extractBlock(src, 'function _handleAsyncInitFailure(e)'); + assert.match(fn, /readyPromise\s*!==\s*rp[\s\S]{0,40}return/, + 'a rejection from a STALE readyPromise (the renderer has since re-init\'d) must be ' + + 'ignored — otherwise a re-initialising renderer is torn down as if it had failed'); + // The renderer-identity check must survive too: a rejection belonging to a + // renderer that has since been REPLACED is also not our problem. + assert.match(fn, /hwState\._renderer\s*!==\s*_installedRenderer[\s\S]{0,20}return/, + 'the renderer-identity guard must remain'); + // ...and a genuine failure of the CURRENT init cycle must still revert. + assert.match(fn, /_emitVizReverted\s*\(\s*'async-init-failure'\s*\)/, + 'a real async-init failure must still fall back to the default renderer'); });