got-feedBack/feedBack
1
0
Fork 0
mirror of https://github.com/got-feedBack/feedBack.git synced 2026-07-21 12:21:49 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

fix(highway): stop stale viz frame bleeding through after switching visualizations (#565)

Browse Source 
Switching between the 3D drum highway (renders onto #highway) and the 3D
guitar highway (renders into its own .h3d-wrap overlay) left the previous
drum frame showing through the gap the overlay did not cover.

Core: _setRenderer now replaces #highway on a genuine viz change (keyed on
viz id via _rendererVizKey, not object identity, so benign same-viz
re-installs don't churn the canvas) as well as on a context-type change.

highway_3d: applySize pins the .h3d-wrap overlay to #highway's exact box,
derived from the same getBoundingClientRect measurements that size the
renderer (sub-pixel correct under zoom). Re-pins once the canvas lays out
(init race) and resets to the static anchor in the not-laid-out fallback.

Reviewed locally via codex (5 rounds, converged clean). CI checks are the
known org Actions billing block, not real failures.
This commit is contained in:
Byron Gamatos 2026-06-22 13:27:21 +02:00 committed by GitHub
parent 5145710a8a
commit 187d0bb978
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 299 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
plugins/highway_3d/screen.js
View File

@ -2703,6 +2703,14 @@
// that CSS-box drift and re-frame, instead of the user having to
// un/re-maximize the window.
let _appliedW = 0, _appliedH = 0;
// True once applySize() has pinned the .h3d-wrap overlay to the
// highway canvas's offset box. Stays false while the canvas has no
// layout yet (init() can run before #highway has a real box, where
// applySize falls back to the parent-panel size and only sets the
// wrap height). The rAF loop re-pins once the canvas lays out even
// when the logical render size is unchanged — otherwise the overlay
// would stay at top:0;left:0;right:0 and expose a strip of #highway.
let _wrapPinned = false;
let mBeatM = null, mBeatQ = null;
let txtCache = {};
// Cloned sprite materials cached on individual sprite instances
@ -12941,7 +12949,55 @@
const baseDPR = _ssActive() ? Math.min(devicePixelRatio, 1.25) : Math.min(devicePixelRatio, 2);
ren.setPixelRatio(_renderScale * baseDPR);
ren.setSize(w, h);
wrap.style.height = h + 'px';
// Pin the overlay to #highway's exact box so it fully covers the
// canvas. The wrap is anchored to top:0/left:0/right:0 of its
// offset parent, which only lines up with #highway when the
// canvas sits at the parent's origin. The v3 player can place
// chrome above the canvas, shifting the wrap up so its lower edge
// falls short of #highway — leaving a strip of the canvas exposed
// (the reported gap, where the previous renderer's frame showed
// through). The wrap is a sibling of highwayCanvas, so they share
// an offset parent; tracking the canvas's box keeps the overlay
// flush in single-player and splitscreen alike.
//
// Derive the box from the SAME getBoundingClientRect measurements
// that drive ren.setSize(w, h) — NOT integer offsetTop/Width — so
// the overlay matches the renderer exactly. Under browser zoom or
// fractional flex layouts the canvas lands on sub-pixel bounds;
// offsetWidth/Top round to whole pixels and would leave the wrap up
// to 1px short of (or shifted from) the canvas, reopening the
// exposed edge strip. Position is taken relative to the containing
// block's padding edge (clientTop/Left strip the parent's border),
// which is what `top`/`left` resolve against for the absolutely
// positioned wrap. Guarded on a laid-out canvas (offsetWidth/Height
// > 0); otherwise fall back to the static top:0/left:0/right:0.
if (highwayCanvas && highwayCanvas.offsetWidth > 0 && highwayCanvas.offsetHeight > 0) {
const _pinParent = wrap.offsetParent || highwayCanvas.parentNode;
const _cr = highwayCanvas.getBoundingClientRect();
const _pr = _pinParent ? _pinParent.getBoundingClientRect() : { top: 0, left: 0 };
const _pbTop = _pinParent ? _pinParent.clientTop : 0;
const _pbLeft = _pinParent ? _pinParent.clientLeft : 0;
wrap.style.top = (_cr.top - _pr.top - _pbTop) + 'px';
wrap.style.left = (_cr.left - _pr.left - _pbLeft) + 'px';
wrap.style.right = 'auto';
wrap.style.width = _cr.width + 'px';
wrap.style.height = _cr.height + 'px';
_wrapPinned = true;
} else {
// Canvas not laid out (e.g. init ran before #highway had a real
// box, or a panel hide/show where canvasSize() falls back to the
// parent panel). Reset to the static anchor — if we had pinned
// before, the old top/left/right:auto/width would otherwise stay
// and the wrap would reappear at a stale horizontal position on
// the next show. Leave _wrapPinned false so the rAF loop re-pins
// once the canvas materializes again.
wrap.style.top = '0';
wrap.style.left = '0';
wrap.style.right = '0';
wrap.style.width = 'auto';
wrap.style.height = h + 'px';
_wrapPinned = false;
}
if (lyricsCanvas) { lyricsCanvas.width = w; lyricsCanvas.height = h; }
_diagRenderCache.clear();
cam.aspect = w / h;
@ -13320,6 +13376,17 @@
} else if (box.w > 0 && box.h > 0 &&
(Math.abs(box.w - _appliedW) > 1 || Math.abs(box.h - _appliedH) > 1)) {
applySize(box.w, box.h);
} else if (!_wrapPinned && box.w > 0 && box.h > 0 &&
highwayCanvas.offsetWidth > 0 && highwayCanvas.offsetHeight > 0) {
// 3. The overlay pin couldn't be applied at init because
// #highway had no layout yet (offsetWidth/Height === 0),
// so applySize() only set the wrap height. The canvas has
// now laid out but to the same logical size, so neither
// drift branch above fires — re-run applySize to pin the
// wrap to the canvas box now that its offsets are real.
// Otherwise the overlay stays at top:0;left:0;right:0 and
// a strip of #highway is exposed on first load / split.
applySize(box.w, box.h);
}
}
update(bundle);
@ -13497,6 +13564,7 @@
_destroyed = true; _isReady = false; _diagChord = null; _diagPrev = null; _diagLastKey = null; _diagRenderCache.clear();
_lastHwW = 0; _lastHwH = 0;
_appliedW = 0; _appliedH = 0;
_wrapPinned = false;
_unsubscribeFocus(); teardown();
highwayCanvas = null;
},

