Repository navigation
fix(core): preview recovers its WebGL scenes and effects after a lost context - #7028
Conversation
…es a lost WebGL context
…after a context restore
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 3 blocker · 0 important · 0 nits · head 7044c1aa
The 'fails without the fix:' line is present and names the two tests. Blockers: one rebuild-ordering bug and two CI gates this diff turns red ↓ inline.
Scope
Read all 6 files end-to-end, plus initVfx, registerVfxHost, buildPasses, releaseRegistry and applySeek on the head. No prior reviews. CI at review time: Comments ✗, Preflight format:check ✗ (which also fails Player perf and preview-regression); CI, CodeQL, Windows and Safari runs still pending.
— Genesis (Lane B)
|
|
||
| /** Rebuilds every chain when a context was lost, so a restored one paints again. */ | ||
| export function rebuildLostVfx(root: HTMLElement): void { | ||
| if (registry.some((entry) => entry.contextLost)) initVfx(root, registryFps); |
There was a problem hiding this comment.
blocker — The rebuild runs as soon as any canvas restores, even if other vfx contexts are still lost. Browsers send webglcontextrestored once per context, so a Three.js canvas or the first .hf-vfx-out can restore a frame before the rest. When that happens, initVfx → registerVfxHost sees the lost context: createShader returns null, buildPasses reports "has no usable program", and out.remove() detaches the output canvas (vfx.ts:596-599). That host is now out of the registry, and its later restore event fires on a detached canvas, which the document capture listener never sees. So the effect stays blank for the whole session, which is the bug this PR is fixing.
A host whose context is still lost but survives registration is also stuck: it gets contextLost: false because its loss event already fired, so rebuildLostVfx never runs for it again.
Fix: rebuild only once no entry is still lost (registry.every((e) => !e.gl.isContextLost()), so the last restore triggers it). Alternatively, skip and keep entries whose gl.isContextLost() is still true. Add a test where two hosts lose their contexts and only one has restored when the frame runs.
There was a problem hiding this comment.
Fixed at 1cefca8. The rebuild now waits until no effect context is still lost, so an early restore no longer drops a host. The test shows it: with the wait removed it fails, with it the rebuild runs once the last context is back. Format and comment-ratchet items are fixed too.
| // Seeking to the time a timeline is already at calls no onUpdate, so step off it first. | ||
| const tl = state.capturedTimeline; | ||
| const at = state.currentTime; | ||
| if (tl && typeof tl.totalTime === "function") tl.totalTime(at > 1e-3 ? at - 1e-4 : at + 1e-4, false); |
There was a problem hiding this comment.
blocker (CI) — Preflight format:check fails on packages/core/src/runtime/init.ts because this line is longer than the formatter's width. The same Preflight failure is why Player perf and preview-regression are red. Run the formatter.
| @@ -0,0 +1,22 @@ | |||
| /** A lost context is allowed to restore; one repaint follows after the scene's own restore handlers ran. */ | |||
There was a problem hiding this comment.
blocker (CI) — The Comments ratchet fails: this new file is 13.0% comments (3/23 lines), and the package is 12.4%. Cutting one of the three comments, or shortening this doc line into the function name, gets it under.
Edit accuracy: accurate 271 (base branch 271), smooth 105 of thoseThe gate passes. Quarantined, measured but not gated (0) |
What
When a GPU process crash drops every WebGL context in the preview, the runtime now recovers what it can:
data-vfx-chain) are rebuilt throughinitVfx, the one rebuild path that already existed, and repainted.onUpdate, so a Three.js scene driven fromonUpdatestayed blank until the next seek. A playing scene redraws on its own next tick.Why
Windows GPU process crashes hit about 1.2% of launches and take every WebGL context with them. Before this, a paused preview showed blank 3D and blank effects until the user seeked, and effects stayed blank for the whole session.
Related work
Refs the earlier grading-only recovery in
colorGrading.ts(its own canvases already rebuild; this adds the rest).How
What goes wrong: the browser only restores a lost context if the page cancels the loss event, scene canvases belong to the composition so nothing cancelled it, effect chains were marked lost for good, and a paused timeline has no reason to call a scene's
onUpdateagain.What the runtime does: one capture-phase listener on the document sees every canvas, cancels the loss, and after the restore asks for one repaint. The effect chains are rebuilt with the existing
initVfxpass and repainted, and a paused timeline is stepped off its current time and back so its scene draws. A playing transport redraws on its next tick. A scene that never re-creates its own GL resources is out of reach (see the table).Test plan
Measured on the devbox in headless Chromium with swiftshader, one page per state, a composition holding a Three.js cube, a custom WebGL triangle and a vfx host. Each context is lost and restored with
WEBGL_lose_context; "lit" is the share of non-black pixels in a real screenshot of that box.The custom WebGL scene stays blank: it creates its program once and never re-creates it after a restore, so only its own code (or a reload of the composition frame) can bring it back. Not done here.
Fails without the fix: with
init.tsrestored to main, the newentry.test.tstest "redraws a paused scene by stepping its timeline off the current time and back after a WebGL restore" fails (1 failed, 114 passed); withvfx.tsstripped of the rebuild, "paints a host again after its lost context is restored and the chains are rebuilt" fails (1 failed, 57 passed). Coresrc/runtimeon the devbox: 113 files, 2729 tests passed at b610439. The rest of the core package has 9 files that fail to import@hyperframes/lintand@hyperframes/studio-serverbecause those packages were not built on that tree; none touch this change.webglContextRecovery.test.ts,vfx.test.ts,entry.test.ts