From 1611c7a2241153f77fc4e501ff1e727d82137da4 Mon Sep 17 00:00:00 2001 From: Alexander Brown Date: Mon, 22 Sep 2025 18:47:26 -0700 Subject: [PATCH] Refactor: Further state management cleanup (#5727) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Going through the GraphNodeManager and VueNodeLifecycle one property at a time and removing the pieces that are not currently wired up or used by the rest of the application Fixes paste location by updating the layoutStore in LGraphCanvas (which already mutates layoutStore elsewhere) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5727-WIP-Refactor-Further-state-management-cleanup-2766d73d36508173b379c6009c194a5a) by [Unito](https://www.unito.io) --- src/composables/graph/useGraphNodeManager.ts | 258 +----------------- src/composables/graph/useVueNodeLifecycle.ts | 21 +- src/lib/litegraph/src/LGraphCanvas.ts | 12 + src/lib/litegraph/src/interfaces.ts | 1 + src/renderer/core/layout/store/layoutStore.ts | 1 + .../composables/useNodeEventHandlers.ts | 1 - 6 files changed, 17 insertions(+), 277 deletions(-) diff --git a/src/composables/graph/useGraphNodeManager.ts b/src/composables/graph/useGraphNodeManager.ts index 962f2b460..ea3f61a14 100644 --- a/src/composables/graph/useGraphNodeManager.ts +++ b/src/composables/graph/useGraphNodeManager.ts @@ -2,42 +2,15 @@ * Vue node lifecycle management for LiteGraph integration * Provides event-driven reactivity with performance optimizations */ -import { nextTick, reactive } from 'vue' +import { reactive } from 'vue' import { useChainCallback } from '@/composables/functional/useChainCallback' import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations' import { LayoutSource } from '@/renderer/core/layout/types' -import { type Bounds, QuadTree } from '@/renderer/core/spatial/QuadTree' import type { WidgetValue } from '@/types/simplifiedWidget' -import type { SpatialIndexDebugInfo } from '@/types/spatialIndex' import type { LGraph, LGraphNode } from '../../lib/litegraph/src/litegraph' -export interface NodeState { - visible: boolean - dirty: boolean - lastUpdate: number - culled: boolean -} - -interface NodeMetadata { - lastRenderTime: number - cachedBounds: DOMRect | null - lodLevel: 'high' | 'medium' | 'low' - spatialIndex?: QuadTree -} - -interface PerformanceMetrics { - fps: number - frameTime: number - updateTime: number - nodeCount: number - culledCount: number - callbackUpdateCount: number - rafUpdateCount: number - adaptiveQuality: boolean -} - export interface SafeWidgetData { name: string type: string @@ -63,39 +36,15 @@ export interface VueNodeData { } } -interface SpatialMetrics { - queryTime: number - nodesInIndex: number -} - export interface GraphNodeManager { // Reactive state - safe data extracted from LiteGraph nodes vueNodeData: ReadonlyMap - nodeState: ReadonlyMap // Access to original LiteGraph nodes (non-reactive) getNode(id: string): LGraphNode | undefined // Lifecycle methods - setupEventListeners(): () => void cleanup(): void - - // Update methods - scheduleUpdate( - nodeId?: string, - priority?: 'critical' | 'normal' | 'low' - ): void - forceSync(): void - - // Spatial queries - getVisibleNodeIds(viewportBounds: Bounds): Set - - // Performance - performanceMetrics: PerformanceMetrics - spatialMetrics: SpatialMetrics - - // Debug - getSpatialIndexDebugInfo(): SpatialIndexDebugInfo | null } export function useGraphNodeManager(graph: LGraph): GraphNodeManager { @@ -103,61 +52,10 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { const { createNode, deleteNode, setSource } = useLayoutMutations() // Safe reactive data extracted from LiteGraph nodes const vueNodeData = reactive(new Map()) - const nodeState = reactive(new Map()) // Non-reactive storage for original LiteGraph nodes const nodeRefs = new Map() - // WeakMap for heavy data that auto-GCs when nodes are removed - const nodeMetadata = new WeakMap() - - // Performance tracking - const performanceMetrics = reactive({ - fps: 0, - frameTime: 0, - updateTime: 0, - nodeCount: 0, - culledCount: 0, - callbackUpdateCount: 0, - rafUpdateCount: 0, - adaptiveQuality: false - }) - - // Spatial indexing using QuadTree - const spatialIndex = new QuadTree( - { x: -10000, y: -10000, width: 20000, height: 20000 }, - { maxDepth: 6, maxItemsPerNode: 4 } - ) - let lastSpatialQueryTime = 0 - - // Spatial metrics - const spatialMetrics = reactive({ - queryTime: 0, - nodesInIndex: 0 - }) - - // Update batching - const pendingUpdates = new Set() - const criticalUpdates = new Set() - const lowPriorityUpdates = new Set() - let updateScheduled = false - let batchTimeoutId: number | null = null - - // Change detection state - const lastNodesSnapshot = new Map< - string, - { pos: [number, number]; size: [number, number] } - >() - - const attachMetadata = (node: LGraphNode) => { - nodeMetadata.set(node, { - lastRenderTime: performance.now(), - cachedBounds: null, - lodLevel: 'high', - spatialIndex: undefined - }) - } - // Extract safe data from LiteGraph node for Vue consumption const extractVueNodeData = (node: LGraphNode): VueNodeData => { // Determine subgraph ID - null for root graph, string for subgraphs @@ -278,7 +176,6 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { ...currentData, widgets: updatedWidgets }) - performanceMetrics.callbackUpdateCount++ } catch (error) { // Ignore widget update errors to prevent cascade failures } @@ -348,70 +245,6 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { }) } - // Uncomment when needed for future features - // const getNodeMetadata = (node: LGraphNode): NodeMetadata => { - // let metadata = nodeMetadata.get(node) - // if (!metadata) { - // attachMetadata(node) - // metadata = nodeMetadata.get(node)! - // } - // return metadata - // } - const scheduleUpdate = ( - nodeId?: string, - priority: 'critical' | 'normal' | 'low' = 'normal' - ) => { - if (nodeId) { - const state = nodeState.get(nodeId) - if (state) state.dirty = true - - // Priority queuing - if (priority === 'critical') { - criticalUpdates.add(nodeId) - flush() // Immediate flush for critical updates - return - } else if (priority === 'low') { - lowPriorityUpdates.add(nodeId) - } else { - pendingUpdates.add(nodeId) - } - } - - if (!updateScheduled) { - updateScheduled = true - - // Adaptive batching strategy - if (pendingUpdates.size > 10) { - // Many updates - batch in nextTick - void nextTick(() => flush()) - } else { - // Few updates - small delay for more batching - batchTimeoutId = window.setTimeout(() => flush(), 4) - } - } - } - - const flush = () => { - const startTime = performance.now() - - if (batchTimeoutId !== null) { - clearTimeout(batchTimeoutId) - batchTimeoutId = null - } - - // Clear all pending updates - criticalUpdates.clear() - pendingUpdates.clear() - lowPriorityUpdates.clear() - updateScheduled = false - - // Sync with graph state - syncWithGraph() - - const endTime = performance.now() - performanceMetrics.updateTime = endTime - startTime - } - const syncWithGraph = () => { if (!graph?._nodes) return @@ -422,9 +255,6 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { if (!currentNodes.has(id)) { nodeRefs.delete(id) vueNodeData.delete(id) - nodeState.delete(id) - lastNodesSnapshot.delete(id) - spatialIndex.remove(id) } } @@ -440,47 +270,7 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { // Extract and store safe data for Vue vueNodeData.set(id, extractVueNodeData(node)) - - if (!nodeState.has(id)) { - nodeState.set(id, { - visible: true, - dirty: false, - lastUpdate: performance.now(), - culled: false - }) - attachMetadata(node) - - // Add to spatial index - const bounds: Bounds = { - x: node.pos[0], - y: node.pos[1], - width: node.size[0], - height: node.size[1] - } - spatialIndex.insert(id, bounds, id) - } }) - - // Update performance metrics - performanceMetrics.nodeCount = vueNodeData.size - performanceMetrics.culledCount = Array.from(nodeState.values()).filter( - (s) => s.culled - ).length - } - - // Most performant: Direct position sync without re-setting entire node - // Query visible nodes using QuadTree spatial index - const getVisibleNodeIds = (viewportBounds: Bounds): Set => { - const startTime = performance.now() - - // Use QuadTree for fast spatial query - const results: string[] = spatialIndex.query(viewportBounds) - const visibleIds = new Set(results) - - lastSpatialQueryTime = performance.now() - startTime - spatialMetrics.queryTime = lastSpatialQueryTime - - return visibleIds } /** @@ -502,30 +292,11 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { // Extract initial data for Vue (may be incomplete during graph configure) vueNodeData.set(id, extractVueNodeData(node)) - // Set up reactive tracking state - nodeState.set(id, { - visible: true, - dirty: false, - lastUpdate: performance.now(), - culled: false - }) - const initializeVueNodeLayout = () => { // Extract actual positions after configure() has potentially updated them const nodePosition = { x: node.pos[0], y: node.pos[1] } const nodeSize = { width: node.size[0], height: node.size[1] } - attachMetadata(node) - - // Add to spatial index for viewport culling with final positions - const nodeBounds: Bounds = { - x: nodePosition.x, - y: nodePosition.y, - width: nodeSize.width, - height: nodeSize.height - } - spatialIndex.insert(id, nodeBounds, id) - // Add node to layout store with final positions setSource(LayoutSource.Canvas) void createNode(id, { @@ -569,9 +340,6 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { ) => { const id = String(node.id) - // Remove from spatial index - spatialIndex.remove(id) - // Remove node from layout store setSource(LayoutSource.Canvas) void deleteNode(id) @@ -579,8 +347,6 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { // Clean up all tracking references nodeRefs.delete(id) vueNodeData.delete(id) - nodeState.delete(id) - lastNodesSnapshot.delete(id) // Call original callback if provided if (originalCallback) { @@ -602,21 +368,9 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { graph.onNodeRemoved = originalOnNodeRemoved || undefined graph.onTrigger = originalOnTrigger || undefined - // Clear pending updates - if (batchTimeoutId !== null) { - clearTimeout(batchTimeoutId) - batchTimeoutId = null - } - // Clear all state maps nodeRefs.clear() vueNodeData.clear() - nodeState.clear() - lastNodesSnapshot.clear() - pendingUpdates.clear() - criticalUpdates.clear() - lowPriorityUpdates.clear() - spatialIndex.clear() } } @@ -712,15 +466,7 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { return { vueNodeData, - nodeState, getNode, - setupEventListeners, - cleanup, - scheduleUpdate, - forceSync: syncWithGraph, - getVisibleNodeIds, - performanceMetrics, - spatialMetrics, - getSpatialIndexDebugInfo: () => spatialIndex.getDebugInfo() + cleanup } } diff --git a/src/composables/graph/useVueNodeLifecycle.ts b/src/composables/graph/useVueNodeLifecycle.ts index 95e68be6a..84e095b5f 100644 --- a/src/composables/graph/useVueNodeLifecycle.ts +++ b/src/composables/graph/useVueNodeLifecycle.ts @@ -1,20 +1,9 @@ -/** - * Vue Node Lifecycle Management Composable - * - * Handles the complete lifecycle of Vue node rendering system including: - * - Node manager initialization and cleanup - * - Layout store synchronization - * - Slot and link sync management - * - Reactive state management for node data, positions, and sizes - * - Memory management and proper cleanup - */ import { createSharedComposable } from '@vueuse/core' -import { computed, readonly, ref, shallowRef, watch } from 'vue' +import { readonly, ref, shallowRef, watch } from 'vue' import { useGraphNodeManager } from '@/composables/graph/useGraphNodeManager' import type { GraphNodeManager, - NodeState, VueNodeData } from '@/composables/graph/useGraphNodeManager' import { useVueFeatureFlags } from '@/composables/useVueFeatureFlags' @@ -42,13 +31,10 @@ function useVueNodeLifecycleIndividual() { // Vue node data state const vueNodeData = ref>(new Map()) - const nodeState = ref>(new Map()) // Trigger for forcing computed re-evaluation const nodeDataTrigger = ref(0) - const isNodeManagerReady = computed(() => nodeManager.value !== null) - const initializeNodeManager = () => { // Use canvas graph if available (handles subgraph contexts), fallback to app graph const activeGraph = comfyApp.canvas?.graph || comfyApp.graph @@ -61,7 +47,6 @@ function useVueNodeLifecycleIndividual() { // Use the manager's data maps vueNodeData.value = manager.vueNodeData - nodeState.value = manager.nodeState // Initialize layout system with existing nodes from active graph const nodes = activeGraph._nodes.map((node: LGraphNode) => ({ @@ -124,7 +109,6 @@ function useVueNodeLifecycleIndividual() { // Reset reactive maps to clean state vueNodeData.value = new Map() - nodeState.value = new Map() } // Watch for Vue nodes enabled state changes @@ -218,10 +202,7 @@ function useVueNodeLifecycleIndividual() { return { vueNodeData, - nodeState, - nodeDataTrigger: readonly(nodeDataTrigger), nodeManager: readonly(nodeManager), - isNodeManagerReady, // Lifecycle methods initializeNodeManager, diff --git a/src/lib/litegraph/src/LGraphCanvas.ts b/src/lib/litegraph/src/LGraphCanvas.ts index 83ce47660..0cf68e3f7 100644 --- a/src/lib/litegraph/src/LGraphCanvas.ts +++ b/src/lib/litegraph/src/LGraphCanvas.ts @@ -4032,6 +4032,18 @@ export class LGraphCanvas // TODO: Report failures, i.e. `failedNodes` + const newPositions = created.map((node) => ({ + nodeId: String(node.id), + bounds: { + x: node.pos[0], + y: node.pos[1], + width: node.size?.[0] ?? 100, + height: node.size?.[1] ?? 200 + } + })) + + layoutStore.batchUpdateNodeBounds(newPositions) + this.selectItems(created) graph.afterChange() diff --git a/src/lib/litegraph/src/interfaces.ts b/src/lib/litegraph/src/interfaces.ts index 359ccfd5f..d8f623712 100644 --- a/src/lib/litegraph/src/interfaces.ts +++ b/src/lib/litegraph/src/interfaces.ts @@ -82,6 +82,7 @@ export interface Positionable extends Parent, HasBoundingRect { * @default 0,0 */ readonly pos: Point + readonly size?: Size /** true if this object is part of the selection, otherwise false. */ selected?: boolean diff --git a/src/renderer/core/layout/store/layoutStore.ts b/src/renderer/core/layout/store/layoutStore.ts index 254b27a2c..15d412814 100644 --- a/src/renderer/core/layout/store/layoutStore.ts +++ b/src/renderer/core/layout/store/layoutStore.ts @@ -1379,6 +1379,7 @@ class LayoutStoreImpl implements LayoutStore { 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 }) } }, this.currentActor) diff --git a/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts b/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts index e3ab1c66c..6adee1e89 100644 --- a/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts +++ b/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts @@ -82,7 +82,6 @@ function useNodeEventHandlersIndividual() { const currentCollapsed = node.flags?.collapsed ?? false if (currentCollapsed !== collapsed) { node.collapse() - nodeManager.value.scheduleUpdate(nodeId, 'critical') } }