From 2d2085deb8d28f8168bb7ce446e3a381b45e6da8 Mon Sep 17 00:00:00 2001 From: Fini Date: Wed, 29 Apr 2026 09:50:42 +0800 Subject: [PATCH] fix(canvas): drag/resize/rotate previews leave ancestor clipStack untouched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A node's `RenderNode.clipStack` carries the ancestor clip chain, NOT this node's own bounds. The previous interaction handlers transformed every entry of clipStack alongside the node's own absX/absY/absW/absH: - drag: translated each entry by (dx, dy) - resize: scaled each entry alongside the resize delta - rotate: rotated each entry around the rotation center That's wrong — the ancestor frames being referenced by those entries aren't being dragged/resized/rotated, so their clip rectangles on screen shouldn't move. The visible result was the clip rectangle drifting away from the actual ancestor during preview. Fix: in all 4 sites (drag mutation loop, resize root preview, resize children iteration, rotate children iteration), restore the snapshot clipStack unchanged (deep-cloned so caller-mutation can't leak back into the snapshot). The node's own pushed clip — if it has clipContent: true — lives in its CHILDREN's clipStack, which the children's flatten will recompute on commit. Brief preview artifact only when the node being transformed has clipContent and its children are simultaneously visible during preview, which is acceptable for an in-flight gesture. Test updated: dragged node's clipStack now stays at its snapshot value. --- .../skia/__tests__/skia-interaction.test.ts | 10 +++- apps/web/src/canvas/skia/skia-interaction.ts | 51 ++++++------------- 2 files changed, 23 insertions(+), 38 deletions(-) diff --git a/apps/web/src/canvas/skia/__tests__/skia-interaction.test.ts b/apps/web/src/canvas/skia/__tests__/skia-interaction.test.ts index e98543eb5..3c16b2268 100644 --- a/apps/web/src/canvas/skia/__tests__/skia-interaction.test.ts +++ b/apps/web/src/canvas/skia/__tests__/skia-interaction.test.ts @@ -312,7 +312,12 @@ describe('SkiaInteractionManager continuous interaction commits', () => { expect(useCanvasStore.getState().selection.activeId).toBe('child-1'); }); - it('moves clip rects together with dragged render nodes', () => { + it('drag preview moves the dragged node bounds but leaves ancestor clipStack untouched', () => { + // clipStack entries are ANCESTOR clips, not the dragged node's own + // bounds. Translating them along with the drag would make the visible + // clip rectangle drift on screen even though the actual ancestor frame + // hasn't moved. The post-commit re-flatten will compute the new + // (and possibly newly-clipped) state. const node = { id: 'frame-1', type: 'frame', @@ -347,7 +352,8 @@ describe('SkiaInteractionManager continuous interaction commits', () => { expect(renderNode.absX).toBe(70); expect(renderNode.absY).toBe(75); - expect(renderNode.clipStack).toEqual([{ x: 65, y: 70, w: 210, h: 130, rx: 8 }]); + // clipStack entries (ancestor clips) stay as snapshot + expect(renderNode.clipStack).toEqual([{ x: 45, y: 55, w: 210, h: 130, rx: 8 }]); expect(engine.rebuildCount).toBeGreaterThan(0); expect(engine.dirtyCount).toBeGreaterThan(0); }); diff --git a/apps/web/src/canvas/skia/skia-interaction.ts b/apps/web/src/canvas/skia/skia-interaction.ts index c27581830..24cb34821 100644 --- a/apps/web/src/canvas/skia/skia-interaction.ts +++ b/apps/web/src/canvas/skia/skia-interaction.ts @@ -649,15 +649,13 @@ export class SkiaInteractionManager { rootRn.absY = nextRootRect.y; rootRn.absW = nextRootRect.w; rootRn.absH = nextRootRect.h; + // clipStack carries ANCESTOR clips, not this node's own bounds. Resizing + // this node doesn't move its ancestors, so the snapshot is still + // accurate — restore unchanged. (If this node has `clipContent: true`, + // that's a clip it pushes for its CHILDREN, lives in children's + // clipStack — not its own.) rootRn.clipStack = rootSnapshot.clipStack - ? rootSnapshot.clipStack.map((clip) => ({ - ...this.scalePreviewRect( - { x: clip.x, y: clip.y, w: clip.w, h: clip.h }, - sourceRect, - nextRootRect, - ), - rx: clip.rx * Math.min(scaleX, scaleY), - })) + ? rootSnapshot.clipStack.map((c) => ({ ...c })) : undefined; for (const [id, snapshot] of previewNodes) { @@ -674,16 +672,9 @@ export class SkiaInteractionManager { rn.absY = scaled.y; rn.absW = scaled.w; rn.absH = scaled.h; - rn.clipStack = snapshot.clipStack - ? snapshot.clipStack.map((clip) => ({ - ...this.scalePreviewRect( - { x: clip.x, y: clip.y, w: clip.w, h: clip.h }, - sourceRect, - nextRootRect, - ), - rx: clip.rx * Math.min(scaleX, scaleY), - })) - : undefined; + // Same as above — preview children's clipStack entries are inherited + // from ancestors that aren't moving. Restore unchanged. + rn.clipStack = snapshot.clipStack ? snapshot.clipStack.map((c) => ({ ...c })) : undefined; } this.resizeLatestPatch = updates as Partial; @@ -744,17 +735,9 @@ export class SkiaInteractionManager { rn.absY = rotated.y; rn.absW = rotated.w; rn.absH = rotated.h; - rn.clipStack = snapshot.clipStack - ? snapshot.clipStack.map((clip) => ({ - ...this.rotatePreviewRect( - { x: clip.x, y: clip.y, w: clip.w, h: clip.h }, - centerX, - centerY, - angleDelta, - ), - rx: clip.rx, - })) - : undefined; + // Rotation only affects the target subtree's bounds; ancestor clips + // are unchanged. Restore the snapshot stack unrotated. + rn.clipStack = snapshot.clipStack ? snapshot.clipStack.map((c) => ({ ...c })) : undefined; } this.rotateLatestAngle = newAngle; @@ -908,13 +891,9 @@ export class SkiaInteractionManager { if (this.dragAllIds!.has(rn.node.id)) { rn.absX += incrDx; rn.absY += incrDy; - if (rn.clipStack && rn.clipStack.length > 0) { - rn.clipStack = rn.clipStack.map((c) => ({ - ...c, - x: c.x + incrDx, - y: c.y + incrDy, - })); - } + // Drag only moves this node's bounds — its clipStack entries are + // ancestor clips that didn't move. Translating them would make the + // visible clip drift away from the actual ancestor on screen. rn.node = { ...rn.node, x: rn.absX, y: rn.absY }; } }