mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-21 12:21:49 +00:00
perf: remove throttled-trace residuals — program churn, per-frame rect, HUD clock
A 4x-CPU-throttled retrace (the honest weak-hardware proxy) surfaced three residual per-frame costs; stack attribution pinned each: - getParameters/getProgramCacheKey (~4% of main thread): every pooled label sprite map swap set material.needsUpdate, bumping material.version and forcing full program re-resolution next render. Swapping between two non-null cached textures never changes the compiled program (USE_MAP define unchanged) — new _setLabelMap() helper only flags needsUpdate on a null<->texture transition, used at all 7 swap sites. - getBoundingClientRect (~1.2%): the 3D highway's per-frame canvas-size self-check forced a layout read every frame. The CSS-box drift read now runs every 10th frame (or when the wrap isn't pinned); the backing-store comparison stays per-frame with cheap property reads and forces an immediate box read + applySize when it fires. - set textContent: the core 60 Hz HUD clock rewrote hud-time (and getElementById'd it) every tick for a display that changes 1/s — now write-on-change with a cached element ref. (The remaining textContent writer in the trace is notedetect's badges.js — external repo, to be filed there.) tests/js: resize-reframe shape test updated for the hoisted _bsChanged gate, incl. an assertion that the throttle can never delay the backing-store path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
1e9741043b
commit
59aa70ce5a
@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
## [Unreleased]
|
||||
|
||||
### Changed
|
||||
- **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.
|
||||
- **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).
|
||||
|
||||
### Added
|
||||
- **The tuner now tracks what tuning your instrument is *actually* in, so it prompts you to retune in BOTH directions — down to a song's tuning, and back up when the next song needs it.** The coverage check used to compare each song against your fixed instrument-profile tuning, so it only ever prompted you *away* from "home" (e.g. E → Drop C#) and stayed silent coming back (Drop C# → E), even though you'd physically retuned. It now reads the host's live **per-instrument working tuning** (`window.feedBack.workingTuning`) — what your selected instrument is currently in — so coverage is measured against your *actual* tuning and fires both ways. When you clear an auto-opened tuner, the tuner publishes that song's tuning as your instrument's live working tuning (`assumed` — an explicit "I tuned / Skip" refines it in a later PR), so the next song is judged against where you now are. **Per-instrument** — your guitar's and bass's tunings are tracked separately (keyed like the selector), so switching instruments uses the right one. Feature-detected: on a host without the working-tuning capability it falls back to the static `/api/settings` tuning (today's behavior). `plugins/tuner/screen.js` (`_playerTuning` reads `workingTuning` keyed by the selected instrument; `_publishWorkingTuning` writes on clear). Builds on the host `workingTuning` foundation (PR 1 of the series) + the instrument→chart routing (PR 2). Tests: `tests/js/tuner_auto_open.test.js` (both-directions coverage via a live Drop-D working tuning; publish-on-clear targets the right instrument slot) — 29 pass.
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
{
|
||||
"id": "highway_3d",
|
||||
"name": "3D Highway",
|
||||
"version": "3.31.0",
|
||||
"version": "3.31.1",
|
||||
"type": "visualization",
|
||||
"bundled": true,
|
||||
"script": "screen.js",
|
||||
|
||||
@ -3983,6 +3983,10 @@
|
||||
let _laneRailBoundsRefTpl = null;
|
||||
let _laneRailBoundsRefNotes = null;
|
||||
let _lastHwW = 0, _lastHwH = 0;
|
||||
// Frame counter for throttling the CSS-box drift check in draw()
|
||||
// (getBoundingClientRect is a forced layout read; see the comment
|
||||
// at the check).
|
||||
let _boxCheckCountdown = 0;
|
||||
// Last logical (CSS px) size handed to applySize(). #highway is a
|
||||
// flex:1 item, so its real rendered box (canvasSize()) can change as
|
||||
// the player layout settles after a song opens WITHOUT the backing
|
||||
@ -9928,6 +9932,23 @@
|
||||
// draw() after each init.
|
||||
// txtMat() rasterises into the unbounded cache these draws hit
|
||||
// anyway; ren.initTexture() forces the GPU upload now.
|
||||
// Swap a pooled label sprite's cached texture WITHOUT recompiling.
|
||||
// Setting material.needsUpdate bumps material.version, which forces
|
||||
// Three.js through getParameters/getProgramCacheKey on the next
|
||||
// render — profiled at ~4% of throttled main-thread time from the
|
||||
// per-frame label map swaps in dense charts. Swapping one non-null
|
||||
// texture for another does NOT change the compiled program (the
|
||||
// USE_MAP define is unchanged); only a null <-> non-null transition
|
||||
// does, and pooled label sprites are constructed with a non-null
|
||||
// map, so in practice this never recompiles.
|
||||
function _setLabelMap(sprite, srcMat) {
|
||||
const m = sprite.material;
|
||||
if (m.map === srcMat.map) return;
|
||||
const nullnessChanged = (m.map == null) !== (srcMat.map == null);
|
||||
m.map = srcMat.map;
|
||||
if (nullnessChanged) m.needsUpdate = true;
|
||||
}
|
||||
|
||||
let _chartPrewarmed = false;
|
||||
function _prewarmTex(mat) {
|
||||
if (mat && mat.map && ren) ren.initTexture(mat.map);
|
||||
@ -11967,7 +11988,7 @@
|
||||
const lblW = 28 * K, lblH = 9 * K;
|
||||
const lbl = pChordLbl.get();
|
||||
const mat = txtMat(chordName, '#e8d080', true, 'chord');
|
||||
if (lbl.material.map !== mat.map) { lbl.material.map = mat.map; lbl.material.needsUpdate = true; }
|
||||
_setLabelMap(lbl, mat);
|
||||
lbl.material.opacity = Math.min(1, 0.3 + fade * 0.7) * chordTailMul;
|
||||
// Gold chord name: slight +X shift from flush-left so it sits farther right.
|
||||
const lblWS = lblW * _textSizeMul;
|
||||
@ -12003,7 +12024,7 @@
|
||||
if (!text) return;
|
||||
const s = pChordLbl.get();
|
||||
const m = txtMat(text, colorHex, true, 'chord');
|
||||
if (s.material.map !== m.map) { s.material.map = m.map; s.material.needsUpdate = true; }
|
||||
_setLabelMap(s, m);
|
||||
s.material.opacity = opacity;
|
||||
s.position.set(baseX, hy, z);
|
||||
s.scale.set(hlW, hlH, 1);
|
||||
@ -12127,10 +12148,7 @@
|
||||
_seenChordFrets.add(f);
|
||||
const lbl = pNoteFretLabel.get();
|
||||
const mat = txtMat(f, FRET_LABEL_GOLD_HEX, false, 'noteFret');
|
||||
if (lbl.material.map !== mat.map) {
|
||||
lbl.material.map = mat.map;
|
||||
lbl.material.needsUpdate = true;
|
||||
}
|
||||
_setLabelMap(lbl, mat);
|
||||
lbl.position.set(xFretMid(f), yMinF, z);
|
||||
lbl.renderOrder = renderOrderForLayerAtZ(z, 'CHORD_FRET_LABEL');
|
||||
const _flS = 7.0 * K * (1 + 0.4 * chDt / AHEAD) * _textSizeMul * fretLabelScaleForFret(f);
|
||||
@ -12753,10 +12771,7 @@
|
||||
const color = '#888888';
|
||||
const sp = pFretColMarker.get();
|
||||
const m = txtMat(f, color, false, 'noteFret');
|
||||
if (sp.material.map !== m.map) {
|
||||
sp.material.map = m.map;
|
||||
sp.material.needsUpdate = true;
|
||||
}
|
||||
_setLabelMap(sp, m);
|
||||
sp.material.opacity = 0.85 * _colFadeIn;
|
||||
sp.position.set(xFretMid(f), labelY, z);
|
||||
// Z-proportional: sits between chord frame and note gem
|
||||
@ -14039,10 +14054,7 @@
|
||||
_frameLabeledKeys.add(_flFrameKey);
|
||||
const fretLabel = pNoteFretLabel.get();
|
||||
const cachedMat = txtMat(n.f, FRET_LABEL_GOLD_HEX, false, 'noteFret');
|
||||
if (fretLabel.material.map !== cachedMat.map) {
|
||||
fretLabel.material.map = cachedMat.map;
|
||||
fretLabel.material.needsUpdate = true;
|
||||
}
|
||||
_setLabelMap(fretLabel, cachedMat);
|
||||
fretLabel.position.set(x, labelY, noteZ);
|
||||
fretLabel.renderOrder = renderOrderForLayerAtZ(noteZ,
|
||||
_isArpNote
|
||||
@ -14067,10 +14079,7 @@
|
||||
if (!text) return;
|
||||
const spr = pTeachMarkLbl.get();
|
||||
const m = txtMat(text, colorHex, false, cacheKey);
|
||||
if (spr.material.map !== m.map) {
|
||||
spr.material.map = m.map;
|
||||
spr.material.needsUpdate = true;
|
||||
}
|
||||
_setLabelMap(spr, m);
|
||||
spr.position.set(x + dx, labelY, noteZ);
|
||||
spr.renderOrder = renderOrderForLayerAtZ(noteZ,
|
||||
_isArpNote ? 'ARP_NOTE_FRET_LABEL' : 'NOTE_FRET_LABEL');
|
||||
@ -14104,10 +14113,7 @@
|
||||
const _isArp2 = arpBounds !== null;
|
||||
const fl2 = pNoteFretLabel.get();
|
||||
const cm2 = txtMat(n.f, FRET_LABEL_GOLD_HEX, false, 'noteFret');
|
||||
if (fl2.material.map !== cm2.map) {
|
||||
fl2.material.map = cm2.map;
|
||||
fl2.material.needsUpdate = true;
|
||||
}
|
||||
_setLabelMap(fl2, cm2);
|
||||
fl2.position.set(x, _labelY2, noteZ);
|
||||
fl2.renderOrder = renderOrderForLayerAtZ(noteZ,
|
||||
_isArp2
|
||||
@ -15163,25 +15169,40 @@
|
||||
// for the pre-settle (too-tall) size and crops the near strings
|
||||
// / fret numbers until the user un/re-maximizes the window.
|
||||
if (highwayCanvas) {
|
||||
const box = canvasSize(highwayCanvas);
|
||||
if (highwayCanvas.width !== _lastHwW || highwayCanvas.height !== _lastHwH) {
|
||||
_lastHwW = highwayCanvas.width;
|
||||
_lastHwH = highwayCanvas.height;
|
||||
if (box.w > 0 && box.h > 0) applySize(box.w, box.h);
|
||||
} 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);
|
||||
// Backing-store drift (branch 1) is detected with cheap
|
||||
// property reads every frame. The CSS-box checks (branches
|
||||
// 2/3) need canvasSize() → getBoundingClientRect(), a
|
||||
// forced layout read — profiled at ~1.2% of throttled
|
||||
// main-thread time when run per frame. Throttle the box
|
||||
// read to every 10th frame (plus whenever the backing
|
||||
// store changed or the wrap isn't pinned yet): the layout
|
||||
// settle it exists to catch plays out over hundreds of ms
|
||||
// right after a song opens, so a ~166 ms detection cadence
|
||||
// loses nothing visible.
|
||||
const _bsChanged = highwayCanvas.width !== _lastHwW
|
||||
|| highwayCanvas.height !== _lastHwH;
|
||||
_boxCheckCountdown = (_boxCheckCountdown + 1) % 10;
|
||||
if (_bsChanged || !_wrapPinned || _boxCheckCountdown === 0) {
|
||||
const box = canvasSize(highwayCanvas);
|
||||
if (_bsChanged) {
|
||||
_lastHwW = highwayCanvas.width;
|
||||
_lastHwH = highwayCanvas.height;
|
||||
if (box.w > 0 && box.h > 0) applySize(box.w, box.h);
|
||||
} 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);
|
||||
|
||||
@ -9777,6 +9777,12 @@ async function startSongCountIn() {
|
||||
|
||||
// Time display + highway sync
|
||||
let lastAudioTime = 0;
|
||||
// hud-time write cache: the 60 Hz tick below used to rewrite textContent
|
||||
// (and getElementById) every tick even though the mm:ss display only
|
||||
// changes once a second — each write invalidates layout. Write-on-change
|
||||
// with a cached element ref (re-resolved if detached).
|
||||
let _hudTimeEl = null;
|
||||
let _hudTimeLast = '';
|
||||
setInterval(() => {
|
||||
let ct = _audioTime();
|
||||
const dur = _audioDuration();
|
||||
@ -9804,7 +9810,12 @@ setInterval(() => {
|
||||
ct = lastAudioTime;
|
||||
}
|
||||
lastAudioTime = ct;
|
||||
document.getElementById('hud-time').textContent = `${formatTime(ct)} / ${formatTime(dur)}`;
|
||||
const hudText = `${formatTime(ct)} / ${formatTime(dur)}`;
|
||||
if (hudText !== _hudTimeLast) {
|
||||
if (!_hudTimeEl || !_hudTimeEl.isConnected) _hudTimeEl = document.getElementById('hud-time');
|
||||
if (_hudTimeEl) _hudTimeEl.textContent = hudText;
|
||||
_hudTimeLast = hudText;
|
||||
}
|
||||
if (dur) {
|
||||
_maybeRefreshSectionPracticeDuration(dur);
|
||||
}
|
||||
|
||||
@ -61,10 +61,25 @@ test('draw() reads the live canvas box once per frame', () => {
|
||||
test('backing-store drift branch is preserved (splitscreen path)', () => {
|
||||
// The original check that catches the splitscreen hw.resize override
|
||||
// resizing the element without calling renderer.resize() must remain.
|
||||
// The comparison is hoisted into _bsChanged (checked with cheap property
|
||||
// reads every frame, and it forces the throttled box read to run on the
|
||||
// same frame); the branch body is unchanged.
|
||||
assert.match(
|
||||
src,
|
||||
/if\s*\(\s*highwayCanvas\.width\s*!==\s*_lastHwW\s*\|\|\s*highwayCanvas\.height\s*!==\s*_lastHwH\s*\)\s*\{\s*_lastHwW\s*=\s*highwayCanvas\.width\s*;\s*_lastHwH\s*=\s*highwayCanvas\.height\s*;\s*if\s*\(\s*box\.w\s*>\s*0\s*&&\s*box\.h\s*>\s*0\s*\)\s*applySize\(\s*box\.w\s*,\s*box\.h\s*\)\s*;/,
|
||||
'the backing-store (canvas.width/height) drift branch must still re-apply',
|
||||
/const\s+_bsChanged\s*=\s*highwayCanvas\.width\s*!==\s*_lastHwW\s*\|\|\s*highwayCanvas\.height\s*!==\s*_lastHwH\s*;/,
|
||||
'the backing-store (canvas.width/height) comparison must run every frame',
|
||||
);
|
||||
assert.match(
|
||||
src,
|
||||
/if\s*\(\s*_bsChanged\s*\)\s*\{\s*_lastHwW\s*=\s*highwayCanvas\.width\s*;\s*_lastHwH\s*=\s*highwayCanvas\.height\s*;\s*if\s*\(\s*box\.w\s*>\s*0\s*&&\s*box\.h\s*>\s*0\s*\)\s*applySize\(\s*box\.w\s*,\s*box\.h\s*\)\s*;/,
|
||||
'the backing-store drift branch must still re-apply',
|
||||
);
|
||||
// The throttle must never delay the backing-store path: _bsChanged is
|
||||
// part of the gate that forces the box read on the same frame.
|
||||
assert.match(
|
||||
src,
|
||||
/if\s*\(\s*_bsChanged\s*\|\|\s*!_wrapPinned\s*\|\|\s*_boxCheckCountdown\s*===\s*0\s*\)/,
|
||||
'the box-read gate must include _bsChanged so backing-store changes re-apply immediately',
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user