mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-05 13:10:24 +00:00
fix: emit layout change for batch node bounds (#5939)
## Summary Fixes issue where node size changes are not serialized by routing DOM-driven node bounds updates through a single CRDT operation so Vue node geometry stays synchronized with LiteGraph. ## Changes - **What**: Added `BatchUpdateBoundsOperation` to the layout store, applied it via the existing Yjs pipeline, notified link sync to recompute touched nodes, and covered the path with a regression test ## Review Focus Correctness of the new batch operation when multiple nodes update simultaneously, especially remote replay/undo scenarios and link geometry recomputation. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5939-fix-emit-layout-change-for-batch-node-bounds-2846d73d365081db8f8cca5bf7b85308) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com>
This commit is contained in:
@@ -12,6 +12,7 @@ import * as Y from 'yjs'
|
||||
import { ACTOR_CONFIG } from '@/renderer/core/layout/constants'
|
||||
import { LayoutSource } from '@/renderer/core/layout/types'
|
||||
import type {
|
||||
BatchUpdateBoundsOperation,
|
||||
Bounds,
|
||||
CreateLinkOperation,
|
||||
CreateNodeOperation,
|
||||
@@ -871,6 +872,12 @@ class LayoutStoreImpl implements LayoutStore {
|
||||
case 'deleteNode':
|
||||
this.handleDeleteNode(operation as DeleteNodeOperation, change)
|
||||
break
|
||||
case 'batchUpdateBounds':
|
||||
this.handleBatchUpdateBounds(
|
||||
operation as BatchUpdateBoundsOperation,
|
||||
change
|
||||
)
|
||||
break
|
||||
case 'createLink':
|
||||
this.handleCreateLink(operation as CreateLinkOperation, change)
|
||||
break
|
||||
@@ -1099,6 +1106,38 @@ class LayoutStoreImpl implements LayoutStore {
|
||||
change.nodeIds.push(operation.nodeId)
|
||||
}
|
||||
|
||||
private handleBatchUpdateBounds(
|
||||
operation: BatchUpdateBoundsOperation,
|
||||
change: LayoutChange
|
||||
): void {
|
||||
const spatialUpdates: Array<{ nodeId: NodeId; bounds: Bounds }> = []
|
||||
|
||||
for (const nodeId of operation.nodeIds) {
|
||||
const data = operation.bounds[nodeId]
|
||||
const ynode = this.ynodes.get(nodeId)
|
||||
if (!ynode || !data) continue
|
||||
|
||||
ynode.set('position', { x: data.bounds.x, y: data.bounds.y })
|
||||
ynode.set('size', {
|
||||
width: data.bounds.width,
|
||||
height: data.bounds.height
|
||||
})
|
||||
ynode.set('bounds', data.bounds)
|
||||
|
||||
spatialUpdates.push({ nodeId, bounds: data.bounds })
|
||||
change.nodeIds.push(nodeId)
|
||||
}
|
||||
|
||||
// Batch update spatial index for better performance
|
||||
if (spatialUpdates.length > 0) {
|
||||
this.spatialIndex.batchUpdate(spatialUpdates)
|
||||
}
|
||||
|
||||
if (change.nodeIds.length) {
|
||||
change.type = 'update'
|
||||
}
|
||||
}
|
||||
|
||||
private handleCreateLink(
|
||||
operation: CreateLinkOperation,
|
||||
change: LayoutChange
|
||||
@@ -1379,19 +1418,38 @@ class LayoutStoreImpl implements LayoutStore {
|
||||
const originalSource = this.currentSource
|
||||
this.currentSource = LayoutSource.Vue
|
||||
|
||||
this.ydoc.transact(() => {
|
||||
for (const { nodeId, bounds } of updates) {
|
||||
const ynode = this.ynodes.get(nodeId)
|
||||
if (!ynode) continue
|
||||
const nodeIds: NodeId[] = []
|
||||
const boundsRecord: BatchUpdateBoundsOperation['bounds'] = {}
|
||||
|
||||
this.spatialIndex.update(nodeId, bounds)
|
||||
ynode.set('bounds', bounds)
|
||||
ynode.set('position', { x: bounds.x, y: bounds.y })
|
||||
ynode.set('size', { width: bounds.width, height: bounds.height })
|
||||
for (const { nodeId, bounds } of updates) {
|
||||
const ynode = this.ynodes.get(nodeId)
|
||||
if (!ynode) continue
|
||||
const currentLayout = yNodeToLayout(ynode)
|
||||
|
||||
boundsRecord[nodeId] = {
|
||||
bounds,
|
||||
previousBounds: currentLayout.bounds
|
||||
}
|
||||
}, this.currentActor)
|
||||
nodeIds.push(nodeId)
|
||||
}
|
||||
|
||||
if (!nodeIds.length) {
|
||||
this.currentSource = originalSource
|
||||
return
|
||||
}
|
||||
|
||||
const operation: BatchUpdateBoundsOperation = {
|
||||
type: 'batchUpdateBounds',
|
||||
entity: 'node',
|
||||
nodeIds,
|
||||
bounds: boundsRecord,
|
||||
timestamp: Date.now(),
|
||||
source: this.currentSource,
|
||||
actor: this.currentActor
|
||||
}
|
||||
|
||||
this.applyOperation(operation)
|
||||
|
||||
// Restore original source
|
||||
this.currentSource = originalSource
|
||||
}
|
||||
}
|
||||
|
||||
@@ -267,6 +267,11 @@ export function useLinkLayoutSync() {
|
||||
case 'resizeNode':
|
||||
recomputeLinksForNode(parseInt(change.operation.nodeId))
|
||||
break
|
||||
case 'batchUpdateBounds':
|
||||
for (const nodeId of change.operation.nodeIds) {
|
||||
recomputeLinksForNode(parseInt(nodeId))
|
||||
}
|
||||
break
|
||||
case 'createLink':
|
||||
recomputeLinkById(change.operation.linkId)
|
||||
break
|
||||
|
||||
@@ -122,7 +122,7 @@ type OperationType =
|
||||
| 'createNode'
|
||||
| 'deleteNode'
|
||||
| 'setNodeVisibility'
|
||||
| 'batchUpdate'
|
||||
| 'batchUpdateBounds'
|
||||
| 'createLink'
|
||||
| 'deleteLink'
|
||||
| 'createReroute'
|
||||
@@ -184,10 +184,11 @@ interface SetNodeVisibilityOperation extends NodeOpBase {
|
||||
/**
|
||||
* Batch update operation for atomic multi-property changes
|
||||
*/
|
||||
interface BatchUpdateOperation extends NodeOpBase {
|
||||
type: 'batchUpdate'
|
||||
updates: Partial<NodeLayout>
|
||||
previousValues: Partial<NodeLayout>
|
||||
export interface BatchUpdateBoundsOperation extends OperationMeta {
|
||||
entity: 'node'
|
||||
type: 'batchUpdateBounds'
|
||||
nodeIds: NodeId[]
|
||||
bounds: Record<NodeId, { bounds: Bounds; previousBounds: Bounds }>
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -244,7 +245,7 @@ export type LayoutOperation =
|
||||
| CreateNodeOperation
|
||||
| DeleteNodeOperation
|
||||
| SetNodeVisibilityOperation
|
||||
| BatchUpdateOperation
|
||||
| BatchUpdateBoundsOperation
|
||||
| CreateLinkOperation
|
||||
| DeleteLinkOperation
|
||||
| CreateRerouteOperation
|
||||
|
||||
@@ -55,6 +55,17 @@ export class SpatialIndexManager {
|
||||
this.invalidateCache()
|
||||
}
|
||||
|
||||
/**
|
||||
* Batch update multiple nodes' bounds in the spatial index
|
||||
* More efficient than calling update() multiple times as it only invalidates cache once
|
||||
*/
|
||||
batchUpdate(updates: Array<{ nodeId: NodeId; bounds: Bounds }>): void {
|
||||
for (const { nodeId, bounds } of updates) {
|
||||
this.quadTree.update(nodeId, bounds)
|
||||
}
|
||||
this.invalidateCache()
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a node from the spatial index
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user