mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-12 19:59:35 +00:00
fix(highway_3d): recover from WebGL context loss instead of crashing on alt-tab (#790)
Switching the active window / alt-tabbing away (most often on Windows) can trigger a GPU context reset. The 3D highway's WebGL renderer had no webglcontextlost handler, so a lost context was left to escalate into a render-process crash -- matching the intermittent "randomly crashes when I change windows" desktop reports. The renderer now binds webglcontextlost/webglcontextrestored on its own WebGL canvas (ren.domElement): the loss is preventDefault()'d so the browser keeps the context restorable, draw() bails while the context is down so no GL work runs on a dead context, and on restore the viewport is re-applied and rendering resumes (Three re-uploads scene resources on the next frame). Listeners are removed in teardown. Root cause is a strong hypothesis -- the crash is intermittent and unreproducible -- but the fix is low-risk and additive and closes a real gap: there was no context-loss handling anywhere in the renderer. plugins/highway_3d 3.31.2 -> 3.31.3. Tests: tests/js/highway_3d_context_loss.test.js (source-contract, like the other highway_* tests). The sibling keys_highway_3d / drum_highway_3d renderers share the same gap -- follow-up in their repos. Claude-Session: https://claude.ai/code/session_01UR2Cr7GEu3yMY7SrfxH6c1 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
de002cdc24
commit
b914612f9d
@@ -16,6 +16,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.
|
- **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
|
### Fixed
|
||||||
|
- **3D Highway: recover from a WebGL context loss instead of crashing on alt-tab.** Switching the active window / alt-tabbing away from the app (most often on Windows) can trigger a GPU context reset; the 3D highway's WebGL renderer had **no `webglcontextlost` handler**, so a lost context was left to escalate into a render-process crash — matching the intermittent "randomly crashes when I change windows" desktop reports. The renderer now binds `webglcontextlost`/`webglcontextrestored` on its own WebGL canvas (`ren.domElement`): the loss is `preventDefault()`'d so the browser keeps the context restorable, `draw()` bails while the context is down so no GL work runs on a dead context, and on restore the viewport is re-applied and rendering resumes (Three re-uploads scene resources on the next frame). Listeners are torn down with the renderer. `plugins/highway_3d` → 3.31.3. Tests: `tests/js/highway_3d_context_loss.test.js`. (The sibling `keys_highway_3d` / `drum_highway_3d` renderers share the same gap — tracked as a follow-up in their repos.)
|
||||||
- **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).
|
- **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).
|
- **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`).
|
- **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`).
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"id": "highway_3d",
|
"id": "highway_3d",
|
||||||
"name": "3D Highway",
|
"name": "3D Highway",
|
||||||
"version": "3.31.2",
|
"version": "3.31.3",
|
||||||
"type": "visualization",
|
"type": "visualization",
|
||||||
"bundled": true,
|
"bundled": true,
|
||||||
"script": "screen.js",
|
"script": "screen.js",
|
||||||
|
|||||||
@@ -3747,6 +3747,15 @@
|
|||||||
// ── Per-instance Three.js state ───────────────────────────────────
|
// ── Per-instance Three.js state ───────────────────────────────────
|
||||||
let scene = null, cam = null, ren = null;
|
let scene = null, cam = null, ren = null;
|
||||||
let wrap = null;
|
let wrap = null;
|
||||||
|
// WebGL context-loss recovery. Switching the active window / alt-tabbing
|
||||||
|
// (especially on Windows) can trigger a GPU context reset; with no
|
||||||
|
// handler the lost context escalates into a render-process crash. The
|
||||||
|
// listeners (bound in initScene on ren.domElement, removed in teardown)
|
||||||
|
// preventDefault the loss so the browser keeps the context restorable,
|
||||||
|
// _ctxLost gates draw() off the dead context, and on restore we reset the
|
||||||
|
// viewport + resume (Three re-uploads scene resources on the next render).
|
||||||
|
let _ctxLost = false;
|
||||||
|
let _onCtxLost = null, _onCtxRestored = null;
|
||||||
let bcCtrl = null; // Butterchurn audio-reactive background (the 'butterchurn' bg-style)
|
let bcCtrl = null; // Butterchurn audio-reactive background (the 'butterchurn' bg-style)
|
||||||
let _chartEnv = 0, _chartPrevT = -1, _bcBeatIdx = 0, _bcNoteIdx = 0, _bcChordIdx = 0, _bcTintTarget = null;
|
let _chartEnv = 0, _chartPrevT = -1, _bcBeatIdx = 0, _bcNoteIdx = 0, _bcChordIdx = 0, _bcTintTarget = null;
|
||||||
let _tintR = 20, _tintG = 24, _tintB = 40; // smoothed instrument-color tint for the bg
|
let _tintR = 20, _tintG = 24, _tintB = 40; // smoothed instrument-color tint for the bg
|
||||||
@@ -6526,6 +6535,26 @@
|
|||||||
ren.setClearColor(0x101820, _bcActive() ? 0 : 1);
|
ren.setClearColor(0x101820, _bcActive() ? 0 : 1);
|
||||||
wrap.appendChild(ren.domElement);
|
wrap.appendChild(ren.domElement);
|
||||||
|
|
||||||
|
// WebGL context-loss recovery (see the _ctxLost declaration). Bound
|
||||||
|
// on Three's own canvas — the context that actually resets on a GPU
|
||||||
|
// reset / alt-tab. preventDefault() keeps the context restorable
|
||||||
|
// instead of letting the loss escalate to a render-process crash;
|
||||||
|
// _ctxLost then makes draw() bail so no GL work runs on the dead
|
||||||
|
// context; on restore we reset the viewport and resume (Three
|
||||||
|
// re-uploads geometry/materials/textures lazily on the next render).
|
||||||
|
_onCtxLost = (e) => {
|
||||||
|
if (e && typeof e.preventDefault === 'function') e.preventDefault();
|
||||||
|
_ctxLost = true;
|
||||||
|
console.warn('[3D-Hwy] WebGL context lost — pausing render until it is restored.');
|
||||||
|
};
|
||||||
|
_onCtxRestored = () => {
|
||||||
|
_ctxLost = false;
|
||||||
|
console.warn('[3D-Hwy] WebGL context restored — resuming render.');
|
||||||
|
try { const s = canvasSize(highwayCanvas); if (s.w > 0 && s.h > 0) applySize(s.w, s.h); } catch (err) {}
|
||||||
|
};
|
||||||
|
ren.domElement.addEventListener('webglcontextlost', _onCtxLost, false);
|
||||||
|
ren.domElement.addEventListener('webglcontextrestored', _onCtxRestored, false);
|
||||||
|
|
||||||
lyricsCanvas = document.createElement('canvas');
|
lyricsCanvas = document.createElement('canvas');
|
||||||
lyricsCanvas.style.cssText = 'position:absolute;top:0;left:0;pointer-events:none;z-index:1;';
|
lyricsCanvas.style.cssText = 'position:absolute;top:0;left:0;pointer-events:none;z-index:1;';
|
||||||
lyricsCtx = lyricsCanvas.getContext('2d');
|
lyricsCtx = lyricsCanvas.getContext('2d');
|
||||||
@@ -14829,6 +14858,15 @@
|
|||||||
// mid-teardown settings change doesn't try to rebuild a torn-
|
// mid-teardown settings change doesn't try to rebuild a torn-
|
||||||
// down scene; then dispose the active style's resources.
|
// down scene; then dispose the active style's resources.
|
||||||
if (_bgListener) { _bgUnsubscribe(_bgListener); _bgListener = null; }
|
if (_bgListener) { _bgUnsubscribe(_bgListener); _bgListener = null; }
|
||||||
|
// WebGL context-loss listeners (bound in initScene on ren.domElement).
|
||||||
|
// Remove before ren is disposed below so a torn-down instance can't
|
||||||
|
// keep firing them; reset the flag so a reused instance starts clean.
|
||||||
|
if (ren && ren.domElement) {
|
||||||
|
if (_onCtxLost) { try { ren.domElement.removeEventListener('webglcontextlost', _onCtxLost, false); } catch (e) {} }
|
||||||
|
if (_onCtxRestored) { try { ren.domElement.removeEventListener('webglcontextrestored', _onCtxRestored, false); } catch (e) {} }
|
||||||
|
}
|
||||||
|
_onCtxLost = _onCtxRestored = null;
|
||||||
|
_ctxLost = false;
|
||||||
// Notedetect listeners (issue #9). Remove on destroy so a
|
// Notedetect listeners (issue #9). Remove on destroy so a
|
||||||
// panel that stops doesn't keep accumulating marks. Marks
|
// panel that stops doesn't keep accumulating marks. Marks
|
||||||
// arrays are cleared too — they hold stale chart positions
|
// arrays are cleared too — they hold stale chart positions
|
||||||
@@ -15154,6 +15192,7 @@
|
|||||||
|
|
||||||
draw(bundle) {
|
draw(bundle) {
|
||||||
if (!_isReady) return;
|
if (!_isReady) return;
|
||||||
|
if (_ctxLost) return; // GPU context lost (alt-tab / reset) — skip until restored
|
||||||
if (!_chartPrewarmed) {
|
if (!_chartPrewarmed) {
|
||||||
_chartPrewarmed = true;
|
_chartPrewarmed = true;
|
||||||
_prewarmChart(bundle);
|
_prewarmChart(bundle);
|
||||||
|
|||||||
@@ -0,0 +1,45 @@
|
|||||||
|
// Contract test: 3D Highway WebGL context-loss recovery.
|
||||||
|
//
|
||||||
|
// Switching the active window / alt-tabbing (especially on Windows) can trigger
|
||||||
|
// a GPU context reset. Without a handler the lost WebGL context escalates into a
|
||||||
|
// render-process crash. The renderer owns its own WebGL canvas + heavy Three.js
|
||||||
|
// lifecycle (too much to construct in a vm sandbox), so — like the other
|
||||||
|
// highway_* source-contract tests here — this pins the wiring at the source
|
||||||
|
// level: the loss must be preventDefault()'d (so the browser restores it), draw
|
||||||
|
// must bail while lost, and the listeners must be torn down.
|
||||||
|
|
||||||
|
const { test } = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const fs = require('node:fs');
|
||||||
|
const path = require('node:path');
|
||||||
|
|
||||||
|
const SCREEN_JS = path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'screen.js');
|
||||||
|
const src = fs.readFileSync(SCREEN_JS, 'utf8');
|
||||||
|
|
||||||
|
test('binds webglcontextlost + webglcontextrestored on the renderer canvas', () => {
|
||||||
|
assert.match(src, /ren\.domElement\.addEventListener\(\s*['"]webglcontextlost['"]/,
|
||||||
|
'must listen for webglcontextlost on ren.domElement (the WebGL canvas)');
|
||||||
|
assert.match(src, /ren\.domElement\.addEventListener\(\s*['"]webglcontextrestored['"]/,
|
||||||
|
'must listen for webglcontextrestored on ren.domElement');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('the context-lost handler preventDefaults and pauses drawing', () => {
|
||||||
|
// Without preventDefault() the browser will not attempt to restore the
|
||||||
|
// context and the loss can escalate to a renderer crash.
|
||||||
|
const m = src.match(/_onCtxLost\s*=\s*\(e\)\s*=>\s*\{[\s\S]*?\};/);
|
||||||
|
assert.ok(m, '_onCtxLost handler must exist');
|
||||||
|
assert.match(m[0], /preventDefault\(\)/, 'context-lost handler must call preventDefault()');
|
||||||
|
assert.match(m[0], /_ctxLost\s*=\s*true/, 'context-lost handler must set _ctxLost = true');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('draw() early-returns while the context is lost', () => {
|
||||||
|
assert.match(src, /draw\(bundle\)\s*\{[\s\S]*?if\s*\(_ctxLost\)\s*return;/,
|
||||||
|
'draw() must bail while _ctxLost is set so no GL work runs on a dead context');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('teardown removes the context-loss listeners', () => {
|
||||||
|
assert.match(src, /removeEventListener\(\s*['"]webglcontextlost['"]/,
|
||||||
|
'teardown must remove the webglcontextlost listener');
|
||||||
|
assert.match(src, /removeEventListener\(\s*['"]webglcontextrestored['"]/,
|
||||||
|
'teardown must remove the webglcontextrestored listener');
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user