fix: 🐛 press m & move w/ arrows
This commit is contained in:
parent
5df7f40b3c
commit
aa9d613f50
6 changed files with 327 additions and 8 deletions
|
|
@ -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 $@
|
||||
|
|
|
|||
93
tests/apps/standalone/warp-pointer/warp-pointer_test.cpp
Normal file
93
tests/apps/standalone/warp-pointer/warp-pointer_test.cpp
Normal file
|
|
@ -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 <emscripten/emscripten.h>
|
||||
#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);
|
||||
36
tests/e2e/warp-pointer.spec.ts
Normal file
36
tests/e2e/warp-pointer.spec.ts
Normal file
|
|
@ -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`);
|
||||
});
|
||||
});
|
||||
174
tests/kicad/pcbnew-move.spec.ts
Normal file
174
tests/kicad/pcbnew-move.spec.ts
Normal file
|
|
@ -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<void> {
|
||||
await page.waitForFunction(
|
||||
() => {
|
||||
const m = (window as unknown as { Module?: Partial<CollabModule> }).Module;
|
||||
return typeof m?.kicadCollabSnapshot === 'function'
|
||||
&& typeof m?.kicadCollabGetPos === 'function';
|
||||
},
|
||||
null,
|
||||
{ timeout: 30000 },
|
||||
);
|
||||
}
|
||||
|
||||
async function snapshotItems(page: Page): Promise<SnapItem[]> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
@ -11,8 +11,8 @@ import {
|
|||
*
|
||||
* This is a faithful port of the proven harness HTML (tests/apps/kicad/<tool>.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
|
||||
* `<tool>.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 `<tool>.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<void> {
|
|||
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/<tool>.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".
|
||||
// <tool>.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})`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
Subproject commit 16339f805b213f7040033d7f410d249926e6df66
|
||||
Subproject commit d8d3b49d5730ae3488480dd38613b29d3b8eacfc
|
||||
Loading…
Reference in a new issue