fix(pcbnew): collab syncs real item layer — devirtualize Get/SetLayer; collab on by default

This commit is contained in:
Gergő Törcsvári 2026-06-09 06:03:17 +02:00
commit b672c54189
No known key found for this signature in database
GPG key ID: 8E75F2CDE64E5322
3 changed files with 39 additions and 13 deletions

View file

@ -191,6 +191,11 @@ test.describe("pcbnew collab bridge — single page", () => {
expect((byId.get(TEXT1) as { text?: string }).text, "board text string emitted").toBe( expect((byId.get(TEXT1) as { text?: string }).text, "board text string emitted").toBe(
"BOARDTEXT", "BOARDTEXT",
); );
// The text is on F.SilkS (=5). Asserting the ACTUAL layer (not just "round-trips") catches the
// GetLayer() emit bug, where every item reported layer 0 (F.Cu) and added items landed on the
// wrong copper layer on the peer.
expect((byId.get(TEXT1) as { layer?: number }).layer, "board text on F.SilkS (not stuck at 0)").toBe(5);
expect((byId.get(SEG1) as { layer?: number }).layer, "segment on F.Cu").toBe(0);
expect(hasAbort(testLogger), "no WASM abort").toBe(false); expect(hasAbort(testLogger), "no WASM abort").toBe(false);
}); });

View file

@ -33,6 +33,7 @@
#include <kiway.h> #include <kiway.h>
#include <kiid.h> #include <kiid.h>
#include <layer_ids.h> #include <layer_ids.h>
#include <lset.h>
#include <tool/coroutine.h> #include <tool/coroutine.h>
#include <nlohmann/json.hpp> #include <nlohmann/json.hpp>
#include <map> #include <map>
@ -107,6 +108,20 @@ bool isTrackType( KICAD_T t )
return t == PCB_TRACE_T || t == PCB_ARC_T || t == PCB_VIA_T; return t == PCB_TRACE_T || t == PCB_ARC_T || t == PCB_VIA_T;
} }
// Read an item's layer WITHOUT the virtual GetLayer(). That virtual mis-dispatches in the
// non-coroutine emit/snapshot context — it returns 0 (F_Cu) for EVERY item (the same asyncify
// call_indirect class as eeschema's Move()), which silently put every collab-added item/track/
// text on the top copper layer on the peer. A class-qualified `BOARD_ITEM::GetLayer()` is a
// statically-bound (direct) call that just reads m_layer, bypassing call_indirect. Zones keep
// their layer in m_layerSet (not m_layer), so use their non-virtual GetFirstLayer().
int itemLayer( BOARD_ITEM* aItem )
{
if( aItem->Type() == PCB_ZONE_T )
return (int) static_cast<ZONE*>( aItem )->GetFirstLayer();
return (int) aItem->BOARD_ITEM::GetLayer();
}
// Iterate every board item the bridge syncs: the top-level items (tracks, footprints, drawings, // Iterate every board item the bridge syncs: the top-level items (tracks, footprints, drawings,
// zones, groups) PLUS each footprint's TEXT children (fields = reference/value/user, and graphic // zones, groups) PLUS each footprint's TEXT children (fields = reference/value/user, and graphic
// PCB_TEXT). The text children are visited by their OWN uuid because a silkscreen reference/value // PCB_TEXT). The text children are visited by their OWN uuid because a silkscreen reference/value
@ -155,7 +170,7 @@ json itemToJson( BOARD_ITEM* aItem )
{ "type", toUtf8( aItem->GetClass() ) }, { "type", toUtf8( aItem->GetClass() ) },
{ "x", p.x }, // internal units (nm); integral, no quantization needed { "x", p.x }, // internal units (nm); integral, no quantization needed
{ "y", p.y }, { "y", p.y },
{ "layer", (int) aItem->GetLayer() }, { "layer", itemLayer( aItem ) }, // devirtualized — aItem->GetLayer() mis-dispatches here
}; };
if( isTrackType( aItem->Type() ) ) if( isTrackType( aItem->Type() ) )
@ -313,7 +328,9 @@ BOARD_ITEM* makeItem( BOARD& aBoard, const json& j )
tr->SetStart( VECTOR2I( j.value( "sx", 0 ), j.value( "sy", 0 ) ) ); tr->SetStart( VECTOR2I( j.value( "sx", 0 ), j.value( "sy", 0 ) ) );
tr->SetEnd( VECTOR2I( j.value( "ex", 0 ), j.value( "ey", 0 ) ) ); tr->SetEnd( VECTOR2I( j.value( "ex", 0 ), j.value( "ey", 0 ) ) );
tr->SetWidth( j.value( "width", 0 ) ); tr->SetWidth( j.value( "width", 0 ) );
tr->SetLayer( (PCB_LAYER_ID) j.value( "layer", (int) F_Cu ) ); // SetLayer is virtual and (like GetLayer) mis-dispatches in this apply context → it no-ops,
// leaving the item on the default layer. Class-qualify to a direct m_layer write.
tr->BOARD_ITEM::SetLayer( (PCB_LAYER_ID) j.value( "layer", (int) F_Cu ) );
item = tr; item = tr;
} }
// Via / zone: reconstruct natively from emitted geometry (the envelope-blob parse is // Via / zone: reconstruct natively from emitted geometry (the envelope-blob parse is
@ -340,7 +357,9 @@ BOARD_ITEM* makeItem( BOARD& aBoard, const json& j )
outline.emplace_back( p[0].get<int>(), p[1].get<int>() ); outline.emplace_back( p[0].get<int>(), p[1].get<int>() );
} }
zone->SetLayer( (PCB_LAYER_ID) j.value( "layer", (int) F_Cu ) ); // Zones keep their layer in m_layerSet via SetLayer→SetLayerSet (both virtual, both no-op
// here). Class-qualify ZONE::SetLayerSet to set it directly (else GetFirstLayer == -1).
zone->ZONE::SetLayerSet( LSET( { (PCB_LAYER_ID) j.value( "layer", (int) F_Cu ) } ) );
if( outline.size() >= 3 ) if( outline.size() >= 3 )
zone->AddPolygon( outline ); zone->AddPolygon( outline );
@ -352,7 +371,7 @@ BOARD_ITEM* makeItem( BOARD& aBoard, const json& j )
auto* txt = new PCB_TEXT( &aBoard ); auto* txt = new PCB_TEXT( &aBoard );
txt->SetText( wxString::FromUTF8( j.value( "text", "" ).c_str() ) ); txt->SetText( wxString::FromUTF8( j.value( "text", "" ).c_str() ) );
txt->SetPosition( VECTOR2I( j.value( "x", 0 ), j.value( "y", 0 ) ) ); txt->SetPosition( VECTOR2I( j.value( "x", 0 ), j.value( "y", 0 ) ) );
txt->SetLayer( (PCB_LAYER_ID) j.value( "layer", (int) F_SilkS ) ); txt->BOARD_ITEM::SetLayer( (PCB_LAYER_ID) j.value( "layer", (int) F_SilkS ) ); // devirt
if( j.contains( "tw" ) ) if( j.contains( "tw" ) )
txt->SetTextSize( VECTOR2I( j.value( "tw", 0 ), j.value( "th", 0 ) ) ); txt->SetTextSize( VECTOR2I( j.value( "tw", 0 ), j.value( "th", 0 ) ) );

