From c6716e6b60fc6a78804d9eae6cb68ab291098801 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Wed, 1 Jul 2026 17:39:42 +0200 Subject: [PATCH] =?UTF-8?q?feat(wasm):=20unify=20editor=20builds=20?= =?UTF-8?q?=E2=80=94=20footprint/symbol=20editors=20via=20runtime=20--fram?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The footprint and symbol editors are no longer separate WASM bundles: the frontend loads the parent pcbnew/eeschema bundle and passes --frame=fpedit / --frame=symedit (TOOL_BUNDLE + TOOL_FRAME -> Module.arguments in boot). Drops the two duplicate build+deploy targets and their wrapper scripts + vestigial embind; adds low-level harnesses (footprint_editor.html, symbol_editor.html) and a runtime-frame spec. Bumps the kicad submodule to the runtime --frame launcher. The frame-runtime spec is listed in PCBNEW_FAMILY_SPECS so CI routes it to the chromium-ci (V8) project — its footprint case boots the pcbnew module, which OOMs SpiderMonkey on x86 CI. Includes the editor-unification dossier (research docs 01-04 + the as-built implementation record 05). Co-Authored-By: Claude Opus 4.8 (1M context) Co-Authored-By: Claude Fable 5 --- docker/build.sh | 13 +- .../01-current-build-structure.md | 133 ++++++++++++++ .../02-kiface-architecture.md | 169 ++++++++++++++++++ .../03-part1-library-editor-unification.md | 110 ++++++++++++ .../04-part2-single-app-merge.md | 142 +++++++++++++++ .../05-part1-implementation.md | 99 ++++++++++ docs/features/editor-unification/README.md | 91 ++++++++++ kicad | 2 +- scripts/deploy/publish-wasm.mjs | 8 +- scripts/kicad/build-footprint_editor.sh | 9 - scripts/kicad/build-kicad-target.sh | 40 ++--- scripts/kicad/build-symbol_editor.sh | 7 - tests/apps/kicad/footprint_editor.html | 153 ++++++++++++++++ tests/apps/kicad/symbol_editor.html | 9 +- tests/kicad/frame-runtime.spec.ts | 67 +++++++ tests/playwright-kicad.config.ts | 3 + tests/scripts/setup-kicad-wasm.sh | 9 +- wasm/bindings/symbol_editor_embind.cpp | 31 ---- web/standalone/src/components/WasmTool.tsx | 5 +- web/standalone/src/wasm/boot.ts | 23 ++- web/standalone/src/wasm/constants.ts | 35 ++++ web/standalone/src/wasm/wasm-assets.ts | 11 +- 22 files changed, 1066 insertions(+), 103 deletions(-) create mode 100644 docs/features/editor-unification/01-current-build-structure.md create mode 100644 docs/features/editor-unification/02-kiface-architecture.md create mode 100644 docs/features/editor-unification/03-part1-library-editor-unification.md create mode 100644 docs/features/editor-unification/04-part2-single-app-merge.md create mode 100644 docs/features/editor-unification/05-part1-implementation.md create mode 100644 docs/features/editor-unification/README.md delete mode 100755 scripts/kicad/build-footprint_editor.sh delete mode 100755 scripts/kicad/build-symbol_editor.sh create mode 100644 tests/apps/kicad/footprint_editor.html create mode 100644 tests/kicad/frame-runtime.spec.ts delete mode 100644 wasm/bindings/symbol_editor_embind.cpp diff --git a/docker/build.sh b/docker/build.sh index 55f567f..f3d7c31 100755 --- a/docker/build.sh +++ b/docker/build.sh @@ -6,11 +6,10 @@ # ./docker/build.sh [,...] [args...] # # Apps: -# pcbnew PCB editor -# eeschema schematic editor +# pcbnew PCB editor (also serves the footprint editor via --frame=fpedit) +# eeschema schematic editor (also serves the symbol editor via --frame=symedit) # calculator PCB calculator # pl_editor drawing-sheet editor -# symbol_editor symbol editor (eeschema kiface, FRAME_SCH_SYMBOL_EDITOR) # gerbview Gerber viewer # all build all of the above # @@ -66,7 +65,7 @@ trap 'kw_fail 130; exit 130' INT TERM cd "$(dirname "$0")/.." -VALID_APPS="pcbnew | eeschema | calculator | pl_editor | symbol_editor | footprint_editor | gerbview | sym_convert | all" +VALID_APPS="pcbnew | eeschema | calculator | pl_editor | gerbview | sym_convert | all" usage() { echo "Usage: ./docker/build.sh [,...] [args...]" >&2 @@ -92,12 +91,12 @@ shift # pcbnew first in "all" — its 90-min host-side wasm-opt chain is the critical # path, so it must start as early as possible (especially with KICAD_PIPELINE=1). if [[ "$APP_NAME" == "all" ]]; then - APPS=(pcbnew eeschema calculator pl_editor symbol_editor footprint_editor gerbview) + APPS=(pcbnew eeschema calculator pl_editor gerbview) else IFS=',' read -r -a APPS <<< "$APP_NAME" for app in "${APPS[@]}"; do case "$app" in - pcbnew|eeschema|calculator|pl_editor|symbol_editor|footprint_editor|gerbview|sym_convert) ;; + pcbnew|eeschema|calculator|pl_editor|gerbview|sym_convert) ;; *) echo "Error: unknown app '$app' (expected: ${VALID_APPS})" >&2 usage @@ -196,8 +195,6 @@ kicad_subdir_for() { case "$1" in calculator) echo "pcb_calculator" ;; pl_editor) echo "pagelayout_editor" ;; - symbol_editor) echo "eeschema" ;; - footprint_editor) echo "pcbnew" ;; sym_convert) echo "eeschema" ;; *) echo "$1" ;; esac diff --git a/docs/features/editor-unification/01-current-build-structure.md b/docs/features/editor-unification/01-current-build-structure.md new file mode 100644 index 0000000..6d3ecb4 --- /dev/null +++ b/docs/features/editor-unification/01-current-build-structure.md @@ -0,0 +1,133 @@ +# 01 — Current build structure (the as-built baseline) + +> What we compile today and why two pairs of apps are duplicates. Evidence from +> `docker/build.sh`, `scripts/kicad/build-kicad-target.sh`, the per-app `CMakeLists.txt`, +> and the on-disk artifact sizes. Read [`02-kiface-architecture.md`](02-kiface-architecture.md) +> for *why* this duplication is avoidable. + +## The 7 apps + +`docker/build.sh` is the orchestrator. Valid apps (`docker/build.sh:69`): + +``` +pcbnew | eeschema | calculator | pl_editor | symbol_editor | footprint_editor | gerbview | sym_convert | all +``` + +`all` (`docker/build.sh:95`) expands to the 7 GUI tools (everything above except `sym_convert`, +which is a headless node CLI `.lib → .kicad_sym` converter, not an editor). Each app dispatches +to `scripts/kicad/build-.sh`, a thin wrapper around +`scripts/kicad/build-kicad-target.sh `, whose `case` (`build-kicad-target.sh:46-79`) maps +each app to a CMake target, a source subdir, and — crucially — a `FRAME_T`: + +| app | CMake target | subdir | `TOP_FRAME` (the baked-in frame) | +|---|---|---|---| +| `pcbnew` | `pcbnew` | `pcbnew` | `FRAME_PCB_EDITOR` (board editor) | +| `footprint_editor` | `footprint_editor` | `pcbnew` | `FRAME_FOOTPRINT_EDITOR` | +| `eeschema` | `eeschema` | `eeschema` | `FRAME_SCH` (schematic editor) | +| `symbol_editor` | `symbol_editor` | `eeschema` | `FRAME_SCH_SYMBOL_EDITOR` | +| `gerbview` | `gerbview` | `gerbview` | `FRAME_GERBER` | +| `pl_editor` | `pl_editor` | `pagelayout_editor` | `FRAME_PL_EDITOR` | +| `calculator` | `pcb_calculator` | `pcb_calculator` | (standalone, `OUTPUT_NAME=calculator`) | + +Note that `footprint_editor`'s subdir is `pcbnew` and `symbol_editor`'s is `eeschema` — because +they are *part of those modules*, not separate codebases. + +## The two duplicate pairs + +### PCB side: `pcbnew` and `footprint_editor` are one kiface, two launchers + +The pcbnew module compiles **once** into an object library `pcbnew_kiface_objects` +(`kicad/pcbnew/CMakeLists.txt:831`) that already contains **both** frame implementations: + +- `pcb_edit_frame.cpp` (the board editor, `PCB_EDIT_FRAME`) +- `footprint_edit_frame.cpp` (the footprint editor, `FOOTPRINT_EDIT_FRAME`) +- plus their shared bases `pcb_base_edit_frame.cpp` / `pcb_base_frame.cpp`, and the footprint + viewer / chooser / wizard frames. + +The `footprint_editor` executable is added in a WASM-only block +(`kicad/pcbnew/CMakeLists.txt:982-1018`) whose own comment says it plainly: + +> *"The footprint editor (FRAME_FOOTPRINT_EDITOR) is served by the pcbnew kiface and its +> sources are already compiled into the pcbnew kiface objects, so we only need a second +> single_top launcher that opens that frame instead of FRAME_PCB_EDITOR. Mirrors eeschema's +> symbol_editor target."* (`kicad/pcbnew/CMakeLists.txt:983-990`) + +It links the **same** `${PCBNEW_KIFACE_LIBRARIES}` as `pcbnew` and differs only in the launcher +define: `TOP_FRAME=FRAME_FOOTPRINT_EDITOR` (`:1001`) vs the board editor's +`TOP_FRAME=FRAME_PCB_EDITOR` (`:807`/`:813`). The build script reflects the sharing — the +footprint editor reuses pcbnew's embind and stubs: `EMBIND_APP="pcbnew"`, `STUB_APP="pcbnew"` +(`build-kicad-target.sh:88`, `:100`). + +### Schematic side: `eeschema` and `symbol_editor`, identically + +`EESCHEMA_LIBEDIT_SRCS` (`kicad/eeschema/CMakeLists.txt:367-378` — the `symbol_editor/*.cpp`) +folds into `eeschema_kiface_objects` (`:656`). The `symbol_editor` executable +(`kicad/eeschema/CMakeLists.txt:758-788`) is a second `single_top` launcher linking the same +`${EESCHEMA_KIFACE_LIBRARIES}` with `TOP_FRAME=FRAME_SCH_SYMBOL_EDITOR;PGM_DATA_FILE_EXT="kicad_sym"` +(`:778`) vs eeschema's `TOP_FRAME=FRAME_SCH` (`:636`/`:641`). (Symbol carries one *extra* +define beyond the frame — the data-file extension — which Part 1 must also make runtime; +footprint has no such extra.) See [`../symbol-editor/0001-symbol-editor-port.md`](../symbol-editor/0001-symbol-editor-port.md). + +### The smoking gun: artifact sizes + +A full build populates `output/` (sizes from the current tree): + +| artifact | size | editor | +|---|---|---| +| `pcbnew.wasm` | **146 MB** | board editor | +| `footprint_editor.wasm` | **146 MB** | footprint editor | +| `eeschema.wasm` | **82 MB** | schematic editor | +| `symbol_editor.wasm` | **82 MB** | symbol editor | +| `gerbview.wasm` | 42 MB | gerber viewer | +| `pl_editor.wasm` | 44 MB | drawing-sheet editor | +| `calculator.wasm` | 30 MB | PCB calculator | + +`146 == 146` and `82 == 82` is the whole story: we are not building four independent +codebases, we are linking **two** kifaces into **four** launchers. Debug builds additionally +emit `.wasm.debug.wasm` DWARF sidecars (~1.6 GB each for pcbnew/footprint). Shared +runtime files (`wx.js`, `wx-dom.js`, `images.tar.gz`) are copied alongside +(`docker/build.sh:236-240`). + +## How the editor is selected: build time, not runtime + +The frame is fixed per-binary at compile time. The universal launcher +`kicad/common/single_top.cpp` opens whatever `TOP_FRAME` names (`:418-420`): + +```cpp +// "TOP_FRAME" is a macro that is passed on compiler command line from CMake, +// and is one of the types in FRAME_T. +KIWAY_PLAYER* frame = Kiway.Player( TOP_FRAME, true ); +``` + +`TOP_FRAME` is a per-target CMake `COMPILE_DEFINITIONS` entry (`-DTOP_FRAME=FRAME_xxx`). The +same `single_top.cpp` source is compiled once per app; the footprint/symbol launchers +`configure_file`-copy it to a private TU so each can carry its own `TOP_FRAME`. + +**The JS side picks an editor purely by which `.wasm`/`.js` bundle it loads — no frame type is +ever passed in.** The routing tables: + +- `web/pcbjam-shared/src/schemas.ts:4-12` — the canonical `TOOLS` enum (the 7 tools). +- `web/pcbjam-shared/src/schemas.ts:28-32` — `EXTENSION_TOOL` (`.kicad_pcb`→`pcbnew`, + `.kicad_sch`→`eeschema`, `.kicad_wks`→`pl_editor`). +- `web/pcbjam-shared/src/schemas.ts:39-42` — `LIB_EXTENSION_TOOL` (`.kicad_sym`→`symbol_editor`, + `.kicad_mod`→`footprint_editor`); `routes.ts:60` routes a footprint/symbol lib to its tool. +- `web/standalone/src/wasm/constants.ts:27-35` — `TOOL_ARGV0` sets `Module.thisProgram = + /usr/bin/` per tool (this is argv[0] for the WASM's DEBUG/identity check, **not** a + frame selector). +- `web/standalone/src/wasm/wasm-assets.ts` / `components/WasmTool.tsx` — resolve the chosen + tool to its CDN bundle (`wasm///`) and boot it. +- `scripts/deploy/publish-wasm.mjs:33-41` — publishes each of the 7 as a separate + `wasm///.{wasm,js}`. + +So: **JS chooses the editor by loading a different wasm module; the module itself can only ever +open its one baked-in frame.** That is precisely the constraint Part 1 removes. + +## Loose ends found + +- `wasm/bindings/symbol_editor_embind.cpp` and `wasm/bindings/footprint_editor_embind.cpp` + exist but are **unused** — the build forces `EMBIND_APP=eeschema`/`pcbnew` for those two + (`build-kicad-target.sh:86-93`), and nothing references the files. Vestigial; no per-app + embind divergence to reconcile when merging. +- All 7 GUI tools are exercised by `tests/web/tools-open.spec.ts:28-34` (boots each, asserts + title + painted canvas + no abort). The footprint editor has no dedicated `tests/kicad/*.spec.ts` + but is covered by the web suite (`tests/web/footprint-*-remote.spec.ts`) and `tools-open`. diff --git a/docs/features/editor-unification/02-kiface-architecture.md b/docs/features/editor-unification/02-kiface-architecture.md new file mode 100644 index 0000000..ba7071c --- /dev/null +++ b/docs/features/editor-unification/02-kiface-architecture.md @@ -0,0 +1,169 @@ +# 02 — KiCad's kiway / kiface / `FRAME_T` architecture (primer) + +> The upstream mechanism that already selects editors at runtime. Understanding this is what +> makes Part 1 obviously cheap and Part 2 precisely scoped. All paths under `kicad/`. + +The one-sentence contract: **one compiled module (a "kiface") implements many editor windows +("frames"), and the specific window is chosen at runtime by passing a `FRAME_T` enum value to a +single factory method, `KIFACE::CreateKiWindow`.** Everything below is the machinery behind that +sentence. + +## `FRAME_T` — the runtime selector + +`include/frame_type.h:34-65` enumerates every top-level editor window. The relevant entries: + +```cpp +enum FRAME_T +{ + FRAME_SCH = 0, + FRAME_SCH_SYMBOL_EDITOR, // symbol editor + FRAME_SCH_VIEWER, + ... + FRAME_PCB_EDITOR, // board editor + FRAME_FOOTPRINT_EDITOR, // footprint editor + FRAME_FOOTPRINT_VIEWER, + ... + FRAME_GERBER, + FRAME_PL_EDITOR, + FRAME_CALC, + ... + KIWAY_PLAYER_COUNT, // count of the editor-window subset (the array bound) + KICAD_MAIN_FRAME_T = KIWAY_PLAYER_COUNT, // the project manager — NOT a player + FRAME_T_COUNT, +``` + +This is the "flag." A board editor *is* `FRAME_PCB_EDITOR`; a footprint editor *is* +`FRAME_FOOTPRINT_EDITOR`. They are sibling values in one enum. + +## `KIFACE::CreateKiWindow` — the factory + +`include/kiway.h:216-217`: + +```cpp +virtual wxWindow* CreateKiWindow( wxWindow* aParent, int aClassId, + KIWAY* aKIWAY, int aCtlBits = 0 ) = 0; +``` + +`aClassId` is a `FRAME_T` (widened to `int` to keep the cross-module ABI mangling-free). Each +kiface implements this as a `switch` that `new`s the concrete frame: + +**pcbnew** (`kicad/pcbnew/pcbnew.cpp:255-290`): + +```cpp +wxWindow* CreateKiWindow( wxWindow* aParent, int aClassId, KIWAY* aKiway, int aCtlBits = 0 ) override +{ + switch( aClassId ) + { + case FRAME_PCB_EDITOR: // :259 + { + auto frame = new PCB_EDIT_FRAME( aKiway, aParent ); + if( Kiface().IsSingle() ) frame->CreateServer( ... ); + return frame; + } + case FRAME_FOOTPRINT_EDITOR: // :277 + return new FOOTPRINT_EDIT_FRAME( aKiway, aParent ); + case FRAME_FOOTPRINT_VIEWER: return new FOOTPRINT_VIEWER_FRAME( aKiway, aParent ); + ... +``` + +**eeschema** (`kicad/eeschema/eeschema.cpp:187-234`) is identical in shape: +`case FRAME_SCH:` → `new SCH_EDIT_FRAME`, `case FRAME_SCH_SYMBOL_EDITOR:` → `new SYMBOL_EDIT_FRAME`. + +A single kiface instance serves all of its frame types — e.g. +`} kiface( "pcbnew", KIWAY::FACE_PCB );` (`pcbnew.cpp:592`). **This is the load-bearing fact for +Part 1:** the board editor and footprint editor are two `case` arms of one switch in one module. +The frame classes are distinct sibling subclasses (`PCB_EDIT_FRAME` /`FOOTPRINT_EDIT_FRAME`, +both deriving `PCB_BASE_EDIT_FRAME → PCB_BASE_FRAME`; `SCH_EDIT_FRAME` /`SYMBOL_EDIT_FRAME`, +both deriving `SCH_BASE_FRAME`) — but they ship in the same compiled object library and are +selected purely by the runtime `FRAME_T`. + +## `KIWAY::Player` — `FRAME_T` → kiface → frame + +`common/kiway.cpp:445-495` is the public entry. Given a `FRAME_T` it (1) maps it to a face, (2) +gets that kiface, (3) calls the factory, (4) caches the result: + +```cpp +KIWAY_PLAYER* KIWAY::Player( FRAME_T aFrameType, bool doCreate, wxTopLevelWindow* aParent ) +{ + KIWAY_PLAYER* frame = GetPlayerFrame( aFrameType ); // already-open singleton? + if( frame ) return frame; + if( doCreate ) { + FACE_T face_type = KifaceType( aFrameType ); // FRAME_T -> FACE_T + KIFACE* kiface = KiFACE( face_type ); // load/return the module + frame = (KIWAY_PLAYER*) kiface->CreateKiWindow( aParent, aFrameType, this, m_ctl ); // :474 + ... + } +} +``` + +`KifaceType()` (`common/kiway.cpp:383-424`) is the **many-frames-per-kiface map** — and the +detail that makes Part 1 free: + +```cpp +case FRAME_SCH: +case FRAME_SCH_SYMBOL_EDITOR: +case FRAME_SCH_VIEWER: return FACE_SCH; // both schematic + symbol -> ONE face + +case FRAME_PCB_EDITOR: +case FRAME_FOOTPRINT_EDITOR: +case FRAME_FOOTPRINT_VIEWER: return FACE_PCB; // both board + footprint -> ONE face +... +``` + +`FACE_PCB` (pcbnew) owns 6 frame types; `FACE_SCH` (eeschema) owns 5. The per-`FACE_T` `m_kiface[]` +array (`include/kiway.h`) and the per-`FRAME_T` `m_playerFrameId[KIWAY_PLAYER_COUNT]` singleton +cache mean: register a face once, and *all* of its frames are reachable, one live instance each. + +## `single_top.cpp` — the standalone bootstrap (and the only build-time pin) + +`common/single_top.cpp` is "a program launcher for a single KIFACE." Its `OnPgmInit` does two +`TOP_FRAME` things on the WASM static path (`#if !defined(BUILD_KIWAY_DLL)`, `:380-396`): + +```cpp +KIFACE_GETTER_FUNC* ki_getter = &KIFACE_GETTER; // :387 the statically-linked getter +KIFACE* kiface = ki_getter( &kiface_version, KIFACE_VERSION, this ); +Kiway.set_kiface( KIWAY::KifaceType( TOP_FRAME ), kiface ); // :396 register its FACE_T slot +... +KIWAY_PLAYER* frame = Kiway.Player( TOP_FRAME, true ); // :420 open the frame +``` + +`TOP_FRAME` is the **single** thing fixed at build time. Note that the `set_kiface` at `:396` +registers the kiface under `KifaceType(TOP_FRAME)` — and since `KifaceType(FRAME_PCB_EDITOR) == +KifaceType(FRAME_FOOTPRINT_EDITOR) == FACE_PCB`, that registration already covers *both* PCB +frames regardless of which `TOP_FRAME` was used. So for a same-kiface pair, only `:420` actually +depends on the build-time value. + +## The WASM "exactly one kiface" assumption + +`common/kiway.cpp:223-258` (`#ifdef __EMSCRIPTEN__`) is the WASM `KiFACE()` path. Because WASM +has no dynamic library loading, it returns the *statically-linked, pre-registered* kiface for a +face, and `nullptr` for any unregistered face — with an explicit warning not to fall back to the +lone getter: + +> *"WASM statically links exactly ONE kiface … Do NOT fall back to the statically-linked +> KIFACE_GETTER here: it returns THIS app's kiface regardless of the requested face, which then +> cannot CreateKiWindow() the other editor's panels."* (`kiway.cpp:238-247`) + +This is a *registration convention*, not a storage limit — `set_kiface` writes into a per-`FACE_T` +array (`include/kiway.h:476-481`), so registering N faces is just calling it N times. Part 2 +relaxes this convention; Part 1 doesn't touch it. + +## Precedent: upstream already does runtime frame selection + +Two upstream patterns prove the runtime-flag model and are worth copying: + +- **`kicad/kicad.cpp:130-164,260-274`** — the project-manager binary maps a `--frame=` + command-line option to a `FRAME_T` (`pcb`→`FRAME_PCB_EDITOR`, `sch`→`FRAME_SCH`, + `fpedit`→`FRAME_FOOTPRINT_EDITOR`, …) and, at runtime, either creates the manager or + `Kiway.Player( appType, true )` for any editor. This is *exactly* the "one binary, runtime + flag, many editors" shape — Part 1 should reuse this `--frame` parser in the WASM launcher. +- **`kicad/tools/kicad_manager_control.cpp:774-797`** — `ShowPlayer` reads a `FRAME_T` out of a + tool-action parameter (`aEvent.Parameter()`) and calls `Kiway().Player(playerType, true)`. + Clicking "PCB Editor" vs "Footprint Editor" in the manager is just two different `FRAME_T` + parameters (`kicad/tools/kicad_manager_actions.cpp:100-142`). + +**Caveat for Part 2:** the project manager achieves *multi-kiface* by loading each kiface as a +**DLL** at runtime. That path does not exist for WASM (no `dlopen` in our build path), and the +`kicad` manager target is **not built for WASM** at all (`kicad/CMakeLists.txt:113`, no +`EMSCRIPTEN` branch; absent from `docker/build.sh`). So Part 2's "many kifaces, one static image" +has no existing template — see [`04-part2-single-app-merge.md`](04-part2-single-app-merge.md). diff --git a/docs/features/editor-unification/03-part1-library-editor-unification.md b/docs/features/editor-unification/03-part1-library-editor-unification.md new file mode 100644 index 0000000..ca1f536 --- /dev/null +++ b/docs/features/editor-unification/03-part1-library-editor-unification.md @@ -0,0 +1,110 @@ +# 03 — Part 1: pair each editor with its own library editor + +> **Verdict: small, free win. Recommended.** Collapse `pcbnew` + `footprint_editor` into one +> build, and `eeschema` + `symbol_editor` into one build, selecting the frame at runtime. +> Prereqs: [`02-kiface-architecture.md`](02-kiface-architecture.md). + +## Why it's trivial + +The pair is already one kiface (see [`01`](01-current-build-structure.md)). Concretely, three +facts mean the editor selection is *already* a runtime decision that we merely refuse to expose: + +1. The kiface's `CreateKiWindow` already has **both** frame arms — + `case FRAME_PCB_EDITOR:` and `case FRAME_FOOTPRINT_EDITOR:` (`kicad/pcbnew/pcbnew.cpp:259,277`); + `case FRAME_SCH:` and `case FRAME_SCH_SYMBOL_EDITOR:` (`kicad/eeschema/eeschema.cpp:193,209`). +2. `KifaceType()` maps **both** frames in a pair to the **same** face + (`FRAME_PCB_EDITOR` & `FRAME_FOOTPRINT_EDITOR` → `FACE_PCB`; + `FRAME_SCH` & `FRAME_SCH_SYMBOL_EDITOR` → `FACE_SCH`; `kicad/common/kiway.cpp:399-405`). So the + `set_kiface()` registration the launcher already does (`single_top.cpp:396`) covers both + frames of the pair *regardless* of which `TOP_FRAME` it was built with. +3. The only build-time pin that actually differs in behavior is `Kiway.Player( TOP_FRAME, true )` + at `common/single_top.cpp:420`. + +So the entire gap between "two builds" and "one build, runtime flag" is the single integer at +`single_top.cpp:420`. + +## The minimal change + +### C++ (one launcher, runtime frame) + +Make the WASM launcher read the desired frame at runtime instead of from `TOP_FRAME`. The +cleanest approach **reuses the upstream pattern already in the tree** — +`kicad/kicad.cpp:130-164` parses a `--frame=` option into a `FRAME_T`. Mirror that in the +WASM `single_top` path: + +- Keep `TOP_FRAME` as the *default* (so nothing regresses if no flag is passed). +- If a runtime frame is supplied (via `Module.arguments`/argv `--frame=fpedit`, or a small embind + setter the JS calls before boot), use it for the `Kiway.Player(...)` call at `:420`. +- `set_kiface()` at `:396` can stay as-is — both frames in a pair share the face, so registering + `FACE_PCB` (or `FACE_SCH`) once is correct either way. + +The C++ delta is essentially: parse one optional argument, and swap one variable into one call. +No frame classes, no kiface, no dispatch logic changes. + +### Build (drop the duplicate targets) + +- Remove the `footprint_editor` executable block (`kicad/pcbnew/CMakeLists.txt:982-1018`) and the + `symbol_editor` block (`kicad/eeschema/CMakeLists.txt:758-788`). +- Remove `footprint_editor` / `symbol_editor` from `docker/build.sh:69,95` and the + `build-kicad-target.sh:46-79` `case`. +- Fold `symbol_editor`'s extra `PGM_DATA_FILE_EXT="kicad_sym"` define into runtime — derive the + library extension from the frame type (symbol → `kicad_sym`, footprint → `kicad_mod`) rather + than baking it. (Footprint had no extra define.) + +### JS routing (load the parent bundle + pass the frame) + +Today JS picks an editor by loading a different bundle; after Part 1 it loads the **parent** +bundle and passes the frame flag: + +- `web/pcbjam-shared/src/schemas.ts:39-42` (`LIB_EXTENSION_TOOL`) — `.kicad_mod` → load + `pcbnew` with `FRAME_FOOTPRINT_EDITOR`; `.kicad_sym` → load `eeschema` with + `FRAME_SCH_SYMBOL_EDITOR`. (`.kicad_pcb`/`.kicad_sch` keep their current + `FRAME_PCB_EDITOR`/`FRAME_SCH` defaults.) +- `web/pcbjam-shared/src/routes.ts:60`, `web/standalone/src/wasm/constants.ts:27-35` + (`TOOL_ARGV0`), `web/standalone/src/wasm/wasm-assets.ts`, + `web/standalone/src/components/WasmTool.tsx` — collapse the two lib tools onto their parent + bundle and thread the frame value into boot. +- `scripts/deploy/publish-wasm.mjs:33-41` — drop `footprint_editor`/`symbol_editor` from the + publish list. + +### Tests + +- `tests/web/tools-open.spec.ts:28-34` and the per-editor specs keep exercising all the *views*; + they now boot the parent bundle with a frame argument instead of a separate `.wasm`. The + footprint/symbol launch-scope coverage stays — it just routes through the merged build. + +## Why it costs nothing to download + +This is the decisive point. `footprint_editor.wasm` is **already** a complete copy of the pcbnew +kiface (it contains `PCB_EDIT_FRAME` *and* `FOOTPRINT_EDIT_FRAME` and all the shared board +machinery — that's why it's the same 146 MB as `pcbnew.wasm`). A user who opens the footprint +editor today *already downloads the whole pcbnew engine*. Merging changes nothing they download — +it deletes the redundant second artifact. + +## Cost / benefit + +| | | +|---|---| +| **Effort** | Small. C++ ≈ "parse one optional `--frame`, swap it into `single_top.cpp:420`"; the rest is JS routing + deleting two build/deploy/test targets. No architecture change. | +| **Download impact** | **None.** The dup bundle is already a full copy of its twin. | +| **Removes** | 228 MB of duplicated deployed WASM (146 + 82), the ~1.6 GB-each debug DWARF sidecars for the footprint twin, and 2 build + 2 deploy + duplicate test targets → faster CI and less R2 storage. | +| **Risk** | Low. The runtime path it relies on (`Kiway.Player(frameType)`) is the same one the build uses today; we're only choosing the argument later. | + +## Caveats (small, known) + +- **argv0 / `thisProgram`.** Today `footprint_editor`/`symbol_editor` set + `thisProgram=/usr/bin/` (`constants.ts:27-35`); after merge both modes report the parent + (`pcbnew`/`eeschema`). `thisProgram` feeds KiCad's DEBUG/app-identity and single-instance + checks — verify nothing keys on the old name. Low risk (the board/footprint editors share one + app identity natively too). +- **Per-app settings.** KiCad keys some config by app; the footprint/symbol editors already store + their settings under their parent module natively, so this should be a non-issue — confirm no + WASM-specific config path assumes the separate binary name. +- **The data-file extension** (`PGM_DATA_FILE_EXT`) must move from a symbol-only compile define to + a runtime value derived from the frame. Mechanical. + +## Relationship to Part 2 + +Part 1 builds exactly the runtime-frame plumbing (a launcher that reads `FRAME_T` at runtime +instead of from `TOP_FRAME`) that [Part 2](04-part2-single-app-merge.md) reuses. Doing Part 1 +first is the natural first step whether or not Part 2 ever happens. diff --git a/docs/features/editor-unification/04-part2-single-app-merge.md b/docs/features/editor-unification/04-part2-single-app-merge.md new file mode 100644 index 0000000..fad8fb9 --- /dev/null +++ b/docs/features/editor-unification/04-part2-single-app-merge.md @@ -0,0 +1,142 @@ +# 04 — Part 2: fuse the editors into one app + +> **Verdict: feasible and well-bounded, but moderate work — and a product decision, not a dedup +> win.** Merging "the four editors" means linking the **two engines** (pcbnew kiface + eeschema +> kiface) into one WASM image and picking the view at runtime. The collision surface is small +> and concentrated; the real cost is bundle size. Prereqs: +> [`02-kiface-architecture.md`](02-kiface-architecture.md), [`03`](03-part1-library-editor-unification.md). + +## What "the four" actually is + +Board + Footprint + Schematic + Symbol = `FACE_PCB` (pcbnew kiface) + `FACE_SCH` (eeschema +kiface) = **two engines in one static image**. Extending to all 7 GUI tools adds `FACE_GERBVIEW`, +`FACE_PL_EDITOR`, `FACE_PCB_CALCULATOR` = 5 kifaces — same mechanism, more faces. + +This is fundamentally different from Part 1. Part 1 is "one engine, many frames" — already +supported, you just expose the selector. Part 2 is "many engines, one image," which has **no +existing template**: upstream gets multi-kiface only by loading DLLs at runtime, the WASM build +has no `dlopen`, the `kicad` project manager isn't built for WASM (`kicad/CMakeLists.txt:113`), +and the WASM `KIWAY` explicitly assumes exactly one kiface (`kiway.cpp:223-258`). So Part 2 is a +real porting task, not a flag flip. + +## The symbol-collision surface (when two kifaces link into one image) + +From most to least trouble: + +### 1. `Kiface()` — the load-bearing item (moderate) + +Each kiface defines a global accessor `KIFACE_BASE& Kiface()` (declared +`include/kiface_base.h:134`; defined `pcbnew.cpp:600`, `eeschema.cpp:452`, +`gerbview.cpp:152`, `pl_editor.cpp:141`, `pcb_calculator.cpp:76`). Linking two kifaces gives two +definitions of `Kiface()` — a duplicate symbol. + +Worse, it is not just a name clash; it is a **semantic** one. `Kiface()` is called ~97 times +(pcbnew 48 calls / 21 files; eeschema 49 / 20 files), **including 5 files in shared `common/`** +that are compiled **once** into libkicommon and linked into every app: + +- `common/eda_base_frame.cpp` (e.g. `:1274 return Kiface().KifaceSettings();`, `:1280`, `:1286`) +- `common/gestfich.cpp` +- `common/design_block_tree_model_adapter.cpp:49` +- `common/dialogs/dialog_color_picker.cpp` +- `common/tool/common_control.cpp` + +That shared code calls `Kiface()` expecting "the one active editor's kiface" — but with two +kifaces linked, it can't statically know whether the active frame is PCB or schematic. So +`Kiface()` must become a **runtime dispatch on the active editor** (a "current active kiface" +pointer keyed off the active frame's `FACE_T`). It's mostly process/editor-level config — +`KifaceSettings()`, `KifaceSearch()`, `GetHelpFileName()`, `IsSingle()` — so a single dispatching +`Kiface()` backed by an active-editor pointer is feasible, but it is a behavioral change touching +~100 call sites including shared code. **This is what lifts Part 2 from "small" to "moderate."** + +> ⚠️ **`--allow-multiple-definition` is not a fix here.** The static kiface link already passes +> `LINKER:--allow-multiple-definition` (see `../symbol-editor/0001-symbol-editor-port.md`). That +> would let two `Kiface()` definitions *link* — by silently taking the first — which means shared +> `common/` code would always resolve to (say) pcbnew's kiface even while the schematic editor is +> active. That is the bug, not the fix. `Kiface()` needs real per-active-editor dispatch. + +### 2. `KIFACE_GETTER` / `KIFACE_1` (trivial) + +Every kiface exports the same `extern "C"` getter — `KIFACE_GETTER` is a macro expanding to the +unmangled symbol `KIFACE_1` (`include/kiway.h:110-115`), defined identically in each kiface +(`pcbnew.cpp:605`, `eeschema.cpp:457`, …), all returning `&kiface`. Two in one image = duplicate +`KIFACE_1`. + +Fix is mechanical: the macro is unconditional at `kiway.h:111`, so guard it with `#ifndef +KIFACE_GETTER` and pass a per-target `-DKIFACE_GETTER=` (the same `COMPILE_DEFINITIONS` +machinery that already sets `TOP_FRAME`). The `extern "C"` declaration at `kiway.h:541/543` +follows the macro automatically. A combined launcher then declares both distinct externs and +registers each face: + +```cpp +extern "C" KIFACE* pcbnew_kiface_getter(int*,int,PGM_BASE*); +extern "C" KIFACE* eeschema_kiface_getter(int*,int,PGM_BASE*); +Kiway.set_kiface( KIWAY::FACE_PCB, pcbnew_kiface_getter(&v, KIFACE_VERSION, this) ); +Kiway.set_kiface( KIWAY::FACE_SCH, eeschema_kiface_getter(&v, KIFACE_VERSION, this) ); +``` + +### 3. Relax the WASM "exactly one kiface" convention (small) + +`kiway.cpp:223-258` enforces single-kiface registration by *convention*, not storage — +`set_kiface` already writes into a per-`FACE_T` array (`include/kiway.h:476-481`) and `KiFACE()` +reads per slot. Registering several faces is just calling `set_kiface` N times. Update the comment +and the single-getter assumption to match the combined launcher above. + +### Non-issues (verified) + +- **`Pgm()` / `program` singleton** (`common/pgm_base.cpp:1051-1058`; `single_top.cpp:137,180`) + — there is exactly one shared instance, which is *correct* for a multi-kiface app. No collision. +- **The `static ::IFACE kiface(...)` instances** (`pcbnew.cpp:592`, `eeschema.cpp:445`, in named + namespaces `PCB`/`SCH`) — file-static, internal linkage, no external symbol. No collision. +- Beyond `KIFACE_1` and `Kiface()`, the kifaces emit no other global non-member symbols at file + scope; the lower libs (libkicommon, libgal, …) are linked once and shared. A one-shot trial + link is the definitive duplicate-symbol audit, but the *by-design* collisions are just those two. + +## The real cost: bundle size + +This is the strategic consideration, and it cuts against our current delivery model. + +- **Today:** 7 separate bundles, each user lazy-loads only the tool they open. Total deployed + ≈ 572 MB raw — but massively duplicated (wx + common + boost replicated in all 7). +- **Merged:** a combined image pays for shared code **once**. A 4-editor (pcbnew + eeschema) + binary ≈ pcbnew's 146 MB (already carries wx/common + the PCB-only OCC) + eeschema's *unique* + code (~30–40 MB; OCC is PCB-only, wx/common/boost are shared) ≈ **~180–190 MB**. All-7 ≈ + ~200–230 MB. *(Estimates — a trial link is the only way to know precisely.)* +- **The catch:** that ~190 MB is downloaded by **every** user — even one who only wants the + calculator. It works *against* the per-tool lazy loading we do now, and a merged binary can't + be lazily sub-loaded: `wasm-split` / `dlopen` are already assessed as a dead end for us (see + [`../perf/bundle-size.md`](../perf/bundle-size.md) and the bundle-composition research). So the + tradeoff is "one shared binary, zero duplication, cross-probing possible" vs. "larger first + load for single-tool users." + +## When Part 2 is worth it + +- **Worth it** if the product goal is a unified, project-based "KiCad in the browser" where a + user has a project open and flips between schematic and PCB — and you want desktop-style + **cross-probing** (select a component in the schematic, highlight it on the board). A combined + image holds the board editor *and* schematic editor live under one `KIWAY`, which is exactly how + desktop KiCad's single process works; the inter-editor `KIWAY` express messaging would light up + for free. This is essentially porting the upstream project-manager model to one static image. +- **Not worth it** if the goal is the fastest possible first load for a single tool. Then keep + the tools as separate lazy-loaded bundles and just take [Part 1's](03-part1-library-editor-unification.md) + free dedup. + +## Suggested sequencing if pursued + +1. Ship [Part 1](03-part1-library-editor-unification.md) first (free; builds the runtime-frame + launcher this reuses). +2. Spike a **2-kiface trial link** (pcbnew + eeschema) with the getter renamed (§2) and + `--allow-multiple-definition` *off*, to enumerate the true duplicate-symbol set and confirm + it's just `Kiface()`. Measure the combined size. +3. Convert `Kiface()` to active-editor dispatch (§1) — the bulk of the work; validate the shared + `common/` call sites resolve correctly with both editors live. +4. Combined launcher registers both faces (§2/§3); JS opens any `FRAME_T` against the one bundle + (and optionally two frames at once for cross-probing). +5. Only then consider folding in gerbview / pl_editor / calculator (3 more faces, same recipe). + +## Bottom line + +The engineering is bounded and the scary part (the getter) is trivial; the genuine work is one +well-understood refactor (`Kiface()` → runtime dispatch). But Part 2 trades per-tool download size +for a unified binary, so decide it on **product** grounds — do we want a single cross-probing +KiCad-in-the-browser? — not as a duplication cleanup. The duplication cleanup is +[Part 1](03-part1-library-editor-unification.md), and it's free. diff --git a/docs/features/editor-unification/05-part1-implementation.md b/docs/features/editor-unification/05-part1-implementation.md new file mode 100644 index 0000000..a2301af --- /dev/null +++ b/docs/features/editor-unification/05-part1-implementation.md @@ -0,0 +1,99 @@ +# 05 — Part 1: implementation record & current state + +> **Status: Part 1 IMPLEMENTED and validated end-to-end (2026-07-02).** This is the +> as-built companion to the [`03`](03-part1-library-editor-unification.md) plan: what +> actually changed, how the runtime mechanism works, what the research got slightly +> wrong, how it was verified, and the current git/build state. Part 2 +> ([`04`](04-part2-single-app-merge.md)) remains deferred by design. + +## One-paragraph summary + +`footprint_editor` and `symbol_editor` are no longer separate WASM builds. The +`pcbnew` / `eeschema` bundle now opens the footprint / symbol editor at **runtime**, +selected by a `--frame=` argument threaded through Emscripten's +`Module.arguments` and parsed in `kicad/common/single_top.cpp` (mirroring the upstream +`--frame` parser in `kicad/kicad.cpp`). The two duplicate build+deploy targets, their +launcher scripts, and a vestigial embind file are gone; the frontend re-points the two +library tools onto their parent bundle. Net effect: **~228 MB of duplicated deployed +WASM removed (plus their ~3 GB of debug DWARF sidecars) with zero download penalty**, +since the footprint/symbol bundle was already a full copy of its twin. + +## What changed + +### C++ — `kicad` submodule (kept close to upstream; all new logic `#ifdef __EMSCRIPTEN__`) + +| File | Change | +|---|---| +| `common/single_top.cpp` | On the WASM path, parse `--frame=` into a `FRAME_T` (tokens: `pcb`, `fpedit`, `sch`, `symedit`, `gerb`, `ds`, `calc`), defaulting to the build-time `TOP_FRAME`; open it with `Kiway.Player(topFrame, true)`. `--frame` is also added to the existing positional-file `wxCmdLineParser` desc so it's ignored there. The default data-file extension is derived from the resolved frame at runtime (`kicad_pcb`/`kicad_mod`/`kicad_sch`/`kicad_sym`) instead of the compile-time `PGM_DATA_FILE_EXT` (native still uses `PGM_DATA_FILE_EXT`). | +| `pcbnew/CMakeLists.txt` | Removed the WASM-only `footprint_editor` executable block (the pcbnew kiface already compiles `footprint_edit_frame.cpp`). | +| `eeschema/CMakeLists.txt` | Removed the WASM-only `symbol_editor` executable block. | + +Native builds are byte-identical (`topFrame == TOP_FRAME` when no flag is passed; the +whole block is WASM-guarded). + +### Build & deploy — root repo + +- `docker/build.sh` — `all` now expands to **5** apps (`pcbnew eeschema calculator pl_editor gerbview`); `symbol_editor`/`footprint_editor` dropped from the valid-apps list, the `all` array, the arg-validation case, and `kicad_subdir_for`. +- `scripts/kicad/build-kicad-target.sh` — removed the two apps' target/subdir, `EMBIND_APP`, and `STUB_APP` case arms + usage strings. +- Deleted `scripts/kicad/build-symbol_editor.sh`, `scripts/kicad/build-footprint_editor.sh`, and the vestigial `wasm/bindings/symbol_editor_embind.cpp`. +- `scripts/deploy/publish-wasm.mjs` — publish list trimmed to the 5 real bundles. + +### Frontend — `web/standalone` (the two tools stay visible in the UI; only *how they boot* changed) + +- `src/wasm/constants.ts` — new `TOOL_BUNDLE` (`footprint_editor→pcbnew`, `symbol_editor→eeschema`, others→self) and `TOOL_FRAME` (`footprint_editor→"fpedit"`, `symbol_editor→"symedit"`, others→`undefined`). +- `src/wasm/wasm-assets.ts` — `resolveWasmBase` resolves the **bundle**'s CDN folder/version (the manifest only lists the 5 published bundles now). +- `src/wasm/boot.ts` — `BootOptions.frame`; computes `bundle = TOOL_BUNDLE[tool]`; fetches `/.{wasm,js}` and the pthread worker from the bundle; sets `Module.arguments = ["--frame="]`. +- `src/components/WasmTool.tsx` — passes `frame: TOOL_FRAME[tool]` into `bootKicadTool`. + +### Tests + +- `tests/apps/kicad/symbol_editor.html` — now loads `eeschema.js` with `Module.arguments=['--frame=symedit']` (and the stale `9.99` config-seed dir corrected to `10.0`). +- `tests/apps/kicad/footprint_editor.html` — new; loads `pcbnew.js` with `--frame=fpedit`. +- `tests/kicad/frame-runtime.spec.ts` — new; parametrized, asserts each harness lands on the **library editor** title (not its parent editor). +- `tests/playwright-kicad.config.ts` — `frame-runtime.spec.ts` added to `PCBNEW_FAMILY_SPECS`: its footprint case boots the pcbnew module, which OOMs SpiderMonkey on x86 CI, so on CI the whole file runs on the `chromium-ci` (V8) project like every other pcbnew-booting spec. Local runs unaffected. +- `tests/scripts/setup-kicad-wasm.sh` — stops copying the two removed bundles. + +## How the runtime frame selection works + +``` +JS boot.ts: Module.arguments = ["--frame=fpedit"] (in the Module literal, before the glue runs) + │ +Emscripten glue (built with -sINVOKE_RUN=0, callMain exported): + Module["arguments"] → arguments_ → run() → callMain(args) → main(argc, argv) + │ +C++ single_top.cpp: wxCmdLineParser(App().argc, App().argv) → "--frame=fpedit" + → FRAME_FOOTPRINT_EDITOR → Kiway.Player(FRAME_FOOTPRINT_EDITOR) +``` + +The pcbnew kiface's `CreateKiWindow` already has both `FRAME_PCB_EDITOR` and +`FRAME_FOOTPRINT_EDITOR` arms (eeschema likewise for `FRAME_SCH` / +`FRAME_SCH_SYMBOL_EDITOR`), and both frames of a pair map to the **same** `FACE_T`, so +one registered kiface serves both — see [`02`](02-kiface-architecture.md). No frame, +kiface, or dispatch code changed. + +## Findings that corrected the research + +- **`Module.arguments` *is* delivered to `App().argv` in our wx WASM port.** This was the one real unknown (files normally open via an embind call, not argv). A temporary diagnostic confirmed `argc=2, argv[1]="--frame=symedit"` in-browser — so the argv approach works and **no `EM_ASM`/embind fallback was needed**. +- **`PGM_DATA_FILE_EXT` is set on *all four* launchers** (`kicad_pcb`/`kicad_mod`/`kicad_sch`/`kicad_sym`), not just symbol — [`03`](03-part1-library-editor-unification.md) said footprint had no extra define. The runtime extension map covers all four. +- **The `kiway.cpp` "exactly one kiface" warning is not a Part-1 blocker.** It concerns cross-*face* requests; both frames of a pair share one face, so registering one kiface serves both (exactly what `footprint_editor.wasm` did before). +- There is no `routes.ts` in `@pcbjam/shared` at the time of writing on this branch's base — extension→tool routing lives in `schemas.ts` + `WasmTool.tsx`'s `fileTool`. (A later `pcbjam-shared` bump adds `routes.ts`; see git state below.) + +## Validation (all green) + +Three tiers, screenshots checked at each: + +1. **Runtime-frame harness** (`frame-runtime.spec.ts`, Firefox): `symbol_editor.html` → title *"… — Symbol Editor"*; `footprint_editor.html` → *"… — Footprint Editor"* — each opening the library editor, not its parent (the parent bundle's default frame would title itself "Schematic/PCB Editor"). +2. **Regression** through the merged bundles: `pcbnew.spec` (drew lines on the board), `eeschema.spec` (drew wires on the schematic), `symbol_editor.spec` (canvas + toolbars + registry) — all pass, wizard-free (the corrected `10.0` config seed skips the first-run wizard). +3. **Real web app + demo project** (`tools-open.spec.ts` via `playwright-web`, frontend :3048 + reference backend :3060 serving `tests/fixtures/demo`): `pcbnew` opens `demo.kicad_pcb`, `eeschema` opens `demo.kicad_sch`, and `footprint_editor` / `symbol_editor` boot from the pcbnew / eeschema bundle via the actual React routing (`resolveWasmBase`→`TOOL_BUNDLE`, `boot`→`--frame`). **4 passed**; screenshots show the footprint layer stack and the symbol library tree + Pins filter, with the real app chrome. + +## Build & git state + +- **Bundles built** for validation via `main`'s warm docker cache at `-O1`: `output/pcbnew.wasm` (147 MB) and `output/eeschema.wasm` (82 MB). `gerbview`/`pl_editor`/`calculator` were **not** rebuilt — they're untouched by this change; `docker/build.sh all` produces the full 5-bundle deploy set. +- **kicad submodule** — branch `editor-unification`, HEAD `e7a27b31` (rebased onto the `wasm-port` tip `e8db3d35`): *"feat(wasm): runtime --frame launcher; drop duplicate footprint/symbol editor targets"*. +- **root** — branch `editor-unification`, one feature commit ahead of `main` (`05127af`): *"feat(wasm): unify editor builds — footprint/symbol editors via runtime --frame"* — includes this dossier and the CI spec routing; pins `kicad e7a27b31` + `pcbjam-shared 9a1a269`. Clean tree. +- **Not pushed.** Ship with `/git-feature-finish` (or a manual push of `editor-unification` in both repos). + +## What's next + +- **Nothing required** for Part 1 — it's complete and validated. +- **Part 2** ([`04`](04-part2-single-app-merge.md)) — fuse the pcbnew + eeschema engines into one image — remains a deferred product call (real +download cost). Part 1 built the runtime-frame plumbing Part 2 would reuse. diff --git a/docs/features/editor-unification/README.md b/docs/features/editor-unification/README.md new file mode 100644 index 0000000..ebdec82 --- /dev/null +++ b/docs/features/editor-unification/README.md @@ -0,0 +1,91 @@ +# Editor build unification — one WASM build per kiface + a runtime frame flag + +> **Status: Part 1 IMPLEMENTED & validated (2026-07-02) — see +> [`05-part1-implementation.md`](05-part1-implementation.md). Part 2 deferred.** Originally +> authored 2026-06-30 as a research / decision record from a 5-agent read of the KiCad +> source (`kicad/` submodule) and our WASM build. The question: +> several of our 7 WASM "apps" are the *same compiled code* differing only by a build-time +> constant — can we collapse them to one build that picks the editor at runtime? +> Companion to [`../symbol-editor/`](../symbol-editor/) (which established the +> second-launcher pattern this doc proposes to retire) and +> [`../perf/bundle-size.md`](../perf/bundle-size.md) (the size context). + +## Why this exists + +We ship **7 separate WASM apps** (`pcbnew`, `footprint_editor`, `eeschema`, `symbol_editor`, +`gerbview`, `pl_editor`, `calculator`). Two pairs of them are byte-for-byte near-duplicates: + +| artifact | size | what it is | differs from its twin by | +|---|---|---|---| +| `pcbnew.wasm` | **146 MB** | board editor | — | +| `footprint_editor.wasm` | **146 MB** | footprint editor | **one** compile macro (`TOP_FRAME`) | +| `eeschema.wasm` | **82 MB** | schematic editor | — | +| `symbol_editor.wasm` | **82 MB** | symbol editor | **one** compile macro (`TOP_FRAME`) | +| `gerbview` / `pl_editor` / `calculator` | 42 / 44 / 30 MB | separate engines | (genuinely different code) | + +The Footprint Editor is not a separate program — it is a *frame* compiled **inside** the +pcbnew module; the Symbol Editor is a frame inside eeschema. KiCad already selects which +editor a module opens **at runtime**, by passing a `FRAME_T` enum value to a single factory +(`KIFACE::CreateKiWindow`). The *only* thing we pin at build time is which frame the launcher +opens (`TOP_FRAME` in `common/single_top.cpp:420`). So `footprint_editor.wasm` is a complete +copy of the pcbnew engine with one integer changed. + +This dossier evaluates two changes the user proposed: + +1. **Part 1 — pair each editor with its own library editor** (PCB Editor + Footprint Editor → + one build; Schematic Editor + Symbol Editor → one build), selecting the frame at runtime. +2. **Part 2 — fuse all four editors** (really the *two* engines, pcbnew + eeschema) into one + app with a runtime flag to switch views. + +## TL;DR / decision + +- **Part 1 — DONE ✅** (implemented + validated end-to-end, incl. the real web app; see + [`05-part1-implementation.md`](05-part1-implementation.md)). A small, free win, as predicted. + The pair is *already one kiface*; the kiface's + `CreateKiWindow` already has both frame arms, and `KifaceType()` maps both frames to the + same face — so the single registered kiface already serves both. The only blocker is the + baked-in `TOP_FRAME` at `common/single_top.cpp:420`. Make that a runtime input (mirror the + upstream `--frame` parser in `kicad/kicad.cpp:130-164`) and the duplicate build disappears. + **Zero download penalty** (the dup bundle is already a full copy of its twin). Removes + **228 MB** of duplicated WASM, their ~1.6 GB-each debug DWARF sidecars, and 2 + build/deploy/test targets. See [`03-part1-library-editor-unification.md`](03-part1-library-editor-unification.md). +- **Part 2 is feasible and well-bounded, but moderate — and a product call, not a dedup + win.** Two engines in one image is *new ground* (no template — the upstream project manager + that does this natively isn't built for WASM, and the WASM `KIWAY` asserts exactly one + kiface). The collision surface is small and concentrated: the load-bearing item is the + global `Kiface()` accessor (two definitions, ~97 call sites incl. shared `common/` code), + which must become a runtime dispatch on the active editor. The `KIFACE_GETTER` symbol clash + everyone fears first is trivial. The real cost is **size**: a merged binary is ~180–190 MB + that *every* user downloads, which fights our per-tool lazy loading. See + [`04-part2-single-app-merge.md`](04-part2-single-app-merge.md). +- **They are not either/or.** Part 1 builds the runtime-frame mechanism that Part 2 reuses, + so Part 1 is the natural first step regardless. Ship Part 1 now; gate Part 2 on whether the + product wants a unified, project-based "KiCad in the browser" (cross-probing schematic↔PCB). +- **Do _not_ build the user's literally-crossed pairing** (PCB + Symbol, or Schematic + + Footprint). Those don't share code, so each bundle would link *both* engines — all of + Part 2's cost with none of Part 1's sharing benefit. + +## Document index + +| File | Contents | +|---|---| +| [`01-current-build-structure.md`](01-current-build-structure.md) | What we build today: the 7 apps, the two duplicate pairs (with sizes), the build-time `TOP_FRAME` selection, and how JS picks an editor by loading a different bundle. The as-built baseline, with evidence. | +| [`02-kiface-architecture.md`](02-kiface-architecture.md) | Primer on KiCad's kiway / kiface / `FRAME_T` / `single_top` system — the upstream mechanism that already selects editors at runtime. The "one module, many frames, chosen by a runtime parameter" contract. | +| [`03-part1-library-editor-unification.md`](03-part1-library-editor-unification.md) | **Part 1.** Pair each editor with its own library editor. Why it's trivial, the exact minimal change, why it costs nothing to download, the cost/benefit, and the caveats. | +| [`04-part2-single-app-merge.md`](04-part2-single-app-merge.md) | **Part 2.** Fuse the engines into one app. The full symbol-collision surface (`Kiface()` is the real work; the getter is trivial), the size tradeoff, when it's worth it, and extending to all 7 tools. | +| [`05-part1-implementation.md`](05-part1-implementation.md) | **Part 1 — as-built (implemented).** The implementation record: exactly what changed (C++ / build / frontend / tests), how the runtime `--frame` mechanism works, corrections to the research (argv *is* delivered; `PGM_DATA_FILE_EXT` on all four), validation (harness + regression + real web app + demo), and the current git/build state. | + +## Relationship to other feature docs + +- [`../symbol-editor/0001-symbol-editor-port.md`](../symbol-editor/0001-symbol-editor-port.md) + — established the "second `single_top` launcher, same kiface, different `TOP_FRAME`" pattern + for `symbol_editor`. Part 1 generalizes that to a *runtime* `TOP_FRAME` and retires the + separate launcher entirely. +- [`../perf/bundle-size.md`](../perf/bundle-size.md) / + [`../perf/README.md`](../perf/README.md) — the size context. Part 1's dedup is orthogonal to + the size levers there (it removes *duplicate artifacts*, not per-binary bloat); Part 2's + size *cost* must be weighed against them (`wasm-split`/dlopen are already a dead end there, + which is why a merged binary can't be lazily sub-loaded). +- Build entry points: `docker/build.sh`, `scripts/kicad/build-kicad-target.sh`. +- Launcher / dispatch: `kicad/common/single_top.cpp`, `kicad/common/kiway.cpp`, + `kicad/pcbnew/pcbnew.cpp`, `kicad/eeschema/eeschema.cpp`, `kicad/include/frame_type.h`. diff --git a/kicad b/kicad index 9d77139..7660513 160000 --- a/kicad +++ b/kicad @@ -1 +1 @@ -Subproject commit 9d771391886ba3877dda90f72f57ad37d931fef9 +Subproject commit 766051316ba7de2c4e4f08340dca08100c3e1565 diff --git a/scripts/deploy/publish-wasm.mjs b/scripts/deploy/publish-wasm.mjs index fc679a1..9d82f52 100644 --- a/scripts/deploy/publish-wasm.mjs +++ b/scripts/deploy/publish-wasm.mjs @@ -29,13 +29,15 @@ import { // --- tools & per-file rules --------------------------------------------------- -// Tools served to the browser editor. sym_convert is a node CLI, not served. +// Bundles served to the browser editor. symbol_editor / footprint_editor are NOT +// separate bundles: they are the eeschema / pcbnew bundle booted with a runtime +// --frame flag (editor-unification), so they publish nothing of their own. The +// frontend maps them onto their parent bundle via TOOL_BUNDLE. sym_convert is a +// node CLI, not served. const TOOLS = [ "pcbnew", "eeschema", "pl_editor", - "symbol_editor", - "footprint_editor", "gerbview", "calculator", ]; diff --git a/scripts/kicad/build-footprint_editor.sh b/scripts/kicad/build-footprint_editor.sh deleted file mode 100755 index 1765868..0000000 --- a/scripts/kicad/build-footprint_editor.sh +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/bash -# Build KiCad Footprint Editor for WebAssembly. -# Thin wrapper around build-kicad-target.sh — see that script for options. -# (The footprint editor is the pcbnew kiface launched at FRAME_FOOTPRINT_EDITOR, -# like symbol_editor is the eeschema kiface at FRAME_SCH_SYMBOL_EDITOR.) - -set -e -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -exec "${SCRIPT_DIR}/build-kicad-target.sh" footprint_editor "$@" diff --git a/scripts/kicad/build-kicad-target.sh b/scripts/kicad/build-kicad-target.sh index f8ebf06..24ca569 100755 --- a/scripts/kicad/build-kicad-target.sh +++ b/scripts/kicad/build-kicad-target.sh @@ -31,7 +31,7 @@ set -e if [ -z "$1" ]; then - echo "Error: missing argument (pcbnew | eeschema | calculator | pl_editor | symbol_editor | gerbview)" >&2 + echo "Error: missing argument (pcbnew | eeschema | calculator | pl_editor | gerbview)" >&2 exit 1 fi APP_NAME="$1" @@ -42,7 +42,8 @@ shift # Most apps share all three names; the exceptions: # - calculator: target+subdir are both pcb_calculator (OUTPUT_NAME=calculator) # - pl_editor: subdir is pagelayout_editor (upstream source dir name) -# - symbol_editor: served by the eeschema kiface, so it builds in eeschema/ +# The footprint/symbol editors have no target of their own — the pcbnew/eeschema +# bundle opens them at runtime via single_top.cpp's --frame flag. case "$APP_NAME" in pcbnew|eeschema|gerbview) KICAD_TARGET="$APP_NAME" @@ -56,15 +57,6 @@ case "$APP_NAME" in KICAD_TARGET="pcb_calculator" KICAD_SUBDIR="pcb_calculator" ;; - symbol_editor) - KICAD_TARGET="symbol_editor" - KICAD_SUBDIR="eeschema" - ;; - footprint_editor) - # served by the pcbnew kiface, so it builds in pcbnew/ (like symbol_editor in eeschema/) - KICAD_TARGET="footprint_editor" - KICAD_SUBDIR="pcbnew" - ;; sym_convert) # Standalone .lib -> .kicad_sym converter (node CLI). Its add_executable # lives in eeschema/CMakeLists.txt (gated by KICAD_SYM_CONVERTER_WASM), so @@ -73,33 +65,23 @@ case "$APP_NAME" in KICAD_SUBDIR="eeschema" ;; *) - echo "Error: unknown app '$APP_NAME' (expected: pcbnew | eeschema | calculator | pl_editor | symbol_editor | footprint_editor | gerbview | sym_convert)" >&2 + echo "Error: unknown app '$APP_NAME' (expected: pcbnew | eeschema | calculator | pl_editor | gerbview | sym_convert)" >&2 exit 1 ;; esac # Which app's embind bindings to compile + link. Most apps use their own; the -# symbol_editor is the eeschema kiface launched at a different TOP_FRAME and has -# no embind of its own, so it reuses eeschema's — whose bindings (kicadCollabOnSave -# et al.) the shared kiface objects reference. Without this the symbol_editor link -# fails with "undefined symbol: kicadCollabOnSave" (the placeholder defines nothing). +# sym_convert CLI links the eeschema kiface objects, which reference eeschema's +# embind symbols (kicadCollabOnSave et al.) — reuse eeschema's embind object. case "$APP_NAME" in - symbol_editor) EMBIND_APP="eeschema" ;; - footprint_editor) EMBIND_APP="pcbnew" ;; - # sym_convert links the eeschema kiface objects, which reference eeschema's - # embind symbols (kicadCollabOnSave et al.) — reuse eeschema's embind object. sym_convert) EMBIND_APP="eeschema" ;; *) EMBIND_APP="$APP_NAME" ;; esac -# Which app's WASM stub libraries (scripting/frame placeholders) to link. Like -# EMBIND_APP, the footprint_editor reuses pcbnew's: it links the pcbnew kiface -# objects, which reference pcbnew's action-plugin scripting symbols -# (pcbnewGetScriptsSearchPaths et al., defined in pcbnew_scripting_stub.cpp). +# Which app's WASM stub libraries (scripting/frame placeholders) to link. +# sym_convert links the eeschema kiface objects, so it needs eeschema's frame +# stub (eeschema_frame_stub.cpp). case "$APP_NAME" in - footprint_editor) STUB_APP="pcbnew" ;; - # sym_convert links the eeschema kiface objects, so it needs eeschema's - # frame stub (eeschema_frame_stub.cpp) like symbol_editor does. sym_convert) STUB_APP="eeschema" ;; *) STUB_APP="$APP_NAME" ;; esac @@ -519,8 +501,8 @@ if [ -f "${EMBIND_SRC}" ]; then # ${KICAD_BUILD}/common by make_lexer custom commands on the pcbcommon target). # On a fresh build dir they don't exist until make runs — build pcbcommon first. # No wasted work: the app target depends on pcbcommon anyway; incremental no-op. - # Guard on EMBIND_APP (not APP_NAME) so footprint_editor — whose embind IS - # pcbnew's — also pre-builds pcbcommon. + # Guard on EMBIND_APP (not APP_NAME) so any app whose embind IS pcbnew's also + # pre-builds pcbcommon. if [ "${EMBIND_APP}" = "pcbnew" ]; then log_info "Pre-building pcbcommon so generated lexer headers exist for the embind compile..." emmake make -j${JOBS} pcbcommon diff --git a/scripts/kicad/build-symbol_editor.sh b/scripts/kicad/build-symbol_editor.sh deleted file mode 100755 index 733a45e..0000000 --- a/scripts/kicad/build-symbol_editor.sh +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/bash -# Build KiCad Symbol Editor for WebAssembly. -# Thin wrapper around build-kicad-target.sh — see that script for options. - -set -e -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -exec "${SCRIPT_DIR}/build-kicad-target.sh" symbol_editor "$@" diff --git a/tests/apps/kicad/footprint_editor.html b/tests/apps/kicad/footprint_editor.html new file mode 100644 index 0000000..a0e173f --- /dev/null +++ b/tests/apps/kicad/footprint_editor.html @@ -0,0 +1,153 @@ + + + + + + KiCad Footprint Editor WASM + + + +
+ +
+
Initializing...
+
+
+ +
+ + + + + + + + + + diff --git a/tests/apps/kicad/symbol_editor.html b/tests/apps/kicad/symbol_editor.html index 18df0cd..7bc25e0 100644 --- a/tests/apps/kicad/symbol_editor.html +++ b/tests/apps/kicad/symbol_editor.html @@ -127,7 +127,7 @@ // before main() so every provider reports NeedsUserInput()==false and the // wizard never opens — same as eeschema.html and the web app's boot.ts. var seedKicadConfig = function() { - var cfgDir = '/home/kicad/.config/kicad/kicad/9.99'; + var cfgDir = '/home/kicad/.config/kicad/kicad/10.0'; FS.mkdirTree(cfgDir); var writeIfAbsent = function(path, contents) { @@ -147,6 +147,10 @@ var Module = { thisProgram: '/usr/bin/symbol_editor', // Fake absolute path for argv[0] (KiCad DEBUG check) + // The symbol editor is served by the eeschema bundle (editor-unification): + // load eeschema.js below and select FRAME_SCH_SYMBOL_EDITOR at runtime. + arguments: ['--frame=symedit'], + preRun: [createCanvas, writeResources, seedKicadConfig], postRun: [], @@ -202,6 +206,7 @@ - + + diff --git a/tests/kicad/frame-runtime.spec.ts b/tests/kicad/frame-runtime.spec.ts new file mode 100644 index 0000000..c71e55b --- /dev/null +++ b/tests/kicad/frame-runtime.spec.ts @@ -0,0 +1,67 @@ +import { test, expect } from '@playwright/test'; + +/** + * Editor-unification runtime-frame validation. + * + * After unification the library editors are no longer separate .wasm bundles: + * - the Symbol Editor is the eeschema bundle booted with --frame=symedit + * - the Footprint Editor is the pcbnew bundle booted with --frame=fpedit + * The frame token is threaded through Module.arguments and parsed in + * kicad/common/single_top.cpp (mirroring kicad/kicad.cpp's --frame parser). + * + * This asserts the decisive fact the launch-smoke specs don't: that the shared + * bundle actually opens the LIBRARY editor frame, not its parent editor. The + * window title is the discriminator — the parent bundle's default frame would + * title itself "Schematic Editor" / "PCB Editor". + */ + +interface FrameCase { + harness: string; + /** title the library-editor frame settles on */ + titleRe: RegExp; + /** the parent editor's title — must NOT appear (would mean --frame was ignored) */ + parentRe: RegExp; +} + +const CASES: FrameCase[] = [ + { harness: 'symbol_editor.html', titleRe: /Symbol Editor/i, parentRe: /Schematic Editor/i }, + { harness: 'footprint_editor.html', titleRe: /Footprint Editor/i, parentRe: /PCB Editor/i }, +]; + +test.describe('editor-unification runtime frame (--frame)', () => { + for (const tc of CASES) { + test(`${tc.harness} opens the library editor frame from its parent bundle`, async ({ page }) => { + const consoleLines: string[] = []; + page.on('console', (m) => consoleLines.push(m.text())); + page.on('pageerror', (e) => consoleLines.push(`pageerror: ${e.message}`)); + + await page.goto(`/kicad/${tc.harness}`); + + // The runtime came up. + await expect(page.locator('#canvas')).toBeVisible({ timeout: 120000 }); + + // The frame sets the document title once it is up; poll until it settles. + await expect + .poll(() => page.title(), { + message: `${tc.harness}: never reached the expected library-editor title`, + timeout: 120000, + intervals: [1000], + }) + .toMatch(tc.titleRe); + + const title = await page.title(); + // eslint-disable-next-line no-console + console.log(`[frame-runtime] ${tc.harness} -> title=${JSON.stringify(title)}`); + + // The runtime --frame flag actually switched frames: the parent editor's + // title must not be what we ended up on. + expect(title, `${tc.harness}: opened the library editor, not its parent`).not.toMatch(tc.parentRe); + + // No WASM abort during load. + const aborted = consoleLines.some((l) => /Aborted\(/.test(l)); + expect(aborted, 'no WASM abort during load').toBe(false); + + await page.screenshot({ path: `test-results/frame-runtime-${tc.harness}.png`, scale: 'device' }); + }); + } +}); diff --git a/tests/playwright-kicad.config.ts b/tests/playwright-kicad.config.ts index a099ff8..54af3b8 100644 --- a/tests/playwright-kicad.config.ts +++ b/tests/playwright-kicad.config.ts @@ -94,6 +94,9 @@ const PCBNEW_FAMILY_SPECS = [ "**/3d-viewer-deadlock.spec.ts", "**/3d-viewer-models.spec.ts", "**/footprint-3d-preview.spec.ts", + // Parametrized over both library editors; the footprint case boots the pcbnew + // module (footprint_editor.html loads pcbnew.js) — same V8 routing, whole file. + "**/frame-runtime.spec.ts", ]; // Runtime-perf specs run ONLY on the Chromium 'perf' project below: they need diff --git a/tests/scripts/setup-kicad-wasm.sh b/tests/scripts/setup-kicad-wasm.sh index f860447..8948b9b 100755 --- a/tests/scripts/setup-kicad-wasm.sh +++ b/tests/scripts/setup-kicad-wasm.sh @@ -39,8 +39,6 @@ kicad_subdir_for() { case "$1" in calculator) echo "pcb_calculator" ;; pl_editor) echo "pagelayout_editor" ;; - symbol_editor) echo "eeschema" ;; - footprint_editor) echo "pcbnew" ;; *) echo "$1" ;; esac } @@ -81,16 +79,17 @@ copy_app() { } found_any=0 +# symbol_editor / footprint_editor are NOT separate bundles: their harness HTMLs +# (symbol_editor.html / footprint_editor.html) load the eeschema / pcbnew bundle +# and select the frame at runtime via --frame (editor-unification). copy_app pcbnew && found_any=1 copy_app eeschema && found_any=1 copy_app calculator && found_any=1 copy_app pl_editor && found_any=1 -copy_app symbol_editor && found_any=1 -copy_app footprint_editor && found_any=1 copy_app gerbview && found_any=1 if [ "$found_any" -eq 0 ]; then - echo "Error: no pcbnew/eeschema/calculator/pl_editor/symbol_editor/footprint_editor/gerbview artifacts found in output/ or docker volume" >&2 + echo "Error: no pcbnew/eeschema/calculator/pl_editor/gerbview artifacts found in output/ or docker volume" >&2 exit 1 fi diff --git a/wasm/bindings/symbol_editor_embind.cpp b/wasm/bindings/symbol_editor_embind.cpp deleted file mode 100644 index ecbdf39..0000000 --- a/wasm/bindings/symbol_editor_embind.cpp +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Embind bindings for KiCad symbol_editor WASM. - * - * Picked up automatically by scripts/kicad/build-kicad-target.sh when building - * the symbol_editor app (it compiles wasm/bindings/_embind.cpp if present). - * - * The symbol editor is the eeschema kiface launched with - * TOP_FRAME=FRAME_SCH_SYMBOL_EDITOR, so it links the same - * eeschema_kiface_objects as eeschema — including files-io.cpp, whose save - * chokepoint references kicadCollabOnSave. eeschema gets the definition from - * eeschema_embind.cpp; this TU provides it for the symbol_editor link. - * eeschema's other bindings are not pulled in here: they assume a - * SCH_EDIT_FRAME top frame, which this app doesn't have. - */ - -#ifdef __EMSCRIPTEN__ -#include - -// C++ → JS save notification (standalone-hardening save routing). Called from -// the kicad fork's save chokepoints after a successful write to MEMFS, so the -// web app can route the saved bytes onward (API upload, local-disk write-back, -// download). No-op without a JS listener. -extern "C" void kicadCollabOnSave( const char* aPath ) -{ - EM_ASM( { - if( window.kicadCollab && window.kicadCollab.onSave ) - window.kicadCollab.onSave( UTF8ToString( $0 ) ); - }, aPath ); -} - -#endif // __EMSCRIPTEN__ diff --git a/web/standalone/src/components/WasmTool.tsx b/web/standalone/src/components/WasmTool.tsx index 4872b4d..259ec30 100644 --- a/web/standalone/src/components/WasmTool.tsx +++ b/web/standalone/src/components/WasmTool.tsx @@ -36,7 +36,7 @@ import { MODELS_LOADING_EVENT, type ModelsLoadingDetail, } from "@/wasm/libs/models-bridge"; -import { memfsFilePath, memfsProjectDir } from "@/wasm/constants"; +import { memfsFilePath, memfsProjectDir, TOOL_FRAME } from "@/wasm/constants"; import { driveProjectIntoTool, type ToolFile } from "@/wasm/kicad-runner"; import { registerSaveHook, type SaveBytes } from "@/wasm/save-flow"; import type { @@ -779,6 +779,9 @@ export function WasmTool({ // 3D models: lazy per-board source (null unless the CDN manifest is // configured) — feeds the board prescan + the viewer's ensure fallback. modelsSource: modelsSourceConfig(), + // footprint_editor/symbol_editor load the pcbnew/eeschema bundle; the + // frame token tells its single_top launcher which editor frame to open. + frame: TOOL_FRAME[tool], }); // Register the save sink before the file opens: from here on, every // editor File→Save (MEMFS write) is routed onward through saveBytes. diff --git a/web/standalone/src/wasm/boot.ts b/web/standalone/src/wasm/boot.ts index b0acc28..9cbc926 100644 --- a/web/standalone/src/wasm/boot.ts +++ b/web/standalone/src/wasm/boot.ts @@ -5,6 +5,7 @@ import { MODELS_3D_ROOT, RESOURCE_PATH, TOOL_ARGV0, + TOOL_BUNDLE, TOOL_LIB_KIND, TOOL_NEEDS_CONFIG_SEED, } from "./constants"; @@ -67,6 +68,11 @@ export interface BootOptions { /** 3D model source (lazy, per-board). Null/omitted ⇒ the viewer renders the * bare board only, exactly as before models existed. */ modelsSource?: Model3dSource | null; + /** Editor frame to open when the bundle serves more than one (e.g. `"fpedit"` + * so the pcbnew bundle opens the Footprint Editor). Passed through as + * `--frame=` in `Module.arguments`; parsed in single_top.cpp. Omitted + * ⇒ the bundle's build-time default frame. See `TOOL_FRAME` in constants.ts. */ + frame?: string; } let booted: { tool: Tool; promise: Promise } | null = null; @@ -195,6 +201,10 @@ async function doBoot(opts: BootOptions): Promise { libsSource, modelsSource, } = opts; + // The deployed bundle backing this tool. footprint_editor/symbol_editor share + // the pcbnew/eeschema engine, so their `.wasm`/`.js`/pthread-worker files are the + // parent's; `tool` still drives identity (thisProgram), config-seed and lib-kind. + const bundle = TOOL_BUNDLE[tool] ?? tool; const w = window as ToolWindow; // The wasm reads the top-level frame geometry from a GLOBAL `mainWindow` @@ -380,6 +390,11 @@ async function doBoot(opts: BootOptions): Promise { w.Module = { thisProgram: TOOL_ARGV0[tool], // argv[0] for KiCad's DEBUG check ...(traceMask ? { ENV: { KICAD_TRACE: traceMask } } : {}), + // Runtime frame selection: emscripten feeds these to main() as argv[1..], which + // single_top.cpp parses ("--frame=") to open the requested editor frame + // from a shared bundle. Set in the Module literal so it's present before the + // glue's run()/callMain fires. Empty ⇒ the bundle's build-time default frame. + arguments: opts.frame ? [`--frame=${opts.frame}`] : [], preRun, postRun: [], print: (...args: unknown[]) => { @@ -415,7 +430,7 @@ async function doBoot(opts: BootOptions): Promise { locateFile: (path: string) => `${base}/${path}`, // Pin the pthread worker script. Same-origin → direct URL; cross-origin CDN // → a same-origin blob shim that importScripts the glue (see helper above). - mainScriptUrlOrBlob: pthreadWorkerScript(base, tool, traceMask), + mainScriptUrlOrBlob: pthreadWorkerScript(base, bundle, traceMask), // Own the wasm fetch so we can report download progress (see helper). Streams // straight into the compiler; passing `module` to the callback lets emscripten // share it with the pthread workers (this hook fires on the main thread only). @@ -426,7 +441,7 @@ async function doBoot(opts: BootOptions): Promise { void (async () => { try { onStatus("Downloading…"); - const resp = await fetchWasmWithProgress(`${base}/${tool}.wasm`, onProgress); + const resp = await fetchWasmWithProgress(`${base}/${bundle}.wasm`, onProgress); const ct = resp.headers.get("content-type") ?? ""; if (ct.includes("application/wasm") && WebAssembly.instantiateStreaming) { const { instance, module } = await WebAssembly.instantiateStreaming( @@ -460,6 +475,6 @@ async function doBoot(opts: BootOptions): Promise { // Emscripten's _scriptName. await loadScript(`${base}/wx.js`); await loadScript(`${base}/wx-dom.js`); - await loadScript(`${base}/${tool}.js`); - log(`[boot] injected wx.js + wx-dom.js + ${tool}.js (base=${base})`); + await loadScript(`${base}/${bundle}.js`); + log(`[boot] injected wx.js + wx-dom.js + ${bundle}.js (base=${base})`); } diff --git a/web/standalone/src/wasm/constants.ts b/web/standalone/src/wasm/constants.ts index d8f4fae..91195dd 100644 --- a/web/standalone/src/wasm/constants.ts +++ b/web/standalone/src/wasm/constants.ts @@ -34,6 +34,41 @@ export const TOOL_ARGV0: Record = { gerbview: "/usr/bin/gerbview", }; +/** + * Which deployed WASM bundle actually backs each tool. `footprint_editor` and + * `symbol_editor` are the SAME compiled engine as `pcbnew` / `eeschema` — they + * were only ever a second launcher pinned to a different build-time frame — so + * after editor-unification they load the parent bundle and select their frame at + * runtime (see `TOOL_FRAME`). Every other tool backs its own bundle. Used to + * resolve the CDN asset folder and the `.{wasm,js}` filenames. + */ +export const TOOL_BUNDLE: Record = { + pcbnew: "pcbnew", + eeschema: "eeschema", + calculator: "calculator", + pl_editor: "pl_editor", + symbol_editor: "eeschema", + footprint_editor: "pcbnew", + gerbview: "gerbview", +}; + +/** + * The frame token passed to the WASM launcher via `Module.arguments` + * (`--frame=`, parsed in `kicad/common/single_top.cpp`) so a shared bundle + * opens a specific editor frame. Tools whose bundle already defaults to the right + * frame need no token (`undefined`). Tokens mirror `kicad/kicad.cpp`'s `--frame` + * parser, plus `symedit` for the symbol editor (which upstream's CLI lacks). + */ +export const TOOL_FRAME: Record = { + pcbnew: undefined, + eeschema: undefined, + calculator: undefined, + pl_editor: undefined, + symbol_editor: "symedit", + footprint_editor: "fpedit", + gerbview: undefined, +}; + /** * Every standalone tool here boots through common/single_top.cpp, which runs * STARTWIZARD::CheckAndRun() — the first-run "KiCad Setup" wizard. It shows diff --git a/web/standalone/src/wasm/wasm-assets.ts b/web/standalone/src/wasm/wasm-assets.ts index a91666d..e09a50d 100644 --- a/web/standalone/src/wasm/wasm-assets.ts +++ b/web/standalone/src/wasm/wasm-assets.ts @@ -1,5 +1,6 @@ import type { Tool } from "@pcbjam/shared"; import { WASM_MANIFEST_FILE, WASM_ROOT } from "@/lib/config"; +import { TOOL_BUNDLE } from "./constants"; /** * Resolve the per-tool WASM asset base at runtime from the CDN release manifest. @@ -42,10 +43,14 @@ export async function resolveWasmBase( ): Promise { if (override) return override.replace(/\/+$/, ""); if (!WASM_MANIFEST_FILE) return WASM_ROOT; // flat (dev / same-origin) + // A tool may be served by a shared bundle (footprint_editor→pcbnew, + // symbol_editor→eeschema); resolve the folder/version of the bundle, not the + // logical tool (the merged bundles are the only ones published). + const bundle = TOOL_BUNDLE[tool] ?? tool; const manifest = await loadManifest(); - const ver = manifest.tools?.[tool]; + const ver = manifest.tools?.[bundle]; if (!ver) { - throw new Error(`no WASM version for "${tool}" in ${WASM_MANIFEST_FILE}`); + throw new Error(`no WASM version for "${bundle}" in ${WASM_MANIFEST_FILE}`); } - return `${WASM_ROOT}/${tool}/${ver}`; + return `${WASM_ROOT}/${bundle}/${ver}`; }