diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index 10353bc95..cbbf3a7ce 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -5820,6 +5820,7 @@ return; } setLiveState('CYCLING'); + hideShaderOverlay(); showOrUpdateCyclingBar(); saveSession(); completeParameterGenerationIfReady(); @@ -7166,6 +7167,7 @@ if (arrivedVariants >= expectedVariants && expectedVariants > 0) { if (state === 'GENERATING') { setLiveState('CYCLING'); + hideShaderOverlay(); showOrUpdateCyclingBar(); disableInlineEdit(); refreshParamsPanel(); @@ -7226,6 +7228,7 @@ pendingAcceptedSession = null; awaitingAcceptResult = null; setLiveState('CYCLING'); + hideShaderOverlay(); updateBarContent('cycling'); showToast('Could not complete accept cleanup. Try Accept again.', 5000); break; @@ -8328,6 +8331,15 @@ void main() { // matches the original off-white risograph paper. const SHADER_PAPER_FALLBACK = [0.975, 0.965, 0.955]; let shaderState = null; // { canvas, gl, program, texture, rafId, startTime } + // showShaderOverlay is async: it appends its canvas, then awaits + // createImageBitmap and the GL setup before it publishes shaderState. A + // teardown that landed inside that window found shaderState still null, + // returned, and then watched the construction publish itself over a session + // that had already left GENERATING, with no teardown left to run. That is + // the generating loader frozen over a page that already cycles (issue #719). + // Every teardown bumps this epoch; a construction abandons its own canvas as + // soon as it sees the epoch move. + let shaderEpoch = 0; // The element's effective background tone, used as the uniform halftone // ground so content dissolves into dots over it. Unlike resolveCanvasBackground @@ -8474,14 +8486,28 @@ void main() { }); } + /** Drop a shader node no shaderState owns (an abandoned construction). */ + function removeStrayShaderNode() { + const stray = uiGetById(PREFIX + '-shader'); + if (stray) stray.remove(); + } + function hideShaderOverlay() { - if (!shaderState) return; + // Bump first, unconditionally: this is what tells an in-flight + // showShaderOverlay to abandon itself rather than publish over a session + // that has already moved on. + shaderEpoch += 1; + if (!shaderState) { + removeStrayShaderNode(); + return; + } if (shaderState.rafId) cancelAnimationFrame(shaderState.rafId); if (shaderState.canvas) shaderState.canvas.remove(); if (shaderState.objectUrl) URL.revokeObjectURL(shaderState.objectUrl); const lose = shaderState.gl?.getExtension?.('WEBGL_lose_context'); try { lose?.loseContext(); } catch {} shaderState = null; + removeStrayShaderNode(); } function showShaderBitmapFallback(canvas, blob) { @@ -8506,6 +8532,16 @@ void main() { async function showShaderOverlay(el, blob, rect, paper) { hideShaderOverlay(); if (!blob || !el) return; + // hideShaderOverlay just bumped the epoch, so this run owns it until the + // next teardown. Every step past an await re-checks before it publishes. + const epoch = shaderEpoch; + const abandoned = (node, gl) => { + if (epoch === shaderEpoch) return false; + node.remove(); + const lose = gl?.getExtension?.('WEBGL_lose_context'); + try { lose?.loseContext(); } catch {} + return true; + }; const canvas = document.createElement('canvas'); canvas.id = PREFIX + '-shader'; const dpr = Math.min(window.devicePixelRatio || 1, 2); @@ -8528,6 +8564,7 @@ void main() { if (!gl) { // WebGL unavailable: use the captured bitmap as a background overlay so // the user still sees something meaningful during generation. + if (abandoned(canvas, null)) return; showShaderBitmapFallback(canvas, blob); return; } @@ -8567,16 +8604,22 @@ void main() { } // Upload the screenshot as a texture + if (abandoned(canvas, gl)) return; let bitmap; try { bitmap = await createImageBitmap(blob); } catch (err) { console.warn('[impeccable] shader bitmap decode failed:', err); + if (abandoned(canvas, gl)) return; const lose = gl.getExtension?.('WEBGL_lose_context'); try { lose?.loseContext(); } catch {} showShaderBitmapFallback(canvas, blob); return; } + if (abandoned(canvas, gl)) { + if (bitmap.close) bitmap.close(); + return; + } texture = gl.createTexture(); gl.bindTexture(gl.TEXTURE_2D, texture); gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_S, gl.CLAMP_TO_EDGE); @@ -8595,6 +8638,7 @@ void main() { const paperRgb = paper || resolvePaperRgb(el); const reduced = window.matchMedia('(prefers-reduced-motion: reduce)').matches; + if (abandoned(canvas, gl)) return; shaderState = { canvas, gl, program, texture, rafId: 0, startTime: performance.now(), reduced }; function frame() { if (!shaderState) return; @@ -8674,6 +8718,7 @@ void main() { .catch(() => { if (pendingAcceptedSession?.id === acceptedSessionId) pendingAcceptedSession = null; setLiveState('CYCLING'); + hideShaderOverlay(); showOrUpdateCyclingBar(); showToast('Could not confirm accept with the live server. Session kept for recovery; try Accept again.', 5000); }); diff --git a/tests/live-browser-source.test.mjs b/tests/live-browser-source.test.mjs index f2aaa0b46..c87db08f6 100644 --- a/tests/live-browser-source.test.mjs +++ b/tests/live-browser-source.test.mjs @@ -564,6 +564,67 @@ describe('live-browser source contracts', () => { ); }); + it('never leaves a shader behind when the teardown races its construction (#719)', () => { + // showShaderOverlay appends its canvas, then awaits createImageBitmap and + // the GL setup before it publishes shaderState. A teardown inside that + // window found shaderState null, returned, and then watched the + // construction publish itself over a session that had already reached + // CYCLING, with no teardown left to run. On a slow runner that is the + // generating loader frozen over a page that already cycles. + const hideStart = SOURCE.indexOf('function hideShaderOverlay()'); + const hide = SOURCE.slice(hideStart, SOURCE.indexOf('\n function ', hideStart + 10)); + assert.match( + hide, + /shaderEpoch \+= 1;[\s\S]{0,120}?if \(!shaderState\) \{/, + 'the epoch must be bumped before the no-state early return, or an in-flight construction never hears about the teardown', + ); + assert.match(hide, /removeStrayShaderNode\(\);/, 'a teardown must also drop a shader node no state owns'); + + const showStart = SOURCE.indexOf('async function showShaderOverlay('); + const show = SOURCE.slice(showStart, SOURCE.indexOf('\n async function handleAccept', showStart)); + assert.match(show, /const epoch = shaderEpoch;/, 'the construction must pin the epoch it owns'); + assert.match( + show, + /const abandoned = \(node, gl\) => \{[\s\S]{0,80}?if \(epoch === shaderEpoch\) return false;[\s\S]{0,200}?return true;/, + 'abandoning must remove the canvas and release the GL context', + ); + assert.match( + show, + /if \(abandoned\(canvas, gl\)\) return;\n shaderState = \{ canvas, gl, program, texture,/, + 'the publish must be guarded by the epoch it pinned', + ); + const awaitIdx = show.indexOf('await createImageBitmap(blob)'); + assert.ok(awaitIdx > 0, 'createImageBitmap is the await this guards'); + assert.ok( + show.indexOf('if (abandoned(canvas, gl))', awaitIdx) > awaitIdx, + 'the bitmap await must be followed by an abandonment check', + ); + for (const call of ['showShaderBitmapFallback(canvas, blob);']) { + let at = show.indexOf(call); + assert.ok(at > 0, call); + while (at > 0) { + const before = show.slice(Math.max(0, at - 220), at); + assert.match(before, /abandoned\(canvas, (?:gl|null)\)/, 'every fallback publish must be epoch guarded'); + at = show.indexOf(call, at + 1); + } + } + }); + + it('lowers the shader on every route that sets CYCLING (#719)', () => { + // resumeSession reaches CYCLING through setLiveState(resumedState), which + // its own block covers; every literal site has to lower the loader too. + const sites = [...SOURCE.matchAll(/setLiveState\('CYCLING'\);/g)].map((m) => m.index); + assert.ok(sites.length >= 8, `expected the known CYCLING sites, saw ${sites.length}`); + for (const at of sites) { + const after = SOURCE.slice(at, at + 260); + assert.match( + after, + /hideShaderOverlay\(\);/, + `a setLiveState('CYCLING') at offset ${at} does not lower the generating shader`, + ); + } + }); + it('prefers the wrapper that actually holds variants over the first match (#719)', () => { // A target inside a `.map()` renders one wrapper per item, and an agent // that relocates the wrapper out of the shared primitive live-wrap