fix(eeschema): collab drag no longer drops segments — batch one edit into one delta
A single SCH_COMMIT::Push fires OnItemsAdded/Removed/Changed separately and
synchronously, then RecalculateConnections once. COLLAB_LISTENER emitted each
category as its own delta, so the peer applied one atomic edit as three separate
commits, each with its own connectivity recompute. On a large connected drag the
junction at the wires' new crossing (added) was applied before the wires moved
(changed) -> dangling junction -> the peer's cleanup deleted it (lost segments).
COLLAB_LISTENER now buffers added/changed/removed (serialized in each synchronous
callback) and flushes one combined delta after Push returns, coalesced via
CallAfter. doApply applies it atomically removed->changed->added in a single
commit with one recompute, so the junction survives. Verified two-tab: a G-drag
of U1A that previously left the peer at 74 items / 7 junctions now emits one
delta {a:1,c:4,r:1} and both tabs converge at 75 / 8.
Root-repo only; kicad and wxwidgets untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b577c05fae
commit
f2a6deaf7b
2 changed files with 79 additions and 17 deletions
|
|
@ -87,6 +87,30 @@ produce the s-expr clipboard blob (`SCH_IO_KICAD_SEXPR::Format` →
|
||||||
until symbol libraries are bundled (you can't place a symbol natively to sync). Symbol
|
until symbol libraries are bundled (you can't place a symbol natively to sync). Symbol
|
||||||
**move/position** already syncs via the generic `changed` path.
|
**move/position** already syncs via the generic `changed` path.
|
||||||
|
|
||||||
|
## Thrust C — collab wire/symbol-drag divergence (lost segments) — fixed
|
||||||
|
|
||||||
|
User-reported follow-up: dragging a wire OR symbol with multiple connections a *large*
|
||||||
|
distance (small drags don't reroute) lost segments on the peer. Reproduced + root-caused
|
||||||
|
in the real app, two tabs.
|
||||||
|
|
||||||
|
**Root cause:** one atomic edit (a single `SCH_COMMIT::Push`) calls `OnItemsAdded`,
|
||||||
|
`OnItemsRemoved`, `OnItemsChanged` **separately and synchronously**, then
|
||||||
|
`RecalculateConnections` once. The old `COLLAB_LISTENER` emitted each category as its own
|
||||||
|
delta, so the peer applied them as THREE separate commits, each with its own connectivity
|
||||||
|
recompute. A `G`-drag of U1A emitted `{added:[junction]}`, `{removed:[wire]}`,
|
||||||
|
`{changed:[3 wires, symbol]}`; the junction (at the wires' new crossing) was applied
|
||||||
|
*before* the wires moved → a **dangling junction on the peer → connectivity cleanup deleted
|
||||||
|
it**. Result: tab A 75 items / 8 junctions, tab B 74 / 7 (the junction lost). Simple
|
||||||
|
translates (`M` tool) always converged — they touch only `changed`.
|
||||||
|
|
||||||
|
**Fix (`wasm/bindings/eeschema_embind.cpp`):** `COLLAB_LISTENER` now buffers the three
|
||||||
|
categories (serializing items in each synchronous callback) and flushes **one combined
|
||||||
|
delta** after Push returns, coalesced via `CallAfter`. The peer's `doApply` applies a
|
||||||
|
combined delta removed→changed→added in a single `SCH_COMMIT` with one recompute, so the
|
||||||
|
junction is added after its wires are in place and survives. **Verified:** the same G-drag
|
||||||
|
now emits 1 delta `{a:1,c:4,r:1}` and both tabs converge identically (75 items, 8 junctions,
|
||||||
|
zero wire/junction diff). Embind-only build; eeschema-collab + eeschema-ui suites green.
|
||||||
|
|
||||||
## Files touched (all root repo)
|
## Files touched (all root repo)
|
||||||
|
|
||||||
- `web/apps/frontend/src/index.css` — `.window` / `.window-canvas` CSS (Thrust A).
|
- `web/apps/frontend/src/index.css` — `.window` / `.window-canvas` CSS (Thrust A).
|
||||||
|
|
|
||||||
|
|
@ -286,17 +286,30 @@ void emit( const json& aDelta )
|
||||||
|
|
||||||
// ChangeSource: native SCHEMATIC_LISTENER. SCH_COMMIT::Push fires these in bulk for
|
// ChangeSource: native SCHEMATIC_LISTENER. SCH_COMMIT::Push fires these in bulk for
|
||||||
// every local edit (move, add, remove, …) — that's our emit trigger.
|
// every local edit (move, add, remove, …) — that's our emit trigger.
|
||||||
|
//
|
||||||
|
// CRUCIAL: one local edit (a single SCH_COMMIT::Push) calls OnItemsAdded, OnItemsRemoved
|
||||||
|
// and OnItemsChanged *separately and synchronously*, then runs RecalculateConnections once
|
||||||
|
// (sch_commit.cpp). Emitting each category as its own delta makes the peer apply them as
|
||||||
|
// THREE separate commits, each followed by its own connectivity recompute — so an item that
|
||||||
|
// only makes sense in the final state is mis-handled mid-sequence. Concretely, a connected
|
||||||
|
// drag adds a junction at the wires' new crossing AND moves those wires; if the junction
|
||||||
|
// (added) is applied before the wires (changed), the peer sees a dangling junction and
|
||||||
|
// connectivity cleanup deletes it → the peer permanently loses it (the "lost segments on a
|
||||||
|
// big drag" divergence). Fix: buffer all three categories and emit ONE combined delta after
|
||||||
|
// Push returns (coalesced via CallAfter), so the peer applies it atomically in a single
|
||||||
|
// SCH_COMMIT — doApply orders it removed→changed→added, with one recompute at the end, so the
|
||||||
|
// junction is added after its wires are in place and survives.
|
||||||
class COLLAB_LISTENER : public SCHEMATIC_LISTENER
|
class COLLAB_LISTENER : public SCHEMATIC_LISTENER
|
||||||
{
|
{
|
||||||
public:
|
public:
|
||||||
void OnSchItemsAdded( SCHEMATIC&, std::vector<SCH_ITEM*>& aItems ) override
|
void OnSchItemsAdded( SCHEMATIC&, std::vector<SCH_ITEM*>& aItems ) override
|
||||||
{
|
{
|
||||||
emitItems( "added", aItems );
|
accumulate( m_added, aItems );
|
||||||
}
|
}
|
||||||
|
|
||||||
void OnSchItemsChanged( SCHEMATIC&, std::vector<SCH_ITEM*>& aItems ) override
|
void OnSchItemsChanged( SCHEMATIC&, std::vector<SCH_ITEM*>& aItems ) override
|
||||||
{
|
{
|
||||||
emitItems( "changed", aItems );
|
accumulate( m_changed, aItems );
|
||||||
}
|
}
|
||||||
|
|
||||||
void OnSchItemsRemoved( SCHEMATIC&, std::vector<SCH_ITEM*>& aItems ) override
|
void OnSchItemsRemoved( SCHEMATIC&, std::vector<SCH_ITEM*>& aItems ) override
|
||||||
|
|
@ -304,35 +317,60 @@ public:
|
||||||
if( s_applyingRemote )
|
if( s_applyingRemote )
|
||||||
return;
|
return;
|
||||||
|
|
||||||
json removed = json::array();
|
|
||||||
|
|
||||||
for( SCH_ITEM* item : aItems )
|
for( SCH_ITEM* item : aItems )
|
||||||
removed.push_back( toUtf8( item->m_Uuid.AsString() ) );
|
m_removed.push_back( toUtf8( item->m_Uuid.AsString() ) );
|
||||||
|
|
||||||
if( !removed.empty() )
|
scheduleFlush();
|
||||||
emit( json{ { "added", json::array() }, { "changed", json::array() },
|
|
||||||
{ "removed", removed } } );
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private:
|
private:
|
||||||
void emitItems( const char* aKey, std::vector<SCH_ITEM*>& aItems )
|
// Serialize the items NOW (they're valid during the synchronous callback); the buffered
|
||||||
|
// json is emitted later by flush().
|
||||||
|
void accumulate( json& aBucket, std::vector<SCH_ITEM*>& aItems )
|
||||||
{
|
{
|
||||||
if( s_applyingRemote )
|
if( s_applyingRemote )
|
||||||
return;
|
return;
|
||||||
|
|
||||||
json arr = json::array();
|
|
||||||
|
|
||||||
for( SCH_ITEM* item : aItems )
|
for( SCH_ITEM* item : aItems )
|
||||||
arr.push_back( itemToJson( item ) );
|
aBucket.push_back( itemToJson( item ) );
|
||||||
|
|
||||||
if( arr.empty() )
|
scheduleFlush();
|
||||||
|
}
|
||||||
|
|
||||||
|
// All three category callbacks for one Push fire synchronously before control returns to
|
||||||
|
// the main loop, so a single CallAfter run after Push drains them into one delta. (Several
|
||||||
|
// commits in the same loop turn coalesce into one delta — harmless for the CRDT.)
|
||||||
|
void scheduleFlush()
|
||||||
|
{
|
||||||
|
if( m_flushScheduled )
|
||||||
return;
|
return;
|
||||||
|
|
||||||
json d = { { "added", json::array() }, { "changed", json::array() },
|
m_flushScheduled = true;
|
||||||
{ "removed", json::array() } };
|
|
||||||
d[aKey] = arr;
|
if( SCH_EDIT_FRAME* fr = schFrame() )
|
||||||
emit( d );
|
fr->CallAfter( [this]() { flush(); } );
|
||||||
|
else
|
||||||
|
flush();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void flush()
|
||||||
|
{
|
||||||
|
m_flushScheduled = false;
|
||||||
|
|
||||||
|
if( m_added.empty() && m_changed.empty() && m_removed.empty() )
|
||||||
|
return;
|
||||||
|
|
||||||
|
emit( json{ { "added", m_added }, { "changed", m_changed }, { "removed", m_removed } } );
|
||||||
|
|
||||||
|
m_added = json::array();
|
||||||
|
m_changed = json::array();
|
||||||
|
m_removed = json::array();
|
||||||
|
}
|
||||||
|
|
||||||
|
json m_added = json::array();
|
||||||
|
json m_changed = json::array();
|
||||||
|
json m_removed = json::array();
|
||||||
|
bool m_flushScheduled = false;
|
||||||
};
|
};
|
||||||
|
|
||||||
COLLAB_LISTENER* g_listener = nullptr;
|
COLLAB_LISTENER* g_listener = nullptr;
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue