From 85ea84635a70c30c636ea9e6f8a2f86442e7a339 Mon Sep 17 00:00:00 2001 From: bymyself Date: Mon, 4 May 2026 00:25:46 -0700 Subject: [PATCH] fix: refresh DomWidget pos when canvas viewport changes The previous equality-check optimization in updateWidgets compared only canvas-space pos, which is unchanged when the user pans or zooms. Because lgCanvas.ds.offset and ds.scale are not reactive, screen-space style only refreshes via the downstream pos/size watcher in DomWidget. With the equality check, no reassignment fired and DOM widgets stayed at their stale screen position while the canvas moved underneath them, intercepting clicks intended for canvas-rendered controls (collapse button, selection toolbox, etc.). Track the viewport between frames and force a pos reassignment when ds.offset or ds.scale changes. Idle frames (stationary canvas and nodes) still skip reassignment, preserving the perf gain. --- src/components/graph/DomWidgets.test.ts | 61 ++++++++++++++++++++++++- src/components/graph/DomWidgets.vue | 26 ++++++++++- 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/components/graph/DomWidgets.test.ts b/src/components/graph/DomWidgets.test.ts index 4d400d326a..87024b18cb 100644 --- a/src/components/graph/DomWidgets.test.ts +++ b/src/components/graph/DomWidgets.test.ts @@ -49,7 +49,8 @@ function createCanvas(graph: LGraph): LGraphCanvas { graph, low_quality: false, read_only: false, - isNodeVisible: vi.fn(() => true) + isNodeVisible: vi.fn(() => true), + ds: { offset: [0, 0], scale: 1 } }) } @@ -148,6 +149,64 @@ describe('DomWidgets transition grace characterization', () => { expect(widgetState.pos).toEqual([310, 428]) }) + it('forces pos reassignment on viewport pan even when canvas-space pos is unchanged', () => { + const canvasStore = useCanvasStore() + const domWidgetStore = useDomWidgetStore() + + const graph = new LGraph() + const node = createNode(graph, 1, 'node', [100, 200]) + const widget = createWidget('viewport-widget', node, 12) + domWidgetStore.registerWidget(widget) + + const canvas = createCanvas(graph) + canvasStore.canvas = canvas + + render(DomWidgets, { + global: { stubs: { DomWidget: true } } + }) + + drawFrame(canvas) + const widgetState = domWidgetStore.widgetStates.get(widget.id) + if (!widgetState) throw new Error('Widget state not registered') + const posAfterFirstFrame = widgetState.pos + expect(posAfterFirstFrame).toEqual([110, 222]) + + // Canvas pan: ds.offset is non-reactive, so the downstream watcher only + // fires if widgetState.pos is reassigned (a new array identity). + canvas.ds.offset[0] = 50 + canvas.ds.offset[1] = 60 + drawFrame(canvas) + + expect(widgetState.pos).not.toBe(posAfterFirstFrame) + }) + + it('skips pos reassignment when viewport and canvas-space pos are both stable', () => { + const canvasStore = useCanvasStore() + const domWidgetStore = useDomWidgetStore() + + const graph = new LGraph() + const node = createNode(graph, 1, 'node', [100, 200]) + const widget = createWidget('idle-widget', node, 12) + domWidgetStore.registerWidget(widget) + + const canvas = createCanvas(graph) + canvasStore.canvas = canvas + + render(DomWidgets, { + global: { stubs: { DomWidget: true } } + }) + + drawFrame(canvas) + const widgetState = domWidgetStore.widgetStates.get(widget.id) + if (!widgetState) throw new Error('Widget state not registered') + const posAfterFirstFrame = widgetState.pos + + // No pan, no node movement — pos array identity must be preserved + // (this is the perf optimization being protected). + drawFrame(canvas) + expect(widgetState.pos).toBe(posAfterFirstFrame) + }) + it('cleans orphaned transition-grace ids after widget removal', () => { const canvasStore = useCanvasStore() const domWidgetStore = useDomWidgetStore() diff --git a/src/components/graph/DomWidgets.vue b/src/components/graph/DomWidgets.vue index 7633b19af0..6f11944117 100644 --- a/src/components/graph/DomWidgets.vue +++ b/src/components/graph/DomWidgets.vue @@ -25,6 +25,15 @@ const overrideTransitionGrace = new Set() const widgetStates = computed(() => [...domWidgetStore.widgetStates.values()]) +// Track canvas viewport between frames. Screen-space position depends on +// lgCanvas.ds.offset and ds.scale, which are non-reactive. When the user +// pans or zooms, canvas-space `pos` is unchanged but the rendered style +// must update — force pos reassignment whenever the viewport changes so +// the downstream watcher in DomWidget recomputes style with current ds. +let lastViewportOffsetX = Number.NaN +let lastViewportOffsetY = Number.NaN +let lastViewportScale = Number.NaN + const updateWidgets = () => { const lgCanvas = canvasStore.canvas if (!lgCanvas) return @@ -33,6 +42,17 @@ const updateWidgets = () => { const currentGraph = lgCanvas.graph const seenWidgetIds = new Set() + const viewportOffsetX = lgCanvas.ds.offset[0] + const viewportOffsetY = lgCanvas.ds.offset[1] + const viewportScale = lgCanvas.ds.scale + const viewportChanged = + lastViewportOffsetX !== viewportOffsetX || + lastViewportOffsetY !== viewportOffsetY || + lastViewportScale !== viewportScale + lastViewportOffsetX = viewportOffsetX + lastViewportOffsetY = viewportOffsetY + lastViewportScale = viewportScale + for (const widgetState of widgetStates.value) { const widget = widgetState.widget seenWidgetIds.add(widget.id) @@ -85,7 +105,11 @@ const updateWidgets = () => { const margin = widget.margin const newPosX = posNode.pos[0] + margin const newPosY = posNode.pos[1] + margin + posWidget.y - if (widgetState.pos[0] !== newPosX || widgetState.pos[1] !== newPosY) { + if ( + viewportChanged || + widgetState.pos[0] !== newPosX || + widgetState.pos[1] !== newPosY + ) { widgetState.pos = [newPosX, newPosY] }