diff --git a/CHANGELOG.md b/CHANGELOG.md index afe1975..58dda22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 carry their gig log; instruments their gig count. ### Changed +- **Folder library renders only the songs on screen** (#965) — a song list used to + render *every* song it held. On a flat 50,944-song library that was one `
` + with 50,938 children and ~1.3 **million** DOM nodes (~4.2 GB of renderer memory), + built even while another screen was showing. A document that size also punishes + unrelated code: any `document.querySelector` that misses has to walk the whole + tree — which is how the song-preview menu check ended up eating ~50% of the + renderer and dropping the app to 2.7 fps. Lists longer than 200 songs are now + windowed (25–31 rows in the DOM instead of 50,000); shorter lists are unchanged. - **The full mix is a stem** (#933) — core no longer depends on `original_audio:`, a top-level manifest key this repo invented (#583) that the feedpak spec never had. The format already carried the pre-separation mixdown as a stem; feedpak 1.15.0 diff --git a/plugins/folder_library/screen.js b/plugins/folder_library/screen.js index 0a9fa0c..8ddec84 100644 --- a/plugins/folder_library/screen.js +++ b/plugins/folder_library/screen.js @@ -878,6 +878,146 @@ function createFolderSurface(cfg) { var _dragRafId = null; var _DRAG_THRESH = 5, _DRAG_ZONE = 150, _DRAG_SPEED = 50; + // ── Windowed song lists ───────────────────────────────────────────── + // A song list used to render EVERY song it held. On a flat 50,944-song + // library that is one
with 50,938 children and ~1.3 MILLION DOM nodes + // (~25 per row) — ~4.2 GB of renderer RSS, for a screen the user may not + // even be looking at. It also poisons unrelated code: any + // `document.querySelector` miss anywhere in the app must walk that whole + // tree, which is how song_preview's per-frame menu check ended up eating + // ~50% of the renderer and dropping the app to 2.7 fps (feedBack#965). + // + // So render only what is on screen. Rows are uniform height (and grid cards + // uniform size), so the window is pure arithmetic — no per-row observers. + // Off-window rows are represented by padding on the list itself rather than + // spacer elements: a spacer
would become a grid ITEM in grid view and + // shift the columns, whereas padding works identically for both layouts. + var VIRTUAL_MIN = 200; // below this, render everything — no behaviour change + var VIRTUAL_BUFFER = 6; // rows kept rendered above/below the viewport + var _virtualCleanups = []; + var _virtualLists = []; // repaint fns, one per live windowed list + + // Which slice of the list is on screen. Pure arithmetic — kept separate from + // the DOM so it can be tested directly (see tests/virtual_list.test.js). + // + // top : list's offset relative to the scroller viewport's top. NEGATIVE + // once the user has scrolled the list's start above the fold. + // rows : total ROWS (grid packs `perRow` songs into one row; list view is 1) + // + // Returns the song index range [start, end) to render, plus how many ROWS of + // padding stand in for the songs above and below it. + function _visibleWindow(top, viewportH, itemH, perRow, rows, total) { + if (!(itemH > 0) || !(rows > 0)) return { start: 0, end: total, padRowsTop: 0, padRowsBottom: 0 }; + var firstRow = Math.max(0, Math.floor(-top / itemH) - VIRTUAL_BUFFER); + var lastRow = Math.min(rows, Math.ceil((-top + viewportH) / itemH) + VIRTUAL_BUFFER); + // Scrolled entirely past the list (either direction): keep one row alive + // rather than emptying it, so the padding math stays anchored. + if (lastRow <= firstRow) { + firstRow = Math.min(firstRow, rows - 1); + lastRow = firstRow + 1; + } + return { + start: firstRow * perRow, + end: Math.min(total, lastRow * perRow), + padRowsTop: firstRow, + padRowsBottom: Math.max(0, rows - lastRow), + }; + } + + function _clearVirtualLists() { + _virtualCleanups.forEach(function (fn) { try { fn(); } catch (_) {} }); + _virtualCleanups = []; + _virtualLists = []; + } + + // Fill `list` with `songs`, windowed when the list is big enough to matter. + // `make(song)` builds one row/card. + function _fillSongList(list, songs, make) { + var sorted = _sortSongs(songs); + if (sorted.length <= VIRTUAL_MIN) { + sorted.forEach(function (s) { list.appendChild(make(s)); }); + return; + } + + var scroller = _getScrollEl(); + var basePadTop = parseFloat(window.getComputedStyle(list).paddingTop) || 0; + var basePadBot = parseFloat(window.getComputedStyle(list).paddingBottom) || 0; + + // Measure one real row once — no hardcoded row height to drift out of + // sync with the CSS. (The list is shown before it is populated, so this + // measures a laid-out row, not a zero-height one.) + var probe = make(sorted[0]); + probe.style.visibility = 'hidden'; + list.appendChild(probe); + var probeRect = probe.getBoundingClientRect(); + var rowH = probeRect.height || 44; + var cardW = probeRect.width || 150; + list.removeChild(probe); + + var GRID_GAP = 12; // matches the grid's `gap:12px` + var raf = 0, lastStart = -1, lastEnd = -1; + + // Recomputed on EVERY paint, not captured once: a window resize changes + // the grid's column count, and therefore the row count and the height of + // the padding standing in for off-window rows. paint() runs on resize, so + // stale metrics would slice the wrong songs and mis-size the list. + function metrics() { + var perRow = 1, itemH = rowH; + if (_view === 'grid') { + perRow = Math.max(1, Math.floor((list.clientWidth + GRID_GAP) / (cardW + GRID_GAP))); + itemH = rowH + GRID_GAP; + } + return { perRow: perRow, itemH: itemH, rows: Math.ceil(sorted.length / perRow) }; + } + + function paint() { + raf = 0; + // Collapsed (display:none) or detached: nothing to paint, and don't + // pay for layout on every scroll tick of a section nobody can see. + // Forget the last window so re-showing repaints from scratch against + // the new position rather than short-circuiting on a stale memo. + if (!list.isConnected || list.offsetParent === null) { + lastStart = -1; lastEnd = -1; + return; + } + var m = metrics(); + // Where the list sits relative to the scroller's viewport. + var top = list.getBoundingClientRect().top - scroller.getBoundingClientRect().top; + var vh = scroller.clientHeight || window.innerHeight; + var w = _visibleWindow(top, vh, m.itemH, m.perRow, m.rows, sorted.length); + if (w.start === lastStart && w.end === lastEnd) return; // nothing moved + lastStart = w.start; lastEnd = w.end; + + var frag = document.createDocumentFragment(); + for (var i = w.start; i < w.end; i++) frag.appendChild(make(sorted[i])); + list.textContent = ''; + list.style.paddingTop = (basePadTop + w.padRowsTop * m.itemH) + 'px'; + list.style.paddingBottom = (basePadBot + w.padRowsBottom * m.itemH) + 'px'; + list.appendChild(frag); + } + function schedule() { if (!raf) raf = window.requestAnimationFrame(paint); } + + scroller.addEventListener('scroll', schedule, { passive: true }); + window.addEventListener('resize', schedule); + // Expanding or collapsing ANY section moves every list below it. Those + // lists' windows are computed from their position, so they must repaint + // too — otherwise they keep the window from their old position and show + // blank padding where songs should be until the user happens to scroll. + _virtualLists.push(schedule); + _virtualCleanups.push(function () { + scroller.removeEventListener('scroll', schedule); + window.removeEventListener('resize', schedule); + if (raf) window.cancelAnimationFrame(raf); + }); + paint(); + } + + // Re-window every live list — call after anything that can move them + // vertically (a folder expanding/collapsing, a section being shown). + function _repaintVirtualLists() { + _virtualLists.forEach(function (fn) { try { fn(); } catch (_) {} }); + } + function _getScrollEl() { var el = _treeEl(); while (el && el !== document.documentElement) { @@ -1159,8 +1299,8 @@ function createFolderSurface(cfg) { var _listPopulated = open; function _populateList() { - _sortSongs(folder.songs).forEach(function (s) { - list.appendChild(_view === 'grid' ? _songCard(s, folder.path) : _songRow(s, folder.path)); + _fillSongList(list, folder.songs, function (s) { + return _view === 'grid' ? _songCard(s, folder.path) : _songRow(s, folder.path); }); (folder.children || []).forEach(function (child) { childrenWrap.appendChild(_folderSection(child, depth + 1)); @@ -1195,12 +1335,18 @@ function createFolderSurface(cfg) { hdr.addEventListener('click', function () { if (_query()) return; var nowOpen = content.style.display === 'none'; - if (nowOpen && !_listPopulated) { _populateList(); _listPopulated = true; } + // Show BEFORE populating: a windowed list measures a real row and the + // scroller viewport, and both are zero while display:none. content.style.display = nowOpen ? '' : 'none'; + if (nowOpen && !_listPopulated) { _populateList(); _listPopulated = true; } chev.style.transform = nowOpen ? 'rotate(90deg)' : ''; if (nowOpen) _openFolders.add(folder.path); else _openFolders.delete(folder.path); _storeJSON('open', [..._openFolders]); + // This toggle moved everything below it — re-window the other lists, + // and re-window THIS one if it was already populated (its saved + // window was computed at its old position). + _repaintVirtualLists(); }); wrap.appendChild(hdr); wrap.appendChild(content); @@ -1245,8 +1391,8 @@ function createFolderSurface(cfg) { } var _populated = _unsortedOpen; function _populate() { - _sortSongs(songs).forEach(function (s) { - list.appendChild(_view === 'grid' ? _songCard(s, '') : _songRow(s, '')); + _fillSongList(list, songs, function (s) { + return _view === 'grid' ? _songCard(s, '') : _songRow(s, ''); }); } if (_unsortedOpen) { _populate(); } else { list.style.display = 'none'; } @@ -1255,10 +1401,12 @@ function createFolderSurface(cfg) { hdr.addEventListener('click', function () { if (_query()) return; _unsortedOpen = list.style.display === 'none'; - if (_unsortedOpen && !_populated) { _populate(); _populated = true; } + // Show BEFORE populating — see the folder toggle above. list.style.display = _unsortedOpen ? (_view === 'grid' ? 'grid' : '') : 'none'; + if (_unsortedOpen && !_populated) { _populate(); _populated = true; } chev.style.transform = _unsortedOpen ? 'rotate(90deg)' : ''; _store(cfg.unsortedKey, String(_unsortedOpen)); + _repaintVirtualLists(); // this toggle moved every list below it }); wrap.appendChild(hdr); wrap.appendChild(list); @@ -1340,6 +1488,10 @@ function createFolderSurface(cfg) { // ── Render ────────────────────────────────────────────────────────── function _render() { _hoveredFolder = null; // DOM is rebuilt; discard any stale reference + // Drop the scroll listeners of the previous render's windowed lists — + // their `list` nodes are about to be detached, and a surviving listener + // would keep painting into orphaned DOM (and leak on every re-render). + _clearVirtualLists(); var treeEl = _treeEl(); if (!treeEl) return; var data = _filtered(); @@ -1451,6 +1603,7 @@ function createFolderSurface(cfg) { // ── Unload (lib surface) ──────────────────────────────────────────── function _unload() { + _clearVirtualLists(); // don't leave scroll listeners behind on teardown if (!cfg.searchInputId) return; var el = _el(cfg.searchInputId); if (el) el.style.maxWidth = ''; @@ -1554,6 +1707,8 @@ function createFolderSurface(cfg) { init: _init, onScreenChanged: _onScreenChanged, render: _render, + // Pure window arithmetic, exposed for tests (no DOM needed). + __test: { visibleWindow: _visibleWindow, VIRTUAL_MIN: VIRTUAL_MIN, VIRTUAL_BUFFER: VIRTUAL_BUFFER }, }; } @@ -1656,6 +1811,7 @@ if (!window.__folderLibraryLib) { window.folderLibrary = { load: function (force) { return _lib.load(force); }, unload: function () { _lib.unload(); }, + __test: _lib.__test, }; // Auto-load if folder view was already active when this script was injected. diff --git a/plugins/folder_library/tests/virtual_list.test.js b/plugins/folder_library/tests/virtual_list.test.js new file mode 100644 index 0000000..9efa703 --- /dev/null +++ b/plugins/folder_library/tests/virtual_list.test.js @@ -0,0 +1,165 @@ +// Windowed song lists (feedBack#965). +// +// A song list used to render EVERY song. On a flat 50,944-song library that is +// one div with 50,938 children and ~1.3 MILLION DOM nodes (~25 per row) — +// ~4.2 GB of renderer RSS, for a screen the user may not even be looking at. It +// also poisoned unrelated code: any `document.querySelector` miss anywhere in +// the app had to walk that whole tree. +// +// _visibleWindow is the arithmetic that decides which slice is on screen. If it +// is wrong the list silently shows the wrong songs, or scrolls to the wrong +// place, so it is tested directly — the DOM glue around it is not the risky bit. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); + +function load() { + const window = { + console, + document: { + readyState: 'complete', + addEventListener() {}, + getElementById() { return null; }, + querySelector() { return null; }, + querySelectorAll() { return []; }, + createElement() { return { style: {}, classList: { add() {}, remove() {}, contains() { return false; } }, addEventListener() {}, appendChild() {} }; }, + }, + addEventListener() {}, + localStorage: { getItem() { return null; }, setItem() {} }, + performance: { now: () => 0 }, + setInterval() { return 0; }, + clearInterval() {}, + requestAnimationFrame() { return 0; }, + cancelAnimationFrame() {}, + getComputedStyle() { return { overflowY: 'visible', paddingTop: '0px', paddingBottom: '0px' }; }, + innerHeight: 800, + }; + window.window = window; + window.globalThis = window; + const ctx = vm.createContext(window); + vm.runInContext(fs.readFileSync(path.join(__dirname, '..', 'screen.js'), 'utf8'), ctx, { filename: 'screen.js' }); + assert.ok(window.folderLibrary && window.folderLibrary.__test, 'plugin must expose __test'); + return window.folderLibrary.__test; +} + +const { visibleWindow, VIRTUAL_BUFFER, VIRTUAL_MIN } = load(); + +// A flat 50k library in list view: 1 song per row, 44px rows, 800px viewport. +const ROW = 44; +const VH = 800; +const TOTAL = 50938; + +test('the whole point: a 50k list renders a bounded window, not 50k rows', () => { + const w = visibleWindow(0, VH, ROW, 1, TOTAL, TOTAL); + const rendered = w.end - w.start; + assert.ok(rendered < 60, `expected a small window, got ${rendered} rows`); + // ~18 rows fit in 800px, plus buffer above and below. + assert.ok(rendered >= Math.ceil(VH / ROW), 'must at least fill the viewport'); +}); + +test('at the top: starts at 0, all remaining rows are bottom padding', () => { + const w = visibleWindow(0, VH, ROW, 1, TOTAL, TOTAL); + assert.equal(w.start, 0); + assert.equal(w.padRowsTop, 0); + assert.equal(w.padRowsBottom, TOTAL - w.end); +}); + +test('scrolled into the middle: window tracks the scroll, padding adds up', () => { + const scrolled = 10000 * ROW; // row 10,000 at the fold + const w = visibleWindow(-scrolled, VH, ROW, 1, TOTAL, TOTAL); + assert.equal(w.start, (10000 - VIRTUAL_BUFFER) * 1); + assert.ok(w.end > w.start); + // The invariant that keeps the scrollbar honest: padding rows + rendered + // rows must account for every song, or the list changes height as you scroll. + assert.equal(w.padRowsTop + (w.end - w.start) + w.padRowsBottom, TOTAL); +}); + +test('at the very bottom: no bottom padding, end lands on the last song', () => { + const rows = TOTAL; + const scrolled = rows * ROW - VH; // scrolled to the end + const w = visibleWindow(-scrolled, VH, ROW, 1, rows, TOTAL); + assert.equal(w.end, TOTAL); + assert.equal(w.padRowsBottom, 0); + assert.equal(w.padRowsTop + (w.end - w.start), TOTAL); +}); + +test('grid view: perRow songs collapse into one row', () => { + const perRow = 6; + const rows = Math.ceil(TOTAL / perRow); + const w = visibleWindow(0, VH, 190, perRow, rows, TOTAL); + assert.equal(w.start, 0); + assert.equal(w.start % perRow, 0, 'a window must start on a row boundary'); + assert.ok(w.end <= TOTAL); + assert.ok((w.end - w.start) < 200, 'grid window must stay bounded'); +}); + +test('scrolled far past the list: keeps one row, never a negative window', () => { + const w = visibleWindow(-99999999, VH, ROW, 1, TOTAL, TOTAL); + assert.ok(w.end > w.start, 'window must never invert'); + assert.ok(w.start >= 0 && w.end <= TOTAL); + assert.equal(w.padRowsTop + (w.end - w.start) + w.padRowsBottom, TOTAL); +}); + +test('list not yet scrolled to (below the fold): still yields a valid window', () => { + const w = visibleWindow(5000, VH, ROW, 1, TOTAL, TOTAL); // list starts below viewport + assert.equal(w.start, 0); + assert.ok(w.end > 0); +}); + +test('degenerate inputs fall back to rendering everything, never to a broken window', () => { + // Measured height of 0 (e.g. list still display:none) must not divide by zero + // and must not silently render an empty list. + const w = visibleWindow(0, VH, 0, 1, TOTAL, TOTAL); + assert.equal(w.start, 0); + assert.equal(w.end, TOTAL); + assert.equal(w.padRowsTop, 0); + assert.equal(w.padRowsBottom, 0); +}); + +test('small lists are below the virtualization threshold', () => { + assert.ok(VIRTUAL_MIN >= 100, 'threshold must be high enough that normal folders are untouched'); +}); + +// ── the grid must be re-measured when the window resizes (CodeRabbit, #967) ── +// perRow and rows were originally captured once at fill time. paint() also runs +// on resize, so a narrower/wider window changed the column count while the +// window maths still used the OLD one — slicing the wrong songs and mis-sizing +// the padding. These pin that the geometry is a function of perRow, so a stale +// perRow cannot silently survive. + +test('resizing the grid to fewer columns re-windows against the new row count', () => { + const total = 10000; + const wide = visibleWindow(0, VH, 190, 6, Math.ceil(total / 6), total); + const narrow = visibleWindow(0, VH, 190, 3, Math.ceil(total / 3), total); + + // Same viewport, half the columns -> about half as many songs on screen. + assert.ok(narrow.end < wide.end, 'fewer columns must render fewer songs per screen'); + // ...and the total must still add up, or the scrollbar lies after a resize. + for (const [w, perRow] of [[wide, 6], [narrow, 3]]) { + const rows = Math.ceil(total / perRow); + assert.equal(w.padRowsTop + Math.ceil((w.end - w.start) / perRow) + w.padRowsBottom, rows, + `rows must account for every song at perRow=${perRow}`); + } +}); + +test('a stale perRow would break the total-height invariant (the bug)', () => { + const total = 10000; + // Grid re-laid out to 3 columns, but windowed with the OLD perRow of 6: + // the row count no longer matches the geometry, and the padding is wrong. + const stalePerRow = 6, actualRows = Math.ceil(total / 3); + const bad = visibleWindow(0, VH, 190, stalePerRow, actualRows, total); + const accounted = bad.padRowsTop + Math.ceil((bad.end - bad.start) / 3) + bad.padRowsBottom; + assert.notEqual(accounted, actualRows, + 'this asserts the FAILURE mode: mismatched perRow/rows must not silently look correct — ' + + 'metrics() recomputes both together on every paint so this cannot happen in practice'); +}); + +test('scrolled grid window always starts on a row boundary', () => { + const total = 10000, perRow = 4; + const rows = Math.ceil(total / perRow); + const w = visibleWindow(-5000, VH, 190, perRow, rows, total); + assert.equal(w.start % perRow, 0, 'a partial row would shift every card in the grid'); +});