AI_Assistant/pcbjam
Watch
1
0
Fork
You've already forked pcbjam
 0
Code Issues Pull requests Projects Releases 7 Packages Wiki Activity Actions

fix(collab): re-push viewport on canvas resize — comment pins sat offset until pan/zoom

Browse source 
emitViewportIfChanged deduped on scale+center only, so a canvas SIZE change
(boot layout settling after the bind-time seed, window resize) never re-pushed
{w,h} to JS. CommentLayer's worldToScreen maps through h/2, so every DOM pin
hit target (and its hover ring) sat vertically offset from its GAL dot by
exactly delta-h/2 css-px until the next pan/zoom finally passed the dedupe.

Size now participates in the dedupe and wxEVT_SIZE re-pushes post-layout
(CallAfter, after the GAL's own onSize). New regression e2e
comments-viewport-resize.spec.ts asserts the DOM pin re-aligns with GAL truth
(fresh kicadCollabGetViewport) across a window resize with no pan/zoom.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011x3h5AzkWDbHmCkeVYAuzU
This commit is contained in:
Gergő Törcsvári 2026-07-09 16:31:41 +02:00
commit c229c75ca6
No known key found for this signature in database
GPG key ID: 8E75F2CDE64E5322
2 changed files with 135 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

111
tests/web/comments-viewport-resize.spec.ts Normal file
View file

@ -0,0 +1,111 @@
import { test, expect, type Page } from '@playwright/test';
/**
* Comment pins vs. canvas resize (collab-presence 0005 regression): the DOM
* hit targets map world→CSS through the viewport transform pushed from C++
* ({cx,cy,scale,w,h} → worldToScreen maps through w/2,h/2). The push used to
* dedupe on scale+center only, so a canvas RESIZE (window resize, boot layout
* settling after the bind-time seed) left a stale w/h in React and every pin
* target sat vertically offset from its GAL dot — by exactly Δh/2 css-px —
* until the next pan/zoom. Now wxEVT_SIZE re-pushes and size participates in
* the dedupe, so pins must re-align after a resize with no pan/zoom at all.
*
* "GAL truth" is computed from a FRESH kicadCollabGetViewport() read + the
* live glcanvas rect — the same mapping the GAL dot itself renders through.
*/
const SCOPE = 'default';
const ROUTE = 'demo.kicad_sch';
const TITLE = /demo — Schematic Editor/i;
async function bootAs(page: Page, user: string): Promise<void> {
await page.goto(`/${SCOPE}/projects/demo/${ROUTE}?user=${user}`);
await expect(page.locator('#canvas')).toBeVisible({ timeout: 120000 });
await expect
.poll(() => page.title(), {
message: `${user}: editor never reached the expected title`,
timeout: 120000,
intervals: [1000],
})
.toMatch(TITLE);
await expect(page.getByTestId('comment-bar-toggle')).toBeVisible({ timeout: 30000 });
await page.getByTestId('comment-bar-toggle').click();
await expect(page.getByTestId('comment-mode-toggle')).toBeVisible();
}
/** Comments persist in the room's ydoc — start each run from a clean slate. */
async function deleteAllThreads(page: Page): Promise<void> {
await page.evaluate(() => {
const ctl = (window as unknown as {
__pcbjamComments?: {
threads(): Array<{ id: string }>;
deleteThread(id: string): void;
};
}).__pcbjamComments;
ctl?.threads().forEach((t) => ctl.deleteThread(t.id));
});
await expect(page.getByTestId('comment-pin')).toHaveCount(0);
}
/** First pin's DOM center minus its GAL-truth CSS position (fresh transform). */
async function pinDelta(page: Page): Promise<{ dx: number; dy: number }> {
return page.evaluate(() => {
const win = window as unknown as {
Module: { kicadCollabGetViewport(): string };
__pcbjamComments: { threads(): Array<{ world: { x: number; y: number } }> };
};
const vp = JSON.parse(win.Module.kicadCollabGetViewport()) as {
cx: number; cy: number; scale: number; w: number; h: number;
};
const gl = Array.from(document.querySelectorAll('[id^="glcanvas-"]')).find((c) => {
const r = (c as HTMLElement).getBoundingClientRect();
return getComputedStyle(c as HTMLElement).display !== 'none' && r.width > 0;
}) as HTMLElement;
const r = gl.getBoundingClientRect();
const ratio = r.width / vp.w;
const world = win.__pcbjamComments.threads()[0].world;
const truthX = r.x + ((world.x - vp.cx) * vp.scale + vp.w / 2) * ratio;
const truthY = r.y + ((world.y - vp.cy) * vp.scale + vp.h / 2) * ratio;
const pin = document.querySelector('[data-testid="comment-pin"]') as HTMLElement;
const pr = pin.getBoundingClientRect();
return { dx: pr.x + pr.width / 2 - truthX, dy: pr.y + pr.height / 2 - truthY };
});
}
test('comment pin targets track the GAL dots across a canvas resize', async ({ page }) => {
test.setTimeout(300000); // one full tool boot
await bootAs(page, 'alice');
await deleteAllThreads(page);
// Place a comment mid-canvas via comment mode.
await page.getByTestId('comment-mode-toggle').click();
const catcher = page.getByTestId('comment-click-catcher');
await expect(catcher).toBeVisible();
const box = (await catcher.boundingBox())!;
await catcher.click({ position: { x: box.width / 2, y: box.height / 2 } });
await page.getByTestId('comment-composer').locator('textarea').fill('resize pin');
await page.getByTestId('comment-submit').click();
await expect(page.getByTestId('comment-pin')).toHaveCount(1);
// Aligned at the boot size (the composer click and the pin share the same
// transform, so this mostly guards the harness itself).
await expect
.poll(async () => Math.abs((await pinDelta(page)).dy), {
message: 'pin never aligned with GAL truth at the boot size',
timeout: 15000,
})
.toBeLessThan(3);
// Shrink the window: the canvas height changes while scale/center stay put.
// The wxEVT_SIZE re-push must realign the DOM target with the GAL dot —
// before the fix it stayed exactly Δh/2 css-px below until a pan/zoom.
await page.setViewportSize({ width: 1280, height: 620 });
await expect
.poll(async () => Math.abs((await pinDelta(page)).dy), {
message: 'pin target did not re-align after the canvas resize (stale viewport w/h?)',
timeout: 15000,
})
.toBeLessThan(3);
expect(Math.abs((await pinDelta(page)).dx)).toBeLessThan(3);
});

28
wasm/bindings/collab_presence_core.h
View file

@ -142,6 +142,7 @@ struct CORE
long long lastCursorEmitMs = 0;
double lastVpScale = 0.0;
VECTOR2D lastVpCenter;
VECTOR2I lastVpSize;
// ── local state emit (selection / cursor / viewport) ──────────────────
@ -203,17 +204,23 @@ struct CORE
if( !fr )
return;
KIGFX::VIEW* view = fr->GetCanvas()->GetView();
double scale = view->GetScale(); // zoom — cheap change detector only
VECTOR2D c = view->GetCenter();
KIGFX::VIEW* view = fr->GetCanvas()->GetView();
double scale = view->GetScale(); // zoom — cheap change detector only
VECTOR2D c = view->GetCenter();
const VECTOR2I& sz = view->GetScreenPixelSize();
if( scale == lastVpScale && c == lastVpCenter )
// Size participates in the dedupe: the JS worldToScreen maps through
// w/2,h/2, so a canvas resize (or the boot layout settling after the
// bind-time seed) with an unchanged scale/center must still re-push —
// else every DOM pin target is vertically offset until the next
// pan/zoom.
if( scale == lastVpScale && c == lastVpCenter && sz == lastVpSize )
return;
lastVpScale = scale;
lastVpCenter = c;
lastVpSize = sz;
const VECTOR2I& sz = view->GetScreenPixelSize();
// px per IU via the GAL matrix — GetScale() is the zoom, not px/IU.
pcbjam_collab::emitViewport( c.x, c.y, view->ToScreen( 1.0 ), sz.x, sz.y );
@ -453,6 +460,17 @@ struct CORE
canvas->Bind( wxEVT_RIGHT_UP, [selAndViewport]( wxMouseEvent& e ) { selAndViewport( e ); } );
canvas->Bind( wxEVT_KEY_UP, [selAndViewport]( wxKeyEvent& e ) { selAndViewport( e ); } );
canvas->Bind( wxEVT_MOUSEWHEEL, [selAndViewport]( wxMouseEvent& e ) { selAndViewport( e ); } );
// Canvas resizes change the w/h half of the world↔screen transform
// without touching scale/center — re-push post-layout (CallAfter runs
// after the GAL's own onSize updated the screen size).
canvas->Bind( wxEVT_SIZE, [this]( wxSizeEvent& e )
{
e.Skip();
if( EDA_DRAW_FRAME* f = frame() )
f->CallAfter( [this]() { emitViewportIfChanged(); } );
} );
}
/** kicadCollabSetRemote: full remote-peers snapshot — `{peers:[{id,name,