From 44f9373a15e4f81a2c0e9621363239903b5ac6a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20T=C3=B6rcsv=C3=A1ri?= Date: Fri, 17 Jul 2026 17:48:10 +0200 Subject: [PATCH] =?UTF-8?q?cvpcb-wasm:=20link=20CvPcb=20as=20third=20kifac?= =?UTF-8?q?e=20=E2=80=94=20eeschema=20Assign=20Footprints=20opens=20in=20W?= =?UTF-8?q?ASM;=20serial-inline=20footprint=20list=20load=20fixes=20pool-t?= =?UTF-8?q?ask-vs-JS-bridge=20deadlock?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Y8of27UBjJmwY7JkALit2j --- kicad | 2 +- scripts/kicad/audit-merged-symbols.sh | 82 +++++++- tests/web/eeschema-assign-footprints.spec.ts | 204 +++++++++++++++++++ wasm/editor/CMakeLists.txt | 20 +- 4 files changed, 295 insertions(+), 13 deletions(-) create mode 100644 tests/web/eeschema-assign-footprints.spec.ts diff --git a/kicad b/kicad index 3dcfea5..d0e1705 160000 --- a/kicad +++ b/kicad @@ -1 +1 @@ -Subproject commit 3dcfea5e458418c1d6cc03b1baecd07424211ff7 +Subproject commit d0e1705a24b5d446c27694aa03a6ed7c8ad10970 diff --git a/scripts/kicad/audit-merged-symbols.sh b/scripts/kicad/audit-merged-symbols.sh index 2776ee1..9fdc856 100755 --- a/scripts/kicad/audit-merged-symbols.sh +++ b/scripts/kicad/audit-merged-symbols.sh @@ -31,12 +31,15 @@ NM="${EMSDK:?EMSDK not set}/upstream/bin/llvm-nm" OUT="${TMPDIR:-/tmp}/merged-symbol-audit" mkdir -p "${OUT}" +SKIP_STANDALONE= if [ ! -d "${P}/pcbnew" ] || [ ! -d "${E}/eeschema" ]; then - echo "ERROR: need built kicad-pcbnew and kicad-eeschema trees under ${BUILD_ROOT}" >&2 - echo " (./docker/build.sh pcbnew,eeschema --compile-only)" >&2 - exit 1 + echo "== pcb/sch standalone audit SKIPPED: need built kicad-pcbnew and kicad-eeschema" >&2 + echo " trees under ${BUILD_ROOT} (./docker/build.sh pcbnew,eeschema --compile-only)" >&2 + SKIP_STANDALONE=1 fi +if [ -z "${SKIP_STANDALONE}" ]; then + pcb_files() { find "${P}/pcbnew/CMakeFiles/pcbnew_kiface_objects.dir" -name '*.o' ls "${P}/common/libpcbcommon.a" \ @@ -95,4 +98,77 @@ comm -12 "${OUT}/pcb_weak.txt" "${OUT}/sch_weak.txt" \ | grep -E '_Z(TV|TI|TS|N)[0-9]+[A-Z]' \ | grep -vE 'magic_enum|wxEventFunctorMethod|wxNavigationEnabled|wxSimplebook|wxDataView|wxMenuBar|wxVector|KIFACE|COLLECTOR|RC_JSON|PARAM_SCALED|EDA_|BOX2|WX_MENUBAR|SEARCH_HANDLER' || true +fi # SKIP_STANDALONE + +# ------------------------------------------------------------------------- +# cvpcb third-kiface audit (feature/cvpcb-wasm). Unlike pcbnew/eeschema above +# (audited from their option-OFF per-app trees), cvpcb_kiface_objects only +# exists in the merged kicad-kicad_editor tree — where the renames ARE applied, +# so these intersections audit exactly what links into the image. Expected +# output: EMPTY strong intersections against both engines. +M="${BUILD_ROOT}/kicad-kicad_editor" + +if [ -d "${M}/cvpcb/CMakeFiles/cvpcb_kiface_objects.dir" ]; then + echo "== collecting cvpcb symbols from merged tree (llvm-nm)..." >&2 + + cv_files() { find "${M}/cvpcb/CMakeFiles/cvpcb_kiface_objects.dir" -name '*.o'; } + mpcb_files() { + find "${M}/pcbnew/CMakeFiles/pcbnew_kiface_objects.dir" -name '*.o' + ls "${M}/common/libpcbcommon.a" \ + "${M}/pcbnew/connectivity/libconnectivity.a" \ + "${M}/pcbnew/router/libpnsrouter.a" \ + "${M}/pcbnew/navlib/libpcbnew_navlib.a" \ + "${M}/utils/idftools/libidf3.a" \ + "${M}"/pcbnew/pcb_io/*/*.a 2>/dev/null || true + find "${M}/3d-viewer" -name '*.o' 2>/dev/null || true + } + msch_files() { + find "${M}/eeschema/CMakeFiles/eeschema_kiface_objects.dir" -name '*.o' + ls "${M}/eeschema/navlib/libeeschema_navlib.a" 2>/dev/null || true + } + + ${NM} --defined-only --extern-only --format=posix $(cv_files) 2>/dev/null \ + | awk '$2 ~ /^[TDBR]$/ {print $1}' | sort -u > "${OUT}/cv_strong.txt" + ${NM} --defined-only --extern-only --format=posix $(cv_files) 2>/dev/null \ + | awk '$2 ~ /^[WVwv]$/ {print $1}' | sort -u > "${OUT}/cv_weak.txt" + ${NM} --defined-only --extern-only --format=posix $(mpcb_files) 2>/dev/null \ + | awk '$2 ~ /^[TDBR]$/ {print $1}' | sort -u > "${OUT}/mpcb_strong.txt" + ${NM} --defined-only --extern-only --format=posix $(mpcb_files) 2>/dev/null \ + | awk '$2 ~ /^[WVwv]$/ {print $1}' | sort -u > "${OUT}/mpcb_weak.txt" + ${NM} --defined-only --extern-only --format=posix $(msch_files) 2>/dev/null \ + | awk '$2 ~ /^[TDBR]$/ {print $1}' | sort -u > "${OUT}/msch_strong.txt" + ${NM} --defined-only --extern-only --format=posix $(msch_files) 2>/dev/null \ + | awk '$2 ~ /^[WVwv]$/ {print $1}' | sort -u > "${OUT}/msch_weak.txt" + + # Shared-lib symbols from the MERGED tree (one definition linked once — safe + # dedup), independent of the standalone trees' shared.txt above. + ${NM} --defined-only --extern-only --format=posix \ + "${M}/common/libcommon.a" "${M}/common/libkicommon.a" \ + "${M}/common/gal/libkigal.a" "${M}/libs/core/libcore.a" \ + "${M}/libs/kimath/libkimath.a" "${M}/libs/kiplatform/libkiplatform.a" \ + "${M}/scripting/libscripting.a" "${M}/api/libkiapi.a" \ + "${M}/libs/sexpr/libsexpr.a" 2>/dev/null \ + | awk '{print $1}' | sort -u > "${OUT}/shared_merged.txt" + + echo "== cvpcb vs pcbnew STRONG duplicates (merged tree — must be empty):" + comm -12 "${OUT}/cv_strong.txt" "${OUT}/mpcb_strong.txt" || true + + echo "== cvpcb vs eeschema STRONG duplicates (merged tree — must be empty):" + comm -12 "${OUT}/cv_strong.txt" "${OUT}/msch_strong.txt" || true + + echo "== cvpcb WEAK duplicates not from shared libs, mentioning class tokens" + echo " (review anything printed — same criteria as the pcb/sch weak audit." + echo " Reviewed 2026-07-17 as identical-definition dedups, now filtered:" + echo " PCB_BASE_FRAME/PCB_VIEWER_TOOLS/PCB_EDITOR_CONDITIONS/NETLIST inline" + echo " members — cvpcb compiles pcbnew's headers with the same renames; plus" + echo " common-lib ACTIONS/COMMON_CONTROL/GetAppSettings.):" + cat "${OUT}/mpcb_weak.txt" "${OUT}/msch_weak.txt" | sort -u > "${OUT}/engines_weak.txt" + comm -12 "${OUT}/cv_weak.txt" "${OUT}/engines_weak.txt" \ + | comm -23 - "${OUT}/shared_merged.txt" \ + | grep -E '_Z(TV|TI|TS|N)[0-9]+[A-Z]' \ + | grep -vE 'magic_enum|wxEventFunctorMethod|wxNavigationEnabled|wxSimplebook|wxDataView|wxMenuBar|wxVector|KIFACE|COLLECTOR|RC_JSON|PARAM_SCALED|EDA_|BOX2|WX_MENUBAR|SEARCH_HANDLER|FOOTPRINT_|LISTBOX|PCB_BASE_FRAME|PCB_VIEWER_TOOLS|PCB_EDITOR_CONDITIONS|COMMON_CONTROL|CVPCB_SETTINGS|7ACTIONS|7NETLIST' || true +else + echo "== cvpcb audit SKIPPED (no merged kicad-kicad_editor tree with cvpcb_kiface_objects under ${BUILD_ROOT})" >&2 +fi + echo "== audit done (details in ${OUT})" diff --git a/tests/web/eeschema-assign-footprints.spec.ts b/tests/web/eeschema-assign-footprints.spec.ts new file mode 100644 index 0000000..0596366 --- /dev/null +++ b/tests/web/eeschema-assign-footprints.spec.ts @@ -0,0 +1,204 @@ +import { test, expect, type Page } from '@playwright/test'; +import { + clickMenuBarItem, + clickMenuItemByText, + waitForRegistry, +} from '../e2e/utils/element-tracker'; + +/** + * Tools → Assign Footprints... must open CvPcb (feature/cvpcb-wasm). + * + * CvPcb is the THIRD statically-linked kiface in the merged kicad_editor + * image. eeschema's OnOpenCvpcb() asks KIWAY::Player(FRAME_CVPCB); before + * this feature the WASM kiway had no FACE_CVPCB registered, Player() returned + * nullptr and the click was a silent no-op. With cvpcb_kiface_getter + * registered in single_top.cpp's merged branch, the click must create and + * raise CVPCB_MAINFRAME ("Assign Footprints") and survive the netlist mail + * round-trip (sendNetlistToCvpcb → MAIL_EESCHEMA_NETLIST). + * + * Assertions: the frame appears (page title or a registered element carrying + * its title), the runtime survives, the screenshot shows the three-pane UI — + * AND the footprint pane's data went through the wasm↔js libs bridge + * (window.kicadLibs: fp-index op and/or per-lib list/bodies), with at least + * one call answered 'ok' by the JS side, i.e. the js↔R2/CDN/backend hop + * actually served data. The symbols pane needs no bridge: components arrive + * as a netlist over kiway mail from eeschema, in-process. + */ + +const SHOT = (n: string) => `test-results/cvpcb-open-${n}.png`; + +/** CvPcb frame visible? The tab title does NOT track the raised frame, and the + * frame itself registers without its title as label — the reliable signal is + * CvPcb's own bottom-row button ("Apply, Save Schematic && Continue"), which + * no other frame has. Title kept as a bonus check. */ +async function cvpcbUp(page: Page): Promise { + if (/Assign Footprints/i.test(await page.title())) return true; + return page.evaluate(() => { + const reg = (window as any).wxElementRegistry; + if (!reg) return false; + return reg + .findAll({ visible: true }) + .some((e: any) => /Assign Footprints|Save Schematic/i.test(e.label ?? '')); + }); +} + +test('Tools → Assign Footprints opens CvPcb (merged third kiface)', async ({ page }) => { + test.setTimeout(420000); + const logs: string[] = []; + page.on('console', (m) => logs.push(`[${m.type()}] ${m.text()}`)); + const pageErrors: string[] = []; + page.on('pageerror', (e) => pageErrors.push(e.message)); + + // Record every window.kicadLibs.request the WASM issues (same wrapper as + // eeschema-fp-selector.spec.ts): window.__libsCalls = [op,lib,arg,kind, + // settle][] where settle is 'ok' | 'null' | 'err' once the JS provider + // (backed by R2/CDN or the reference backend) answers. + await page.addInitScript(() => { + const calls: unknown[][] = ((window as any).__libsCalls = []); + let inner: any; + Object.defineProperty(window, 'kicadLibs', { + configurable: true, + get: () => inner, + set: (v: any) => { + if (v && typeof v.request === 'function') { + const orig = v.request.bind(v); + v = { + ...v, + request: (...a: unknown[]) => { + const entry = a.slice(0, 4); + calls.push(entry); + const p = orig(...a); + Promise.resolve(p).then( + (r: unknown) => entry.push(r === null ? 'null' : 'ok'), + () => entry.push('err'), + ); + return p; + }, + }; + } + inner = v; + }, + }); + }); + + // The FILE deep link (not /-/eeschema, which opens the tool without a file + // and leaves an untitled empty schematic — no components to assign). + // ?trace= mirrors the WASM's print/printErr to the browser console and turns + // on the KICAD_LIBRARIES wxLogTrace channel (library adapter load states). + await page.goto('/default/projects/demo/demo.kicad_sch?trace=KICAD_LIBRARIES'); + await expect(page.locator('#canvas')).toBeVisible({ timeout: 150000 }); + await waitForRegistry(page, 150000); + await expect + .poll(() => page.title(), { timeout: 150000, intervals: [1000] }) + .toMatch(/Schematic Editor/i); + await page.screenshot({ path: SHOT('01-boot'), scale: 'css' }); + + expect(await clickMenuBarItem(page, 'Tools'), 'Tools menu opened').toBe(true); + // Do NOT use clickMenuItemByText: Playwright's mouse.click awaits the input + // ack from the content process, and if the wx handler synchronously blocks + // (the failure mode this spec exists to catch) the await deadlocks the test + // with zero diagnostics. Find the item via the registry, then dispatch the + // mouse events synthetically — dispatchEvent returns even when the handler + // later wedges the runtime, so the poll below can report what happened. + const item = await page.waitForFunction( + () => { + const norm = (s: string) => (s || '').replace(/&/g, '').replace(/[.…\s]+$/u, '').trim(); + const reg = (window as any).wxElementRegistry; + if (!reg || !reg.findAllRendered) return null; + const hit = reg + .findAllRendered({ elementType: 'menuitem' }) + .find((e: any) => norm(e.label) === 'Assign Footprints' && e.enabled !== false); + return hit ? { x: hit.centerX, y: hit.centerY } : null; + }, + null, + { timeout: 30000 }, + ).then((h) => h.jsonValue() as Promise<{ x: number; y: number }>); + await page.evaluate(({ x, y }) => { + const target = document.elementFromPoint(x, y) ?? document.body; + for (const type of ['mousedown', 'mouseup', 'click'] as const) { + target.dispatchEvent( + new MouseEvent(type, { + bubbles: true, cancelable: true, view: window, + clientX: x, clientY: y, button: 0, buttons: type === 'mousedown' ? 1 : 0, + }), + ); + } + }, item); + + // The frame construction + netlist mail runs off wxPostEvent'd follow-ups, + // which the wx WASM port only flushes on input events — wiggle the mouse + // while polling. (No auto-answering of prompts here: CvPcb itself shows an + // OK button, so a blanket "click any OK" would close the very frame under + // test. The demo schematic is annotated; if ReadyToNetlist() ever prompts, + // this poll times out and the failure dump + screenshot will show it.) + const c = { x: 400, y: 300 }; + let up = false; + for (let i = 0; i < 60 && !up; i++) { + await page.mouse.move(c.x + (i % 5) * 4, c.y + (i % 3) * 4); + up = await cvpcbUp(page); + if (!up) await page.waitForTimeout(1000); + } + + if (!up) { + // Diagnostics before failing: what IS on screen, and the console tail. + const dump = await page.evaluate(() => + (window as any).wxElementRegistry + .findAll({ visible: true }) + .map((e: any) => `${e.typeName ?? '?'}|${e.elementType ?? '?'}|${e.label ?? ''}`) + .slice(0, 120), + ); + console.log(`[cvpcb-open] no CvPcb frame; visible elements:\n${dump.join('\n')}`); + console.log(`[cvpcb-open] console tail:\n${logs.slice(-60).join('\n')}`); + console.log( + `[cvpcb-open] libs bridge calls so far:\n${JSON.stringify( + await page.evaluate(() => (window as any).__libsCalls), + )}`, + ); + await page.screenshot({ path: SHOT('99-stuck'), scale: 'css' }); + } + expect(up, 'CvPcb ("Assign Footprints") frame appeared').toBe(true); + + // Let the frame finish its first paint (footprint/symbol panes), then shoot + // for eyeball review of the three-pane UI. + await page.mouse.move(c.x + 2, c.y + 2); + await page.waitForTimeout(2000); + await page.screenshot({ path: SHOT('02-cvpcb'), scale: 'css' }); + + // The footprint pane is fed over the wasm↔js libs bridge — poll until + // footprint-kind traffic shows up (index op for CDN sources and/or per-lib + // list/bodies fat-loads), pumping the wx loop with mouse moves. + let fpCalls: string[][] = []; + for (let i = 0; i < 30; i++) { + fpCalls = ((await page.evaluate(() => (window as any).__libsCalls)) as string[][]).filter( + (call) => call[3] === 'footprint', + ); + if (fpCalls.length > 0) break; + await page.mouse.move(c.x + (i % 5) * 3, c.y + (i % 3) * 3); + await page.waitForTimeout(1000); + } + console.log( + `[cvpcb-open] fp bridge calls: ${JSON.stringify(fpCalls.map((call) => [call[0], call[1], call[2], call[4]]))}`, + ); + expect( + fpCalls.length > 0, + `CvPcb requested footprints over the wasm↔js bridge; all calls:\n${JSON.stringify( + await page.evaluate(() => (window as any).__libsCalls), + )}`, + ).toBe(true); + // At least one bridge call must have been ANSWERED by the JS side — that + // answer is the js↔R2/CDN/backend hop actually serving data ('null' means + // the source has no such artifact, 'err' a failed fetch). + expect( + fpCalls.some((call) => call[4] === 'ok'), + `a footprint bridge call settled 'ok'; fp calls:\n${JSON.stringify(fpCalls)}`, + ).toBe(true); + + // Runtime survived opening the third kiface + the netlist mail. Match real + // crash signatures only — app text legitimately contains the word "abort" + // (AbortAsyncLoad, "aborted=0" traces). + expect(pageErrors, `no page errors, got:\n${pageErrors.join('\n')}`).toEqual([]); + expect( + logs.some((l) => /RuntimeError|Aborted\(|\[boot\] abort|pump error/i.test(l)), + `no wasm abort; console tail:\n${logs.slice(-15).join('\n')}`, + ).toBe(false); +}); diff --git a/wasm/editor/CMakeLists.txt b/wasm/editor/CMakeLists.txt index 5365807..8111ded 100644 --- a/wasm/editor/CMakeLists.txt +++ b/wasm/editor/CMakeLists.txt @@ -1,10 +1,12 @@ # Merged pcbnew+eeschema WASM editor (editor-unification Part 2). # -# ONE executable links BOTH kifaces; the editor frame (PCB / Footprint / Schematic / -# Symbol) is chosen at runtime by single_top.cpp's --frame flag. Added from the kicad -# fork's top-level CMakeLists.txt via add_subdirectory( ${KICAD_WASM_LAYER}/editor ) -# when EMSCRIPTEN AND KICAD_WASM_MERGED_EDITOR — after eeschema/ and pcbnew/, so both -# kifaces' CACHE INTERNAL library lists exist here. +# ONE executable links THREE kifaces (pcbnew, eeschema, cvpcb); the editor frame +# (PCB / Footprint / Schematic / Symbol) is chosen at runtime by single_top.cpp's +# --frame flag, while cvpcb only opens in-session via eeschema's Assign Footprints +# (KIWAY::Player( FRAME_CVPCB )). Added from the kicad fork's top-level CMakeLists.txt +# via add_subdirectory( ${KICAD_WASM_LAYER}/editor ) when EMSCRIPTEN AND +# KICAD_WASM_MERGED_EDITOR — after eeschema/, pcbnew/ and cvpcb/, so the kifaces' +# CACHE INTERNAL library lists exist here. # # Collision handling (see KICAD_WASM_PCB_SIDE_RENAMES in the fork's top-level # CMakeLists.txt and docs/features/editor-unification/): the PCB-side targets compile @@ -37,10 +39,10 @@ set_source_files_properties( ${CMAKE_SOURCE_DIR}/common/single_top.cpp PROPERTIE COMPILE_DEFINITIONS "TOP_FRAME=FRAME_PCB_EDITOR;KICAD_MERGED_KIFACES" ) -# Both kifaces' full library sets (exported CACHE INTERNAL by pcbnew/ and eeschema/). -# The overlap (common, kicommon, gal, ...) is deduped; generator-expression entries -# (native-only pads_common/pcm) are inert strings that evaluate to nothing here. -set( KICAD_EDITOR_LIBS ${PCBNEW_KIFACE_LIBRARIES} ${EESCHEMA_KIFACE_LIBRARIES} ) +# All three kifaces' full library sets (exported CACHE INTERNAL by pcbnew/, eeschema/ +# and cvpcb/). The overlap (common, kicommon, gal, ...) is deduped; generator-expression +# entries (native-only pads_common/pcm) are inert strings that evaluate to nothing here. +set( KICAD_EDITOR_LIBS ${PCBNEW_KIFACE_LIBRARIES} ${EESCHEMA_KIFACE_LIBRARIES} ${CVPCB_KIFACE_LIBRARIES} ) list( REMOVE_DUPLICATES KICAD_EDITOR_LIBS ) target_link_libraries( kicad_editor PRIVATE ${KICAD_EDITOR_LIBS} )