pcbjam/tests/kicad/3d-viewer-reopen.spec.ts
Istvan Matejcsok 3722891d48 fix(3d): blank render + lost position after 3D viewer close/reopen (gl1 context guard)
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 <noreply@anthropic.com>
2026-08-24 16:18:22 +02:00

202 lines
11 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

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<number> {
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<string>();
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<string> {
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([]);
});
});