diff --git a/tests/apps/Makefile.wasm b/tests/apps/Makefile.wasm index 071d3e8..b807331 100644 --- a/tests/apps/Makefile.wasm +++ b/tests/apps/Makefile.wasm @@ -186,7 +186,8 @@ all: minimal_test.html \ $(S)/coroutine-pthread/nested_repro_ex.html \ $(S)/coroutine-pthread/vcall_repro.html \ $(S)/textctrl-reentry/textctrl-reentry_test.html \ - $(S)/tooltip-lifetime/tooltip-lifetime_test.html + $(S)/tooltip-lifetime/tooltip-lifetime_test.html \ + $(S)/warp-pointer/warp-pointer_test.html # Main test app minimal_test.o: minimal_test.cpp @@ -216,6 +217,15 @@ $(S)/tooltip-lifetime/tooltip-lifetime_test.html: $(S)/tooltip-lifetime/tooltip- tooltip-lifetime: $(S)/tooltip-lifetime/tooltip-lifetime_test.html +# wxWindow::WarpPointer must update the cached mouse position (wxGetMousePosition). +$(S)/warp-pointer/warp-pointer_test.o: $(S)/warp-pointer/warp-pointer_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/warp-pointer/warp-pointer_test.html: $(S)/warp-pointer/warp-pointer_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +warp-pointer: $(S)/warp-pointer/warp-pointer_test.html + # Menu test (no GL) $(S)/menu/menu_test.o: $(S)/menu/menu_test.cpp $(CXX) -c $(CXXFLAGS) $< -o $@ diff --git a/tests/apps/standalone/warp-pointer/warp-pointer_test.cpp b/tests/apps/standalone/warp-pointer/warp-pointer_test.cpp new file mode 100644 index 0000000..3aa9609 --- /dev/null +++ b/tests/apps/standalone/warp-pointer/warp-pointer_test.cpp @@ -0,0 +1,93 @@ +// wxWindow::WarpPointer must update the cached mouse position (DOM port). +// +// Bug (src/wasm/window.cpp): wxWindowWasm::WarpPointer() is a no-op because the +// browser cannot move the OS pointer. KiCad nudges the cursor with the arrow +// keys by warping the pointer and then reading it back via wxGetMousePosition() +// (WX_VIEW_CONTROLS::SetCursorPosition -> WarpMouseCursor -> WarpPointer, then +// the interactive-move loop reads GetViewControls()->GetMousePosition()). With +// the warp a no-op the cached position never changes, so a selected item never +// follows the arrow keys and snaps to the stale cursor on grab. +// +// The invariant the bug violates: after WarpPointer(x, y), wxGetMousePosition() +// must report the screen-space equivalent of (x, y) — exactly what a real OS +// pointer warp produces on desktop. +// +// RED (bug present): wxGetMousePosition() is unchanged by the warp. +// GREEN (fixed): wxGetMousePosition() == ClientToScreen({x, y}). + +#include "wx/wxprec.h" +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#ifdef __EMSCRIPTEN__ +#include +#endif + +static void Report(const char *name, bool pass, const wxString &detail) +{ +#ifdef __EMSCRIPTEN__ + EM_ASM({ + var msg = '[REPRO] ' + UTF8ToString($0) + ': ' + ($1 ? 'PASS' : 'FAIL') + + ' - ' + UTF8ToString($2); + if ($1) { console.log(msg); } else { console.error(msg); } + }, name, pass ? 1 : 0, (const char *)detail.utf8_str()); +#endif +} + +class ReproFrame : public wxFrame +{ +public: + ReproFrame(); + +private: + void RunTest(); +}; + +ReproFrame::ReproFrame() + : wxFrame(nullptr, wxID_ANY, "wxWindow::WarpPointer repro") +{ + CallAfter(&ReproFrame::RunTest); +} + +void ReproFrame::RunTest() +{ + // Two distinct targets so a stale/default cached position cannot accidentally + // match. Warp to each, then require wxGetMousePosition() to report the + // screen-space equivalent (the same conversion a real OS warp performs). + const wxPoint targets[] = { wxPoint(137, 211), wxPoint(56, 92) }; + + bool allPass = true; + wxString detail; + + for (const wxPoint &client : targets) + { + WarpPointer(client.x, client.y); + + const wxPoint expected = ClientToScreen(client); + const wxPoint actual = wxGetMousePosition(); + const bool pass = (actual == expected); + allPass = allPass && pass; + + detail += wxString::Format("[client=(%d,%d) expected=(%d,%d) actual=(%d,%d) %s] ", + client.x, client.y, expected.x, expected.y, + actual.x, actual.y, pass ? "ok" : "MISMATCH"); + } + + Report("warppointer_updates_position", allPass, detail); +} + +class ReproApp : public wxApp +{ +public: + bool OnInit() override + { + if (!wxApp::OnInit()) + return false; + + (new ReproFrame())->Show(true); + return true; + } +}; + +wxIMPLEMENT_APP(ReproApp); diff --git a/tests/e2e/warp-pointer.spec.ts b/tests/e2e/warp-pointer.spec.ts new file mode 100644 index 0000000..05d51d7 --- /dev/null +++ b/tests/e2e/warp-pointer.spec.ts @@ -0,0 +1,36 @@ +import { test, expect, tryLoadApp } from './utils/fixtures'; + +// Red-green reproduction for the wxWindow::WarpPointer no-op (src/wasm/window.cpp). +// +// WarpPointer() was a no-op because the browser cannot move the OS pointer. But +// KiCad's arrow-key cursor nudge warps the pointer and then reads it back via +// wxGetMousePosition() (SetCursorPosition -> WarpMouseCursor -> WarpPointer, then +// the interactive-move loop reads GetMousePosition()). With the warp dead the +// read is stale, so a moved item never follows the arrow keys and snaps to the +// cursor on grab (pcbnew issue #9). The standalone app warps to known points and +// self-reports whether wxGetMousePosition() tracked them. +// +// RED (bug present): wxGetMousePosition() unchanged by the warp. +// GREEN (fixed): wxGetMousePosition() == ClientToScreen({x, y}). + +function reproLine(logs: string[], name: string): string | undefined { + return logs.find((l) => l.includes(`[REPRO] ${name}:`)); +} + +test.describe('wxWindow::WarpPointer cached mouse position', () => { + test('WarpPointer updates wxGetMousePosition()', async ({ page, testLogger }) => { + const name = 'warppointer_updates_position'; + await page.goto('/standalone/warp-pointer/warp-pointer_test.html'); + expect(await tryLoadApp(page, 30000), 'repro app should load').toBe(true); + + await expect + .poll(() => reproLine(testLogger.consoleLogs, name) ?? null, { + timeout: 30000, + message: `repro app should emit its [REPRO] ${name} result line`, + }) + .not.toBeNull(); + + const line = reproLine(testLogger.consoleLogs, name)!; + expect(line, `repro line was: ${line}`).toContain(`[REPRO] ${name}: PASS`); + }); +}); diff --git a/tests/kicad/pcbnew-move.spec.ts b/tests/kicad/pcbnew-move.spec.ts new file mode 100644 index 0000000..8396d1e --- /dev/null +++ b/tests/kicad/pcbnew-move.spec.ts @@ -0,0 +1,174 @@ +import type { Page } from '@playwright/test'; +import { test, expect } from './fixtures'; +import { clickByTooltip, findByTooltip } from '../e2e/utils/element-tracker'; +import { completeWizard, hideCursor } from './utils/screenshot-compare'; + +/** + * PCBnew "m" move regression — GitHub issue #9. + * + * On desktop you select an item, press `m`, then nudge it with the arrow keys. + * In the WASM build the arrow keys did nothing and the item snapped to the + * cursor on grab, because wxWindowWasm::WarpPointer() was a no-op: KiCad's + * arrow-key cursor nudge warps the pointer and reads it back via + * wxGetMousePosition(), so a dead warp left the move loop reading a stale + * position. The fix makes WarpPointer update the cached mouse position. + * + * This drives the real path — draw a graphic line, select it, press `m`, then + * ArrowRight, and COMMIT WITH ENTER (a click would drop the item at the cursor + * and hide the arrow nudges) — and asserts via the embind position hooks that + * the item actually moved right. + * + * RED (bug present): the line does not move; delta == 0. + * GREEN (fixed): the line moves right; delta_x > 0. + */ + +type SnapItem = { id: string; type: string; x: number; y: number }; +type CollabModule = { + kicadCollabSnapshot(): string; + kicadCollabGetPos(id: string): string; +}; + +async function waitForCollabModule(page: Page): Promise { + await page.waitForFunction( + () => { + const m = (window as unknown as { Module?: Partial }).Module; + return typeof m?.kicadCollabSnapshot === 'function' + && typeof m?.kicadCollabGetPos === 'function'; + }, + null, + { timeout: 30000 }, + ); +} + +async function snapshotItems(page: Page): Promise { + return page.evaluate(() => { + const m = (window as unknown as { Module: CollabModule }).Module; + const snap = JSON.parse(m.kicadCollabSnapshot()) as { added: SnapItem[] }; + return snap.added; + }); +} + +async function getPos(page: Page, id: string): Promise<{ x: number; y: number }> { + const raw = await page.evaluate( + (i) => (window as unknown as { Module: CollabModule }).Module.kicadCollabGetPos(i), + id, + ); + const [x, y] = raw.split(',').map(Number); + return { x, y }; +} + +async function visibleGlCanvasBox(page: Page) { + const glCanvasId = await page.evaluate(() => { + const glCanvas = + Array.from(document.querySelectorAll('[id^="glcanvas-"]')) + .map((c) => c as HTMLCanvasElement) + .find((c) => { + const rect = c.getBoundingClientRect(); + const style = window.getComputedStyle(c); + return style.display !== 'none' && rect.width > 0 && rect.height > 0; + }) ?? (document.querySelector('[id^="glcanvas-"]') as HTMLCanvasElement | null); + return glCanvas?.id ?? null; + }); + expect(glCanvasId, 'visible GL canvas').not.toBeNull(); + const box = await page.locator(`#${glCanvasId}`).boundingBox(); + expect(box, 'GL canvas bounding box').not.toBeNull(); + return box!; +} + +test.describe('PCBnew move with "m" (#9)', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/kicad/pcbnew.html'); + }); + + test('selected item moves with the arrow keys after pressing m', async ({ page, testLogger }) => { + await completeWizard(page, { screenshots: false }); + await hideCursor(page); + await waitForCollabModule(page); + + // Wait until the Draw Lines tool is registered, then select it. + await page.waitForFunction(() => { + const registry = window.wxElementRegistry; + return !!registry?.findAllRendered + && registry.findAllRendered({ elementType: 'tool' }) + .some((t) => t.tooltip?.includes('Draw Lines')); + }, null, { timeout: 15000 }); + + const isToolChecked = (t: { label?: string } | null | undefined) => + (t?.label ?? '').includes('[checked]'); + + const idsBeforeDraw = new Set((await snapshotItems(page)).map((i) => i.id)); + + expect(await clickByTooltip(page, 'Draw Lines', { elementType: 'tool' })).toBe(true); + await expect.poll(async () => + isToolChecked(await findByTooltip(page, 'Draw Lines', { elementType: 'tool' })), { + message: 'Draw Lines tool should stay selected', + timeout: 5000, + }).toBe(true); + + // Draw a horizontal segment at known canvas pixels. Settle after each + // move so the asyncified pointer-move handler updates the world cursor + // before the click lands (see pcbnew.spec.ts draw-lines test). + const glBox = await visibleGlCanvasBox(page); + const startPoint = { x: Math.round(glBox.x + glBox.width * 0.35), y: Math.round(glBox.y + glBox.height * 0.45) }; + const endPoint = { x: Math.round(glBox.x + glBox.width * 0.55), y: Math.round(glBox.y + glBox.height * 0.45) }; + const midPoint = { x: Math.round((startPoint.x + endPoint.x) / 2), y: startPoint.y }; + + await page.mouse.move(startPoint.x, startPoint.y); + await page.waitForTimeout(350); + await page.mouse.down(); + await page.mouse.up(); + await page.waitForTimeout(350); + await page.mouse.move(endPoint.x, endPoint.y); + await page.waitForTimeout(350); + await page.mouse.down(); + await page.mouse.up(); + await page.waitForTimeout(500); + // Finish the segment and return to the selection tool. + await page.keyboard.press('Escape'); + await page.waitForTimeout(250); + await page.keyboard.press('Escape'); + await page.waitForTimeout(250); + + // Identify the drawn item and its starting position. + const newItems = (await snapshotItems(page)).filter((i) => !idsBeforeDraw.has(i.id)); + expect(newItems.length, `exactly one new board item was drawn (got ${JSON.stringify(newItems)})`).toBe(1); + const drawnId = newItems[0].id; + const pos0 = await getPos(page, drawnId); + + const beforeMove = await page.screenshot({ path: 'test-results/pcbnew-move-00-before.png', scale: 'device' }); + + // Hover the cursor onto the line and select it, then move with the keyboard. + await page.mouse.move(midPoint.x, midPoint.y); + await page.waitForTimeout(350); + await page.mouse.down(); + await page.mouse.up(); + await page.waitForTimeout(350); + + const NUDGES = 10; + await page.keyboard.press('m'); + await page.waitForTimeout(400); + for (let i = 0; i < NUDGES; i++) { + await page.keyboard.press('ArrowRight'); + await page.waitForTimeout(150); + } + // Commit at the nudged position WITHOUT moving the cursor (Enter, not click). + await page.keyboard.press('Enter'); + await page.waitForTimeout(500); + + const afterMove = await page.screenshot({ path: 'test-results/pcbnew-move-01-after.png', scale: 'device' }); + + const pos1 = await getPos(page, drawnId); + const dx = pos1.x - pos0.x; + const dy = pos1.y - pos0.y; + testLogger; // logs captured by fixture + console.log(`[TEST] pcbnew move dx=${dx} dy=${dy} pos0=${JSON.stringify(pos0)} pos1=${JSON.stringify(pos1)}`); + + // Core regression: ArrowRight after `m` must move the item to the right. + // RED (no-op warp): dx == 0. GREEN (fixed): dx > 0, predominantly horizontal. + expect(dx, 'item should move right by the arrow keys (issue #9)').toBeGreaterThan(0); + expect(Math.abs(dy), 'ArrowRight move should be horizontal').toBeLessThanOrEqual(Math.abs(dx)); + + expect(beforeMove.length).toBeGreaterThan(0); + expect(afterMove.length).toBeGreaterThan(0); + }); +}); diff --git a/web/standalone/src/wasm/boot.ts b/web/standalone/src/wasm/boot.ts index 148aba3..3ff2279 100644 --- a/web/standalone/src/wasm/boot.ts +++ b/web/standalone/src/wasm/boot.ts @@ -11,8 +11,8 @@ import { * * This is a faithful port of the proven harness HTML (tests/apps/kicad/.html): * it builds the same global `Module` config, runs the same preRun steps (create - * canvas, write images.tar.gz, seed config), then injects `wx.js` followed by - * `.js`. The KiCad WASM build is NON-modularized, so it reads a global + * canvas, write images.tar.gz, seed config), then injects `wx.js`, `wx-dom.js`, + * and `.js`. The KiCad WASM build is NON-modularized, so it reads a global * `var Module` and publishes `FS`/`wxElementRegistry` onto `window` — exactly the * surface the iframe approach used, only now in the top-level window. * @@ -220,10 +220,16 @@ async function doBoot(opts: BootOptions): Promise { mainScriptUrlOrBlob: `${base}/${tool}.js`, }; - // wx.js MUST load first: it defines globals the wasm imports (getConfigEntryLength, - // …) and the wxElementRegistry the open-flow drives. Then the tool glue, whose - // execution captures currentScript.src as Emscripten's _scriptName. + // Load order mirrors the harness HTML (tests/apps/kicad/.html): + // wx.js — defines globals the wasm imports (getConfigEntryLength, …) and + // the wxElementRegistry the open-flow drives. + // wx-dom.js — the DOM-port shim that defines window.wxDomCreateControl and the + // other DOM widget hooks the wasm invokes via EM_ASM. Without it the + // tool aborts at startup with "wxDomCreateControl is not defined". + // .js — the tool glue, whose execution captures currentScript.src as + // Emscripten's _scriptName. await loadScript(`${base}/wx.js`); + await loadScript(`${base}/wx-dom.js`); await loadScript(`${base}/${tool}.js`); - log(`[boot] injected wx.js + ${tool}.js (base=${base})`); + log(`[boot] injected wx.js + wx-dom.js + ${tool}.js (base=${base})`); } diff --git a/wxwidgets b/wxwidgets index 16339f8..d8d3b49 160000 --- a/wxwidgets +++ b/wxwidgets @@ -1 +1 @@ -Subproject commit 16339f805b213f7040033d7f410d249926e6df66 +Subproject commit d8d3b49d5730ae3488480dd38613b29d3b8eacfc