mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
fix: resync slot layouts when switching between app mode and graph mode (#10273)
## Summary Fix broken link rendering (noodles disappearing or going to wrong positions) when switching between app mode and graph mode tabs. ## Changes - **What**: When the graph canvas is hidden via `display: none` in app mode, slot elements lose valid DOM measurements. On switching back, links rendered at stale coordinates or disappeared. This PR rekeys `LGraphNode` components by workflow path, adds measurability guards to skip hidden slots, clears stale layouts, and watches `linearMode` to trigger a full slot layout resync on mode transitions. ## Review Focus - The `isSlotElementMeasurable` guard skips elements that are disconnected or have zero-size rects — verify this doesn't inadvertently skip slots during normal graph rendering. - The `linearMode` watcher clears all slot layouts when entering app mode and requests a full resync when leaving — confirm no flicker or race with the RAF-based sync scheduler. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10273-fix-resync-slot-layouts-when-switching-between-app-mode-and-graph-mode-3276d73d3650812f9366dae53c7b2d37) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -164,9 +164,11 @@ import { useWorkflowAutoSave } from '@/platform/workflow/persistence/composables
|
||||
import { useWorkflowPersistenceV2 as useWorkflowPersistence } from '@/platform/workflow/persistence/composables/useWorkflowPersistenceV2'
|
||||
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
|
||||
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
import TransformPane from '@/renderer/core/layout/transform/TransformPane.vue'
|
||||
import MiniMap from '@/renderer/extensions/minimap/MiniMap.vue'
|
||||
import LGraphNode from '@/renderer/extensions/vueNodes/components/LGraphNode.vue'
|
||||
import { requestSlotLayoutSyncForAllNodes } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'
|
||||
import { UnauthorizedError } from '@/scripts/api'
|
||||
import { app as comfyApp } from '@/scripts/app'
|
||||
import { ChangeTracker } from '@/scripts/changeTracker'
|
||||
@@ -207,6 +209,7 @@ const workspaceStore = useWorkspaceStore()
|
||||
const { isBuilderMode } = useAppMode()
|
||||
const canvasStore = useCanvasStore()
|
||||
const workflowStore = useWorkflowStore()
|
||||
const { linearMode } = storeToRefs(canvasStore)
|
||||
const executionStore = useExecutionStore()
|
||||
const executionErrorStore = useExecutionErrorStore()
|
||||
const toastStore = useToastStore()
|
||||
@@ -279,6 +282,22 @@ watch(
|
||||
const allNodes = computed((): VueNodeData[] =>
|
||||
Array.from(vueNodeLifecycle.nodeManager.value?.vueNodeData?.values() ?? [])
|
||||
)
|
||||
watch(
|
||||
() => linearMode.value,
|
||||
(isLinearMode) => {
|
||||
if (!shouldRenderVueNodes.value) return
|
||||
|
||||
if (isLinearMode) {
|
||||
layoutStore.clearAllSlotLayouts()
|
||||
} else {
|
||||
// App mode hides the graph canvas with `display: none`, so slot connectors
|
||||
// need a fresh DOM measurement pass before links can render correctly.
|
||||
requestSlotLayoutSyncForAllNodes()
|
||||
}
|
||||
|
||||
layoutStore.setPendingSlotSync(true)
|
||||
}
|
||||
)
|
||||
|
||||
function onLinkOverlayReady(el: HTMLCanvasElement) {
|
||||
if (!canvasStore.canvas) return
|
||||
|
||||
@@ -14,6 +14,7 @@ import { useNodeSlotRegistryStore } from '@/renderer/extensions/vueNodes/stores/
|
||||
import {
|
||||
syncNodeSlotLayoutsFromDOM,
|
||||
flushScheduledSlotLayoutSync,
|
||||
requestSlotLayoutSyncForAllNodes,
|
||||
useSlotElementTracking
|
||||
} from './useSlotElementTracking'
|
||||
|
||||
@@ -55,7 +56,10 @@ function createWrapperComponent(type: 'input' | 'output') {
|
||||
*/
|
||||
async function mountAndRegisterSlot(type: 'input' | 'output') {
|
||||
const wrapper = mount(createWrapperComponent(type))
|
||||
wrapper.vm.el = document.createElement('div')
|
||||
const slotEl = document.createElement('div')
|
||||
slotEl.getBoundingClientRect = vi.fn(() => new DOMRect(100, 200, 16, 16))
|
||||
document.body.append(slotEl)
|
||||
wrapper.vm.el = slotEl
|
||||
await nextTick()
|
||||
flushScheduledSlotLayoutSync()
|
||||
return wrapper
|
||||
@@ -64,6 +68,7 @@ async function mountAndRegisterSlot(type: 'input' | 'output') {
|
||||
describe('useSlotElementTracking', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
document.body.innerHTML = ''
|
||||
layoutStore.initializeFromLiteGraph([])
|
||||
layoutStore.applyOperation({
|
||||
type: 'createNode',
|
||||
@@ -134,9 +139,55 @@ describe('useSlotElementTracking', () => {
|
||||
expect(layoutStore.pendingSlotSync).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps pendingSlotSync when all registered slots are hidden', () => {
|
||||
const slotKey = getSlotKey(NODE_ID, SLOT_INDEX, true)
|
||||
const hiddenSlot = document.createElement('div')
|
||||
|
||||
const registryStore = useNodeSlotRegistryStore()
|
||||
const node = registryStore.ensureNode(NODE_ID)
|
||||
node.slots.set(slotKey, {
|
||||
el: hiddenSlot,
|
||||
index: SLOT_INDEX,
|
||||
type: 'input'
|
||||
})
|
||||
|
||||
layoutStore.setPendingSlotSync(true)
|
||||
requestSlotLayoutSyncForAllNodes()
|
||||
|
||||
expect(layoutStore.pendingSlotSync).toBe(true)
|
||||
expect(layoutStore.getSlotLayout(slotKey)).toBeNull()
|
||||
})
|
||||
|
||||
it('removes stale slot layouts when slot element is hidden', () => {
|
||||
const slotKey = getSlotKey(NODE_ID, SLOT_INDEX, true)
|
||||
const hiddenSlot = document.createElement('div')
|
||||
|
||||
const staleLayout: SlotLayout = {
|
||||
nodeId: NODE_ID,
|
||||
index: SLOT_INDEX,
|
||||
type: 'input',
|
||||
position: { x: 10, y: 20 },
|
||||
bounds: { x: 6, y: 16, width: 8, height: 8 }
|
||||
}
|
||||
layoutStore.batchUpdateSlotLayouts([{ key: slotKey, layout: staleLayout }])
|
||||
|
||||
const registryStore = useNodeSlotRegistryStore()
|
||||
const node = registryStore.ensureNode(NODE_ID)
|
||||
node.slots.set(slotKey, {
|
||||
el: hiddenSlot,
|
||||
index: SLOT_INDEX,
|
||||
type: 'input'
|
||||
})
|
||||
|
||||
syncNodeSlotLayoutsFromDOM(NODE_ID)
|
||||
|
||||
expect(layoutStore.getSlotLayout(slotKey)).toBeNull()
|
||||
})
|
||||
|
||||
it('skips slot layout writeback when measured slot geometry is unchanged', () => {
|
||||
const slotKey = getSlotKey(NODE_ID, SLOT_INDEX, true)
|
||||
const slotEl = document.createElement('div')
|
||||
document.body.append(slotEl)
|
||||
slotEl.getBoundingClientRect = vi.fn(() => new DOMRect(100, 200, 16, 16))
|
||||
|
||||
const registryStore = useNodeSlotRegistryStore()
|
||||
|
||||
@@ -33,6 +33,38 @@ function scheduleSlotLayoutSync(nodeId: string) {
|
||||
raf.schedule()
|
||||
}
|
||||
|
||||
function shouldWaitForSlotLayouts(): boolean {
|
||||
const graph = app.canvas?.graph
|
||||
const hasNodes = Boolean(graph && graph._nodes && graph._nodes.length > 0)
|
||||
return hasNodes && !layoutStore.hasSlotLayouts
|
||||
}
|
||||
|
||||
function completePendingSlotSync(): void {
|
||||
layoutStore.setPendingSlotSync(false)
|
||||
app.canvas?.setDirty(true, true)
|
||||
}
|
||||
|
||||
function getSlotElementRect(el: HTMLElement): DOMRect | null {
|
||||
if (!el.isConnected) return null
|
||||
|
||||
const rect = el.getBoundingClientRect()
|
||||
if (rect.width <= 0 || rect.height <= 0) return null
|
||||
return rect
|
||||
}
|
||||
|
||||
export function requestSlotLayoutSyncForAllNodes(): void {
|
||||
const nodeSlotRegistryStore = useNodeSlotRegistryStore()
|
||||
for (const nodeId of nodeSlotRegistryStore.getNodeIds()) {
|
||||
scheduleSlotLayoutSync(nodeId)
|
||||
}
|
||||
|
||||
// If no slots are currently registered, run the completion check immediately
|
||||
// so pendingSlotSync can be cleared when the graph has no nodes.
|
||||
if (pendingNodes.size === 0) {
|
||||
flushScheduledSlotLayoutSync()
|
||||
}
|
||||
}
|
||||
|
||||
function createSlotLayout(options: {
|
||||
nodeId: string
|
||||
index: number
|
||||
@@ -60,17 +92,14 @@ function createSlotLayout(options: {
|
||||
export function flushScheduledSlotLayoutSync() {
|
||||
if (pendingNodes.size === 0) {
|
||||
// No pending nodes - check if we should wait for Vue components to mount
|
||||
const graph = app.canvas?.graph
|
||||
const hasNodes = graph && graph._nodes && graph._nodes.length > 0
|
||||
if (hasNodes && !layoutStore.hasSlotLayouts) {
|
||||
if (shouldWaitForSlotLayouts()) {
|
||||
// Graph has nodes but no slot layouts yet - Vue hasn't mounted.
|
||||
// Keep flag set so late mounts can re-assert via scheduleSlotLayoutSync()
|
||||
return
|
||||
}
|
||||
// Either no nodes (nothing to wait for) or slot layouts already exist
|
||||
// (undo/redo preserved them). Clear the flag so links can render.
|
||||
layoutStore.setPendingSlotSync(false)
|
||||
app.canvas?.setDirty(true, true)
|
||||
completePendingSlotSync()
|
||||
return
|
||||
}
|
||||
const conv = useSharedCanvasPositionConversion()
|
||||
@@ -78,10 +107,12 @@ export function flushScheduledSlotLayoutSync() {
|
||||
pendingNodes.delete(nodeId)
|
||||
syncNodeSlotLayoutsFromDOM(nodeId, conv)
|
||||
}
|
||||
// Clear the pending sync flag - slots are now synced
|
||||
layoutStore.setPendingSlotSync(false)
|
||||
// Trigger canvas redraw now that links can render with correct positions
|
||||
app.canvas?.setDirty(true, true)
|
||||
|
||||
// Keep pending sync active until at least one measurable slot layout has
|
||||
// been captured for the current graph.
|
||||
if (shouldWaitForSlotLayouts()) return
|
||||
|
||||
completePendingSlotSync()
|
||||
}
|
||||
|
||||
export function syncNodeSlotLayoutsFromDOM(
|
||||
@@ -99,7 +130,14 @@ export function syncNodeSlotLayoutsFromDOM(
|
||||
const positionConv = conv ?? useSharedCanvasPositionConversion()
|
||||
|
||||
for (const [slotKey, entry] of node.slots) {
|
||||
const rect = entry.el.getBoundingClientRect()
|
||||
const rect = getSlotElementRect(entry.el)
|
||||
if (!rect) {
|
||||
// Drop stale layout values while the slot is hidden so we don't render
|
||||
// links with off-screen coordinates from a previous graph/tab state.
|
||||
layoutStore.deleteSlotLayout(slotKey)
|
||||
continue
|
||||
}
|
||||
|
||||
const screenCenter: [number, number] = [
|
||||
rect.left + rect.width / 2,
|
||||
rect.top + rect.height / 2
|
||||
|
||||
@@ -41,10 +41,15 @@ export const useNodeSlotRegistryStore = defineStore('nodeSlotRegistry', () => {
|
||||
registry.clear()
|
||||
}
|
||||
|
||||
function getNodeIds(): string[] {
|
||||
return Array.from(registry.keys())
|
||||
}
|
||||
|
||||
return {
|
||||
getNode,
|
||||
ensureNode,
|
||||
deleteNode,
|
||||
clear
|
||||
clear,
|
||||
getNodeIds
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user