From 4f8c2d1f56f7b3da52f5bd6256aae69bb8d5ebb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20T=C3=B6rcsv=C3=A1ri?= Date: Wed, 3 Jun 2026 17:49:11 +0200 Subject: [PATCH] feat(eeschema): collab bridge read/emit + build fixes; apply WIP (yjs-bridge commit 3) eeschema's half of the Yjs collaborative bridge, reusing the generic reconciler / BroadcastChannel transport unchanged. Zero kicad-fork change: native SCH_ITEM uuid + native SCHEMATIC_LISTENER. All in the wasm layer (wasm/bindings/eeschema_embind.cpp). Working (verified in the web app): - kicadCollabSnapshot(): enumerate sch.Hierarchy() -> LastScreen()->Items() as {id,type,x,y}; registers the listener on first call - emit: SCHEMATIC_LISTENER subclass -> per-item delta via window.kicadCollab.onDelta; fires on real SCH_COMMIT::Push (a real wire move broadcasts added/removed/changed) Apply is a documented follow-up (gated off so a peer tab can't crash): SCH_ITEM::Move traps with 'indirect call signature mismatch' when invoked outside a KiCad tool coroutine (Asyncify+fiber+exception-trampoline). Modify/Clone/GetPosition all work; only the virtual Move write traps. Fix direction: route apply through TOOL_MANAGER. Also: build-kicad-target.sh now force-relinks when only _embind.cpp changed (the embind .o isn't a make dep, so new bindings silently vanished), and adds the expected/rtree/fmt thirdparty includes the eeschema bindings need. Tests: eeschema-collab.spec.ts covers snapshot (green); apply/two-tab skipped with the blocker noted. WasmTool gates collab to pl_editor only until eeschema apply works. Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/kicad/build-kicad-target.sh | 15 + tests/kicad/eeschema-collab.spec.ts | 121 +++++++ wasm/bindings/eeschema_embind.cpp | 316 ++++++++++++++++++ web/apps/frontend/src/components/WasmTool.tsx | 19 +- 4 files changed, 464 insertions(+), 7 deletions(-) create mode 100644 tests/kicad/eeschema-collab.spec.ts diff --git a/scripts/kicad/build-kicad-target.sh b/scripts/kicad/build-kicad-target.sh index 87f4393..0432102 100755 --- a/scripts/kicad/build-kicad-target.sh +++ b/scripts/kicad/build-kicad-target.sh @@ -398,6 +398,9 @@ if [ -f "${EMBIND_SRC}" ]; then KICAD_INCLUDES+=" -I${KICAD_DIR}/libs/core/include -I${KICAD_DIR}/libs/kimath/include -I${KICAD_DIR}/libs/kiplatform/include" KICAD_INCLUDES+=" -I${KICAD_DIR}/thirdparty/clipper2/Clipper2Lib/include" KICAD_INCLUDES+=" -I${KICAD_DIR}/thirdparty/nlohmann_json" + KICAD_INCLUDES+=" -I${KICAD_DIR}/thirdparty/expected/include" + KICAD_INCLUDES+=" -I${KICAD_DIR}/thirdparty/rtree" + KICAD_INCLUDES+=" -I${KICAD_DIR}/thirdparty/fmt" KICAD_INCLUDES+=" -I${KICAD_DIR}/thirdparty/dynamic_bitset" KICAD_INCLUDES+=" -I${KICAD_DIR}/thirdparty/nanodbc" KICAD_INCLUDES+=" -I${KICAD_DIR}/thirdparty/picosha2" @@ -415,6 +418,18 @@ fi # Step 8: Build the app target kw_stage kicad-compile log_info "Building ${APP_NAME} (CMake target: ${KICAD_TARGET})..." + +# The embind object (step 7.1) is injected via linker flags, NOT tracked as a CMake/ +# make dependency — so when ONLY _embind.cpp changes, make sees no changed source, +# skips the link, and the new bindings silently vanish from the binary. Force a relink +# whenever the freshly-compiled embind object is newer than the linked output. +LINK_OUT_JS="${KICAD_BUILD}/${KICAD_SUBDIR}/${APP_NAME}.js" +LINK_OUT_WASM="${KICAD_BUILD}/${KICAD_SUBDIR}/${APP_NAME}.wasm" +if [ -f "${EMBIND_OBJ}" ] && [ -f "${LINK_OUT_JS}" ] && [ "${EMBIND_OBJ}" -nt "${LINK_OUT_JS}" ]; then + log_info "Embind object newer than ${APP_NAME}.js — forcing relink to pick up new bindings" + rm -f "${LINK_OUT_JS}" "${LINK_OUT_WASM}" +fi + emmake make -j${JOBS} "${KICAD_TARGET}" # Step 8.1: Build bitmap resources (images.tar.gz) diff --git a/tests/kicad/eeschema-collab.spec.ts b/tests/kicad/eeschema-collab.spec.ts new file mode 100644 index 0000000..dbd13a9 --- /dev/null +++ b/tests/kicad/eeschema-collab.spec.ts @@ -0,0 +1,121 @@ +import type { Page } from "@playwright/test"; +import { test, expect } from "./fixtures"; + +/** + * eeschema Yjs collaborative bridge (features/yjs-bridge commit 3) — READ side. + * + * eeschema reuses the same wire contract + generic JS reconciler as pl_editor; the new + * code is the C++ adapter (native SCHEMATIC_LISTENER emit + SCH_COMMIT apply). The + * read/emit half works (verified in the real web app: a real edit fires the listener and + * broadcasts a delta). This spec covers what is reproducible headlessly: kicadCollabSnapshot + * reflecting the schematic by uuid/type/position. + * + * APPLY is a known open follow-up (0003): editor write ops — specifically + * SCH_ITEM::Move — trap with "indirect call signature mismatch" when invoked outside a + * KiCad tool coroutine (Asyncify+fiber interaction). The apply/two-tab tests are skipped + * until apply is routed through the tool framework. See + * memory/eeschema-collab-asyncify-apply. + */ + +const WIRE1 = "22222222-0000-0000-0000-000000000001"; +const WIRE2 = "22222222-0000-0000-0000-000000000002"; +const SAMPLE_SCH = `(kicad_sch +\t(version 20250114) +\t(generator "eeschema") +\t(generator_version "9.0") +\t(uuid "11111111-1111-1111-1111-111111111111") +\t(paper "A4") +\t(lib_symbols) +\t(wire (pts (xy 50.8 50.8) (xy 101.6 50.8)) (stroke (width 0) (type default)) (uuid "${WIRE1}")) +\t(wire (pts (xy 50.8 76.2) (xy 101.6 76.2)) (stroke (width 0) (type default)) (uuid "${WIRE2}")) +\t(sheet_instances (path "/" (page "1"))) +) +`; + +type FS = { mkdirTree(p: string): void; writeFile(p: string, d: string): void }; +type Mod = { + kicadOpenFile(p: string): unknown; + kicadCollabSnapshot(): string; + kicadCollabGetPos(id: string): string; +}; + +function hasAbort(l: { consoleLogs: string[]; errors: string[] }): boolean { + return [...l.consoleLogs, ...l.errors].some((s) => s.includes("Aborted(")); +} + +async function bootAndOpen(page: Page, name: string): Promise { + await page.goto("/kicad/eeschema.html"); + await expect(page.locator("#canvas")).toBeVisible({ timeout: 90000 }); + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: 90000 }); + await page.waitForFunction( + () => { + const m = (window as unknown as { Module?: Mod }).Module; + return ( + typeof m?.kicadOpenFile === "function" && + typeof m?.kicadCollabSnapshot === "function" + ); + }, + null, + { timeout: 90000 }, + ); + await page.waitForFunction( + () => + !!window.wxElementRegistry && + window.wxElementRegistry + .findAll({ visible: true }) + .some((e) => /Frame$/.test(e.typeName) || (e.name || "").endsWith("Frame")), + null, + { timeout: 90000 }, + ); + + await page.evaluate( + ({ content, name }) => { + const w = window as unknown as { FS: FS; Module: Mod }; + const dir = "/home/kicad/documents"; + try { + w.FS.mkdirTree(dir); + } catch { + /* exists */ + } + const p = `${dir}/${name}.kicad_sch`; + w.FS.writeFile(p, content); + w.Module.kicadOpenFile(p); + }, + { content: SAMPLE_SCH, name }, + ); + + await expect.poll(() => page.title(), { timeout: 30000 }).toMatch(new RegExp(name, "i")); +} + +test.describe("eeschema collab bridge — snapshot (read side)", () => { + test("kicadCollabSnapshot reflects schematic items by uuid/type/position", async ({ + page, + testLogger, + }) => { + await bootAndOpen(page, "single"); + + const snap = await page.evaluate(() => JSON.parse(window.Module.kicadCollabSnapshot())); + const byId = new Map( + snap.added.map((i: { id: string; type: string; x: number; y: number }) => [i.id, i]), + ); + + expect(byId.has(WIRE1)).toBe(true); + expect(byId.has(WIRE2)).toBe(true); + expect(byId.get(WIRE1)!.type).toBe("SCH_LINE"); + // 50.8 mm in eeschema internal units (×10000) = 508000. + expect(byId.get(WIRE1)!.x).toBe(508000); + expect(byId.get(WIRE1)!.y).toBe(508000); + + // getPos resolves the same item by uuid. + const pos = await page.evaluate((id) => window.Module.kicadCollabGetPos(id), WIRE1); + expect(pos).toBe("508000,508000"); + + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + // BLOCKED (0003 follow-up): apply traps on SCH_ITEM::Move outside a tool coroutine. + // Re-enable once apply is routed through the TOOL_MANAGER. The read/emit side is proven + // working in the real web app; only programmatic apply is affected. + test.skip("apply moves an item by uuid (blocked: SCH_ITEM::Move coroutine trap)", () => {}); + test.skip("two-tab move propagates A→B (blocked: same apply trap)", () => {}); +}); diff --git a/wasm/bindings/eeschema_embind.cpp b/wasm/bindings/eeschema_embind.cpp index 116caa4..726444f 100644 --- a/wasm/bindings/eeschema_embind.cpp +++ b/wasm/bindings/eeschema_embind.cpp @@ -6,15 +6,28 @@ */ #ifdef __EMSCRIPTEN__ +#include #include #include #include +#include +#include +#include #include #include #include #include +#include +#include +#include +#include +#include +#include +#include +#include using namespace emscripten; +using json = nlohmann::json; // Programmatically open a project file (schematic) in the running editor frame, // without UI automation. Mirrors single_top.cpp's MacOpenFile path: the editor @@ -36,8 +49,311 @@ bool kicadOpenFile( std::string path ) std::vector( 1, wxString::FromUTF8( path.c_str() ) ) ); } +// ───────────────────────────── Yjs collaborative bridge ───────────────────────────── +// +// eeschema's half of the unified bridge contract (features/yjs-bridge/0001, 0003). +// Unlike pl_editor it needs NO kicad-fork change: SCH_ITEM already carries a stable +// KIID, and eeschema has native change machinery, so the adapter is a thin re-use: +// ChangeSource (emit) = a SCHEMATIC_LISTENER subclass (SCH_COMMIT::Push fires it) +// apply = SCH_COMMIT Modify/Remove + Push (drives connectivity recompute) +// The generic JS reconciler / transport / WasmTool wiring are reused unchanged. +// +// Scope of this first commit (0003 §"first PoC"): position-level sync of existing +// items — changed (move/edit) and removed. Decomposed field coverage via reflection, +// constructing arbitrary new item types on `added`, and symbol-instance / multi-sheet +// scoping are deferred (see TODOs). Items are resolved by globally-unique uuid, so +// changed/removed already work across the whole hierarchy without sheet scoping. +namespace { + +// Guard so SCH_COMMIT::Push's listener callbacks during apply() aren't re-emitted. +bool s_applyingRemote = false; + +std::string toUtf8( const wxString& s ) { return std::string( s.utf8_str() ); } + +SCH_EDIT_FRAME* schFrame() +{ + return wxTheApp ? dynamic_cast( wxTheApp->GetTopWindow() ) : nullptr; +} + +json itemToJson( SCH_ITEM* aItem ) +{ + VECTOR2I p = aItem->GetPosition(); + return json{ + { "id", toUtf8( aItem->m_Uuid.AsString() ) }, + { "type", toUtf8( aItem->GetClass() ) }, + { "x", p.x }, // internal units (nm); integral, so no quantization needed + { "y", p.y }, + }; +} + +// Full current model as an array of item json, deduped by uuid across the hierarchy. +json snapshotItems( SCHEMATIC& aSch ) +{ + json arr = json::array(); + std::set seen; + + for( const SCH_SHEET_PATH& path : aSch.Hierarchy() ) + { + SCH_SCREEN* screen = const_cast( path ).LastScreen(); + + if( !screen ) + continue; + + for( SCH_ITEM* item : screen->Items() ) + { + std::string id = toUtf8( item->m_Uuid.AsString() ); + + if( seen.insert( id ).second ) + arr.push_back( itemToJson( item ) ); + } + } + + return arr; +} + +void emit( const json& aDelta ) +{ + std::string s = aDelta.dump(); + EM_ASM( { + if( window.kicadCollab && window.kicadCollab.onDelta ) + window.kicadCollab.onDelta( UTF8ToString( $0 ) ); + }, s.c_str() ); +} + +// ChangeSource: native SCHEMATIC_LISTENER. SCH_COMMIT::Push fires these in bulk for +// every local edit (move, add, remove, …) — that's our emit trigger. +class COLLAB_LISTENER : public SCHEMATIC_LISTENER +{ +public: + void OnSchItemsAdded( SCHEMATIC&, std::vector& aItems ) override + { + emitItems( "added", aItems ); + } + + void OnSchItemsChanged( SCHEMATIC&, std::vector& aItems ) override + { + emitItems( "changed", aItems ); + } + + void OnSchItemsRemoved( SCHEMATIC&, std::vector& aItems ) override + { + if( s_applyingRemote ) + return; + + json removed = json::array(); + + for( SCH_ITEM* item : aItems ) + removed.push_back( toUtf8( item->m_Uuid.AsString() ) ); + + if( !removed.empty() ) + emit( json{ { "added", json::array() }, { "changed", json::array() }, + { "removed", removed } } ); + } + +private: + void emitItems( const char* aKey, std::vector& aItems ) + { + if( s_applyingRemote ) + return; + + json arr = json::array(); + + for( SCH_ITEM* item : aItems ) + arr.push_back( itemToJson( item ) ); + + if( arr.empty() ) + return; + + json d = { { "added", json::array() }, { "changed", json::array() }, + { "removed", json::array() } }; + d[aKey] = arr; + emit( d ); + } +}; + +COLLAB_LISTENER* g_listener = nullptr; + +// Get the live SCHEMATIC and ensure our listener is registered on it (idempotent). +SCHEMATIC* ensureBridge() +{ + SCH_EDIT_FRAME* fr = schFrame(); + + if( !fr ) + return nullptr; + + SCHEMATIC& sch = fr->Schematic(); + + if( !g_listener ) + { + g_listener = new COLLAB_LISTENER(); + sch.AddListener( g_listener ); + } + + return &sch; +} + +} // namespace + + +namespace { + +// The actual model mutation. Must run inside a KiCad tool coroutine — calling editor +// write ops (notably SCH_ITEM::Move) outside one traps with "indirect call signature +// mismatch" (the Asyncify+fiber+exception-trampoline machinery; see +// memory/eeschema-collab-asyncify-apply / 0003). Routing apply through the tool +// framework is the open follow-up; reads (GetPosition) and Clone/Modify already work. +void doApply( SCH_EDIT_FRAME* aFrame, const json& aDelta ) +{ + SCHEMATIC& sch = aFrame->Schematic(); + + s_applyingRemote = true; + + SCH_COMMIT commit( aFrame ); + bool staged = false; + + for( const json& rid : aDelta.value( "removed", json::array() ) ) + { + SCH_SHEET_PATH path; + KIID id( wxString::FromUTF8( rid.get().c_str() ) ); + + if( SCH_ITEM* item = sch.ResolveItem( id, &path, /*allowNull*/ true ) ) + { + commit.Remove( item, path.LastScreen() ); + staged = true; + } + } + + for( const json& j : aDelta.value( "changed", json::array() ) ) + { + SCH_SHEET_PATH path; + KIID id( wxString::FromUTF8( j.value( "id", "" ).c_str() ) ); + + if( SCH_ITEM* item = sch.ResolveItem( id, &path, /*allowNull*/ true ) ) + { + commit.Modify( item, path.LastScreen() ); + + if( j.contains( "x" ) && j.contains( "y" ) ) + { + VECTOR2I newPos( j["x"].get(), j["y"].get() ); + item->Move( newPos - item->GetPosition() ); + } + + staged = true; + } + } + + // TODO(0003 follow-up): `added` requires constructing the right SCH_ITEM subclass + // per KICAD_T from JSON (no per-item blob path in eeschema — §serialization note). + + if( staged ) + commit.Push( wxT( "Collaborative edit" ) ); + + s_applyingRemote = false; +} + +} // namespace + + +// JS → C++. Apply a remote per-item delta by uuid, through SCH_COMMIT so connectivity/ +// ERC/hierarchy recompute the same way a UI edit would (0003 §apply). +// +// SCH_COMMIT must run in the editor's Asyncify-rooted main loop — invoking it from this +// embind ccall, or from an emscripten_async_call/setTimeout callback, traps with an +// "indirect call signature mismatch" because those are not the asyncify root (0001 §5). +// wxEvtHandler::CallAfter queues onto the app's pending-event list, which the wasm main +// loop drains every frame via ProcessPendingEvents() (src/wasm/evtloop.cpp) — i.e. the +// exact context real UI edits run in. So defer the whole mutation there. +void kicadCollabApply( std::string aJson ) +{ + json delta = json::parse( aJson, nullptr, /*allow_exceptions*/ false ); + + if( delta.is_discarded() ) + return; + + SCH_EDIT_FRAME* fr = schFrame(); + + if( !fr ) + return; + + fr->CallAfter( [fr, delta]() { doApply( fr, delta ); } ); +} + + +// JS pull of the full current model as an all-"added" delta (seed/baseline). Also +// registers the change listener on first call. +std::string kicadCollabSnapshot() +{ + SCHEMATIC* sch = ensureBridge(); + json added = sch ? snapshotItems( *sch ) : json::array(); + + return json{ { "added", added }, { "changed", json::array() }, + { "removed", json::array() } }.dump(); +} + + +// Test/PoC helper: move the first schematic item by (dx,dy) IU via a real SCH_COMMIT, +// firing the listener — a deterministic local edit for the two-tab demo / e2e. +// Returns the moved item's uuid. +std::string kicadCollabTestMoveFirst( int aDx, int aDy ) +{ + SCH_EDIT_FRAME* fr = schFrame(); + + if( !fr ) + return ""; + + SCHEMATIC& sch = fr->Schematic(); + + for( const SCH_SHEET_PATH& path : sch.Hierarchy() ) + { + SCH_SCREEN* screen = const_cast( path ).LastScreen(); + + if( !screen ) + continue; + + for( SCH_ITEM* item : screen->Items() ) + { + // Defer the SCH_COMMIT to the main loop (same Asyncify reason as apply). + fr->CallAfter( [fr, item, screen, aDx, aDy]() { + SCH_COMMIT commit( fr ); + commit.Modify( item, screen ); + item->Move( VECTOR2I( aDx, aDy ) ); + commit.Push( wxT( "Collab test move" ) ); + } ); + return toUtf8( item->m_Uuid.AsString() ); + } + } + + return ""; +} + + +// Test helper: read an item's position by uuid as "x,y" (internal units). +std::string kicadCollabGetPos( std::string aId ) +{ + SCH_EDIT_FRAME* fr = schFrame(); + + if( !fr ) + return ""; + + KIID id( wxString::FromUTF8( aId.c_str() ) ); + + if( SCH_ITEM* item = fr->Schematic().ResolveItem( id, nullptr, /*allowNull*/ true ) ) + { + VECTOR2I p = item->GetPosition(); + return std::to_string( p.x ) + "," + std::to_string( p.y ); + } + + return ""; +} + + EMSCRIPTEN_BINDINGS(eeschema) { // Programmatic file open (preferred over UI automation from the web app). function("kicadOpenFile", &kicadOpenFile); + // Yjs collaborative bridge entry points (same contract as pl_editor). + function("kicadCollabApply", &kicadCollabApply); + function("kicadCollabSnapshot", &kicadCollabSnapshot); + function("kicadCollabTestMoveFirst", &kicadCollabTestMoveFirst); + function("kicadCollabGetPos", &kicadCollabGetPos); } #endif diff --git a/web/apps/frontend/src/components/WasmTool.tsx b/web/apps/frontend/src/components/WasmTool.tsx index 75d5aef..32a5a90 100644 --- a/web/apps/frontend/src/components/WasmTool.tsx +++ b/web/apps/frontend/src/components/WasmTool.tsx @@ -8,12 +8,17 @@ import { driveProjectIntoTool } from "@/wasm/kicad-runner"; import type { CollabWindow } from "@/wasm/collab"; import { clog, cwarn } from "@/wasm/collab/debug"; +// Tools with a *fully working* collab bridge. eeschema's bridge exists (read/emit work) +// but its apply traps on SCH_ITEM::Move outside a tool coroutine (features/yjs-bridge +// 0003 follow-up), so it stays gated off to avoid crashing a peer tab. +const COLLAB_TOOLS = new Set(["pl_editor"]); + /** * Opt-in collaborative editing (features/yjs-bridge). Enabled when the URL carries - * `?collab=1` and the tool is pl_editor (the only tool with the collab bridge so far). - * Open the same project URL in two tabs with `?collab=1` to edit together: the channel - * is keyed to project+file, so both tabs share one Y.Doc over BroadcastChannel. Edits - * in the editor (add/move text, lines, …) fire OnModify → the differ → the peer tab. + * `?collab=1` and the tool has the collab bridge. Open the same project URL in two tabs + * with `?collab=1` to edit together: the channel is keyed to project+file, so both tabs + * share one Y.Doc over BroadcastChannel. Editor edits (add/move items) fire the tool's + * change hook → the bridge → the peer tab. */ async function maybeStartCollab( win: ToolWindow, @@ -40,15 +45,15 @@ async function maybeStartCollab( clog("disabled (no ?collab=1) — skipping"); return; } - if (opts.tool !== "pl_editor") { - clog(`tool is ${opts.tool}, not pl_editor — skipping`); + if (!COLLAB_TOOLS.has(opts.tool)) { + clog(`tool ${opts.tool} has no collab bridge — skipping`); return; } if (typeof mod?.kicadCollabSnapshot !== "function") { cwarn( "BRIDGE NOT PRESENT: Module.kicadCollabSnapshot is", typeof mod?.kicadCollabSnapshot, - "— the loaded pl_editor.wasm predates the collab bridge. Rebuild + `npm run setup:kicad` and restart the dev server.", + `— the loaded ${opts.tool}.wasm predates the collab bridge. Rebuild + \`npm run setup:kicad\` and restart the dev server.`, ); return; }