From 2ee87889faab382ca56e79a61702f20af72e09d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20T=C3=B6rcsv=C3=A1ri?= Date: Mon, 8 Jun 2026 16:02:57 +0200 Subject: [PATCH] feat(pcbnew): collab adds board text natively (PCB_TEXT) incl. justification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A board-level graphic text (Place→Text) lands in Drawings() and would otherwise hit the same asyncify-fragile (kicad_pcb …) envelope-blob wall as via/zone on add. So PCB_TEXT reconstructs NATIVELY: itemToJson emits size/thickness/angle plus horizontal & vertical justification, mirror, and bold/italic (the text string was already emitted for any EDA_TEXT); makeItem builds a fresh PCB_TEXT and restores all of them. flushDiff skips the blob for PCB_TEXT_T. Footprint child text is unaffected — it syncs by move, and its add is carried by the footprint blob. Justification matters: it anchors the glyphs relative to the text POSITION, so without it a left-justified text reconstructed centered on the peer and rendered visibly offset even though GetPosition() matched (which is why the anchor-only headless check missed it). Verified two-tab in the real app: a left/bottom-justified bold text added in tab A reconstructs in tab B at the exact position with matching justification + string. Headless: a board-text round-trip add test (sample text is left/bottom-justified) asserts the position AND that hjust/vjust/text round-trip. 7 passed, 1 skipped, 0 aborts. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/kicad/pcbnew-collab.spec.ts | 28 +++++++++++++- wasm/bindings/pcbnew_embind.cpp | 61 +++++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/tests/kicad/pcbnew-collab.spec.ts b/tests/kicad/pcbnew-collab.spec.ts index b04a2cf..98db7d4 100644 --- a/tests/kicad/pcbnew-collab.spec.ts +++ b/tests/kicad/pcbnew-collab.spec.ts @@ -28,6 +28,7 @@ const FP1_REF = "66666666-0000-0000-0000-0000000000aa"; // Reference field (PCB_ const FP1_TXT = "66666666-0000-0000-0000-0000000000cc"; // user fp_text (PCB_TEXT, F.SilkS) const VIA1 = "77777777-0000-0000-0000-000000000001"; // a through via (PCB_VIA) const ZONE1 = "77777777-0000-0000-0000-000000000002"; // a copper zone (ZONE) +const TEXT1 = "77777777-0000-0000-0000-000000000003"; // a board graphic text (PCB_TEXT) const SAMPLE_PCB = `(kicad_pcb \t(version 20241229) \t(generator "pcbnew") @@ -79,6 +80,12 @@ const SAMPLE_PCB = `(kicad_pcb \t\t(min_thickness 0.25) \t\t(polygon (pts (xy 60 110) (xy 75 110) (xy 75 125) (xy 60 125))) \t) +\t(gr_text "BOARDTEXT" +\t\t(at 90 95 0) +\t\t(layer "F.SilkS") +\t\t(uuid "${TEXT1}") +\t\t(effects (font (size 1 1) (thickness 0.15)) (justify left bottom)) +\t) \t(segment (start 50.8 50.8) (end 101.6 50.8) (width 0.2) (layer "F.Cu") (net 0) (uuid "${SEG1}")) \t(segment (start 50.8 76.2) (end 101.6 76.2) (width 0.2) (layer "F.Cu") (net 0) (uuid "${SEG2}")) ) @@ -179,6 +186,11 @@ test.describe("pcbnew collab bridge — single page", () => { (byId.get(ZONE1) as { poly?: number[][] }).poly?.length, "zone outline emitted", ).toBeGreaterThanOrEqual(3); + expect(byId.has(TEXT1), "board text present").toBe(true); + expect(byId.get(TEXT1)!.type).toBe("PCB_TEXT"); + expect((byId.get(TEXT1) as { text?: string }).text, "board text string emitted").toBe( + "BOARDTEXT", + ); expect(hasAbort(testLogger), "no WASM abort").toBe(false); }); @@ -191,8 +203,9 @@ test.describe("pcbnew collab bridge — single page", () => { ["footprint", FP1, "FOOTPRINT"], ["via", VIA1, "PCB_VIA"], ["zone", ZONE1, "ZONE"], + ["board text", TEXT1, "PCB_TEXT"], ] as const) { - test(`apply adds a ${label} (footprint via blob, via/zone native)`, async ({ page, testLogger }) => { + test(`apply adds a ${label} (footprint via blob, via/zone/text native)`, async ({ page, testLogger }) => { await bootAndOpen(page, `add-${label}`); // Full emit-equivalent payload: snapshot item (native geometry fields) + the clipboard blob. @@ -228,6 +241,19 @@ test.describe("pcbnew collab bridge — single page", () => { intervals: [200], }) .toBe(posBefore); + + // Fidelity beyond the anchor: a text's justification anchors its glyphs, so a peer that + // dropped it would render the text visibly offset even though GetPosition matches. Confirm + // the reconstructed item round-trips its type-defining fields. + const after = await page.evaluate((i) => { + const snap = JSON.parse(window.Module.kicadCollabSnapshot()); + return snap.added.find((it: { id: string }) => it.id === i); + }, id); + if (type === "PCB_TEXT") { + expect(after.text, "text string preserved").toBe(payload.text); + expect(after.hjust, "text horizontal justification preserved").toBe(payload.hjust); + expect(after.vjust, "text vertical justification preserved").toBe(payload.vjust); + } expect(hasAbort(testLogger), "no WASM abort").toBe(false); }); } diff --git a/wasm/bindings/pcbnew_embind.cpp b/wasm/bindings/pcbnew_embind.cpp index e67f1be..d320974 100644 --- a/wasm/bindings/pcbnew_embind.cpp +++ b/wasm/bindings/pcbnew_embind.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -196,9 +197,28 @@ json itemToJson( BOARD_ITEM* aItem ) j["poly"] = pts; } + // A board-level graphic text (Place→Text) also reconstructs NATIVELY (same asyncify reason as + // via/zone): emit its size / stroke / angle so makeItem can rebuild it. Footprint child text + // is synced by move, not `added`, so this is only the board PCB_TEXT case. + else if( aItem->Type() == PCB_TEXT_T ) + { + auto* txt = static_cast( aItem ); + j["tw"] = txt->GetTextSize().x; + j["th"] = txt->GetTextSize().y; + j["thick"] = txt->GetTextThickness(); + j["angle"] = txt->GetTextAngle().AsTenthsOfADegree(); + // Justification + mirror anchor the glyphs relative to the text POSITION; without them a + // left-justified text reconstructs centered and renders visibly offset from the same + // GetPosition() (peers diverge visually though GetPosition matches). Bold/italic for looks. + j["hjust"] = (int) txt->GetHorizJustify(); + j["vjust"] = (int) txt->GetVertJustify(); + j["mirror"] = txt->IsMirrored(); + j["bold"] = txt->IsBold(); + j["italic"] = txt->IsItalic(); + } // Text items (incl. footprint fields / graphic text): carry the string so a move diff is - // legible and a future text `added` can reconstruct. Position-only sync uses x/y above. + // legible and a text `added` can reconstruct. Position-only sync uses x/y above. if( EDA_TEXT* txt = dynamic_cast( aItem ) ) j["text"] = toUtf8( txt->GetText() ); @@ -327,6 +347,40 @@ BOARD_ITEM* makeItem( BOARD& aBoard, const json& j ) item = zone; } + else if( type == "PCB_TEXT" && j.contains( "text" ) ) + { + auto* txt = new PCB_TEXT( &aBoard ); + txt->SetText( wxString::FromUTF8( j.value( "text", "" ).c_str() ) ); + txt->SetPosition( VECTOR2I( j.value( "x", 0 ), j.value( "y", 0 ) ) ); + txt->SetLayer( (PCB_LAYER_ID) j.value( "layer", (int) F_SilkS ) ); + + if( j.contains( "tw" ) ) + txt->SetTextSize( VECTOR2I( j.value( "tw", 0 ), j.value( "th", 0 ) ) ); + + if( j.contains( "thick" ) ) + txt->SetTextThickness( j.value( "thick", 0 ) ); + + if( j.contains( "angle" ) ) + txt->SetTextAngle( EDA_ANGLE( j.value( "angle", 0 ), TENTHS_OF_A_DEGREE_T ) ); + + // Restore justification/mirror so the glyphs sit at the same place relative to position. + if( j.contains( "hjust" ) ) + txt->SetHorizJustify( (GR_TEXT_H_ALIGN_T) j.value( "hjust", (int) GR_TEXT_H_ALIGN_LEFT ) ); + + if( j.contains( "vjust" ) ) + txt->SetVertJustify( (GR_TEXT_V_ALIGN_T) j.value( "vjust", (int) GR_TEXT_V_ALIGN_CENTER ) ); + + if( j.contains( "mirror" ) ) + txt->SetMirrored( j.value( "mirror", false ) ); + + if( j.contains( "bold" ) ) + txt->SetBold( j.value( "bold", false ) ); + + if( j.contains( "italic" ) ) + txt->SetItalic( j.value( "italic", false ) ); + + item = txt; + } else if( j.contains( "sexpr" ) ) { item = makeFromBlob( aBoard, j.value( "sexpr", "" ) ); @@ -439,10 +493,11 @@ void flushDiff() json withBlob = j; // Attach an s-expr clipboard blob ONLY for types makeItem reconstructs from it - // (footprints, board shapes/text, …). Tracks/vias/zones rebuild NATIVELY from the + // (footprints, board graphics, …). Tracks/vias/zones/text rebuild NATIVELY from the // fields itemToJson already emitted, so they need no blob — and skipping it avoids a // wasted SaveSelection plus the asyncify-fragile envelope parse for those. - if( live && !isTrackType( live->Type() ) && live->Type() != PCB_ZONE_T ) + if( live && !isTrackType( live->Type() ) && live->Type() != PCB_ZONE_T + && live->Type() != PCB_TEXT_T ) withBlob["sexpr"] = blobForItem( board, live ); added.push_back( withBlob );