mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-05 13:10:24 +00:00
fix: cache canvas cursor style to avoid redundant DOM writes (#9171)
## Summary Cache `canvas.style.cursor` to avoid redundant DOM writes that dirty Firefox's style tree. ## Changes - **What**: Add `_lastCursor` field to `LGraphCanvas._updateCursorStyle()` — only writes `canvas.style.cursor` when the value changes. Eliminates ~347 redundant style mutations per profiling session. ## Review Focus - The fix is 2 lines (cache field + comparison). The unit test validates the caching pattern without requiring full LGraphCanvas instantiation. - This is one of several contributors to Firefox's cascading style recalculation freeze. Each `canvas.style.cursor` write dirties the style tree, which is flushed during the next paint in the canvas render loop. ## Stack 2 of 4 in Firefox perf fix stack. Depends on #9170. <!-- Fixes #ISSUE_NUMBER --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9171-fix-cache-canvas-cursor-style-to-avoid-redundant-DOM-writes-3116d73d36508139827fe1d644fa1bd0) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -12,6 +12,7 @@ import { forEachNode } from '@/utils/graphTraversalUtil'
|
||||
|
||||
import { CanvasPointer } from './CanvasPointer'
|
||||
import type { ContextMenu } from './ContextMenu'
|
||||
import { createCursorCache } from './cursorCache'
|
||||
import { DragAndScale } from './DragAndScale'
|
||||
import type { AnimationOptions } from './DragAndScale'
|
||||
import type { LGraph } from './LGraph'
|
||||
@@ -364,6 +365,8 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
this.canvas.dispatchEvent(new CustomEvent(type, { detail }))
|
||||
}
|
||||
|
||||
private _setCursor!: ReturnType<typeof createCursorCache>
|
||||
|
||||
private _updateCursorStyle() {
|
||||
if (!this.state.shouldSetCursor) return
|
||||
|
||||
@@ -386,7 +389,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
cursor = 'grab'
|
||||
}
|
||||
|
||||
this.canvas.style.cursor = cursor
|
||||
this._setCursor(cursor)
|
||||
}
|
||||
|
||||
// Whether the canvas was previously being dragged prior to pressing space key.
|
||||
@@ -1911,6 +1914,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
this.pointer.element = element
|
||||
|
||||
if (!element) return
|
||||
this._setCursor = createCursorCache(element)
|
||||
|
||||
// TODO: classList.add
|
||||
element.className += ' lgraphcanvas'
|
||||
@@ -2970,7 +2974,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
}
|
||||
|
||||
// Set appropriate cursor for resize direction
|
||||
this.canvas.style.cursor = cursors[resizeDirection]
|
||||
this._setCursor(cursors[resizeDirection])
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
59
src/lib/litegraph/src/cursorCache.test.ts
Normal file
59
src/lib/litegraph/src/cursorCache.test.ts
Normal file
@@ -0,0 +1,59 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { createCursorCache } from './cursorCache'
|
||||
|
||||
function createMockElement() {
|
||||
let cursorValue = ''
|
||||
const setter = vi.fn((value: string) => {
|
||||
cursorValue = value
|
||||
})
|
||||
|
||||
const element = document.createElement('div')
|
||||
Object.defineProperty(element.style, 'cursor', {
|
||||
get: () => cursorValue,
|
||||
set: setter
|
||||
})
|
||||
|
||||
return { element, setter }
|
||||
}
|
||||
|
||||
describe('createCursorCache', () => {
|
||||
it('should only write to DOM when cursor value changes', () => {
|
||||
const { element, setter } = createMockElement()
|
||||
const setCursor = createCursorCache(element)
|
||||
|
||||
setCursor('crosshair')
|
||||
setCursor('crosshair')
|
||||
setCursor('crosshair')
|
||||
|
||||
expect(setter).toHaveBeenCalledTimes(1)
|
||||
expect(setter).toHaveBeenCalledWith('crosshair')
|
||||
})
|
||||
|
||||
it('should write to DOM when cursor value differs', () => {
|
||||
const { element, setter } = createMockElement()
|
||||
const setCursor = createCursorCache(element)
|
||||
|
||||
setCursor('default')
|
||||
setCursor('crosshair')
|
||||
setCursor('grabbing')
|
||||
|
||||
expect(setter).toHaveBeenCalledTimes(3)
|
||||
expect(setter).toHaveBeenNthCalledWith(1, 'default')
|
||||
expect(setter).toHaveBeenNthCalledWith(2, 'crosshair')
|
||||
expect(setter).toHaveBeenNthCalledWith(3, 'grabbing')
|
||||
})
|
||||
|
||||
it('should skip repeated values interspersed with changes', () => {
|
||||
const { element, setter } = createMockElement()
|
||||
const setCursor = createCursorCache(element)
|
||||
|
||||
setCursor('default')
|
||||
setCursor('default')
|
||||
setCursor('grab')
|
||||
setCursor('grab')
|
||||
setCursor('default')
|
||||
|
||||
expect(setter).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
})
|
||||
8
src/lib/litegraph/src/cursorCache.ts
Normal file
8
src/lib/litegraph/src/cursorCache.ts
Normal file
@@ -0,0 +1,8 @@
|
||||
export function createCursorCache(element: HTMLElement) {
|
||||
let lastCursor = ''
|
||||
return function setCursor(cursor: string) {
|
||||
if (cursor === lastCursor) return
|
||||
lastCursor = cursor
|
||||
element.style.cursor = cursor
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user