mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-17 14:08:22 +00:00
fix(highway): a SUPERSEDED renderer init is not a FAILED one (#970)
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.
This commit is contained in:
@@ -15398,15 +15398,29 @@
|
|||||||
// highway throttled the whole room. Pausing the song dropped the
|
// highway throttled the whole room. Pausing the song dropped the
|
||||||
// venue, the crowd and the stage to 10 fps.
|
// venue, the crowd and the stage to 10 fps.
|
||||||
//
|
//
|
||||||
// Only claim continuous frames while a crowd video is actually
|
// Two independent sources of motion, and BOTH must keep their frames:
|
||||||
// rolling: with no venue pack (the common case) the paused scene IS
|
//
|
||||||
// static and the throttle should still save the GPU.
|
// • 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() {
|
needsContinuousFrames() {
|
||||||
if (!_isReady || _ctxLost) return false;
|
if (!_isReady || _ctxLost) return false;
|
||||||
for (const v of _venueCrowdVideos) {
|
for (const v of _venueCrowdVideos) {
|
||||||
if (v && !v.paused && !v.ended && v.readyState >= 2) return true;
|
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) {
|
draw(bundle) {
|
||||||
|
|||||||
@@ -986,6 +986,22 @@ function createHighway() {
|
|||||||
// inline arrow function.
|
// inline arrow function.
|
||||||
function _handleAsyncInitFailure(e) {
|
function _handleAsyncInitFailure(e) {
|
||||||
if (hwState._renderer !== _installedRenderer) return;
|
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);
|
console.error('renderer async init failure:', e);
|
||||||
_destroyCurrentIfInited();
|
_destroyCurrentIfInited();
|
||||||
hwState._renderer = _defaultRenderer;
|
hwState._renderer = _defaultRenderer;
|
||||||
|
|||||||
@@ -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');
|
'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(
|
const h3d = fs.readFileSync(
|
||||||
path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'screen.js'), 'utf8');
|
path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'screen.js'), 'utf8');
|
||||||
const fn = extractBlock(h3d, 'needsContinuousFrames()');
|
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, /_venueCrowdVideos/, 'must key off the actual crowd video elements');
|
||||||
assert.match(fn, /\.paused/, 'a paused video is a still frame — throttle should still apply');
|
assert.match(fn, /\.paused/, 'a paused video is a still frame');
|
||||||
// With no venue pack (the common case) the paused scene really is static and
|
// (2) the venue scene's OWN fake-depth motion — backdrop breathe, haze drift,
|
||||||
// the GPU saving must survive: the method has to be able to return false.
|
// warmth pulse, shimmer. Math.sin(t) in the draw loop, so it only moves while
|
||||||
assert.match(fn, /return false;/, 'must fall through to false with no live video');
|
// 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');
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user