mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-10-02 19:21:50 +00:00
fix(v3): recycle library grid cards on scroll instead of rebuilding the window
The virtualized v3 Songs grid rebuilt its entire visible window (grid.innerHTML = _renderCardsRange(...) + a full wireCards pass) every time it slid by one row. Each row-boundary crossing was therefore a heavy synchronous frame — reparse ~60 cards, re-attach hundreds of listeners, reflow — that stalled the main thread and buffered held-arrow key-repeats, flushing them in a burst. Testers saw the library "go super fast for a second then slow down," skipping "every so many scrolls," up or down, at the same spots each time. It hitched scrolling back up over already-loaded songs too, because the cost was DOM teardown, not fetching. renderWindow() now reconciles the window in place: it reuses the card nodes that stay on-screen and builds only the row that enters/leaves (~6 nodes per slide instead of ~60). Nodes are keyed by absolute index (data-idx) with a real-vs-skeleton + select-mode signature (data-sig) so hole-fills after a page fetch and select-mode toggles still rebuild exactly the nodes that changed. wireCards()'s existing data-wired guard then wires only the freshly-built nodes, so per-slide listener churn drops with it. Everything keyed off data-fn (favorites, ⋮ menu, right-click, selection, accuracy badges, A–Z rail) is unaffected. Follow-up to the stage-2 virtualized grid (#636 item 3). Frontend-only. Tests: tests/js/v3_songs_window_recycle.test.js — window stays [start,end) contiguous and in-window node identity is reused across a down-then-up scroll; select-mode toggle and a rail-seek jump rebuild correctly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
f5c9c34291
commit
815b03eae7
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- **Player frame-time hotspots removed (trace-backed) + weak-hardware hardening.** A Chrome performance trace of a 3D-highway session surfaced two core per-frame layout-thrash sources, now fixed: the highway's visibility check read `canvas.offsetParent` every rAF frame (forces style/layout recalc — now sampled every 10th frame with a cached value, force-refreshed on init/canvas-replace/resize/override-clear), and the v3 player chrome loop called `matches(':hover')` per frame and unconditionally rewrote the Up-Next pill's `textContent`/bar width at 6 Hz (now hover-tracked via mouseenter/mouseleave, DOM writes only on value change, progress bar moved from `width` to compositor-only `scaleX`). The 3D highway pre-warms shader programs (`ren.compile`) and deterministic label textures at init — and chart-dependent chord/section label textures on first draw — so first-appearance shader-compile/texture-upload frame spikes move into the load spinner. For weaker hardware: the per-frame renderer bundle is now a single reused object instead of a fresh ~35-field allocation per frame (object identity is stable and meaningless; array fields still swap reference on chart changes), custom viz get `bundle.lowerBoundT`/`bundle.lowerBoundTime` binary-search helpers for visible-window culling, the default 2D highway's beat lines no longer scan every beat in the song per frame, and the 3D highway stops reading `localStorage` per frame (1 Hz poll) and caches its lyrics text-measurement layout per displayed line instead of re-measuring every syllable every frame. A second, throttled-CPU trace pass additionally removed: shader-program re-resolution churn from label texture swaps (`material.needsUpdate` is now only set on a null↔texture transition — swapping between two cached label textures never changes the compiled program), the 3D highway's per-frame `getBoundingClientRect` layout read in its canvas-size self-check (now every 10th frame, still immediate on backing-store change), and the core 60 Hz HUD clock rewriting `textContent` on every tick (now write-on-change, ~1/s). The dominant residual — steady `getParameters` shader-program re-resolution (~4% of throttled main thread) — turned out to be Three r158+'s transparent-DoubleSide two-pass rendering, which sets `material.needsUpdate` twice per object per frame; all 18 of the 3D highway's transparent DoubleSide materials are flat unlit quads (labels, rails, chord frames, lanes), so they now declare `forceSinglePass: true`, eliminating the recompile churn and halving those objects' draw calls.
|
||||
|
||||
### Fixed
|
||||
- **v3 Songs grid: fixed the scroll stutter that "skips every so many scrolls," up or down.** The virtualized grid rebuilt its **entire** visible window (`grid.innerHTML = …` + a full `wireCards` pass) every time it slid by one row, so each row-boundary crossing was a heavy synchronous frame that stalled the main thread and buffered held-arrow key-repeats into a visible lurch (a tester's "super fast for a second then slowed down") at fixed scroll offsets — in **both directions and regardless of whether the page was already loaded** (the cost was DOM teardown, not fetching, which is why scrolling back up over cached songs hitched too). `renderWindow()` now **reconciles the window in place**: it reuses the card nodes that stay on-screen and builds only the row that enters/leaves (~6 nodes per slide instead of ~60), keyed by absolute index with a real-vs-skeleton + select-mode signature so hole-fills (after a page fetch) and select-mode toggles still rebuild exactly the nodes that changed. `wireCards`'s `data-wired` guard then wires only the freshly-built nodes, so per-slide listener churn drops with it. Follow-up to the stage-2 virtualized grid (got-feedback/feedBack#636 item 3). Frontend-only: `static/v3/songs.js`. Tests: `tests/js/v3_songs_window_recycle.test.js` (window stays `[start,end)` contiguous + in-window node identity reused across a down-then-up scroll; select-mode toggle and rail-seek jump rebuild correctly).
|
||||
- **Edit Metadata now writes into `.feedpak` files, not just legacy `.sloppak` ones.** `lib/songmeta.py`'s suffix gate predated the format rename — core reads both suffixes everywhere else (`sloppak.SONG_EXTS`), but the metadata writer only dispatched on `.sloppak`, so editing a zip-form `.feedpak`'s title/artist/album/year silently fell back to a DB-only update. That looked fine until the next **full library rescan** re-derived metadata from the file and reverted the edit (directory-form packages were unaffected — they dispatch on manifest presence, not suffix). The gate now accepts both package suffixes. Tests: `tests/test_songmeta.py` `TestWriteSongMetadata` (both zip suffixes, mixed-case suffix, directory form, unknown-suffix fallback).
|
||||
- **3D Drum & Keys highways now re-frame on fullscreen/layout drift under splitscreen.** The guitar/bass `highway_3d` self-detects when its panel canvas changes size and re-runs `applySize()` every frame, because the splitscreen host overrides `hw.resize` and never calls `renderer.resize()`. The drum and keys highways lacked that fallback — they only re-framed when the host explicitly called `resize(w, h)` — so their panels stayed framed for the pre-fullscreen size while the guitar/bass panels adapted (visible as a too-small, off-center highway after maximizing a split-screen session). Both draw loops now port `highway_3d`'s per-frame drift check: they re-apply on backing-store change (`canvas.width/height`) AND on CSS-box drift (`clientWidth/clientHeight` vs the last applied logical size, throttled to every 10th frame), and reset the tracking in `destroy()` so a reused instance re-frames on the next song. `plugins/drum_highway_3d` → 0.3.1, `plugins/keys_highway_3d` → 0.1.1. Tests: `tests/js/drum_keys_highway_3d_resize_reframe.test.js`.
|
||||
- **Tuner: finished the "remove unused settings" cleanup and fixed the sidebar panel position.** The Floating Button and Tuning Visibility settings sections were removed, but their config was still live: `disabledTunings` still filtered the tuner menu (with no UI left to re-enable a hidden tuning — a one-way trap) and `showFloatingButton` still gated the floating launcher. Both are now fully retired — the enforcement paths in `plugins/tuner/screen.js`/`utils/ui.js` and the persistence in `plugins/tuner/routes.py` are gone (and `routes.py` strips the retired keys on write, so stale values are purged). The tuner panel opened from the v3 sidebar Plugins rail popover now anchors beside it via the host's stable plugin-control slot API (falling back to the popover id), is **clamped to the viewport** so it can't open off the right/bottom edge on narrow/short windows, and re-anchors on window resize. `plugins/tuner` → 1.3.3.
|
||||
|
||||
+50
-6
@@ -1759,13 +1759,57 @@
|
||||
'</div>';
|
||||
}
|
||||
|
||||
function _renderCardsRange(start, end) {
|
||||
let html = '';
|
||||
// Signature of the card at absolute index i: real-card vs skeleton, plus the
|
||||
// select-mode it was built under. A change here is the ONLY reason a recycled
|
||||
// node must be rebuilt (a hole filled after a fetch, or select mode toggled) —
|
||||
// otherwise the node is reused as-is across window slides.
|
||||
function _cardSig(i) {
|
||||
return (state.songs[i] ? 'r' : 's') + (state.selectMode ? '1' : '0');
|
||||
}
|
||||
|
||||
function _buildCardNode(i) {
|
||||
const s = state.songs[i];
|
||||
const tmp = document.createElement('div');
|
||||
tmp.innerHTML = s ? songCard(s) : _skeletonCard();
|
||||
const node = tmp.firstElementChild;
|
||||
node.setAttribute('data-idx', String(i));
|
||||
node.setAttribute('data-sig', _cardSig(i));
|
||||
return node;
|
||||
}
|
||||
|
||||
// Reconcile the grid's children to exactly cover [start, end) in ascending
|
||||
// index order, REUSING the card nodes that stay in-window. Sliding the window
|
||||
// one row now mutates only the row that entered/left instead of tearing down +
|
||||
// rebuilding (+ re-wiring) the whole ~60-card window every frame — that
|
||||
// per-slide teardown was the main-thread stall behind the "library skips every
|
||||
// so many scrolls, up or down" report (the stall buffers held-arrow key-repeats
|
||||
// that then flush in a burst). wireCards()'s data-wired guard wires only the
|
||||
// freshly-built nodes.
|
||||
function _syncWindow(grid, start, end) {
|
||||
// Pass 1: drop nodes that left the window, are untagged, or whose content
|
||||
// signature is stale (skeleton→real, or select-mode toggled). What remains
|
||||
// is a reusable, correctly-rendered subset in ascending DOM order.
|
||||
for (const el of Array.from(grid.children)) {
|
||||
const a = el.getAttribute('data-idx');
|
||||
const idx = a == null ? NaN : Number(a);
|
||||
if (!(idx >= start && idx < end) || el.getAttribute('data-sig') !== _cardSig(idx)) {
|
||||
el.remove();
|
||||
}
|
||||
}
|
||||
// Pass 2: walk [start, end) in order, reusing survivors and inserting new
|
||||
// nodes into their correct slot; `ref` tracks the child expected next.
|
||||
const existing = new Map();
|
||||
for (const el of grid.children) existing.set(Number(el.getAttribute('data-idx')), el);
|
||||
let ref = grid.firstChild;
|
||||
for (let i = start; i < end; i++) {
|
||||
const s = state.songs[i];
|
||||
html += s ? songCard(s) : _skeletonCard();
|
||||
let node = existing.get(i);
|
||||
if (!node) node = _buildCardNode(i);
|
||||
if (node === ref) {
|
||||
ref = ref.nextSibling;
|
||||
} else {
|
||||
grid.insertBefore(node, ref);
|
||||
}
|
||||
}
|
||||
return html;
|
||||
}
|
||||
|
||||
// Fetch a single OFFSET page into the sparse store. Uses the stage-1 keyset
|
||||
@@ -1861,7 +1905,7 @@
|
||||
}
|
||||
if (_closeCardMenu) _closeCardMenu(); // its DOM is about to be replaced
|
||||
grid.style.top = (firstRow * rowH) + 'px';
|
||||
grid.innerHTML = _renderCardsRange(start, end);
|
||||
_syncWindow(grid, start, end); // recycle in-window nodes; only the entering/leaving row rebuilds
|
||||
wireCards(grid);
|
||||
decorateTuningChips(grid); // colour tuning chips by working-tuning match (async, feature-detected)
|
||||
state.winRange = { start, end };
|
||||
|
||||
@@ -0,0 +1,143 @@
|
||||
'use strict';
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert');
|
||||
|
||||
// Mirror of static/v3/songs.js _cardSig / _buildCardNode / _syncWindow (the
|
||||
// windowed-grid recycle path, #636 item 3 follow-up) — keep in sync. Exercised
|
||||
// against a minimal DOM shim so the reconcile invariants are covered off-browser:
|
||||
// (1) after every slide the grid's children are exactly [start,end) ascending,
|
||||
// (2) card nodes for indices that stay in-window are REUSED (identity kept) —
|
||||
// i.e. sliding one row never tears down + rebuilds the whole window (the
|
||||
// per-slide stall behind the "skips every so many scrolls" report), and
|
||||
// (3) a select-mode toggle rebuilds the visible window (checkbox/ring change).
|
||||
|
||||
let NODE_SEQ = 0;
|
||||
function makeNode() {
|
||||
const attrs = {};
|
||||
return {
|
||||
_uid: ++NODE_SEQ,
|
||||
parent: null,
|
||||
getAttribute(k) { return k in attrs ? attrs[k] : null; },
|
||||
setAttribute(k, v) { attrs[k] = String(v); },
|
||||
get nextSibling() {
|
||||
const p = this.parent; if (!p) return null;
|
||||
const i = p._kids.indexOf(this);
|
||||
return i >= 0 && i + 1 < p._kids.length ? p._kids[i + 1] : null;
|
||||
},
|
||||
remove() {
|
||||
const p = this.parent; if (!p) return;
|
||||
const i = p._kids.indexOf(this);
|
||||
if (i >= 0) p._kids.splice(i, 1);
|
||||
this.parent = null;
|
||||
},
|
||||
};
|
||||
}
|
||||
function makeGrid() {
|
||||
return {
|
||||
_kids: [],
|
||||
get children() { return this._kids.slice(); },
|
||||
get firstChild() { return this._kids[0] || null; },
|
||||
insertBefore(node, ref) {
|
||||
if (node.parent) node.remove();
|
||||
if (ref == null) this._kids.push(node);
|
||||
else { const i = this._kids.indexOf(ref); this._kids.splice(i < 0 ? this._kids.length : i, 0, node); }
|
||||
node.parent = this;
|
||||
return node;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// --- state + the three helpers, mirrored from songs.js ---
|
||||
const state = { songs: [], selectMode: false };
|
||||
for (let i = 0; i < 5000; i++) state.songs[i] = { filename: 'song' + i };
|
||||
|
||||
function _cardSig(i) { return (state.songs[i] ? 'r' : 's') + (state.selectMode ? '1' : '0'); }
|
||||
function _buildCardNode(i) {
|
||||
const node = makeNode();
|
||||
node.setAttribute('data-idx', String(i));
|
||||
node.setAttribute('data-sig', _cardSig(i));
|
||||
return node;
|
||||
}
|
||||
function _syncWindow(grid, start, end) {
|
||||
for (const el of Array.from(grid.children)) {
|
||||
const a = el.getAttribute('data-idx');
|
||||
const idx = a == null ? NaN : Number(a);
|
||||
if (!(idx >= start && idx < end) || el.getAttribute('data-sig') !== _cardSig(idx)) el.remove();
|
||||
}
|
||||
const existing = new Map();
|
||||
for (const el of grid.children) existing.set(Number(el.getAttribute('data-idx')), el);
|
||||
let ref = grid.firstChild;
|
||||
for (let i = start; i < end; i++) {
|
||||
let node = existing.get(i);
|
||||
if (!node) node = _buildCardNode(i);
|
||||
if (node === ref) ref = ref.nextSibling;
|
||||
else grid.insertBefore(node, ref);
|
||||
}
|
||||
}
|
||||
|
||||
const idxOf = (g) => g._kids.map((n) => Number(n.getAttribute('data-idx')));
|
||||
const uidOf = (g) => { const m = new Map(); for (const n of g._kids) m.set(Number(n.getAttribute('data-idx')), n._uid); return m; };
|
||||
function assertContig(g, start, end) {
|
||||
const a = idxOf(g);
|
||||
assert.strictEqual(a.length, end - start, `len == ${end - start}`);
|
||||
for (let k = 0; k < a.length; k++) assert.strictEqual(a[k], start + k, `child ${k} == ${start + k}`);
|
||||
}
|
||||
|
||||
const COLS = 6, WIN = 12 * COLS; // 12 rows visible
|
||||
|
||||
test('window stays [start,end) contiguous scrolling down, one row at a time', () => {
|
||||
const grid = makeGrid();
|
||||
for (let row = 0; row < 40; row++) {
|
||||
const start = row * COLS;
|
||||
_syncWindow(grid, start, start + WIN);
|
||||
assertContig(grid, start, start + WIN);
|
||||
}
|
||||
});
|
||||
|
||||
test('in-window card nodes are reused across a slide (no whole-window teardown)', () => {
|
||||
const grid = makeGrid();
|
||||
_syncWindow(grid, 0, WIN);
|
||||
const before = uidOf(grid);
|
||||
_syncWindow(grid, COLS, COLS + WIN); // slide down one row
|
||||
const after = uidOf(grid);
|
||||
let reused = 0, built = 0;
|
||||
for (const [i, uid] of after) (before.get(i) === uid ? reused++ : built++);
|
||||
assert.strictEqual(built, COLS, `only the entering row is built (${COLS}), got ${built}`);
|
||||
assert.strictEqual(reused, WIN - COLS, 'every overlapping card node is reused');
|
||||
});
|
||||
|
||||
test('scrolling back UP reuses nodes too and keeps order', () => {
|
||||
const grid = makeGrid();
|
||||
for (let row = 0; row < 30; row++) _syncWindow(grid, row * COLS, row * COLS + WIN);
|
||||
let prev = uidOf(grid);
|
||||
for (let row = 29; row >= 0; row--) {
|
||||
const start = row * COLS;
|
||||
_syncWindow(grid, start, start + WIN);
|
||||
assertContig(grid, start, start + WIN);
|
||||
const now = uidOf(grid);
|
||||
for (const [i, uid] of prev) if (i >= start && i < start + WIN) assert.strictEqual(now.get(i), uid, `idx ${i} reused going up`);
|
||||
prev = now;
|
||||
}
|
||||
});
|
||||
|
||||
test('a select-mode toggle rebuilds the visible window', () => {
|
||||
const grid = makeGrid();
|
||||
const start = 6 * COLS;
|
||||
_syncWindow(grid, start, start + WIN);
|
||||
const before = uidOf(grid);
|
||||
state.selectMode = true;
|
||||
_syncWindow(grid, start, start + WIN);
|
||||
const after = uidOf(grid);
|
||||
let rebuilt = 0;
|
||||
for (const [i, uid] of before) if (after.get(i) !== uid) rebuilt++;
|
||||
assert.strictEqual(rebuilt, WIN, 'select-mode change rebuilds every visible card');
|
||||
assertContig(grid, start, start + WIN);
|
||||
state.selectMode = false;
|
||||
});
|
||||
|
||||
test('a large jump (rail seek) rebuilds cleanly with no stale survivors', () => {
|
||||
const grid = makeGrid();
|
||||
_syncWindow(grid, 0, WIN);
|
||||
_syncWindow(grid, 1000 * COLS, 1000 * COLS + WIN); // non-overlapping jump
|
||||
assertContig(grid, 1000 * COLS, 1000 * COLS + WIN);
|
||||
});
|
||||
Reference in New Issue
Block a user