Refactor vue slot tracking (#5463)

* add dom element resize observer registry for vue node components

* Update src/renderer/extensions/vueNodes/composables/useVueNodeResizeTracking.ts

Co-authored-by: AustinMroz <austin@comfy.org>

* refactor(vue-nodes): typed TransformState InjectionKey, safer ResizeObserver sizing, centralized slot tracking, and small readability updates

* chore: make TransformState interface non-exported to satisfy knip pre-push

* Revert "chore: make TransformState interface non-exported to satisfy knip pre-push"

This reverts commit 110ecf31da.

* Revert "refactor(vue-nodes): typed TransformState InjectionKey, safer ResizeObserver sizing, centralized slot tracking, and small readability updates"

This reverts commit 428752619c.

* [refactor] Improve resize tracking composable documentation and test utilities

- Rename parameters in useVueElementTracking for clarity (appIdentifier, trackingType)
- Add comprehensive docstring with examples to prevent DOM attribute confusion
- Extract mountLGraphNode test utility to eliminate repetitive mock setup
- Add technical implementation notes documenting optimization decisions

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* remove typo comment

* convert to functional bounds collection

* remove inline import

* add interfaces for bounds mutations

* remove change log

* fix bounds collection when vue nodes turned off

* fix title offset on y

* move from resize observer to selection toolbox bounds

* refactor(vue-nodes): typed TransformState InjectionKey, safer ResizeObserver sizing, centralized slot tracking, and small readability updates

* Fix conversion

* Readd padding

* revert churn reducings from layoutStore.ts

* Rely on RO for resize, and batch

* Improve churn

* Cache canvas offset

* rename from measure

* remove unused

* address review comments

* Update legacy injection

* nit

* Split into store

* nit

* perf improvement

---------

Co-authored-by: bymyself <cbyrne@comfy.org>
Co-authored-by: AustinMroz <austin@comfy.org>
Co-authored-by: Claude <noreply@anthropic.com>

src/renderer/extensions/vueNodes/components/InputOutputSlot.test.ts

@@ -4,15 +4,18 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { createI18n } from 'vue-i18n'
 
 import enMessages from '@/locales/en/main.json'
-import { useDomSlotRegistration } from '@/renderer/core/layout/slots/useDomSlotRegistration'
+import { useSlotElementTracking } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'
 
 import InputSlot from './InputSlot.vue'
 import OutputSlot from './OutputSlot.vue'
 
 // Mock composable used by InputSlot/OutputSlot so we can assert call params
-vi.mock('@/renderer/core/layout/slots/useDomSlotRegistration', () => ({
-  useDomSlotRegistration: vi.fn(() => ({ remeasure: vi.fn() }))
-}))
+vi.mock(
+  '@/renderer/extensions/vueNodes/composables/useSlotElementTracking',
+  () => ({
+    useSlotElementTracking: vi.fn(() => ({ stop: vi.fn() }))
+  })
+)
 
 type InputSlotProps = ComponentMountingOptions<typeof InputSlot>['props']
 type OutputSlotProps = ComponentMountingOptions<typeof OutputSlot>['props']
@@ -49,7 +52,7 @@ const mountOutputSlot = (props: OutputSlotProps) =>
 
 describe('InputSlot/OutputSlot', () => {
   beforeEach(() => {
-    vi.mocked(useDomSlotRegistration).mockClear()
+    vi.mocked(useSlotElementTracking).mockClear()
   })
 
   it('InputSlot registers with correct options', () => {
@@ -59,11 +62,11 @@ describe('InputSlot/OutputSlot', () => {
       slotData: { name: 'A', type: 'any', boundingRect: [0, 0, 0, 0] }
     })
 
-    expect(useDomSlotRegistration).toHaveBeenLastCalledWith(
+    expect(useSlotElementTracking).toHaveBeenLastCalledWith(
       expect.objectContaining({
         nodeId: 'node-1',
-        slotIndex: 3,
-        isInput: true
+        index: 3,
+        type: 'input'
       })
     )
   })
@@ -75,11 +78,11 @@ describe('InputSlot/OutputSlot', () => {
       slotData: { name: 'B', type: 'any', boundingRect: [0, 0, 0, 0] }
     })
 
-    expect(useDomSlotRegistration).toHaveBeenLastCalledWith(
+    expect(useSlotElementTracking).toHaveBeenLastCalledWith(
       expect.objectContaining({
         nodeId: 'node-2',
-        slotIndex: 1,
-        isInput: false
+        index: 1,
+        type: 'output'
       })
     )
   })

src/renderer/extensions/vueNodes/components/InputSlot.vue

@@ -32,7 +32,6 @@
 import {
   type ComponentPublicInstance,
   computed,
-  inject,
   onErrorCaptured,
   ref,
   watchEffect
@@ -41,11 +40,7 @@ import {
 import { useErrorHandling } from '@/composables/useErrorHandling'
 import { getSlotColor } from '@/constants/slotColors'
 import { INodeSlot, LGraphNode } from '@/lib/litegraph/src/litegraph'
-// DOM-based slot registration for arbitrary positioning
-import {
-  type TransformState,
-  useDomSlotRegistration
-} from '@/renderer/core/layout/slots/useDomSlotRegistration'
+import { useSlotElementTracking } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'
 
 import SlotConnectionDot from './SlotConnectionDot.vue'
 
@@ -75,11 +70,6 @@ onErrorCaptured((error) => {
 // Get slot color based on type
 const slotColor = computed(() => getSlotColor(props.slotData.type))
 
-const transformState = inject<TransformState | undefined>(
-  'transformState',
-  undefined
-)
-
 const connectionDotRef = ref<ComponentPublicInstance<{
   slotElRef: HTMLElement | undefined
 }> | null>(null)
@@ -92,11 +82,10 @@ watchEffect(() => {
   slotElRef.value = el || null
 })
 
-useDomSlotRegistration({
+useSlotElementTracking({
   nodeId: props.nodeId ?? '',
-  slotIndex: props.index,
-  isInput: true,
-  element: slotElRef,
-  transform: transformState
+  index: props.index,
+  type: 'input',
+  element: slotElRef
 })
 </script>

src/renderer/extensions/vueNodes/components/LGraphNode.vue

@@ -147,6 +147,7 @@ import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
 import { useErrorHandling } from '@/composables/useErrorHandling'
 import { LiteGraph } from '@/lib/litegraph/src/litegraph'
 import { SelectedNodeIdsKey } from '@/renderer/core/canvas/injectionKeys'
+import { TransformStateKey } from '@/renderer/core/layout/injectionKeys'
 import { useNodeExecutionState } from '@/renderer/extensions/vueNodes/execution/useNodeExecutionState'
 import { useNodeLayout } from '@/renderer/extensions/vueNodes/layout/useNodeLayout'
 import { LODLevel, useLOD } from '@/renderer/extensions/vueNodes/lod/useLOD'
@@ -212,19 +213,7 @@ if (!selectedNodeIds) {
 }
 
 // Inject transform state for coordinate conversion
-const transformState = inject('transformState') as
-  | {
-      camera: { z: number }
-      canvasToScreen: (point: { x: number; y: number }) => {
-        x: number
-        y: number
-      }
-      screenToCanvas: (point: { x: number; y: number }) => {
-        x: number
-        y: number
-      }
-    }
-  | undefined
+const transformState = inject(TransformStateKey)
 
 // Computed selection state - only this node re-evaluates when its selection changes
 const isSelected = computed(() => {
@@ -281,7 +270,7 @@ const {
 } = useNodeLayout(nodeData.id)
 
 onMounted(() => {
-  if (size && transformState) {
+  if (size && transformState?.camera) {
     const scale = transformState.camera.z
     const screenSize = {
       width: size.width * scale,

src/renderer/extensions/vueNodes/components/OutputSlot.vue

@@ -33,7 +33,6 @@
 import {
   type ComponentPublicInstance,
   computed,
-  inject,
   onErrorCaptured,
   ref,
   watchEffect
@@ -42,11 +41,7 @@ import {
 import { useErrorHandling } from '@/composables/useErrorHandling'
 import { getSlotColor } from '@/constants/slotColors'
 import type { INodeSlot, LGraphNode } from '@/lib/litegraph/src/litegraph'
-// DOM-based slot registration for arbitrary positioning
-import {
-  type TransformState,
-  useDomSlotRegistration
-} from '@/renderer/core/layout/slots/useDomSlotRegistration'
+import { useSlotElementTracking } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'
 
 import SlotConnectionDot from './SlotConnectionDot.vue'
 
@@ -77,11 +72,6 @@ onErrorCaptured((error) => {
 // Get slot color based on type
 const slotColor = computed(() => getSlotColor(props.slotData.type))
 
-const transformState = inject<TransformState | undefined>(
-  'transformState',
-  undefined
-)
-
 const connectionDotRef = ref<ComponentPublicInstance<{
   slotElRef: HTMLElement | undefined
 }> | null>(null)
@@ -94,11 +84,10 @@ watchEffect(() => {
   slotElRef.value = el || null
 })
 
-useDomSlotRegistration({
+useSlotElementTracking({
   nodeId: props.nodeId ?? '',
-  slotIndex: props.index,
-  isInput: false,
-  element: slotElRef,
-  transform: transformState
+  index: props.index,
+  type: 'output',
+  element: slotElRef
 })
 </script>

src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts

@@ -0,0 +1,220 @@
+/**
+ * Centralized Slot Element Tracking
+ *
+ * Registers slot connector DOM elements per node, measures their canvas-space
+ * positions in a single batched pass, and caches offsets so that node moves
+ * update slot positions without DOM reads.
+ */
+import { type Ref, onMounted, onUnmounted, watch } from 'vue'
+
+import { useSharedCanvasPositionConversion } from '@/composables/element/useCanvasPositionConversion'
+import { LiteGraph } from '@/lib/litegraph/src/litegraph'
+import { getSlotKey } from '@/renderer/core/layout/slots/slotIdentifier'
+import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
+import type { SlotLayout } from '@/renderer/core/layout/types'
+import {
+  isPointEqual,
+  isSizeEqual
+} from '@/renderer/core/layout/utils/geometry'
+import { useNodeSlotRegistryStore } from '@/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore'
+
+// RAF batching
+const pendingNodes = new Set<string>()
+let rafId: number | null = null
+
+function scheduleSlotLayoutSync(nodeId: string) {
+  pendingNodes.add(nodeId)
+  if (rafId == null) {
+    rafId = requestAnimationFrame(() => {
+      rafId = null
+      flushScheduledSlotLayoutSync()
+    })
+  }
+}
+
+function flushScheduledSlotLayoutSync() {
+  if (pendingNodes.size === 0) return
+  const conv = useSharedCanvasPositionConversion()
+  for (const nodeId of Array.from(pendingNodes)) {
+    pendingNodes.delete(nodeId)
+    syncNodeSlotLayoutsFromDOM(nodeId, conv)
+  }
+}
+
+export function syncNodeSlotLayoutsFromDOM(
+  nodeId: string,
+  conv?: ReturnType<typeof useSharedCanvasPositionConversion>
+) {
+  const nodeSlotRegistryStore = useNodeSlotRegistryStore()
+  const node = nodeSlotRegistryStore.getNode(nodeId)
+  if (!node) return
+  const nodeLayout = layoutStore.getNodeLayoutRef(nodeId).value
+  if (!nodeLayout) return
+
+  const batch: Array<{ key: string; layout: SlotLayout }> = []
+
+  for (const [slotKey, entry] of node.slots) {
+    const rect = entry.el.getBoundingClientRect()
+    const screenCenter: [number, number] = [
+      rect.left + rect.width / 2,
+      rect.top + rect.height / 2
+    ]
+    const [x, y] = (
+      conv ?? useSharedCanvasPositionConversion()
+    ).clientPosToCanvasPos(screenCenter)
+    const centerCanvas = { x, y }
+
+    // Cache offset relative to node position for fast updates later
+    entry.cachedOffset = {
+      x: centerCanvas.x - nodeLayout.position.x,
+      y: centerCanvas.y - nodeLayout.position.y
+    }
+
+    // Persist layout in canvas coordinates
+    const size = LiteGraph.NODE_SLOT_HEIGHT
+    const half = size / 2
+    batch.push({
+      key: slotKey,
+      layout: {
+        nodeId,
+        index: entry.index,
+        type: entry.type,
+        position: { x: centerCanvas.x, y: centerCanvas.y },
+        bounds: {
+          x: centerCanvas.x - half,
+          y: centerCanvas.y - half,
+          width: size,
+          height: size
+        }
+      }
+    })
+  }
+  if (batch.length) layoutStore.batchUpdateSlotLayouts(batch)
+}
+
+function updateNodeSlotsFromCache(nodeId: string) {
+  const nodeSlotRegistryStore = useNodeSlotRegistryStore()
+  const node = nodeSlotRegistryStore.getNode(nodeId)
+  if (!node) return
+  const nodeLayout = layoutStore.getNodeLayoutRef(nodeId).value
+  if (!nodeLayout) return
+
+  const batch: Array<{ key: string; layout: SlotLayout }> = []
+
+  for (const [slotKey, entry] of node.slots) {
+    if (!entry.cachedOffset) {
+      // schedule a sync to seed offset
+      scheduleSlotLayoutSync(nodeId)
+      continue
+    }
+
+    const centerCanvas = {
+      x: nodeLayout.position.x + entry.cachedOffset.x,
+      y: nodeLayout.position.y + entry.cachedOffset.y
+    }
+    const size = LiteGraph.NODE_SLOT_HEIGHT
+    const half = size / 2
+    batch.push({
+      key: slotKey,
+      layout: {
+        nodeId,
+        index: entry.index,
+        type: entry.type,
+        position: { x: centerCanvas.x, y: centerCanvas.y },
+        bounds: {
+          x: centerCanvas.x - half,
+          y: centerCanvas.y - half,
+          width: size,
+          height: size
+        }
+      }
+    })
+  }
+
+  if (batch.length) layoutStore.batchUpdateSlotLayouts(batch)
+}
+
+export function useSlotElementTracking(options: {
+  nodeId: string
+  index: number
+  type: 'input' | 'output'
+  element: Ref<HTMLElement | null>
+}) {
+  const { nodeId, index, type, element } = options
+  const nodeSlotRegistryStore = useNodeSlotRegistryStore()
+
+  onMounted(() => {
+    if (!nodeId) return
+    const stop = watch(
+      element,
+      (el) => {
+        if (!el) return
+
+        // Ensure node entry
+        const node = nodeSlotRegistryStore.ensureNode(nodeId)
+
+        if (!node.stopWatch) {
+          const layoutRef = layoutStore.getNodeLayoutRef(nodeId)
+
+          const stopPositionWatch = watch(
+            () => layoutRef.value?.position,
+            (newPosition, oldPosition) => {
+              if (!newPosition) return
+              if (!oldPosition || !isPointEqual(newPosition, oldPosition)) {
+                updateNodeSlotsFromCache(nodeId)
+              }
+            }
+          )
+
+          const stopSizeWatch = watch(
+            () => layoutRef.value?.size,
+            (newSize, oldSize) => {
+              if (!newSize) return
+              if (!oldSize || !isSizeEqual(newSize, oldSize)) {
+                scheduleSlotLayoutSync(nodeId)
+              }
+            }
+          )
+
+          node.stopWatch = () => {
+            stopPositionWatch()
+            stopSizeWatch()
+          }
+        }
+
+        // Register slot
+        const slotKey = getSlotKey(nodeId, index, type === 'input')
+        node.slots.set(slotKey, { el, index, type })
+
+        // Seed initial sync from DOM
+        scheduleSlotLayoutSync(nodeId)
+
+        // Stop watching once registered
+        stop()
+      },
+      { immediate: true, flush: 'post' }
+    )
+  })
+
+  onUnmounted(() => {
+    if (!nodeId) return
+    const node = nodeSlotRegistryStore.getNode(nodeId)
+    if (!node) return
+
+    // Remove this slot from registry and layout
+    const slotKey = getSlotKey(nodeId, index, type === 'input')
+    node.slots.delete(slotKey)
+    layoutStore.deleteSlotLayout(slotKey)
+
+    // If node has no more slots, clean up
+    if (node.slots.size === 0) {
+      // Stop the node-level watcher when the last slot is gone
+      if (node.stopWatch) node.stopWatch()
+      nodeSlotRegistryStore.deleteNode(nodeId)
+    }
+  })
+
+  return {
+    requestSlotLayoutSync: () => scheduleSlotLayoutSync(nodeId)
+  }
+}

src/renderer/extensions/vueNodes/composables/useVueNodeResizeTracking.ts

@@ -10,9 +10,13 @@
  */
 import { getCurrentInstance, onMounted, onUnmounted } from 'vue'
 
+import { useSharedCanvasPositionConversion } from '@/composables/element/useCanvasPositionConversion'
+import { LiteGraph } from '@/lib/litegraph/src/litegraph'
 import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
 import type { Bounds, NodeId } from '@/renderer/core/layout/types'
 
+import { syncNodeSlotLayoutsFromDOM } from './useSlotElementTracking'
+
 /**
  * Generic update item for element bounds tracking
  */
@@ -54,8 +58,12 @@ const trackingConfigs: Map<string, ElementTrackingConfig> = new Map([
 
 // Single ResizeObserver instance for all Vue elements
 const resizeObserver = new ResizeObserver((entries) => {
-  // Group updates by element type
+  // Canvas is ready when this code runs; no defensive guards needed.
+  const conv = useSharedCanvasPositionConversion()
+  // Group updates by type, then flush via each config's handler
   const updatesByType = new Map<string, ElementBoundsUpdate[]>()
+  // Track nodes whose slots should be resynced after node size changes
+  const nodesNeedingSlotResync = new Set<string>()
 
   for (const entry of entries) {
     if (!(entry.target instanceof HTMLElement)) continue
@@ -76,30 +84,50 @@ const resizeObserver = new ResizeObserver((entries) => {
 
     if (!elementType || !elementId) continue
 
-    const { inlineSize: width, blockSize: height } = entry.contentBoxSize[0]
+    // Use contentBoxSize when available; fall back to contentRect for older engines/tests
+    const contentBox = Array.isArray(entry.contentBoxSize)
+      ? entry.contentBoxSize[0]
+      : {
+          inlineSize: entry.contentRect.width,
+          blockSize: entry.contentRect.height
+        }
+    const width = contentBox.inlineSize
+    const height = contentBox.blockSize
+
+    // Screen-space rect
     const rect = element.getBoundingClientRect()
-
+    const [cx, cy] = conv.clientPosToCanvasPos([rect.left, rect.top])
+    const topLeftCanvas = { x: cx, y: cy }
     const bounds: Bounds = {
-      x: rect.left,
-      y: rect.top,
-      width,
-      height: height
+      x: topLeftCanvas.x,
+      y: topLeftCanvas.y + LiteGraph.NODE_TITLE_HEIGHT,
+      width: Math.max(0, width),
+      height: Math.max(0, height - LiteGraph.NODE_TITLE_HEIGHT)
     }
 
-    if (!updatesByType.has(elementType)) {
-      updatesByType.set(elementType, [])
+    let updates = updatesByType.get(elementType)
+    if (!updates) {
+      updates = []
+      updatesByType.set(elementType, updates)
     }
-    const updates = updatesByType.get(elementType)
-    if (updates) {
-      updates.push({ id: elementId, bounds })
+    updates.push({ id: elementId, bounds })
+
+    // If this entry is a node, mark it for slot layout resync
+    if (elementType === 'node' && elementId) {
+      nodesNeedingSlotResync.add(elementId)
     }
   }
 
-  // Process updates by type
+  // Flush per-type
   for (const [type, updates] of updatesByType) {
     const config = trackingConfigs.get(type)
-    if (config && updates.length > 0) {
-      config.updateHandler(updates)
+    if (config && updates.length) config.updateHandler(updates)
+  }
+
+  // After node bounds are updated, refresh slot cached offsets and layouts
+  if (nodesNeedingSlotResync.size > 0) {
+    for (const nodeId of nodesNeedingSlotResync) {
+      syncNodeSlotLayoutsFromDOM(nodeId)
     }
   }
 })
@@ -134,11 +162,11 @@ export function useVueElementTracking(
     if (!(element instanceof HTMLElement) || !appIdentifier) return
 
     const config = trackingConfigs.get(trackingType)
-    if (config) {
-      // Set the appropriate data attribute
-      element.dataset[config.dataAttribute] = appIdentifier
-      resizeObserver.observe(element)
-    }
+    if (!config) return
+
+    // Set the data attribute expected by the RO pipeline for this type
+    element.dataset[config.dataAttribute] = appIdentifier
+    resizeObserver.observe(element)
   })
 
   onUnmounted(() => {
@@ -146,10 +174,10 @@ export function useVueElementTracking(
     if (!(element instanceof HTMLElement)) return
 
     const config = trackingConfigs.get(trackingType)
-    if (config) {
-      // Remove the data attribute
-      delete element.dataset[config.dataAttribute]
-      resizeObserver.unobserve(element)
-    }
+    if (!config) return
+
+    // Remove the data attribute and observer
+    delete element.dataset[config.dataAttribute]
+    resizeObserver.unobserve(element)
   })
 }

src/renderer/extensions/vueNodes/layout/useNodeLayout.ts

@@ -7,6 +7,7 @@
 import { computed, inject } from 'vue'
 
 import { SelectedNodeIdsKey } from '@/renderer/core/canvas/injectionKeys'
+import { TransformStateKey } from '@/renderer/core/layout/injectionKeys'
 import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations'
 import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
 import { LayoutSource, type Point } from '@/renderer/core/layout/types'
@@ -20,12 +21,7 @@ export function useNodeLayout(nodeId: string) {
   const mutations = useLayoutMutations()
 
   // Get transform utilities from TransformPane if available
-  const transformState = inject('transformState') as
-    | {
-        canvasToScreen: (point: Point) => Point
-        screenToCanvas: (point: Point) => Point
-      }
-    | undefined
+  const transformState = inject(TransformStateKey)
 
   // Get the customRef for this node (shared write access)
   const layoutRef = store.getNodeLayoutRef(nodeId)

src/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore.ts

@@ -0,0 +1,50 @@
+import { defineStore } from 'pinia'
+import { markRaw } from 'vue'
+
+type SlotEntry = {
+  el: HTMLElement
+  index: number
+  type: 'input' | 'output'
+  cachedOffset?: { x: number; y: number }
+}
+
+type NodeEntry = {
+  nodeId: string
+  slots: Map<string, SlotEntry>
+  stopWatch?: () => void
+}
+
+export const useNodeSlotRegistryStore = defineStore('nodeSlotRegistry', () => {
+  const registry = markRaw(new Map<string, NodeEntry>())
+
+  function getNode(nodeId: string) {
+    return registry.get(nodeId)
+  }
+
+  function ensureNode(nodeId: string) {
+    let node = registry.get(nodeId)
+    if (!node) {
+      node = {
+        nodeId,
+        slots: markRaw(new Map<string, SlotEntry>())
+      }
+      registry.set(nodeId, node)
+    }
+    return node
+  }
+
+  function deleteNode(nodeId: string) {
+    registry.delete(nodeId)
+  }
+
+  function clear() {
+    registry.clear()
+  }
+
+  return {
+    getNode,
+    ensureNode,
+    deleteNode,
+    clear
+  }
+})