mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 19:09:52 +00:00
disable transform settling reflow when panning the graph (#6186)
## 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 <DrJKL@users.noreply.github.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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, {
|
||||
|
||||
@@ -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<HTMLElement | null | undefined>,
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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<HTMLElement | null>(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: '<div>{{ isTransforming }}</div>'
|
||||
@@ -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 })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user