diff --git a/CHANGELOG.md b/CHANGELOG.md index 33067f4..91d7719 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **Guitar Pro 6 (`.gpx`) import no longer fails on every real file.** The GPX BCFS container reader (`lib/gp2rs_gpx.py`) rejected any file whose final sector wasn't a full `0x1000` block — but a real `.gpx`'s BCFZ-declared decompressed size isn't sector-aligned, so the last (small) container file always lands in a partial trailing sector. The bounds check *raised* `GPX BCFS sector pointer out of range (malformed file)` instead of clamping the tail read, so `_load_gpif` threw before `score.gpif` could be extracted and **no GP6 file could be imported into the song editor** (both real test files failed identically — this wasn't file-specific). GP7/GP8 `.gp` files were unaffected — they take the ZIP path, not BCFS, which is why prior GP-import work didn't surface it. The reader now **clamps the final sector read to the buffer end** (the per-file size field trims the padding anyway), matching canonical GPX readers (alphaTab / PyGuitarPro); a sector whose *start* is past the end still raises, preserving the malformed-file guard. Verified against two real GP6 files — both now unpack to valid GPIF with all tracks. Tests: `tests/test_gp2rs_gpx.py` (partial-final-sector round-trip, multi-file container, sector-aligned baseline, and the preserved out-of-range guard). +- **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). - **Starter content seeds again (and now ships The Adicts' "Ode to Joy").** `_BUILTIN_STARTER_SOURCES` still listed `beethoven-ode_to_joy.feedpak` after that pack was deleted, and never wired up its replacement `the_adicts-ode-to-joy_vst_cover.feedpak` that landed on disk. The listed-but-missing file made the all-present gate never fire, so **no** starter songs seeded on first run. Synced the manifest to what's on disk (Für Elise, Star Spangled Banner, The Adicts' Ode to Joy). Tests: `tests/test_builtin_starter_seed.py` (the present/unlisted guards were red on `main`). - **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`. diff --git a/static/v3/songs.js b/static/v3/songs.js index 845c954..9d6fe9d 100644 --- a/static/v3/songs.js +++ b/static/v3/songs.js @@ -1859,13 +1859,57 @@ ''; } - function _renderCardsRange(start, end) { - let html = ''; - for (let i = start; i < end; i++) { - const s = state.songs[i]; - html += s ? songCard(s) : _skeletonCard(); + // 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++) { + 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 @@ -1983,7 +2027,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 }; diff --git a/tests/js/v3_songs_window_recycle.test.js b/tests/js/v3_songs_window_recycle.test.js new file mode 100644 index 0000000..47639c2 --- /dev/null +++ b/tests/js/v3_songs_window_recycle.test.js @@ -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); +});