mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-13 04:09:26 +00:00
review: address PR #694 findings
- isVisible() forces a fresh DOM sample (was serving the rAF loop's
throttled cache, contradicting its 'live DOM check' docstring).
- v3 chrome: reconcile the edge-driven overControls hover flag against
matches(':hover') on the throttled ~6 Hz tick — covers a missed
mouseleave (flag stuck true, transport never hides) and a re-created
#player-controls node with lost listeners.
- highway_3d pre-warm now also covers teachFg/teachSd label textures
and the technique sprite factories (mute X, hammer/pull triangles,
bend chevrons, slide arrows) per active-palette string colour, plus
a maintenance note tying new label styles to the warm list.
- Document that the visibility throttle's manual invalidations are
latency-only (periodic resample self-heals within ~10 frames), and
why highway_3d keeps its local lowerBoundT (downlevel hosts).
External-repo audit (finding 1): staffview, tabview, piano, drums,
keys_highway_3d, drum_highway_3d grepped — no cross-frame bundle
retention or bundle-identity checks found.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
77547af110
commit
1e9741043b
@@ -1236,6 +1236,11 @@
|
|||||||
|
|
||||||
// Binary lower-bound: returns the first index i in arr where arr[i].t >= t.
|
// Binary lower-bound: returns the first index i in arr where arr[i].t >= t.
|
||||||
// Assumes arr is sorted ascending by .t (bundle.notes / bundle.chords always are).
|
// Assumes arr is sorted ascending by .t (bundle.notes / bundle.chords always are).
|
||||||
|
// Byte-identical to core's bundle.lowerBoundT — kept as a local because this
|
||||||
|
// plugin must run on downlevel hosts whose bundles don't carry the helper
|
||||||
|
// (it's called from ~30 sites incl. top-level helpers that don't receive a
|
||||||
|
// bundle). New code that already holds a bundle should prefer
|
||||||
|
// bundle.lowerBoundT / bundle.lowerBoundTime.
|
||||||
function lowerBoundT(arr, t) {
|
function lowerBoundT(arr, t) {
|
||||||
let lo = 0, hi = arr.length;
|
let lo = 0, hi = arr.length;
|
||||||
while (lo < hi) {
|
while (lo < hi) {
|
||||||
@@ -9928,16 +9933,47 @@
|
|||||||
if (mat && mat.map && ren) ren.initTexture(mat.map);
|
if (mat && mat.map && ren) ren.initTexture(mat.map);
|
||||||
}
|
}
|
||||||
function _prewarmStatic() {
|
function _prewarmStatic() {
|
||||||
|
// MAINTENANCE NOTE: this list must cover every deterministic
|
||||||
|
// (chart-independent) material/texture the per-frame paths can
|
||||||
|
// request lazily. Adding a new label style or sprite factory to
|
||||||
|
// drawNote()/update() without warming it here silently
|
||||||
|
// reintroduces a first-appearance texSubImage2D/compile spike
|
||||||
|
// mid-song. Chart-dependent labels (chord names, section names)
|
||||||
|
// live in _prewarmChart.
|
||||||
try {
|
try {
|
||||||
if (ren && scene && cam) ren.compile(scene, cam);
|
if (ren && scene && cam) ren.compile(scene, cam);
|
||||||
} catch (e) { console.warn('[3D-Hwy] prewarm compile:', e); }
|
} catch (e) { console.warn('[3D-Hwy] prewarm compile:', e); }
|
||||||
try {
|
try {
|
||||||
|
// Fret-number labels in the per-frame style/colour combos.
|
||||||
for (let f = 0; f <= NFRETS; f++) {
|
for (let f = 0; f <= NFRETS; f++) {
|
||||||
_prewarmTex(txtMat(f, FRET_LABEL_GOLD_HEX, false, 'noteFret'));
|
_prewarmTex(txtMat(f, FRET_LABEL_GOLD_HEX, false, 'noteFret'));
|
||||||
_prewarmTex(txtMat(f, FRET_LABEL_GOLD_HEX, false, 'fretRow'));
|
_prewarmTex(txtMat(f, FRET_LABEL_GOLD_HEX, false, 'fretRow'));
|
||||||
_prewarmTex(txtMat(f, FRET_LABEL_IDLE_HEX, false, 'fretRow'));
|
_prewarmTex(txtMat(f, FRET_LABEL_IDLE_HEX, false, 'fretRow'));
|
||||||
_prewarmTex(txtMat(f, '#ffffff', false, 'ghostFret'));
|
_prewarmTex(txtMat(f, '#ffffff', false, 'ghostFret'));
|
||||||
}
|
}
|
||||||
|
// Teaching marks (drawNote _drawTeachMark): finger hints
|
||||||
|
// T/1-4 (teachFg) and scale degrees 0-11 (teachSd).
|
||||||
|
_prewarmTex(txtMat('T', '#7fd1ff', false, 'teachFg'));
|
||||||
|
for (let i = 1; i <= 4; i++) _prewarmTex(txtMat(String(i), '#7fd1ff', false, 'teachFg'));
|
||||||
|
for (let i = 0; i <= 11; i++) _prewarmTex(txtMat(String(i), '#ffcc66', false, 'teachSd'));
|
||||||
|
// Technique sprite factories (own caches, keyed by packed
|
||||||
|
// number): PM/FH mute X, hammer/pull triangles, bend
|
||||||
|
// chevron stacks, slide direction arrows — per string
|
||||||
|
// colour of the active palette.
|
||||||
|
_prewarmTex(palmMuteXSpriteMat());
|
||||||
|
_prewarmTex(fretHandMuteXSpriteMat());
|
||||||
|
const _nWarm = Math.min(
|
||||||
|
Math.max(nStr, 6),
|
||||||
|
(activePalette && activePalette.length) || 0);
|
||||||
|
for (let s = 0; s < _nWarm; s++) {
|
||||||
|
const hex = activePalette[s] || 0xffffff;
|
||||||
|
_prewarmTex(triMat(true, hex));
|
||||||
|
_prewarmTex(triMat(false, hex));
|
||||||
|
for (let st = 1; st <= 4; st++) _prewarmTex(bendChevronMat(st, hex));
|
||||||
|
const arrowHex = darkenHex(hex, 0.55);
|
||||||
|
_prewarmTex(slideArrowMat(true, arrowHex));
|
||||||
|
_prewarmTex(slideArrowMat(false, arrowHex));
|
||||||
|
}
|
||||||
} catch (e) { console.warn('[3D-Hwy] prewarm labels:', e); }
|
} catch (e) { console.warn('[3D-Hwy] prewarm labels:', e); }
|
||||||
}
|
}
|
||||||
function _prewarmChart(bundle) {
|
function _prewarmChart(bundle) {
|
||||||
|
|||||||
@@ -132,6 +132,11 @@ function createHighway() {
|
|||||||
// Set _domVisSampledFrame to NaN to force a fresh sample on the next
|
// Set _domVisSampledFrame to NaN to force a fresh sample on the next
|
||||||
// check (done on init, canvas replace, resize, and override-clear so
|
// check (done on init, canvas replace, resize, and override-clear so
|
||||||
// deliberate transitions don't wait out the throttle window).
|
// deliberate transitions don't wait out the throttle window).
|
||||||
|
// NOTE those manual resets are LATENCY optimizations, not correctness
|
||||||
|
// requirements: the periodic re-sample runs every _DOM_VIS_CHECK_FRAMES
|
||||||
|
// frames regardless, so a visibility-affecting path that forgets to
|
||||||
|
// reset self-heals within ~10 frames — stale visibility can never be
|
||||||
|
// served indefinitely.
|
||||||
const _DOM_VIS_CHECK_FRAMES = 10;
|
const _DOM_VIS_CHECK_FRAMES = 10;
|
||||||
let _domVisCached = false;
|
let _domVisCached = false;
|
||||||
let _domVisSampledFrame = NaN;
|
let _domVisSampledFrame = NaN;
|
||||||
@@ -3845,6 +3850,12 @@ function createHighway() {
|
|||||||
// can call this once to sync their initial state — the event
|
// can call this once to sync their initial state — the event
|
||||||
// is transition-only and won't re-fire for late subscribers.
|
// is transition-only and won't re-fire for late subscribers.
|
||||||
isVisible() {
|
isVisible() {
|
||||||
|
// Force a fresh DOM sample — this is a documented "live DOM
|
||||||
|
// check" for late subscribers seeding initial state, so it
|
||||||
|
// must not serve the rAF loop's throttled cache (up to
|
||||||
|
// ~166 ms stale). Called rarely; the layout-read cost that
|
||||||
|
// motivated the throttle only matters per-frame.
|
||||||
|
_domVisSampledFrame = NaN;
|
||||||
return _isHighwayVisible();
|
return _isHighwayVisible();
|
||||||
},
|
},
|
||||||
getNotes() { return notes; },
|
getNotes() { return notes; },
|
||||||
|
|||||||
@@ -291,6 +291,16 @@
|
|||||||
// Re-sync the lyrics icon so programmatic highway.setLyricsVisible()
|
// Re-sync the lyrics icon so programmatic highway.setLyricsVisible()
|
||||||
// (e.g. from lyrics_karaoke) isn't left stale; cheap + idempotent.
|
// (e.g. from lyrics_karaoke) isn't left stale; cheap + idempotent.
|
||||||
syncLyricsIcon();
|
syncLyricsIcon();
|
||||||
|
// Reconcile the edge-driven hover flag against ground truth at
|
||||||
|
// this throttled cadence (~6 Hz, not per frame). Covers both
|
||||||
|
// failure modes of pure mouseenter/mouseleave tracking: a
|
||||||
|
// missed mouseleave (controls hidden/detached under the
|
||||||
|
// pointer → flag stuck true, transport never auto-hides) and
|
||||||
|
// a re-created #player-controls node whose listeners were
|
||||||
|
// lost (flag stuck false-ish / dead). matches(':hover') on a
|
||||||
|
// detached node is simply false, so this also self-clears.
|
||||||
|
const c = $('player-controls');
|
||||||
|
overControls = !!(c && typeof c.matches === 'function' && c.matches(':hover'));
|
||||||
}
|
}
|
||||||
tickIdle();
|
tickIdle();
|
||||||
rafId = requestAnimationFrame(loop);
|
rafId = requestAnimationFrame(loop);
|
||||||
|
|||||||
Reference in New Issue
Block a user