Refactor: Viewport Culling improvements (#5767)

## Summary

Reduce the number of DOM changes, but also restructure the logic to
create a clean point of separation for when we can make DragAndScale
reactive.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5767-Refactor-Viewport-Culling-improvements-2796d73d365081708f02cb8033aac9b1)
by [Unito](https://www.unito.io)

package.json

@@ -18,10 +18,11 @@
     "format:check": "prettier --check './**/*.{js,ts,tsx,vue,mts}' --cache",
     "format:no-cache": "prettier --write './**/*.{js,ts,tsx,vue,mts}' --list-different",
     "format:check:no-cache": "prettier --check './**/*.{js,ts,tsx,vue,mts}'",
+    "test:all": "nx run test",
     "test:browser": "npx nx e2e",
-    "test:unit": "nx run test tests-ui/tests",
     "test:component": "nx run test src/components/",
     "test:litegraph": "vitest run --config vitest.litegraph.config.ts",
+    "test:unit": "nx run test tests-ui/tests",
     "preinstall": "npx only-allow pnpm",
     "prepare": "husky || true && git config blame.ignoreRevsFile .git-blame-ignore-revs || true",
     "preview": "nx preview",

src/components/graph/GraphCanvas.vue

@@ -165,7 +165,7 @@ const { shouldRenderVueNodes } = useVueFeatureFlags()
 
 // Vue node system
 const vueNodeLifecycle = useVueNodeLifecycle()
-const viewportCulling = useViewportCulling()
+const { handleTransformUpdate } = useViewportCulling()
 
 const handleVueNodeLifecycleReset = async () => {
   if (shouldRenderVueNodes.value) {
@@ -187,8 +187,9 @@ watch(
   }
 )
 
-const allNodes = viewportCulling.allNodes
-const handleTransformUpdate = viewportCulling.handleTransformUpdate
+const allNodes = computed(() =>
+  Array.from(vueNodeLifecycle.vueNodeData.value.values())
+)
 
 watchEffect(() => {
   nodeDefStore.showDeprecated = settingStore.get('Comfy.Node.ShowDeprecated')

src/composables/graph/useViewportCulling.ts

@@ -6,78 +6,98 @@
  * 2. Set display none on element to avoid cascade resolution overhead
  * 3. Only run when transform changes (event driven)
  */
-import { useThrottleFn } from '@vueuse/core'
+import { createSharedComposable, useThrottleFn } from '@vueuse/core'
 import { computed } from 'vue'
 
 import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
+import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
 import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
-import { app as comfyApp } from '@/scripts/app'
+import { app } from '@/scripts/app'
 
-export function useViewportCulling() {
+type Bounds = [left: number, right: number, top: number, bottom: number]
+
+function getNodeBounds(node: LGraphNode): Bounds {
+  const [nodeLeft, nodeTop] = node.pos
+  const nodeRight = nodeLeft + node.size[0]
+  const nodeBottom = nodeTop + node.size[1]
+  return [nodeLeft, nodeRight, nodeTop, nodeBottom]
+}
+
+function viewportEdges(
+  canvas: ReturnType<typeof useCanvasStore>['canvas']
+): Bounds | undefined {
+  if (!canvas) {
+    return
+  }
+  const ds = canvas.ds
+  const viewport_width = canvas.canvas.width
+  const viewport_height = canvas.canvas.height
+  const margin = 500 * ds.scale
+
+  const [xOffset, yOffset] = ds.offset
+
+  const leftEdge = -margin / ds.scale - xOffset
+  const rightEdge = (viewport_width + margin) / ds.scale - xOffset
+  const topEdge = -margin / ds.scale - yOffset
+  const bottomEdge = (viewport_height + margin) / ds.scale - yOffset
+  return [leftEdge, rightEdge, topEdge, bottomEdge]
+}
+
+function boundsIntersect(boxA: Bounds, boxB: Bounds): boolean {
+  const [aLeft, aRight, aTop, aBottom] = boxA
+  const [bLeft, bRight, bTop, bBottom] = boxB
+
+  const leftOf = aRight < bLeft
+  const rightOf = aLeft > bRight
+  const above = aBottom < bTop
+  const below = aTop > bBottom
+  return !(leftOf || rightOf || above || below)
+}
+
+function useViewportCullingIndividual() {
   const canvasStore = useCanvasStore()
-  const { vueNodeData, nodeManager } = useVueNodeLifecycle()
+  const { nodeManager } = useVueNodeLifecycle()
 
-  const allNodes = computed(() => {
-    return Array.from(vueNodeData.value.values())
-  })
+  const viewport = computed(() => viewportEdges(canvasStore.canvas))
+
+  function inViewport(node: LGraphNode | undefined): boolean {
+    if (!viewport.value || !node) {
+      return true
+    }
+    const nodeBounds = getNodeBounds(node)
+    return boundsIntersect(nodeBounds, viewport.value)
+  }
 
   /**
    * Update visibility of all nodes based on viewport
    * Queries DOM directly - no cache maintenance needed
    */
   function updateVisibility() {
-    if (!nodeManager.value || !canvasStore.canvas || !comfyApp.canvas) return
+    if (!nodeManager.value || !app.canvas) return // load bearing app.canvas check for workflows being loaded.
 
-    const canvas = canvasStore.canvas
-    const manager = nodeManager.value
-    const ds = canvas.ds
-
-    // Viewport bounds
-    const viewport_width = canvas.canvas.width
-    const viewport_height = canvas.canvas.height
-    const margin = 500 * ds.scale
-
-    // Get all node elements at once
     const nodeElements = document.querySelectorAll('[data-node-id]')
-
-    // Update each element's visibility
     for (const element of nodeElements) {
       const nodeId = element.getAttribute('data-node-id')
       if (!nodeId) continue
 
-      const node = manager.getNode(nodeId)
+      const node = nodeManager.value.getNode(nodeId)
       if (!node) continue
 
-      // Calculate if node is outside viewport
-      const screen_x = (node.pos[0] + ds.offset[0]) * ds.scale
-      const screen_y = (node.pos[1] + ds.offset[1]) * ds.scale
-      const screen_width = node.size[0] * ds.scale
-      const screen_height = node.size[1] * ds.scale
-
-      const isNodeOutsideViewport =
-        screen_x + screen_width < -margin ||
-        screen_x > viewport_width + margin ||
-        screen_y + screen_height < -margin ||
-        screen_y > viewport_height + margin
-
-      // Setting display none directly avoid potential cascade resolution
-      if (element instanceof HTMLElement) {
-        element.style.display = isNodeOutsideViewport ? 'none' : ''
+      const displayValue = inViewport(node) ? '' : 'none'
+      if (
+        element instanceof HTMLElement &&
+        element.style.display !== displayValue
+      ) {
+        element.style.display = displayValue
       }
     }
   }
 
-  const updateVisibilityDebounced = useThrottleFn(updateVisibility, 20)
+  const handleTransformUpdate = useThrottleFn(() => updateVisibility, 100, true)
 
-  // RAF throttling for smooth updates during continuous panning
-  function handleTransformUpdate() {
-    requestAnimationFrame(async () => {
-      await updateVisibilityDebounced()
-    })
-  }
-
-  return {
-    allNodes,
-    handleTransformUpdate
-  }
+  return { handleTransformUpdate }
 }
+
+export const useViewportCulling = createSharedComposable(
+  useViewportCullingIndividual
+)

src/renderer/core/layout/__tests__/TransformPane.spec.ts

@@ -1,97 +1,96 @@
 import { mount } from '@vue/test-utils'
-import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
-import { nextTick, ref } from 'vue'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { computed, nextTick } from 'vue'
+
+import type { LGraphCanvas } from '@/lib/litegraph/src/LGraphCanvas'
+import { useTransformState } from '@/renderer/core/layout/transform/useTransformState'
 
 import TransformPane from '../transform/TransformPane.vue'
 
-// Mock the transform state composable
-const mockTransformState = {
-  camera: ref({ x: 0, y: 0, z: 1 }),
-  transformStyle: ref({
+const mockData = vi.hoisted(() => ({
+  mockTransformStyle: {
     transform: 'scale(1) translate(0px, 0px)',
     transformOrigin: '0 0'
-  }),
-  syncWithCanvas: vi.fn(),
-  canvasToScreen: vi.fn(),
-  screenToCanvas: vi.fn(),
-  isNodeInViewport: vi.fn()
-}
-
-vi.mock('@/renderer/core/spatial/useTransformState', () => ({
-  useTransformState: () => mockTransformState
+  },
+  mockCamera: { x: 0, y: 0, z: 1 }
 }))
 
-// Mock requestAnimationFrame/cancelAnimationFrame
-global.requestAnimationFrame = vi.fn((cb) => {
-  setTimeout(cb, 16)
-  return 1
+vi.mock('@/renderer/core/layout/transform/useTransformState', () => {
+  const syncWithCanvas = vi.fn()
+  return {
+    useTransformState: () => ({
+      camera: computed(() => mockData.mockCamera),
+      transformStyle: computed(() => mockData.mockTransformStyle),
+      canvasToScreen: vi.fn(),
+      screenToCanvas: vi.fn(),
+      isNodeInViewport: vi.fn(),
+      syncWithCanvas
+    })
+  }
 })
-global.cancelAnimationFrame = vi.fn()
+
+vi.mock('@/renderer/extensions/vueNodes/lod/useLOD', () => ({
+  useLOD: vi.fn(() => ({
+    isLOD: false
+  }))
+}))
+
+function createMockCanvas(): LGraphCanvas {
+  return {
+    canvas: {
+      addEventListener: vi.fn(),
+      removeEventListener: vi.fn()
+    },
+    ds: {
+      offset: [0, 0],
+      scale: 1
+    }
+  } as unknown as LGraphCanvas
+}
 
 describe('TransformPane', () => {
-  let wrapper: ReturnType<typeof mount>
-  let mockCanvas: any
-
   beforeEach(() => {
-    vi.clearAllMocks()
+    vi.useFakeTimers()
+    vi.resetAllMocks()
 
     // Create mock canvas with LiteGraph interface
-    mockCanvas = {
-      canvas: {
-        addEventListener: vi.fn(),
-        removeEventListener: vi.fn()
-      },
-      ds: {
-        offset: [0, 0],
-        scale: 1
-      }
-    }
-
-    // Reset mock transform state
-    mockTransformState.camera.value = { x: 0, y: 0, z: 1 }
-    mockTransformState.transformStyle.value = {
-      transform: 'scale(1) translate(0px, 0px)',
-      transformOrigin: '0 0'
-    }
-  })
-
-  afterEach(() => {
-    if (wrapper) {
-      wrapper.unmount()
-    }
   })
 
   describe('component mounting', () => {
     it('should mount successfully with minimal props', () => {
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      const wrapper = mount(TransformPane, {
         props: {
           canvas: mockCanvas
         }
       })
 
       expect(wrapper.exists()).toBe(true)
-      expect(wrapper.find('.transform-pane').exists()).toBe(true)
+      expect(wrapper.find('[data-testid="transform-pane"]').exists()).toBe(true)
     })
 
-    it('should apply transform style from composable', () => {
-      mockTransformState.transformStyle.value = {
+    it('should apply transform style from composable', async () => {
+      mockData.mockTransformStyle = {
         transform: 'scale(2) translate(100px, 50px)',
         transformOrigin: '0 0'
       }
 
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      const wrapper = mount(TransformPane, {
         props: {
           canvas: mockCanvas
         }
       })
+      await nextTick()
 
-      const transformPane = wrapper.find('.transform-pane')
+      const transformPane = wrapper.find('[data-testid="transform-pane"]')
       const style = transformPane.attributes('style')
       expect(style).toContain('transform: scale(2) translate(100px, 50px)')
     })
 
     it('should render slot content', () => {
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      const wrapper = mount(TransformPane, {
         props: {
           canvas: mockCanvas
         },
@@ -106,22 +105,9 @@ describe('TransformPane', () => {
   })
 
   describe('RAF synchronization', () => {
-    it('should start RAF sync on mount', async () => {
-      wrapper = mount(TransformPane, {
-        props: {
-          canvas: mockCanvas
-        }
-      })
-
-      await nextTick()
-
-      // Should emit RAF status change to true
-      expect(wrapper.emitted('rafStatusChange')).toBeTruthy()
-      expect(wrapper.emitted('rafStatusChange')?.[0]).toEqual([true])
-    })
-
     it('should call syncWithCanvas during RAF updates', async () => {
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      mount(TransformPane, {
         props: {
           canvas: mockCanvas
         }
@@ -130,13 +116,15 @@ describe('TransformPane', () => {
       await nextTick()
 
       // Allow RAF to execute
-      await new Promise((resolve) => setTimeout(resolve, 20))
+      vi.advanceTimersToNextFrame()
 
-      expect(mockTransformState.syncWithCanvas).toHaveBeenCalledWith(mockCanvas)
+      const transformState = useTransformState()
+      expect(transformState.syncWithCanvas).toHaveBeenCalledWith(mockCanvas)
     })
 
     it('should emit transform update timing', async () => {
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      const wrapper = mount(TransformPane, {
         props: {
           canvas: mockCanvas
         }
@@ -145,34 +133,16 @@ describe('TransformPane', () => {
       await nextTick()
 
       // Allow RAF to execute
-      await new Promise((resolve) => setTimeout(resolve, 20))
+      vi.advanceTimersToNextFrame()
 
       expect(wrapper.emitted('transformUpdate')).toBeTruthy()
-      const updateEvent = wrapper.emitted('transformUpdate')?.[0]
-      expect(typeof updateEvent?.[0]).toBe('number')
-      expect(updateEvent?.[0]).toBeGreaterThanOrEqual(0)
-    })
-
-    it('should stop RAF sync on unmount', async () => {
-      wrapper = mount(TransformPane, {
-        props: {
-          canvas: mockCanvas
-        }
-      })
-
-      await nextTick()
-      wrapper.unmount()
-
-      expect(wrapper.emitted('rafStatusChange')).toBeTruthy()
-      const events = wrapper.emitted('rafStatusChange') as any[]
-      expect(events[events.length - 1]).toEqual([false])
-      expect(global.cancelAnimationFrame).toHaveBeenCalled()
     })
   })
 
   describe('canvas event listeners', () => {
     it('should add event listeners to canvas on mount', async () => {
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      mount(TransformPane, {
         props: {
           canvas: mockCanvas
         }
@@ -203,7 +173,8 @@ describe('TransformPane', () => {
     })
 
     it('should remove event listeners on unmount', async () => {
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      const wrapper = mount(TransformPane, {
         props: {
           canvas: mockCanvas
         }
@@ -237,7 +208,8 @@ describe('TransformPane', () => {
 
   describe('interaction state management', () => {
     it('should apply interacting class during interactions', async () => {
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      const wrapper = mount(TransformPane, {
         props: {
           canvas: mockCanvas
         }
@@ -245,7 +217,7 @@ describe('TransformPane', () => {
 
       // Simulate interaction start by checking internal state
       // Note: This tests the CSS class application logic
-      const transformPane = wrapper.find('.transform-pane')
+      const transformPane = wrapper.find('[data-testid="transform-pane"]')
 
       // Initially should not have interacting class
       expect(transformPane.classes()).not.toContain(
@@ -254,13 +226,14 @@ describe('TransformPane', () => {
     })
 
     it('should handle pointer events for node delegation', async () => {
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      const wrapper = mount(TransformPane, {
         props: {
           canvas: mockCanvas
         }
       })
 
-      const transformPane = wrapper.find('.transform-pane')
+      const transformPane = wrapper.find('[data-testid="transform-pane"]')
 
       // Simulate pointer down - we can't test the exact delegation logic
       // in unit tests due to vue-test-utils limitations, but we can verify
@@ -274,77 +247,32 @@ describe('TransformPane', () => {
 
   describe('transform state integration', () => {
     it('should provide transform utilities to child components', () => {
-      wrapper = mount(TransformPane, {
+      const mockCanvas = createMockCanvas()
+      mount(TransformPane, {
         props: {
           canvas: mockCanvas
         }
       })
 
+      const transformState = useTransformState()
       // The component should provide transform state via Vue's provide/inject
       // This is tested indirectly through the composable integration
-      expect(mockTransformState.syncWithCanvas).toBeDefined()
-      expect(mockTransformState.canvasToScreen).toBeDefined()
-      expect(mockTransformState.screenToCanvas).toBeDefined()
+      expect(transformState.syncWithCanvas).toBeDefined()
+      expect(transformState.canvasToScreen).toBeDefined()
+      expect(transformState.screenToCanvas).toBeDefined()
     })
   })
 
   describe('error handling', () => {
     it('should handle null canvas gracefully', () => {
-      wrapper = mount(TransformPane, {
+      const wrapper = mount(TransformPane, {
         props: {
           canvas: undefined
         }
       })
 
       expect(wrapper.exists()).toBe(true)
-      expect(wrapper.find('.transform-pane').exists()).toBe(true)
-    })
-
-    it('should handle missing canvas properties', () => {
-      const incompleteCanvas = {} as any
-
-      wrapper = mount(TransformPane, {
-        props: {
-          canvas: incompleteCanvas
-        }
-      })
-
-      expect(wrapper.exists()).toBe(true)
-      // Should not throw errors during mount
-    })
-  })
-
-  describe('performance optimizations', () => {
-    it('should use contain CSS property for layout optimization', () => {
-      wrapper = mount(TransformPane, {
-        props: {
-          canvas: mockCanvas
-        }
-      })
-
-      const transformPane = wrapper.find('.transform-pane')
-
-      // This test verifies the CSS contains the performance optimization
-      // Note: In JSDOM, computed styles might not reflect all CSS properties
-      expect(transformPane.element.className).toContain('transform-pane')
-    })
-
-    it('should disable pointer events on container but allow on children', () => {
-      wrapper = mount(TransformPane, {
-        props: {
-          canvas: mockCanvas
-        },
-        slots: {
-          default: '<div data-node-id="test">Test Node</div>'
-        }
-      })
-
-      const transformPane = wrapper.find('.transform-pane')
-
-      // The CSS should handle pointer events optimization
-      // This is primarily a CSS concern, but we verify the structure
-      expect(transformPane.exists()).toBe(true)
-      expect(wrapper.find('[data-node-id="test"]').exists()).toBe(true)
+      expect(wrapper.find('[data-testid="transform-pane"]').exists()).toBe(true)
     })
   })
 })

src/renderer/core/layout/transform/TransformPane.vue

@@ -1,14 +1,14 @@
 <template>
   <div
-    class="absolute inset-0 w-full h-full pointer-events-none"
+    data-testid="transform-pane"
     :class="
       cn(
+        'absolute inset-0 w-full h-full pointer-events-none',
         isInteracting ? 'transform-pane--interacting' : 'will-change-auto',
-        isLOD ? 'isLOD' : ''
+        isLOD && 'isLOD'
       )
     "
     :style="transformStyle"
-    @pointerdown="handlePointerDown"
   >
     <!-- Vue nodes will be rendered here -->
     <slot />
@@ -56,18 +56,7 @@ provide(TransformStateKey, {
   isNodeInViewport
 })
 
-const handlePointerDown = (event: PointerEvent) => {
-  const target = event.target as HTMLElement
-  const nodeElement = target.closest('[data-node-id]')
-
-  if (nodeElement) {
-    // TODO: Emit event for node interaction
-    // Node interaction with nodeId will be handled in future implementation
-  }
-}
-
 const emit = defineEmits<{
-  rafStatusChange: [active: boolean]
   transformUpdate: []
 }>()
 
@@ -84,23 +73,7 @@ useRafFn(
 </script>
 
 <style scoped>
-.transform-pane {
-  position: absolute;
-  inset: 0;
-  transform-origin: 0 0;
-  pointer-events: none;
-  top: 0;
-  left: 0;
-  width: 100%;
-  height: 100%;
-}
-
 .transform-pane--interacting {
   will-change: transform;
 }
-
-/* Allow pointer events on nodes */
-.transform-pane :deep([data-node-id]) {
-  pointer-events: auto;
-}
 </style>