fix(venue/highway): flyover replay on arrangement switch, venue on Virtuoso, and the paused throttle starving the venue (#968)

* fix(venue): don't replay the flyover on an arrangement switch; keep the venue off other screens

Two bugs from a live career session.

1. CHANGING ARRANGEMENT REPLAYED THE ARRIVAL FLYOVER.

   changeArrangement() reloads the song through the normal load path, so
   highway.js re-emits `song:loaded` — same filename, new arrangement. The venue
   could not tell that from a fresh arrival, so it reset the machine and flew the
   camera in from the back of the room again, mid-set, every time the player
   switched lead -> rhythm. The player is already on stage.

   onSongLoaded now compares the filename. A repeat of the song already on stage
   keeps the video pipeline running and only re-syncs the mood: the performance
   restarts, so the loop follows the reset machine with a quiet crossfade, never
   the intro. A genuinely different song still gets the full teardown + flyover.

2. THE VENUE SHOWED UP ON THE VIRTUOSO HIGHWAY.

   The venue was gated purely on `isVenueViz()` — the selected visualization,
   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 with
   Venue selected it inherited the backdrop: the crowd and the stage behind a
   chromatic exercise.

   Selecting Venue is a preference for the PLAYER; it is not a licence to paint
   the venue over whatever else happens to be using the renderer. The venue is now
   gated on viz AND screen (`shouldBeActive`), and follows `screen:changed` — it
   tears down on leaving the player and rebuilds on return. Nothing else changes:
   stop() already unbinds the videos from the renderer, so deactivating is enough
   to clear the backdrop.

Tests: both decisions exposed as pure predicates and pinned — arrangement switch
vs new song (including the first load, and a malformed payload that must not
suppress the flyover forever), and the venue's screen scope. The existing syncViz
test encoded the OLD contract (activate regardless of screen), so it now states
the new one and additionally asserts the venue does NOT activate on virtuoso.

Includes a guard test: with Venue selected AND on the player, the venue IS
active — without it, every "not active" assertion could pass vacuously.

All 8 new/updated assertions fail against the pre-fix source. eslint clean;
JS 1199/1199; pytest 2597 passed.

* fix(highway): the paused-frame throttle was throttling the whole venue

Pausing the song dropped the venue, the crowd and the stage to ~10 fps —
"everything around the highway drops fps by a lot".

draw() caps paused frames to one per _PAUSED_FRAME_INTERVAL_MS (100ms), on an
assumption stated plainly in highway-constants.js: a heavy WebGL renderer "does
a full render every frame even while paused. That is pure waste." That was true
when a paused chart was a still picture.

The venue broke the assumption. Its video backdrop keeps playing and its crowd
reacts on a clock of their own, and BOTH are drawn into the same canvas as the
notes — so a throttle aimed at static notes throttled the entire room. The
scene only got a texture upload 10 times a second while the transport sat
paused.

Renderers can now declare that their picture is not static while the chart
clock is stopped: an optional needsContinuousFrames(). The throttle is skipped
only when it returns exactly true, and the probe fails closed — a renderer that
doesn't implement it, or one that throws, keeps the throttle unchanged. So the
GPU saving that motivated #654 survives everywhere it was actually valid.

highway_3d implements it and claims continuous frames ONLY while a crowd video
is genuinely rolling (bound, unpaused, not ended, readyState >= 2). With no
venue pack — the common case — the paused scene really is static, so it keeps
the throttle and the GPU still idles.

Tests extend tests/js/highway_pause_throttle.test.js, which guards this code
path source-level (the draw loop owns the rAF + WebGL lifecycle and is
deliberately not reproduced in a vm — see the file header). The new guards pin
that the capability GATES the early return rather than merely being called near
it, that the probe fails closed on absent/non-function/throwing/truthy-but-not-
true, and that the 3D renderer keys off the real video elements and can still
return false. All 3 fail against the pre-fix source.

eslint 0 errors; JS 1202/1202; pytest 2597 passed.
This commit is contained in:
Byron Gamatos
2026-07-14 22:11:52 +02:00
committed by GitHub
parent 8ef97708ef
commit 4e0e3c5417
7 changed files with 298 additions and 6 deletions
+20 -1
View File
@@ -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;
}
}
+37 -1
View File
@@ -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;
+37 -3
View File
@@ -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,