46
static/highway.js
View File

@ -973,6 +973,23 @@ function createHighway() {
return '2d';
}
// Logical identity of a renderer for swap detection: its viz id, not
// its object reference. app.js stamps `pluginId`/`source` (the viz
// picker id) onto every renderer it installs via _tagVizRenderer, and
// setViz()/_autoMatchViz() build a FRESH renderer object with factory()
// on every (re)apply — even when the selected viz did not change (e.g.
// Auto re-resolving to the same viz across consecutive songs). Keying
// the canvas-replacement decision on object identity would therefore
// treat those benign re-installs as a swap and needlessly clone
// #highway, dropping listeners/expando state attached to the element.
// The default renderer has no id, so it keys on its own singleton; an
// untagged custom renderer falls back to its object reference (the safe
// "treat as a swap" answer).
function _rendererVizKey(r) {
if (r === _defaultRenderer) return _defaultRenderer;
return (r && (r.pluginId || r.source)) || r;
}
// Replace the underlying <canvas> element with a fresh one because
// the next renderer needs a different context type than the current
// canvas is locked to. Preserves the DOM position, id, classes,
@ -1038,6 +1055,11 @@ function createHighway() {
}
function _setRenderer(r) {
// Capture the outgoing renderer before _destroyCurrentIfInited /
// the `_renderer = next` assignment below overwrite it — the
// canvas-replacement decision needs to know whether we're
// actually swapping to a *different* renderer instance.
const prev = _renderer;
_destroyCurrentIfInited();
// null/undefined reverts to default. Anything else must provide
// at minimum a draw(bundle) function — without it the rAF loop
@ -1070,7 +1092,29 @@ function createHighway() {
// destroyed by _destroyCurrentIfInited above, so it's safe to
// detach the element from the DOM here.
const nextType = _resolveRendererContextType(next);
if (nextType !== _currentCanvasContextType) {
// Replace the underlying <canvas> when (a) the new renderer needs
// a different context type than the one currently bound, OR (b)
// we're swapping to a *different* visualization while keeping the
// same context type. Case (b) prevents a stale frame from the
// previous renderer bleeding through: renderers that draw into a
// sibling overlay (e.g. 3D Highway's `.h3d-wrap`) never paint
// #highway, so whatever the *previous* renderer left on the shared
// canvas would otherwise stay visible in any area the overlay does
// not cover. The reported symptom was the 3D drum highway (which
// renders directly onto #highway) showing through the gap below
// the 3D guitar highway's overlay after switching between them —
// both are webgl2, so the type-change check alone never fired. A
// fresh canvas starts blank/transparent, so the incoming renderer
// always begins over a clean surface. The swap test keys on the
// viz id (_rendererVizKey), NOT object identity: setViz/_autoMatchViz
// build a fresh renderer object on every apply, so an object-identity
// check would clone #highway on every benign same-viz re-install
// (e.g. Auto re-resolving to the same viz across songs) and drop the
// element's listeners/expando state. Skipped when re-installing the
// same viz, and on the very first install (prev === null) where
// there is no prior frame to clear.
const _vizChanged = prev && _rendererVizKey(next) !== _rendererVizKey(prev);
if (nextType !== _currentCanvasContextType || _vizChanged) {
_replaceCanvas(nextType);
}
const bundle = _makeBundle();

94
tests/js/highway_3d_overlay_full_cover.test.js Normal file
View File

@ -0,0 +1,94 @@
// Source-level guard for the 3D Highway overlay fully covering #highway.
//
// The `.h3d-wrap` overlay is anchored to top:0/left:0/right:0 of its offset
// parent, which only lines up with #highway when the canvas sits at the
// parent's origin. The v3 player can place chrome above the canvas, shifting
// the wrap up so its lower edge falls short of #highway and exposes a strip
// of the canvas (the reported gap). applySize() must pin the wrap to the
// canvas's actual offset box so it stays flush. createHighway's WebGL
// lifecycle is too heavy for a vm sandbox, so this locks in the wiring.
const { test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const screenJs = path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'screen.js');
function extractBlock(src, signature) {
const start = src.indexOf(signature);
assert.ok(start !== -1, `signature '${signature}' not found`);
const openBrace = src.indexOf('{', start);
let depth = 1;
let i = openBrace + 1;
while (i < src.length && depth > 0) {
const ch = src[i];
if (ch === '{') depth++;
else if (ch === '}') depth--;
i++;
}
assert.ok(depth === 0, `unbalanced braces after '${signature}'`);
return src.slice(start, i);
}
test('applySize pins the .h3d-wrap overlay to the highway canvas rect box', () => {
const src = fs.readFileSync(screenJs, 'utf8');
const fn = extractBlock(src, 'function applySize(w, h)');
// Guarded on a laid-out canvas so we never pin to a zero box.
assert.match(
fn,
/highwayCanvas\s*&&\s*highwayCanvas\.offsetWidth\s*>\s*0\s*&&\s*highwayCanvas\.offsetHeight\s*>\s*0/,
'must guard the pin on a laid-out canvas (offsetWidth/Height > 0)',
);
// Size/position must come from getBoundingClientRect (fractional, matches
// ren.setSize), NOT integer offset* props which round and reopen the strip.
assert.match(fn, /highwayCanvas\.getBoundingClientRect\(\)/, 'must measure the canvas via getBoundingClientRect');
assert.doesNotMatch(fn, /wrap\.style\.width\s*=\s*highwayCanvas\.offsetWidth/, 'must NOT size to integer offsetWidth');
assert.doesNotMatch(fn, /wrap\.style\.height\s*=\s*highwayCanvas\.offsetHeight/, 'must NOT size to integer offsetHeight');
// Width/height set from the rect; position is parent-relative (padding edge).
assert.match(fn, /wrap\.style\.width\s*=\s*_cr\.width/, 'must size width to the canvas rect width');
assert.match(fn, /wrap\.style\.height\s*=\s*_cr\.height/, 'must size height to the canvas rect height');
assert.match(fn, /wrap\.style\.top\s*=\s*\(\s*_cr\.top\s*-\s*_pr\.top\s*-\s*_pbTop\s*\)/, 'top must be canvas rect relative to the containing block padding edge');
assert.match(fn, /wrap\.style\.left\s*=\s*\(\s*_cr\.left\s*-\s*_pr\.left\s*-\s*_pbLeft\s*\)/, 'left must be canvas rect relative to the containing block padding edge');
assert.match(fn, /clientTop/, 'must strip the parent border via clientTop');
// right:0 must be released so the explicit width takes effect.
assert.match(fn, /wrap\.style\.right\s*=\s*['"]auto['"]/, "must release right:0 (set 'auto') when pinning width");
});
test('applySize fallback resets the static anchor and the computed height', () => {
const src = fs.readFileSync(screenJs, 'utf8');
const fn = extractBlock(src, 'function applySize(w, h)');
// The not-laid-out fallback must clear any stale pin styles (a prior pin
// leaves top/left/right:auto/width set) back to the original
// top:0;left:0;right:0;width:auto anchor, or the wrap reappears at a
// stale horizontal position after a panel hide/show.
const fallback = fn.slice(fn.indexOf('} else {'));
assert.match(fallback, /wrap\.style\.top\s*=\s*['"]0['"]/, 'fallback must reset top:0');
assert.match(fallback, /wrap\.style\.left\s*=\s*['"]0['"]/, 'fallback must reset left:0');
assert.match(fallback, /wrap\.style\.right\s*=\s*['"]0['"]/, 'fallback must reset right:0');
assert.match(fallback, /wrap\.style\.width\s*=\s*['"]auto['"]/, 'fallback must reset width:auto');
assert.match(fallback, /wrap\.style\.height\s*=\s*h\s*\+\s*['"]px['"]/, 'fallback must keep the computed height');
});
test('applySize records whether the overlay pin was applied (_wrapPinned)', () => {
const src = fs.readFileSync(screenJs, 'utf8');
const fn = extractBlock(src, 'function applySize(w, h)');
// Pin path sets the flag true; the not-laid-out fallback sets it false
// so the rAF loop knows the pin is still pending.
assert.match(fn, /_wrapPinned\s*=\s*true/, 'pin path must set _wrapPinned = true');
assert.match(fn, /_wrapPinned\s*=\s*false/, 'fallback path must set _wrapPinned = false');
});
test('the rAF loop re-pins the overlay once the canvas lays out (Codex P1)', () => {
// When init() pins via the parent-panel fallback (offset box still 0) and
// the canvas later lays out to the SAME logical size, neither size-drift
// branch fires. A dedicated branch must re-run applySize so the overlay
// gets pinned to the now-real canvas box instead of leaving the exposed
// strip the fix was meant to close.
const src = fs.readFileSync(screenJs, 'utf8');
assert.match(
src,
/else if\s*\(\s*!_wrapPinned\s*&&\s*box\.w\s*>\s*0\s*&&\s*box\.h\s*>\s*0\s*&&\s*highwayCanvas\.offsetWidth\s*>\s*0\s*&&\s*highwayCanvas\.offsetHeight\s*>\s*0\s*\)\s*\{\s*[\s\S]*?applySize\(\s*box\.w\s*,\s*box\.h\s*\)\s*;/,
'must re-pin via applySize when !_wrapPinned and the canvas has laid out',
);
});

91
tests/js/highway_renderer_swap_canvas_reset.test.js Normal file
View File

@ -0,0 +1,91 @@
// Source-level guard for the renderer-swap canvas reset.
//
// Both 3D visualizations are webgl2 renderers, but they paint to
// different surfaces: the 3D *drum* highway renders directly onto the
// shared #highway canvas, while the 3D *guitar* highway renders into its
// own `.h3d-wrap` sibling overlay and never touches #highway. Switching
// drum -> guitar is webgl2 -> webgl2, so the context-type check alone
// never replaced the canvas — the last drum frame stayed painted on
// #highway and bled through the gap the guitar overlay does not cover.
//
// The fix replaces the underlying <canvas> on ANY swap to a different
// renderer instance (not just on a context-type change) so the incoming
// renderer always starts over a blank surface. These checks lock in the
// wiring; the createHighway closure owns a WebGL lifecycle too heavy to
// reproduce in a vm sandbox.
const { test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const highwayJs = path.join(__dirname, '..', '..', 'static', 'highway.js');
function extractBlock(src, signature) {
const start = src.indexOf(signature);
assert.ok(start !== -1, `signature '${signature}' not found`);
const openBrace = src.indexOf('{', start);
assert.ok(openBrace !== -1, `opening brace after '${signature}' not found`);
let depth = 1;
let i = openBrace + 1;
while (i < src.length && depth > 0) {
const ch = src[i];
if (ch === '{') depth++;
else if (ch === '}') depth--;
i++;
}
assert.ok(depth === 0, `unbalanced braces after '${signature}'`);
return src.slice(start, i);
}
test('_setRenderer captures the outgoing renderer before overwriting it', () => {
const src = fs.readFileSync(highwayJs, 'utf8');
const fn = extractBlock(src, 'function _setRenderer(r)');
// prev must be captured BEFORE _destroyCurrentIfInited and the
// `_renderer = next` assignment, otherwise the swap detection below
// would always compare next against itself.
const prevIdx = fn.search(/const\s+prev\s*=\s*_renderer/);
const destroyIdx = fn.search(/_destroyCurrentIfInited\(\)/);
const assignIdx = fn.search(/^\s*_renderer\s*=\s*next\s*;/m);
assert.ok(prevIdx !== -1, 'must capture `const prev = _renderer`');
assert.ok(destroyIdx !== -1, 'must call _destroyCurrentIfInited');
assert.ok(assignIdx !== -1, 'must assign `_renderer = next`');
assert.ok(prevIdx < destroyIdx, 'prev must be captured before _destroyCurrentIfInited');
assert.ok(prevIdx < assignIdx, 'prev must be captured before `_renderer = next`');
});
test('_setRenderer replaces the canvas on a context-type change OR a viz change', () => {
const src = fs.readFileSync(highwayJs, 'utf8');
const fn = extractBlock(src, 'function _setRenderer(r)');
// The replace guard must fire on EITHER a context-type change OR a
// swap to a different visualization. A regression that drops the
// viz-change clause would let a stale frame bleed through.
assert.match(
fn,
/const\s+_vizChanged\s*=\s*prev\s*&&\s*_rendererVizKey\(next\)\s*!==\s*_rendererVizKey\(prev\)/,
'_vizChanged must compare _rendererVizKey(next) vs _rendererVizKey(prev), guarded by prev',
);
assert.match(
fn,
/if\s*\(\s*nextType\s*!==\s*_currentCanvasContextType\s*\|\|\s*_vizChanged\s*\)\s*\{\s*_replaceCanvas\(nextType\)/,
'replace guard must be `nextType !== _currentCanvasContextType || _vizChanged`',
);
});
test('_rendererVizKey keys on the viz id, not object identity (avoids churn on same-viz re-install)', () => {
const src = fs.readFileSync(highwayJs, 'utf8');
const fn = extractBlock(src, 'function _rendererVizKey(r)');
// Default renderer keys on its singleton; custom renderers key on the
// viz picker id (pluginId/source) stamped by app.js's _tagVizRenderer,
// falling back to the object reference only when untagged.
assert.match(fn, /r\s*===\s*_defaultRenderer/, 'default renderer must key on its own singleton');
assert.match(fn, /r\.pluginId\s*\|\|\s*r\.source/, 'custom renderers must key on pluginId/source (the viz id)');
});
test('_setRenderer skips the extra replace on first install (prev === null)', () => {
const src = fs.readFileSync(highwayJs, 'utf8');
const fn = extractBlock(src, 'function _setRenderer(r)');
// The `prev &&` guard avoids a needless swap on the very first install
// (prev === null) where there is no prior frame to clear.
assert.match(fn, /_vizChanged\s*=\s*prev\s*&&/, 'must short-circuit _vizChanged when prev is null');
});