From 3722891d48e35230e94808a57459a4ff96ea7709 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:55:31 +0200 Subject: [PATCH] fix(3d): blank render + lost position after 3D viewer close/reopen (gl1 context guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing the viewer destroys its wxGLCanvas's WebGL context; reopening mints a new one. The gl1 shim cached GL names (FFP program, stream/scratch VBOs) in never-reset statics behind `if (!handle)` guards — in the new context every draw died with INVALID_OPERATION and the viewer showed only the clear color ("Reload time 0.031 s" is benign: warm model caches make the rebuild fast). contextSync() (gl1_state.cpp) now detects the context change in programSync() — the one choke point every shim draw crosses, and a path the 2D GAL never reaches (a first attempt checking in the glBindTexture wrap saw the GAL's context and thrash-rebuilt the program 23x per run) — and drops the cached names for lazy rebuild in the new context. Context identity is a monotonic id stamped on Emscripten's per-context record: the numeric EMSCRIPTEN_WEBGL_CONTEXT_HANDLE is recycled, so a destroy-then-create can return the same number and a handle comparison detects nothing. The lost-position half is a wxwidgets wasm fix (pointer bump: GetFromWindow reports display 0; saved geometry used to carry display=(unsigned)-1, which LoadWindowState treats as "display not found" and re-centres the frame). TDD (red observed before each fix, green after): - tests/kicad/3d-viewer-reopen.spec.ts (new, own worker like the deadlock spec): load board, open viewer, render-gate, drag by the titlebar, close via the x, reopen; asserts the board re-renders (was: 1 distinct colour for 90 s) and the window position is restored (was: re-centred to 0,0 after closing at 40,90). Green run logs exactly one [gl1] context-change line. - 3d-regression harness: recreateContext() destroys the context AND swaps in a fresh canvas element (a browser canvas keeps its context for life, so same-element recreation hands back the live old context and hides the bug); the new 3d-webgl spec test renders redraw-mini-board-navigator before and after recreation and requires pixel-identical output. Parity: 47/47, zero drift. Co-Authored-By: Claude Fable 5 --- tests/3d-regression/wasm/3d_webgl_test.cpp | 88 +++++++-- tests/3d-regression/wasm/3d_webgl_test.html | 1 + tests/3d-regression/wasm/Makefile | 2 +- tests/apps/3d-webgl/3d_webgl_test.html | 1 + tests/e2e/3d-webgl.spec.ts | 101 ++++++++++ tests/kicad/3d-viewer-reopen.spec.ts | 202 ++++++++++++++++++++ wasm/gl1/README.md | 16 ++ wasm/gl1/include/gl1_shim.h | 17 ++ wasm/gl1/src/gl1_draw.cpp | 9 + wasm/gl1/src/gl1_entry_wrapped.cpp | 7 + wasm/gl1/src/gl1_shaders.cpp | 16 ++ wasm/gl1/src/gl1_state.cpp | 62 ++++++ wxwidgets | 2 +- 13 files changed, 504 insertions(+), 20 deletions(-) create mode 100644 tests/kicad/3d-viewer-reopen.spec.ts diff --git a/tests/3d-regression/wasm/3d_webgl_test.cpp b/tests/3d-regression/wasm/3d_webgl_test.cpp index fa78d61..530f5b7 100644 --- a/tests/3d-regression/wasm/3d_webgl_test.cpp +++ b/tests/3d-regression/wasm/3d_webgl_test.cpp @@ -25,6 +25,33 @@ static const int CAPTURE_HEIGHT = 600; static EMSCRIPTEN_WEBGL_CONTEXT_HANDLE g_context = 0; static SCENE3D_CTX* g_ctx = nullptr; +// One context-creation recipe for the initial context (main) and recreateContext(): +// the recreated context must carry the exact attributes the suite requires, or a +// close/reopen render would differ from the goldens for attribute reasons alone. +static bool createContext() +{ + EmscriptenWebGLContextAttributes attrs; + emscripten_webgl_init_context_attributes( &attrs ); + + attrs.majorVersion = 2; + attrs.minorVersion = 0; + attrs.alpha = false; + attrs.depth = true; + attrs.stencil = true; // DrawCulled hole subtraction + attrs.antialias = false; // native goldens are single-sample + attrs.preserveDrawingBuffer = true; // Playwright canvas.screenshot() + + emscripten_set_canvas_element_size( "#canvas", CAPTURE_WIDTH, CAPTURE_HEIGHT ); + + g_context = emscripten_webgl_create_context( "#canvas", &attrs ); + + if( g_context <= 0 ) + return false; + + emscripten_webgl_make_context_current( g_context ); + return true; +} + extern "C" { @@ -82,35 +109,60 @@ int runScenario( int aIndex ) return 0; } +// Destroy the current WebGL context and mint a fresh one on a fresh canvas +// element — the harness model of the app's 3D-viewer close/reopen (~wxGLCanvas +// destroys its context AND its DOM canvas; a new frame creates new ones). +// The element swap is essential: a browser canvas keeps its WebGL context for +// life, so recreating on the SAME element would hand back the same live context +// and old GL object names would still work — hiding the very bug this models. +// Deliberately NO shim call here: the gl1 layer itself must detect the context +// change on its next draw and rebuild its cached GL objects, exactly as +// production code paths require. +EMSCRIPTEN_KEEPALIVE +int recreateContext() +{ + // The scene ctx is rebuilt per context, like the app's per-canvas renderer. + // Delete it while the old context is still alive (mirrors releaseOpenGL()). + delete g_ctx; + g_ctx = nullptr; + + if( g_context > 0 ) + emscripten_webgl_destroy_context( g_context ); + + g_context = 0; + + EM_ASM( { + var old = Module['canvas']; + var fresh = old.cloneNode( false ); // same id/width/height, no context yet + old.parentNode.replaceChild( fresh, old ); + Module['canvas'] = fresh; + // '#canvas' resolves through specialHTMLTargets before querySelector. + if( typeof specialHTMLTargets !== 'undefined' ) + specialHTMLTargets['#canvas'] = fresh; + } ); + + if( !createContext() ) + { + std::fprintf( stderr, "[3d-webgl] recreateContext: failed to create WebGL2 context\n" ); + return -1; + } + + std::printf( "[3d-webgl] context recreated\n" ); + return 0; +} + } // extern "C" int main() { - EmscriptenWebGLContextAttributes attrs; - emscripten_webgl_init_context_attributes( &attrs ); - - attrs.majorVersion = 2; - attrs.minorVersion = 0; - attrs.alpha = false; - attrs.depth = true; - attrs.stencil = true; // DrawCulled hole subtraction - attrs.antialias = false; // native goldens are single-sample - attrs.preserveDrawingBuffer = true; // Playwright canvas.screenshot() - - emscripten_set_canvas_element_size( "#canvas", CAPTURE_WIDTH, CAPTURE_HEIGHT ); - - g_context = emscripten_webgl_create_context( "#canvas", &attrs ); - - if( g_context <= 0 ) + if( !createContext() ) { std::fprintf( stderr, "[3d-webgl] failed to create WebGL2 context (%ld)\n", (long) g_context ); return 1; } - emscripten_webgl_make_context_current( g_context ); - std::printf( "[3d-webgl] ready: %d scenarios, %dx%d\n", Scene3DTest::GetScenarioCount(), CAPTURE_WIDTH, CAPTURE_HEIGHT ); diff --git a/tests/3d-regression/wasm/3d_webgl_test.html b/tests/3d-regression/wasm/3d_webgl_test.html index 519c42c..63b70d0 100644 --- a/tests/3d-regression/wasm/3d_webgl_test.html +++ b/tests/3d-regression/wasm/3d_webgl_test.html @@ -23,6 +23,7 @@ getScenarioName: (i) => wasmModule.ccall('getScenarioName', 'string', ['number'], [i]), getCanvasWidth: () => wasmModule.ccall('getCanvasWidth', 'number', [], []), getCanvasHeight: () => wasmModule.ccall('getCanvasHeight', 'number', [], []), + recreateContext: () => wasmModule.ccall('recreateContext', 'number', [], []), }; diff --git a/tests/3d-regression/wasm/Makefile b/tests/3d-regression/wasm/Makefile index 58f28e9..339cfc1 100644 --- a/tests/3d-regression/wasm/Makefile +++ b/tests/3d-regression/wasm/Makefile @@ -69,7 +69,7 @@ CFLAGS = $(OPT_FLAGS) $(DEPS_EH_FLAGS) -I$(PROJECT_ROOT)/wasm/stubs BASE_LDFLAGS = $(DEPS_EH_FLAGS) \ -sALLOW_MEMORY_GROWTH=1 \ -sERROR_ON_UNDEFINED_SYMBOLS=0 \ - -sEXPORTED_FUNCTIONS=['_main','_runScenario','_getTotalScenarios','_getScenarioName','_getCanvasWidth','_getCanvasHeight'] \ + -sEXPORTED_FUNCTIONS=['_main','_runScenario','_getTotalScenarios','_getScenarioName','_getCanvasWidth','_getCanvasHeight','_recreateContext'] \ -sEXPORTED_RUNTIME_METHODS=['ccall','cwrap'] \ -sMODULARIZE=1 \ -sEXPORT_NAME='create3DTest' \ diff --git a/tests/apps/3d-webgl/3d_webgl_test.html b/tests/apps/3d-webgl/3d_webgl_test.html index 519c42c..63b70d0 100644 --- a/tests/apps/3d-webgl/3d_webgl_test.html +++ b/tests/apps/3d-webgl/3d_webgl_test.html @@ -23,6 +23,7 @@ getScenarioName: (i) => wasmModule.ccall('getScenarioName', 'string', ['number'], [i]), getCanvasWidth: () => wasmModule.ccall('getCanvasWidth', 'number', [], []), getCanvasHeight: () => wasmModule.ccall('getCanvasHeight', 'number', [], []), + recreateContext: () => wasmModule.ccall('recreateContext', 'number', [], []), }; diff --git a/tests/e2e/3d-webgl.spec.ts b/tests/e2e/3d-webgl.spec.ts index ff45033..3d90e48 100644 --- a/tests/e2e/3d-webgl.spec.ts +++ b/tests/e2e/3d-webgl.spec.ts @@ -58,6 +58,107 @@ test.describe('3D WebGL Regression', () => { expect(height).toBe(MANIFEST.height); }); + /** + * Regression: WebGL context recreation (the 3D-viewer close/reopen model). + * + * The app's viewer close destroys the wxGLCanvas's WebGL context and DOM canvas; + * reopen creates fresh ones. The gl1 shim caches GL object names (FFP program, + * stream/scratch VBOs) in statics — pre-fix it kept using names owned by the + * destroyed context in the new one, so every draw died with INVALID_OPERATION and + * the reopened viewer rendered blank. recreateContext() reproduces exactly that + * (fresh canvas element + fresh context, no shim call — the shim must self-detect). + * + * Renders redraw-mini-board-navigator (the port-complete gate scenario: display + * lists, VBO models, grid, gizmo, materials) before and after recreation and + * requires the second render to be non-blank and pixel-identical-ish to the first. + */ + test('re-renders identically after WebGL context recreation', async ({ page }) => { + test.setTimeout(120000); + + await page.goto('/3d-webgl/3d_webgl_test.html'); + await page.waitForFunction(() => (window as any).threeDTest?.isReady(), undefined, { + timeout: 60000, + }); + + const name = 'redraw-mini-board-navigator'; + const idx = MANIFEST.scenarios.indexOf(name); + expect(idx, `${name} must exist in the committed manifest`).toBeGreaterThanOrEqual(0); + + // Snapshot the canvas backing store into an in-page buffer (one drawImage readback, + // CPU-backed 2D canvas — SwiftShader-safe) plus a 16x16 distinct-colour count. + const capture = (slot: string) => + page.evaluate((s) => { + const el = document.getElementById('canvas') as HTMLCanvasElement; + const tmp = document.createElement('canvas'); + tmp.width = el.width; + tmp.height = el.height; + const ctx = tmp.getContext('2d', { willReadFrequently: true })!; + ctx.drawImage(el, 0, 0); + const img = ctx.getImageData(0, 0, el.width, el.height).data; + (window as any)[s] = img; + const colors = new Set(); + for (let i = 0; i < 16; i++) { + for (let j = 0; j < 16; j++) { + const p = + (Math.floor((el.height * j) / 16) * el.width + Math.floor((el.width * i) / 16)) * 4; + colors.add(`${img[p]},${img[p + 1]},${img[p + 2]}`); + } + } + return colors.size; + }, slot); + + const first = await page.evaluate((i) => (window as any).threeDTest.runScenario(i), idx); + expect(first, `runScenario(${idx}) [${name}] before recreation`).toBe(0); + await page.evaluate(() => new Promise(requestAnimationFrame)); + const colorsBefore = await capture('__reopenFirst'); + expect(colorsBefore, 'the scenario must render before the context swap').toBeGreaterThan(8); + // NOT in OUTPUT_DIR: the parity compare treats that dir as scenario renders. + await page + .locator('#canvas') + .screenshot({ path: path.join(OUTPUT_DIR, '..', `3d-ctx-recreate-before.png`) }); + + const rc = await page.evaluate(() => (window as any).threeDTest.recreateContext()); + expect(rc, 'recreateContext() should mint a fresh WebGL context').toBe(0); + + const second = await page.evaluate((i) => (window as any).threeDTest.runScenario(i), idx); + expect(second, `runScenario(${idx}) [${name}] after recreation`).toBe(0); + await page.evaluate(() => new Promise(requestAnimationFrame)); + const colorsAfter = await capture('__reopenSecond'); + await page + .locator('#canvas') + .screenshot({ path: path.join(OUTPUT_DIR, '..', `3d-ctx-recreate-after.png`) }); + + expect( + colorsAfter, + 'the re-rendered scenario must not be blank — a uniform canvas means the gl1 shim ' + + 'drew with GL object names from the destroyed context' + ).toBeGreaterThan(8); + + // Pixel-level identity check (same code, same context attributes → deterministic). + // Tolerance mirrors the suite's pixelmatch spirit: <0.1% differing pixels. + const diff = await page.evaluate(() => { + const a = (window as any).__reopenFirst as Uint8ClampedArray; + const b = (window as any).__reopenSecond as Uint8ClampedArray; + if (!a || !b || a.length !== b.length) return { changed: -1, total: 0 }; + let changed = 0; + for (let p = 0; p < a.length; p += 4) { + if ( + Math.abs(a[p] - b[p]) > 2 || + Math.abs(a[p + 1] - b[p + 1]) > 2 || + Math.abs(a[p + 2] - b[p + 2]) > 2 + ) + changed++; + } + return { changed, total: a.length / 4 }; + }); + expect(diff.changed, 'both captures must exist and agree in size').toBeGreaterThanOrEqual(0); + expect( + diff.changed / diff.total, + `render after context recreation must match the one before ` + + `(${diff.changed}/${diff.total} pixels differ)` + ).toBeLessThan(0.001); + }); + test('render all scenarios', async ({ page }) => { test.setTimeout(300000); diff --git a/tests/kicad/3d-viewer-reopen.spec.ts b/tests/kicad/3d-viewer-reopen.spec.ts new file mode 100644 index 0000000..7c04f07 --- /dev/null +++ b/tests/kicad/3d-viewer-reopen.spec.ts @@ -0,0 +1,202 @@ +import { test, expect } from './fixtures'; +import { shotPath } from '../e2e/utils/element-tracker'; +import { waitForPcbnew } from './utils/pcbnew-ready'; +import { DEMO, loadBoard, countGlCanvases, logThreeDDiag, openThreeDViewer, waitForThreeDRender } + from './utils/threed-viewer'; + +/** + * Regression: close the 3D viewer, reopen it — it must render again, in the same place. + * + * Closing the viewer Destroy()s the frame; ~wxGLCanvas destroys the WebGL context and its + * DOM canvas (wxwidgets/src/wasm/glcanvas.cpp). Reopening creates a fresh frame + canvas + + * context. Two independent bugs surfaced on that path: + * + * 1. BLANK CANVAS — the wasm/gl1 FFP shim cached GL object names (FFP program, stream and + * scratch VBOs) in file-scope statics guarded by `if (!handle)`, so in the new context it + * kept using names owned by the destroyed context: every draw died with INVALID_OPERATION + * and the reopened viewer showed only the clear color ("Reload time 0.031 s" — the scene + * rebuild itself is fine, warm model caches make it fast). The shim now detects the + * context change and rebuilds its GL objects. + * + * 2. LOST POSITION — wxWindowWasm::GetHandle() returns NULL, so wxDisplay::GetFromWindow() + * was wxNOT_FOUND (-1); EDA_BASE_FRAME::SaveWindowSettings stored display = (unsigned)-1, + * and LoadWindowState's "previous display not found" branch re-centred the frame on every + * reopen instead of restoring the saved position. The wasm display factory now reports + * display 0 for any created window. + * + * ISOLATED in its own spec file (own Playwright worker → own browser process) like the + * deadlock spec: one heavy pcbnew load per process keeps the emscripten Worker pool and the + * shared GPU-process context budget predictable on CI. + */ + +// Sample the NEWEST glcanvas backing store: distinct colours on a 16x16 grid. One +// drawImage readback per call (CPU-backed 2D canvas) — same technique as +// waitForThreeDRender, kept SwiftShader-safe (no per-pixel GPU round-trips). +function sampleNewestGlCanvasColors(page: import('@playwright/test').Page): Promise { + return page.evaluate(() => { + const list = document.querySelectorAll('canvas[id^="glcanvas-"]'); + const el = list[list.length - 1] as HTMLCanvasElement | undefined; + if (!el || !el.width || !el.height) return 0; + const tmp = document.createElement('canvas'); + tmp.width = el.width; tmp.height = el.height; + const ctx = tmp.getContext('2d', { willReadFrequently: true })!; + ctx.drawImage(el, 0, 0); + const img = ctx.getImageData(0, 0, el.width, el.height).data; + const colors = new Set(); + for (let i = 0; i < 16; i++) { + for (let j = 0; j < 16; j++) { + const p = (Math.floor(el.height * j / 16) * el.width + + Math.floor(el.width * i / 16)) * 4; + colors.add(`${img[p]},${img[p + 1]},${img[p + 2]}`); + } + } + return colors.size; + }); +} + +// The viewer's top-level window div position, from its inline style (the wasm DOM port +// positions windows via style.left/top). +function windowPos(page: import('@playwright/test').Page, winId: string) { + return page.evaluate((id) => { + const el = document.getElementById(id) as HTMLElement | null; + if (!el) return null; + return { + left: parseInt(el.style.left || '0', 10) || 0, + top: parseInt(el.style.top || '0', 10) || 0, + }; + }, winId); +} + +// Newest window-N div beyond a recorded set (same detection as 3d-viewer.spec.ts). +async function newestWindowId(page: import('@playwright/test').Page, + before: string[]): Promise { + await expect.poll(async () => page.evaluate((prev: string[]) => { + const all = Array.from(document.querySelectorAll('#window-container [id^="window-"]')) + .map((e) => e.id); + return all.find((id) => !prev.includes(id)) ?? null; + }, before), { timeout: 60000, intervals: [300] }).not.toBeNull(); + const winId = await page.evaluate((prev: string[]) => { + const all = Array.from(document.querySelectorAll('#window-container [id^="window-"]')) + .map((e) => e.id); + return all.find((id) => !prev.includes(id)) ?? all[all.length - 1] ?? null; + }, before); + expect(winId, 'the 3D viewer should open a new top-level window').toBeTruthy(); + return winId as string; +} + +test.describe('3D viewer close and reopen', () => { + // Two full viewer opens over one heavy pcbnew load: serial + generous. + test.describe.configure({ mode: 'serial' }); + test.setTimeout(480000); + + test('re-renders the board and keeps its window position after close + reopen', + async ({ page, testLogger }) => { + await page.goto('/kicad/pcbnew.html'); + await waitForPcbnew(page); + await loadBoard(page, testLogger); + + // ── First open: must render (precondition, same gate as 3d-viewer.spec.ts). ── + const winsBefore = await page.evaluate(() => + Array.from(document.querySelectorAll('#window-container [id^="window-"]')) + .map((e) => e.id)); + const glBefore = await countGlCanvases(page); + await openThreeDViewer(page, glBefore); + const winId = await newestWindowId(page, winsBefore); + await waitForThreeDRender(page); + await logThreeDDiag(page, 'first open rendered'); + + // Drag the viewer by its DOM title bar to a distinctive position: with pristine + // settings the frame opens display-sized at (0,0), where "restored" and + // "re-centred" coincide and a position regression is invisible. The drag makes + // the two outcomes distinguishable. (Same machinery as the titlebar spec.) + const posInitial = await windowPos(page, winId); + expect(posInitial, 'the 3D viewer window should have a position').not.toBeNull(); + const bar = page.locator(`#${winId} .window-titlebar`); + const barBox = await bar.boundingBox(); + expect(barBox, 'the 3D viewer should have a DOM title bar to drag').not.toBeNull(); + const cx = barBox!.x + barBox!.width / 2; + const cy = barBox!.y + barBox!.height / 2; + await page.mouse.move(cx, cy); + await page.mouse.down(); + await page.mouse.move(cx + 40, cy + 90, { steps: 10 }); + await page.mouse.up(); + // DOM style updates first; poll it as the observable... + await expect.poll(async () => (await windowPos(page, winId))?.top, + { timeout: 15000, intervals: [200] }).not.toBe(posInitial!.top); + // ...then let the wx-side op (wx_window_move → wxWindow::Move) fully land before + // close saves the frame position from the wx side (documented interaction dwell — + // the DOM moves before the wx op completes; see 3d-viewer.spec.ts titlebar test). + await page.waitForTimeout(500); // eslint-disable-line -- documented interaction dwell + + const posBefore = await windowPos(page, winId); + expect(posBefore, 'the 3D viewer window should have a position').not.toBeNull(); + await page.screenshot({ path: shotPath(page, `3d-reopen-01-first-${DEMO.name}.png`), + scale: 'css' }); + + // ── Close via the titlebar ×; wait for the frame AND its GL canvas to go away + // (the canvas removal is the observable for wxGLCanvas/WebGL-context teardown). ── + await page.locator(`#${winId} .window-titlebar-close`).click(); + await expect.poll(async () => page.evaluate((wid) => { + const el = document.getElementById(wid); + return !el || getComputedStyle(el).display === 'none'; + }, winId), { timeout: 30000, intervals: [200] }).toBe(true); + await expect.poll(() => countGlCanvases(page), { timeout: 30000, intervals: [200] }) + .toBe(glBefore); + await logThreeDDiag(page, 'viewer closed'); + + // ── Reopen. ── + const winsBeforeReopen = await page.evaluate(() => + Array.from(document.querySelectorAll('#window-container [id^="window-"]')) + .map((e) => e.id)); + await openThreeDViewer(page, glBefore); + const winId2 = await newestWindowId(page, winsBeforeReopen); + + // Position must be restored, not re-centred. Read once the frame exists (position is + // applied during frame construction, which precedes the GL canvas the open-wait saw). + // Soft: a position regression must not mask the render assertion below (and vice + // versa) — both defects come from one user action and should report together. + const posAfter = await windowPos(page, winId2); + expect.soft(posAfter, 'the reopened 3D viewer window should have a position') + .not.toBeNull(); + if (posBefore && posAfter) { + expect.soft(Math.abs(posAfter.left - posBefore.left), + `reopened 3D viewer should keep its window position (closed at ` + + `${posBefore.left},${posBefore.top}; reopened at ${posAfter.left},${posAfter.top}` + + ` — re-centring means the saved display index was invalid)`) + .toBeLessThanOrEqual(2); + expect.soft(Math.abs(posAfter.top - posBefore.top), + `reopened 3D viewer should keep its window position (closed at ` + + `${posBefore.left},${posBefore.top}; reopened at ${posAfter.left},${posAfter.top}` + + ` — re-centring means the saved display index was invalid)`) + .toBeLessThanOrEqual(2); + } + + // THE render regression assertion: the reopened viewer must show the board again. + // A blank canvas (stale gl1 GL handles in the new WebGL context) stays at ~1 colour. + await expect.poll(() => sampleNewestGlCanvasColors(page), { + message: 'reopened 3D viewer canvas should render the board again ' + + '(a blank/uniform canvas means the gl1 shim is drawing with GL object ' + + 'names from the destroyed WebGL context)', + timeout: 90000, + intervals: [1000], + }).toBeGreaterThan(8); + + await page.screenshot({ path: shotPath(page, `3d-reopen-02-reopened-${DEMO.name}.png`), + scale: 'css' }); + + // ── Console-clean gates (same signatures as 3d-viewer.spec.ts). ── + const allLines = [...testLogger.consoleLogs, ...testLogger.errors]; + const aborts = allLines.filter((l) => l.includes('Aborted(')); + expect(aborts, `WASM aborted during close/reopen:\n${aborts.join('\n\n')}`).toEqual([]); + + const wasmTrapSignatures = [ + 'index out of bounds', 'indirect call to null', 'uncaught exception: unwind', + 'invalid state', 'is not a function', + ]; + const wasmTrapErrors = allLines.filter((l) => + wasmTrapSignatures.some((sig) => l.toLowerCase().includes(sig))); + expect(wasmTrapErrors, + `wasm trap surfaced during close/reopen:\n${wasmTrapErrors.join('\n\n')}`) + .toEqual([]); + }); +}); diff --git a/wasm/gl1/README.md b/wasm/gl1/README.md index cf7c399..af0a211 100644 --- a/wasm/gl1/README.md +++ b/wasm/gl1/README.md @@ -55,6 +55,22 @@ untouched. the goldens document; tolerating it would *diverge* from native. - **Lighting is per-vertex (Gouraud)** to match fixed-function output; per-fragment lighting visibly mismatches speculars on coarse meshes. +- **GL object caches are per-context, and the context is mortal.** Closing the + 3D viewer destroys its wxGLCanvas's WebGL context; reopening mints a new one + in which the cached names (FFP program, stream/scratch VBOs) are invalid — + every draw then dies with `INVALID_OPERATION` and the viewer is blank. + `contextSync()` (gl1_state.cpp) detects the change in `programSync()` — the + one choke point every shim draw crosses and a path the 2D GAL never reaches + (a check in any `__wrap_*` would see the GAL's context and ping-pong the + owner on 2D↔3D paint alternation) — and drops the caches so they rebuild + lazily. Identity comes from a monotonic id stamped on Emscripten's + per-context record — NOT the `EMSCRIPTEN_WEBGL_CONTEXT_HANDLE`, which + Emscripten recycles (a destroy-then-create can return the same number). + Display-list/immediate state is deliberately untouched: it is CPU-only, and + the change can be detected mid-scene-rebuild (even inside `glNewList`). + Known limit (pre-existing): two *simultaneously live* FFP contexts would + thrash the caches on every alternation — the shim still assumes one live + 3D-viewer context at a time. ## Layout diff --git a/wasm/gl1/include/gl1_shim.h b/wasm/gl1/include/gl1_shim.h index 46ae4ed..d1f2220 100644 --- a/wasm/gl1/include/gl1_shim.h +++ b/wasm/gl1/include/gl1_shim.h @@ -217,6 +217,23 @@ void stateBlendFunc( GLenum sfactor, GLenum dfactor ); void stateLineWidth( GLfloat width ); void stateAlphaFunc( GLenum func, GLclampf ref ); +// --- context-generation guard (gl1_state.cpp) --- +// GL object names live and die with the WebGL context, and the 3D viewer's +// close/reopen destroys and recreates it (~wxGLCanvas destroys the context +// with the canvas; the next open mints new ones). Called from programSync() +// ONLY: that is the single choke point every shim draw crosses, it always +// runs under the 3D context, and — critically — it is a path the 2D GAL never +// reaches, so alternating 2D/3D paints cannot ping-pong the owner (a check in +// the glBindTexture wrap did exactly that: every caller crosses a wrap). +// On a context change it drops every cached name so the shim rebuilds lazily +// in the new context. Deliberately never touches display-list or +// immediate-mode state: the change can be detected mid-scene-rebuild, and +// those modules are context-agnostic CPU state. +void contextSync(); +// Per-TU cache drops invoked by contextSync() on a context change. +void shadersDropContextObjects(); // FFP program + uniform locations + fail latch +void drawDropContextObjects(); // stream/scratch VBOs + // GL1 normalized-attribute rule: integer colors and normals are normalized, // floats are not (positions/texcoords are float-only in this codebase). bool attribNormalized( int arrayIndex, GLenum type ); diff --git a/wasm/gl1/src/gl1_draw.cpp b/wasm/gl1/src/gl1_draw.cpp index 95c2937..7c43774 100644 --- a/wasm/gl1/src/gl1_draw.cpp +++ b/wasm/gl1/src/gl1_draw.cpp @@ -22,6 +22,15 @@ namespace gl1 static GLuint s_streamVBO = 0; // immediate-mode interleaved stream static GLuint s_scratchVBO = 0; // client-array upload staging + +void drawDropContextObjects() +{ + // The owning context is gone; the buffer names are invalid in the current + // one. No glDeleteBuffers — just forget them so the draws re-gen lazily. + s_streamVBO = 0; + s_scratchVBO = 0; +} + enum { ATTR_POSITION = 0, diff --git a/wasm/gl1/src/gl1_entry_wrapped.cpp b/wasm/gl1/src/gl1_entry_wrapped.cpp index 1fc7777..64c8109 100644 --- a/wasm/gl1/src/gl1_entry_wrapped.cpp +++ b/wasm/gl1/src/gl1_entry_wrapped.cpp @@ -116,6 +116,13 @@ void __wrap_glDrawElements( GLenum mode, GLsizei count, GLenum type, const GLvoi void __wrap_glBindTexture( GLenum target, GLuint texture ) { + // NO contextSync() here: this wrap intercepts EVERY caller, including the + // 2D GAL binding its own textures under its own context — a check here + // ping-pongs the owner on 2D<->3D paint alternation and thrash-rebuilds + // the FFP program each flip (observed: 23 resets in one e2e run). The + // boundTexture2D mirror this site feeds is write-only bookkeeping, so a + // reset zeroing it after a new-context bind loses nothing. + if( dlistRecording() ) { dlistRecordBindTexture( target, texture ); diff --git a/wasm/gl1/src/gl1_shaders.cpp b/wasm/gl1/src/gl1_shaders.cpp index 53ace7a..ca168c1 100644 --- a/wasm/gl1/src/gl1_shaders.cpp +++ b/wasm/gl1/src/gl1_shaders.cpp @@ -301,6 +301,17 @@ static bool s_buildFailed = false; static ProgramLocs s_locs; +void shadersDropContextObjects() +{ + // The owning context is gone; the program name is invalid in the current + // one. No glDeleteProgram — just forget it so programSync() rebuilds. + // The fail latch resets too: a fresh context gets a fresh build attempt. + s_program = 0; + s_buildFailed = false; + s_locs = ProgramLocs(); +} + + static GLuint compileShader( GLenum type, const char* source ) { GLuint shader = glCreateShader( type ); @@ -476,6 +487,11 @@ static int encodeCombineFunc( GLenum func ) bool programSync() { + // Every shim draw funnels through here, so this is the single choke point + // where a recreated WebGL context (3D viewer close/reopen) gets detected + // before any cached GL name is used. + contextSync(); + if( s_buildFailed ) return false; diff --git a/wasm/gl1/src/gl1_state.cpp b/wasm/gl1/src/gl1_state.cpp index 9aec5d7..026ab0f 100644 --- a/wasm/gl1/src/gl1_state.cpp +++ b/wasm/gl1/src/gl1_state.cpp @@ -4,6 +4,8 @@ #include "gl1_shim.h" +#include + namespace gl1 { @@ -14,6 +16,66 @@ State& S() } +// Identity of the current WebGL context, stable for the context's lifetime and +// never reused. The EMSCRIPTEN_WEBGL_CONTEXT_HANDLE is NOT that: Emscripten +// recycles freed handle slots, so the context created after a destroy can get +// the very same numeric handle. Stamp a monotonic id on Emscripten's +// per-context record (a fresh JS object per createContext) instead. +static int currentContextId() +{ + return EM_ASM_INT( { + var ctx = ( typeof GL !== 'undefined' ) ? GL.currentContext : null; + if( !ctx ) + return 0; + if( !ctx.gl1ContextId ) + { + GL.gl1NextContextId = ( GL.gl1NextContextId | 0 ) + 1; + ctx.gl1ContextId = GL.gl1NextContextId; + } + return ctx.gl1ContextId; + } ); +} + + +// The context generation the shim's cached GL objects belong to. 0 until the +// first contextSync() under a live context. +static int s_ownerContext = 0; + +void contextSync() +{ + int cur = currentContextId(); + + if( cur == s_ownerContext || cur == 0 ) + return; + + if( s_ownerContext != 0 ) + { + // The context that owned the cached names is gone (3D viewer closed and + // reopened). No glDelete*: the names are invalid in the current context + // — forget them and let each module rebuild lazily. + std::printf( "[gl1] WebGL context changed — dropping cached GL objects\n" ); + + shadersDropContextObjects(); + drawDropContextObjects(); + + State& s = S(); + s.boundTexture2D = 0; + + for( int i = 0; i < CA_COUNT; ++i ) + s.clientArrays[i].boundBuffer = 0; + + // The new program starts with default-initialized uniforms; force a + // full re-upload on its first sync. + s.matricesDirty = true; + s.lightingDirty = true; + s.texEnvDirty = true; + s.miscDirty = true; + } + + s_ownerContext = cur; +} + + bool* ffpCapSlot( GLenum cap ) { State& s = S(); diff --git a/wxwidgets b/wxwidgets index f56511f..b8060a0 160000 --- a/wxwidgets +++ b/wxwidgets @@ -1 +1 @@ -Subproject commit f56511f965f0d8b9d2c2a1019a942f4e1d343e23 +Subproject commit b8060a0635341b078fcb9ac8d2212898e21c7208