View file

@ -12,11 +12,12 @@ import { clog, cwarn } from "@/wasm/collab/debug";
const COLLAB_TOOLS = new Set<Tool>(["pl_editor", "eeschema", "pcbnew"]); const COLLAB_TOOLS = new Set<Tool>(["pl_editor", "eeschema", "pcbnew"]);
/** /**
* Opt-in collaborative editing (features/yjs-bridge). Enabled when the URL carries * Collaborative editing (features/yjs-bridge), ON BY DEFAULT for any tool that has the
* `?collab=1` and the tool has the collab bridge. Open the same project URL in two tabs * collab bridge. Open the same project URL in two tabs to edit together: the channel is
* with `?collab=1` to edit together: the channel is keyed to project+file, so both tabs * keyed to project+file, so both tabs share one Y.Doc over BroadcastChannel. Editor edits
* share one Y.Doc over BroadcastChannel. Editor edits (add/move items) fire the tool's * (add/move items) fire the tool's change hook → the bridge → the peer tab.
* change hook → the bridge → the peer tab. *
* Opt OUT with `?collab=0` (or `collab=false`). Tools without a bridge are skipped anyway.
*/ */
async function maybeStartCollab( async function maybeStartCollab(
win: ToolWindow, win: ToolWindow,
@ -28,10 +29,10 @@ async function maybeStartCollab(
onStatus: (t: string) => void; onStatus: (t: string) => void;
}, },
): Promise<void> { ): Promise<void> {
const enabled = new URLSearchParams(win.location.search).get("collab"); const collabParam = new URLSearchParams(win.location.search).get("collab");
const mod = win.Module; const mod = win.Module;
clog("maybeStartCollab gate:", { clog("maybeStartCollab gate:", {
collabParam: enabled, collabParam,
tool: opts.tool, tool: opts.tool,
hasModule: !!mod, hasModule: !!mod,
hasSnapshot: typeof mod?.kicadCollabSnapshot, hasSnapshot: typeof mod?.kicadCollabSnapshot,
@ -39,8 +40,9 @@ async function maybeStartCollab(
url: win.location.href, url: win.location.href,
}); });
if (!enabled || enabled === "0" || enabled === "false") { // On by default; only an explicit opt-out disables it.
clog("disabled (no ?collab=1) — skipping"); if (collabParam === "0" || collabParam === "false") {
clog("disabled (?collab=0) — skipping");
return; return;
} }
if (!COLLAB_TOOLS.has(opts.tool)) { if (!COLLAB_TOOLS.has(opts.tool)) {