mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-14 01:36:14 +00:00
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.
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -25,6 +25,15 @@ const overrideTransitionGrace = new Set<string>()
|
||||
|
||||
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<string>()
|
||||
|
||||
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]
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user