diff --git a/plugins/highway_3d/screen.js b/plugins/highway_3d/screen.js index 8d9ffe43..577726f9 100644 --- a/plugins/highway_3d/screen.js +++ b/plugins/highway_3d/screen.js @@ -15388,6 +15388,27 @@ }); }, + // The host throttles paused frames to ~10 fps, on the assumption + // that a paused chart is a static picture and re-rendering it is + // pure waste (highway-constants._PAUSED_FRAME_INTERVAL_MS). + // + // That stopped being true when the venue landed. The venue backdrop + // is a PLAYING VIDEO and the crowd reacts on its own clock, and they + // are drawn into this same canvas as the highway — so throttling the + // 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. + needsContinuousFrames() { + if (!_isReady || _ctxLost) return false; + for (const v of _venueCrowdVideos) { + if (v && !v.paused && !v.ended && v.readyState >= 2) return true; + } + return false; + }, + draw(bundle) { if (!_isReady) return; if (_ctxLost) return; // GPU context lost (alt-tab / reset) — skip until restored diff --git a/static/highway.js b/static/highway.js index 071e7d13..cc74ae45 100644 --- a/static/highway.js +++ b/static/highway.js @@ -1159,6 +1159,17 @@ function createHighway() { ' (user ' + hwState._renderScale.toFixed(2) + ' / auto ' + hwState._autoScale.toFixed(2) + ')'; } + // Optional renderer capability: "my picture keeps moving even when the chart + // clock is stopped". Anything a renderer animates on its own clock (the 3D + // highway's venue video + crowd) has to opt out of the paused-frame throttle + // or it renders at 10 fps while the song is paused. Absent / throwing = + // false, so every existing renderer keeps the throttle unchanged. + function _rendererNeedsContinuousFrames() { + const r = hwState._renderer; + if (!r || typeof r.needsContinuousFrames !== 'function') return false; + try { return r.needsContinuousFrames() === true; } catch (_) { return false; } + } + function draw() { hwState.animFrame = requestAnimationFrame(draw); if (!hwState.canvas || !hwState._renderer) return; @@ -1223,7 +1234,15 @@ function createHighway() { const _nowP = performance.now(); if (_nowP - hwState._chartLastAdvanceAt > _CHART_MAX_INTERP_MS) { _paused = true; - if (_nowP - hwState._lastPausedDrawAt < _PAUSED_FRAME_INTERVAL_MS) return; + // ...unless the renderer says its picture is NOT static while + // paused. The throttle assumes a paused chart is a still frame, + // but a renderer can own content on a clock of its own — the 3D + // highway draws the venue's video backdrop and its reactive crowd + // into this same canvas, so throttling the highway throttled the + // whole room to 10 fps whenever the song was paused. Optional + // method: renderers that don't implement it keep the throttle. + if (!_rendererNeedsContinuousFrames() + && _nowP - hwState._lastPausedDrawAt < _PAUSED_FRAME_INTERVAL_MS) return; hwState._lastPausedDrawAt = _nowP; } } diff --git a/static/v3/venue-crowd.js b/static/v3/venue-crowd.js index dea8789b..fbb2c5d1 100644 --- a/static/v3/venue-crowd.js +++ b/static/v3/venue-crowd.js @@ -133,6 +133,11 @@ let _lastStingerAt = -Infinity; let _prevStreak = 0; let _lastAccuracyPct = null; // from perf events; stats:recorded carries none + // Filename of the song song:loaded last reported. An arrangement switch + // re-emits song:loaded for the SAME file (changeArrangement reloads through + // the normal load path), and that must not be mistaken for arriving at the + // venue with a new song — see onSongLoaded. + let _lastSongFile = ''; let _bound = false; function now() { return Date.now(); } @@ -478,10 +483,40 @@ } } - function onSongLoaded() { + // song:loaded for the SAME file is an arrangement switch, not an arrival at + // the venue. changeArrangement() reloads through the normal load path, so + // the event is indistinguishable from a fresh load except by filename. + function isArrangementSwitch(prevFile, nextFile) { + return !!nextFile && nextFile === prevFile; + } + + function onSongLoaded(song) { + const file = String((song && song.filename) || ''); + const sameSong = isArrangementSwitch(_lastSongFile, file); + _lastSongFile = file; + machine.reset(); _prevStreak = 0; _lastAccuracyPct = null; + + // Switching arrangement is NOT arriving at the venue. + // + // changeArrangement() reloads the song through the same path as a fresh + // load, so highway.js emits song:loaded again — same filename, new + // arrangement. Treated as a new song, that replayed the arrival flyover: + // the camera flew in from the back of the room again mid-set, every time + // the player switched from lead to rhythm. The player is already on + // stage; the room should just carry on. + // + // So keep the video pipeline running and only re-sync the mood: the + // performance restarts, so the loop must follow the reset machine (a + // quiet crossfade), never the intro. + if (sameSong) { + if (_venueActive && _manifest && !_introActive) showLoop(machine.current, FADE_MS); + return; + } + + // A genuinely different song — full teardown. // Abort any stinger/pending state from the previous song: its ended // handler must not fade back into the old song's layers. cancelFade(); @@ -651,6 +686,7 @@ bindRuntime, getState, celebrate, + isArrangementSwitch, }; if (root) root.v3VenueCrowd = api; diff --git a/static/v3/venue-scene-3d.js b/static/v3/venue-scene-3d.js index 5dec46bb..dc81127c 100644 --- a/static/v3/venue-scene-3d.js +++ b/static/v3/venue-scene-3d.js @@ -18,6 +18,30 @@ let _lastMood = 'idle'; let _bound = false; + // The venue belongs to the SONG player and nowhere else. + // + // isVenueViz() only answers "is Venue the selected visualization" — a global + // preference. It says nothing about what is on screen. Other surfaces borrow + // the same highway_3d renderer (Virtuoso runs its practice charts on it), so + // with Venue selected they inherited the venue backdrop: the crowd and the + // stage showed up behind a chromatic exercise. The viz picker is a + // preference for the player; it is not a licence to paint the venue over + // whatever else happens to be using the renderer. + // + // So gate on both: Venue selected AND the player screen is the one showing. + function isPlayerScreen() { + try { + const active = document.querySelector('.screen.active'); + return !!active && active.id === 'player'; + } catch (_) { + return false; + } + } + + function shouldBeActive() { + return isVenueViz() && isPlayerScreen(); + } + function isVenueViz() { if (root && root.v3VenueViz && typeof root.v3VenueViz.isVenueVisualization === 'function') { const sel = root.v3VenueViz.getSelectedVizId @@ -146,7 +170,8 @@ function syncViz(vizId) { const id = String(vizId || ''); - if (id === 'venue') { + // Venue selected is necessary but not sufficient — see shouldBeActive. + if (id === 'venue' && isPlayerScreen()) { activate(); } else { deactivate(); @@ -192,12 +217,19 @@ if (_active) syncInstrumentPov(); }); sm.on('viz:renderer:ready', () => { - if (isVenueViz()) activate(); + if (shouldBeActive()) activate(); else deactivate(); }); sm.on('viz:reverted', () => deactivate()); + // Leaving the player tears the venue down; coming back rebuilds it. + // Without this the backdrop followed the renderer onto every other + // surface that borrows it (Virtuoso's practice highway). + sm.on('screen:changed', () => { + if (shouldBeActive()) activate(); + else deactivate(); + }); } - if (isVenueViz()) activate(); + if (shouldBeActive()) activate(); } function getState() { @@ -234,6 +266,8 @@ activate, deactivate, syncViz, + isPlayerScreen, + shouldBeActive, onAssetsLoaded, onAssetsFailed, onPerformanceState, diff --git a/tests/js/highway_pause_throttle.test.js b/tests/js/highway_pause_throttle.test.js index 99455d9a..bc5ffbc9 100644 --- a/tests/js/highway_pause_throttle.test.js +++ b/tests/js/highway_pause_throttle.test.js @@ -77,3 +77,50 @@ test('throttle runs after the ready gate, before bundle/draw', () => { assert.ok(readyIdx < throttleIdx, 'throttle must come after the ready gate'); assert.ok(throttleIdx < drawIdx, 'throttle must come before the renderer draw'); }); + +// ── The throttle must not starve a renderer that animates on its own clock ── +// +// The throttle assumes a paused chart is a still picture, so re-rendering it is +// waste. That stopped being true when the venue landed: the 3D highway draws the +// venue's VIDEO backdrop and its reactive crowd into the same canvas as the +// notes, so capping paused frames capped the whole room — pausing the song +// dropped the venue to ~10 fps ("everything around the highway drops fps"). +// +// Renderers now opt out via an optional needsContinuousFrames(). Absent or +// throwing must mean false, so every other renderer keeps the throttle. + +test('paused throttle defers to a renderer that needs continuous frames', () => { + const src = highwaySources(); + const fn = extractBlock(src, 'function draw()'); + assert.match(fn, /_rendererNeedsContinuousFrames\s*\(\s*\)/, + 'the paused throttle must consult the renderer capability'); + // The capability must GATE the early-return, not merely be called near it: + // the throttle only applies when the renderer does NOT need every frame. + assert.match( + fn, + /!\s*_rendererNeedsContinuousFrames\s*\(\s*\)[\s\S]{0,160}_PAUSED_FRAME_INTERVAL_MS[\s\S]{0,40}return;/, + 'throttle must be skipped when the renderer needs continuous frames', + ); +}); + +test('the capability probe fails closed (absent / non-function / throwing)', () => { + const src = highwaySources(); + const fn = extractBlock(src, 'function _rendererNeedsContinuousFrames()'); + assert.match(fn, /typeof\s+r\.needsContinuousFrames\s*!==\s*'function'[\s\S]{0,40}return false/, + 'a renderer without the method must keep the throttle'); + assert.match(fn, /catch[\s\S]{0,40}return false/, + 'a throwing renderer must keep the throttle, not crash the draw loop'); + assert.match(fn, /===\s*true/, + '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', () => { + const h3d = fs.readFileSync( + path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'screen.js'), 'utf8'); + const fn = extractBlock(h3d, 'needsContinuousFrames()'); + 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'); +}); diff --git a/tests/js/venue_scene_3d.test.js b/tests/js/venue_scene_3d.test.js index 69272d7b..b8f61fdd 100644 --- a/tests/js/venue_scene_3d.test.js +++ b/tests/js/venue_scene_3d.test.js @@ -208,7 +208,7 @@ test('index.html loads venue deps before venue-scene-3d', () => { assert.ok(vizIdx < moodIdx && moodIdx < sceneIdx); }); -test('syncViz activates only for venue visualization id', () => { +test('syncViz activates only for venue visualization id, and only on the player', () => { global.h3dVenueSceneSetActive = (on) => { global._h3dActive = on; }; global.h3dVenueSceneSetMood = (s) => { global._h3dMood = s; }; global.h3dVenueSceneSetInstrumentPov = () => {}; @@ -216,7 +216,14 @@ test('syncViz activates only for venue visualization id', () => { global.v3VenueViz = venueViz; global.v3VenueInstrumentPov = pov; global.feedBack = { on() {} }; + // The venue is scoped to the song player: selecting Venue is a preference + // for THAT screen, not a licence to paint the venue over anything else that + // borrows the highway_3d renderer (Virtuoso's practice charts did exactly + // that). syncViz therefore needs to know which screen is showing. + const onScreen = (id) => { global.document = { querySelector: (s) => (s === '.screen.active' && id ? { id } : null) }; }; + const prevDoc = global.document; try { + onScreen('player'); venueScene.deactivate(); venueScene.syncViz('highway_3d'); assert.equal(global._h3dActive, false); @@ -224,7 +231,16 @@ test('syncViz activates only for venue visualization id', () => { assert.equal(global._h3dActive, true); assert.equal(venueScene.getState().active, true); assert.equal(venueScene.getState().themeId, 'small-club'); + + // ...and the same call OFF the player must not activate it. + venueScene.deactivate(); + onScreen('virtuoso'); + venueScene.syncViz('venue'); + assert.equal(global._h3dActive, false, + 'Venue selected must NOT paint the venue onto the Virtuoso highway'); + assert.equal(venueScene.getState().active, false); } finally { + global.document = prevDoc; venueScene.deactivate(); delete global.h3dVenueSceneSetActive; delete global.h3dVenueSceneSetMood; diff --git a/tests/js/venue_scope.test.js b/tests/js/venue_scope.test.js new file mode 100644 index 00000000..af9564b3 --- /dev/null +++ b/tests/js/venue_scope.test.js @@ -0,0 +1,119 @@ +// Two venue bugs reported from a live career session. +// +// 1. Changing arrangement mid-song replayed the venue arrival flyover. The +// camera flew in from the back of the room again, every time the player +// switched lead -> rhythm. changeArrangement() reloads the song through the +// normal load path, so highway.js re-emits `song:loaded` — same filename, +// new arrangement — and the venue could not tell that from a fresh arrival. +// The player is already on stage; the room should just carry on. +// +// 2. With Venue selected, the venue backdrop showed up on the VIRTUOSO highway. +// The venue was gated purely on the viz selection, which is a global +// preference and says nothing about what is on screen. Virtuoso borrows the +// same highway_3d renderer for its practice charts, so it inherited the +// crowd and the stage behind a chromatic exercise. The venue belongs to the +// song player and nowhere else. + +'use strict'; + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); + +const crowd = require('../../static/v3/venue-crowd.js'); + +// ── 1. arrangement switch is not an arrival ──────────────────────────────── + +test('same filename = arrangement switch (no arrival flyover)', () => { + // changeArrangement() re-emits song:loaded for the song already on stage. + assert.equal(crowd.isArrangementSwitch('song.feedpak', 'song.feedpak'), true); +}); + +test('different filename = a genuinely new song (flyover is correct)', () => { + assert.equal(crowd.isArrangementSwitch('a.feedpak', 'b.feedpak'), false); +}); + +test('first load of the session is an arrival, not a switch', () => { + // No previous song -> the flyover must play. + assert.equal(crowd.isArrangementSwitch('', 'a.feedpak'), false); +}); + +test('a missing filename is never treated as a switch', () => { + // Otherwise a malformed payload would silently suppress the flyover for the + // rest of the session. + assert.equal(crowd.isArrangementSwitch('a.feedpak', ''), false); + assert.equal(crowd.isArrangementSwitch('a.feedpak', undefined), false); + assert.equal(crowd.isArrangementSwitch('', ''), false); +}); + +// ── 2. the venue belongs to the player screen ────────────────────────────── + +const scene = require('../../static/v3/venue-scene-3d.js'); + +// Venue MUST be the selected visualization for these to mean anything: if the +// viz were unset, shouldBeActive() would be false for the wrong reason and the +// virtuoso assertion below would pass vacuously. Force the viz on, so the only +// thing under test is the SCREEN gate. +function withScreen(id, fn) { + const prevDoc = global.document; + const prevViz = global.v3VenueViz; + global.v3VenueViz = { + isVenueVisualization: (v) => String(v) === 'venue', + getSelectedVizId: () => 'venue', + }; + global.document = { + querySelector(sel) { + if (sel !== '.screen.active') return null; + return id ? { id } : null; + }, + }; + try { return fn(); } finally { global.document = prevDoc; global.v3VenueViz = prevViz; } +} + +test('guard: with Venue selected AND on the player, the venue IS active', () => { + // If this ever fails, every "not active" test below is vacuous. + withScreen('player', () => { + assert.equal(scene.shouldBeActive(), true, + 'the screen gate must not break the normal case'); + }); +}); + +test('venue is active on the player screen', () => { + withScreen('player', () => { + assert.equal(scene.isPlayerScreen(), true); + }); +}); + +test('venue is NOT active on the virtuoso screen (the bug)', () => { + withScreen('virtuoso', () => { + assert.equal(scene.isPlayerScreen(), false, + 'Virtuoso borrows the same highway_3d renderer — the venue backdrop ' + + 'must not follow it there'); + assert.equal(scene.shouldBeActive(), false, + 'selecting Venue is a preference for the PLAYER; it is not a licence ' + + 'to paint the venue over whatever else is using the renderer'); + }); +}); + +test('venue is not active on any other screen either', () => { + for (const id of ['v3-home', 'plugin-folder_library', 'settings', 'career']) { + withScreen(id, () => { + assert.equal(scene.shouldBeActive(), false, `venue must not be active on ${id}`); + }); + } +}); + +test('no active screen at all is not the player', () => { + withScreen(null, () => { + assert.equal(scene.isPlayerScreen(), false); + }); +}); + +test('a throwing document does not take the venue down with it', () => { + const prev = global.document; + global.document = { querySelector() { throw new Error('detached'); } }; + try { + assert.equal(scene.isPlayerScreen(), false, 'must fail closed, not throw'); + } finally { + global.document = prev; + } +});