From dc5d41642d9d0059f360fa15bfe4b41d3c6d98cd Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Wed, 22 Oct 2025 14:33:05 -0700 Subject: [PATCH] disable transform settling reflow when panning the graph (#6186) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - disable pan tracking in `useTransformSettling` so we stop wiring high-frequency pointer listeners during canvas drags - the post-navigation-interaction forced reflow is only necessary when zooming since it is for fixing pixel stretch that results from `scale` (which doesn't happen during panning/`translate`) - extend settle delay to 512ms to reduce unnecessary reflow while preserving post-zoom pixel fix After this PR, there should be 0 reflows when panning the graph. First PR in series to address: - https://github.com/Comfy-Org/ComfyUI_frontend/issues/6151 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6186-disable-transform-settling-reflow-when-panning-the-graph-2936d73d365081c2b357e3c72d711439) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL --- .../layout/__tests__/TransformPane.test.ts | 12 +- .../core/layout/transform/TransformPane.vue | 3 +- .../layout/transform/useTransformSettling.ts | 85 ++---------- .../useNodePointerInteractions.test.ts | 2 + .../graph/useTransformSettling.test.ts | 122 +----------------- 5 files changed, 24 insertions(+), 200 deletions(-) diff --git a/src/renderer/core/layout/__tests__/TransformPane.test.ts b/src/renderer/core/layout/__tests__/TransformPane.test.ts index f6b4f463e..ae18ffbac 100644 --- a/src/renderer/core/layout/__tests__/TransformPane.test.ts +++ b/src/renderer/core/layout/__tests__/TransformPane.test.ts @@ -155,17 +155,17 @@ describe('TransformPane', () => { expect.any(Function), expect.any(Object) ) - expect(mockCanvas.canvas.addEventListener).toHaveBeenCalledWith( + expect(mockCanvas.canvas.addEventListener).not.toHaveBeenCalledWith( 'pointerdown', expect.any(Function), expect.any(Object) ) - expect(mockCanvas.canvas.addEventListener).toHaveBeenCalledWith( + expect(mockCanvas.canvas.addEventListener).not.toHaveBeenCalledWith( 'pointerup', expect.any(Function), expect.any(Object) ) - expect(mockCanvas.canvas.addEventListener).toHaveBeenCalledWith( + expect(mockCanvas.canvas.addEventListener).not.toHaveBeenCalledWith( 'pointercancel', expect.any(Function), expect.any(Object) @@ -188,17 +188,17 @@ describe('TransformPane', () => { expect.any(Function), expect.any(Object) ) - expect(mockCanvas.canvas.removeEventListener).toHaveBeenCalledWith( + expect(mockCanvas.canvas.removeEventListener).not.toHaveBeenCalledWith( 'pointerdown', expect.any(Function), expect.any(Object) ) - expect(mockCanvas.canvas.removeEventListener).toHaveBeenCalledWith( + expect(mockCanvas.canvas.removeEventListener).not.toHaveBeenCalledWith( 'pointerup', expect.any(Function), expect.any(Object) ) - expect(mockCanvas.canvas.removeEventListener).toHaveBeenCalledWith( + expect(mockCanvas.canvas.removeEventListener).not.toHaveBeenCalledWith( 'pointercancel', expect.any(Function), expect.any(Object) diff --git a/src/renderer/core/layout/transform/TransformPane.vue b/src/renderer/core/layout/transform/TransformPane.vue index 0ed23a90c..9675248a7 100644 --- a/src/renderer/core/layout/transform/TransformPane.vue +++ b/src/renderer/core/layout/transform/TransformPane.vue @@ -45,8 +45,7 @@ const { isLOD } = useLOD(camera) const canvasElement = computed(() => props.canvas?.canvas) const { isTransforming: isInteracting } = useTransformSettling(canvasElement, { - settleDelay: 200, - trackPan: true + settleDelay: 512 }) provide(TransformStateKey, { diff --git a/src/renderer/core/layout/transform/useTransformSettling.ts b/src/renderer/core/layout/transform/useTransformSettling.ts index bc3d0fc06..3df6dba2b 100644 --- a/src/renderer/core/layout/transform/useTransformSettling.ts +++ b/src/renderer/core/layout/transform/useTransformSettling.ts @@ -1,4 +1,4 @@ -import { useDebounceFn, useEventListener, useThrottleFn } from '@vueuse/core' +import { useDebounceFn, useEventListener } from '@vueuse/core' import { ref } from 'vue' import type { MaybeRefOrGetter } from 'vue' @@ -8,16 +8,6 @@ interface TransformSettlingOptions { * @default 200 */ settleDelay?: number - /** - * Whether to track both zoom (wheel) and pan (pointer drag) interactions - * @default false - */ - trackPan?: boolean - /** - * Throttle delay for high-frequency pointermove events (only used when trackPan is true) - * @default 16 (~60fps) - */ - pointerMoveThrottle?: number /** * Whether to use passive event listeners (better performance but can't preventDefault) * @default true @@ -26,10 +16,10 @@ interface TransformSettlingOptions { } /** - * Tracks when canvas transforms (zoom/pan) are actively changing vs settled. + * Tracks when canvas zoom transforms are actively changing vs settled. * - * This composable helps optimize rendering quality during transformations. - * When the user is actively zooming or panning, we can reduce rendering quality + * This composable helps optimize rendering quality during zoom transformations. + * When the user is actively zooming, we can reduce rendering quality * for better performance. Once the transform "settles" (stops changing), we can * trigger high-quality re-rasterization. * @@ -42,8 +32,7 @@ interface TransformSettlingOptions { * @example * ```ts * const { isTransforming } = useTransformSettling(canvasRef, { - * settleDelay: 200, - * trackPan: true + * settleDelay: 200 * }) * * // Use in CSS classes or rendering logic @@ -57,15 +46,9 @@ export function useTransformSettling( target: MaybeRefOrGetter, options: TransformSettlingOptions = {} ) { - const { - settleDelay = 200, - trackPan = false, - pointerMoveThrottle = 16, - passive = true - } = options + const { settleDelay = 200, passive = true } = options const isTransforming = ref(false) - let isPanning = false /** * Mark transform as active @@ -82,69 +65,19 @@ export function useTransformSettling( }, settleDelay) /** - * Handle any transform event - mark active then queue settle + * Handle zoom transform event - mark active then queue settle */ - const handleTransformEvent = () => { + const handleWheel = () => { markTransformActive() void markTransformSettled() } - // Wheel handler - const handleWheel = () => { - handleTransformEvent() - } - - // Pointer handlers for panning - const handlePointerDown = () => { - if (trackPan) { - isPanning = true - handleTransformEvent() - } - } - - // Throttled pointer move handler for performance - const handlePointerMove = trackPan - ? useThrottleFn(() => { - if (isPanning) { - handleTransformEvent() - } - }, pointerMoveThrottle) - : undefined - - const handlePointerEnd = () => { - if (trackPan) { - isPanning = false - // Don't immediately stop - let the debounced settle handle it - } - } - - // Register event listeners with auto-cleanup + // Register wheel event listener with auto-cleanup useEventListener(target, 'wheel', handleWheel, { capture: true, passive }) - if (trackPan) { - useEventListener(target, 'pointerdown', handlePointerDown, { - capture: true - }) - - if (handlePointerMove) { - useEventListener(target, 'pointermove', handlePointerMove, { - capture: true, - passive - }) - } - - useEventListener(target, 'pointerup', handlePointerEnd, { - capture: true - }) - - useEventListener(target, 'pointercancel', handlePointerEnd, { - capture: true - }) - } - return { isTransforming } diff --git a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts index 61cc4035a..b4bd1d839 100644 --- a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts +++ b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts @@ -1,3 +1,4 @@ +import { createPinia, setActivePinia } from 'pinia' import { beforeEach, describe, expect, it, vi } from 'vitest' import { nextTick, ref } from 'vue' @@ -71,6 +72,7 @@ const createMouseEvent = ( describe('useNodePointerInteractions', () => { beforeEach(async () => { vi.clearAllMocks() + setActivePinia(createPinia()) // Reset layout store state between tests const { layoutStore } = await import( '@/renderer/core/layout/store/layoutStore' diff --git a/tests-ui/tests/composables/graph/useTransformSettling.test.ts b/tests-ui/tests/composables/graph/useTransformSettling.test.ts index 6b2d106d5..bbf0f5d7b 100644 --- a/tests-ui/tests/composables/graph/useTransformSettling.test.ts +++ b/tests-ui/tests/composables/graph/useTransformSettling.test.ts @@ -67,41 +67,8 @@ describe('useTransformSettling', () => { expect(isTransforming.value).toBe(false) }) - it('should track pan events when trackPan is enabled', async () => { - const { isTransforming } = useTransformSettling(element, { - trackPan: true, - settleDelay: 200 - }) - - // Pointer down should start transform - element.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) - await nextTick() - expect(isTransforming.value).toBe(true) - - // Pointer move should keep it active - vi.advanceTimersByTime(100) - element.dispatchEvent(new PointerEvent('pointermove', { bubbles: true })) - await nextTick() - - // Should still be transforming - expect(isTransforming.value).toBe(true) - - // Pointer up - element.dispatchEvent(new PointerEvent('pointerup', { bubbles: true })) - await nextTick() - - // Should still be transforming until settle delay - expect(isTransforming.value).toBe(true) - - // Advance past settle delay - vi.advanceTimersByTime(200) - expect(isTransforming.value).toBe(false) - }) - - it('should not track pan events when trackPan is disabled', async () => { - const { isTransforming } = useTransformSettling(element, { - trackPan: false - }) + it('should not track pan events', async () => { + const { isTransforming } = useTransformSettling(element) // Pointer events should not trigger transform element.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) @@ -111,26 +78,6 @@ describe('useTransformSettling', () => { expect(isTransforming.value).toBe(false) }) - it('should handle pointer cancel events', async () => { - const { isTransforming } = useTransformSettling(element, { - trackPan: true, - settleDelay: 200 - }) - - // Start panning - element.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) - await nextTick() - expect(isTransforming.value).toBe(true) - - // Cancel instead of up - element.dispatchEvent(new PointerEvent('pointercancel', { bubbles: true })) - await nextTick() - - // Should still settle normally - vi.advanceTimersByTime(200) - expect(isTransforming.value).toBe(false) - }) - it('should work with ref target', async () => { const targetRef = ref(null) const { isTransforming } = useTransformSettling(targetRef) @@ -177,44 +124,13 @@ describe('useTransformSettling', () => { element.removeEventListener('wheel', bubbleHandler, false) }) - it('should throttle pointer move events', async () => { - const { isTransforming } = useTransformSettling(element, { - trackPan: true, - pointerMoveThrottle: 50, - settleDelay: 100 - }) - - // Start panning - element.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) - await nextTick() - - // Fire many pointer move events rapidly - for (let i = 0; i < 10; i++) { - element.dispatchEvent(new PointerEvent('pointermove', { bubbles: true })) - vi.advanceTimersByTime(5) // 5ms between events - } - await nextTick() - - // Should still be transforming - expect(isTransforming.value).toBe(true) - - // End panning - element.dispatchEvent(new PointerEvent('pointerup', { bubbles: true })) - - // Advance past settle delay - vi.advanceTimersByTime(100) - expect(isTransforming.value).toBe(false) - }) - it('should clean up event listeners when component unmounts', async () => { const removeEventListenerSpy = vi.spyOn(element, 'removeEventListener') // Create a test component const TestComponent = { setup() { - const { isTransforming } = useTransformSettling(element, { - trackPan: true - }) + const { isTransforming } = useTransformSettling(element) return { isTransforming } }, template: '
{{ isTransforming }}
' @@ -226,52 +142,26 @@ describe('useTransformSettling', () => { // Unmount component wrapper.unmount() - // Should have removed all event listeners + // Should have removed wheel event listener expect(removeEventListenerSpy).toHaveBeenCalledWith( 'wheel', expect.any(Function), expect.objectContaining({ capture: true }) ) - expect(removeEventListenerSpy).toHaveBeenCalledWith( - 'pointerdown', - expect.any(Function), - expect.objectContaining({ capture: true }) - ) - expect(removeEventListenerSpy).toHaveBeenCalledWith( - 'pointermove', - expect.any(Function), - expect.objectContaining({ capture: true }) - ) - expect(removeEventListenerSpy).toHaveBeenCalledWith( - 'pointerup', - expect.any(Function), - expect.objectContaining({ capture: true }) - ) - expect(removeEventListenerSpy).toHaveBeenCalledWith( - 'pointercancel', - expect.any(Function), - expect.objectContaining({ capture: true }) - ) }) it('should use passive listeners when specified', async () => { const addEventListenerSpy = vi.spyOn(element, 'addEventListener') useTransformSettling(element, { - passive: true, - trackPan: true + passive: true }) - // Check that passive option was used for appropriate events + // Check that passive option was used for wheel event expect(addEventListenerSpy).toHaveBeenCalledWith( 'wheel', expect.any(Function), expect.objectContaining({ passive: true, capture: true }) ) - expect(addEventListenerSpy).toHaveBeenCalledWith( - 'pointermove', - expect.any(Function), - expect.objectContaining({ passive: true, capture: true }) - ) }) })