Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions browser_tests/fixtures/ComfyMouse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ export class ComfyMouse implements Omit<Mouse, 'move'> {
)
}

async hold(...args: Parameters<Mouse['down']>) {
await this.mouse.down(...args)
const release = new AsyncDisposableStack()
release.defer(() => this.mouse.up(...args))
return release
}

//#region Pass-through
async click(...args: Parameters<Mouse['click']>) {
return await this.mouse.click(...args)
Expand Down
7 changes: 7 additions & 0 deletions browser_tests/fixtures/helpers/KeyboardHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ export class KeyboardHelper {
await nextFrame(this.page)
}

async hold(key: string): Promise<AsyncDisposableStack> {
await this.page.keyboard.down(key)
const release = new AsyncDisposableStack()
release.defer(() => this.page.keyboard.up(key))
return release
}

async delete(locator?: Locator | null): Promise<void> {
await this.press('Delete', locator)
}
Expand Down
140 changes: 119 additions & 21 deletions browser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,37 +27,135 @@ test.describe('Vue Nodes Canvas Pan', { tag: '@vue-nodes' }, () => {
}
)

test.describe('spacebar panning', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.settings.setSetting(
'Comfy.Canvas.NavigationMode',
'standard'
)
await comfyPage.workflow.loadWorkflow('vueNodes/simple-triple')
})
test('spacebar panning', async ({ comfyPage, comfyMouse }) => {
await comfyPage.settings.setSetting(
'Comfy.Canvas.NavigationMode',
'standard'
)
await comfyPage.workflow.loadWorkflow('vueNodes/simple-triple')
const node = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
const [nodeRef] = await comfyPage.nodeOps.getNodeRefsByTitle('KSampler')
if (!nodeRef) throw new Error('KSampler is not rendered')
const softExpect = expect.configure({ soft: true })

test('Space + left-drag on a Vue node pans canvas', async ({
comfyPage,
comfyMouse
}) => {
const node = comfyPage.vueNodes.getNodeByTitle('KSampler')
await test.step('Space + click on a node starts a pan', async () => {
const offsetBefore = await comfyPage.canvasOps.getOffset()

await comfyPage.canvas.focus()
await comfyPage.page.keyboard.down('Space')
await expect.poll(() => comfyPage.canvasOps.isReadOnly()).toBe(true)
try {
await comfyMouse.dragElementBy(node, { x: 140, y: 90 })
} finally {
await comfyPage.page.keyboard.up('Space')
}
await using releaseSpace = await comfyPage.keyboard.hold('Space')
await softExpect.poll(() => comfyPage.canvasOps.isReadOnly()).toBe(true)
await comfyMouse.dragElementBy(node.root, { x: -300, y: 0 })
await releaseSpace.disposeAsync()

await expect
await softExpect
.poll(() => comfyPage.canvasOps.getOffset())
.not.toEqual(offsetBefore)
})

await test.step('Space switches node dragging to canvas panning', async () => {
await node.header.hover()
await using mouseRelease = await comfyMouse.hold()
await comfyPage.page.mouse.move(500, 500, { steps: 5 })
const offsetBeforePan = await comfyPage.canvasOps.getOffset()

await using spaceRelease = await comfyPage.keyboard.hold('Space')
await comfyPage.page.mouse.move(400, 400, { steps: 5 })
await softExpect
.poll(() => comfyPage.canvasOps.getOffset())
.not.toEqual(offsetBeforePan)

await test.step('Releasing Space resumes node dragging', async () => {
await spaceRelease.disposeAsync()
const offsetAfterPan = await comfyPage.canvasOps.getOffset()
const positionBeforeResume = [
...(await nodeRef.getProperty<[number, number]>('pos'))
]
await comfyPage.page.mouse.move(500, 500, { steps: 5 })
await comfyPage.nextFrame()

softExpect(await comfyPage.canvasOps.getOffset()).toEqual(
offsetAfterPan
)
await softExpect
.poll(async () => [
...(await nodeRef.getProperty<[number, number]>('pos'))
])
.not.toEqual(positionBeforeResume)
await mouseRelease.disposeAsync()
})
})
})

test(
'Space in a focused text widget does not start canvas panning',
{ tag: ['@canvas', '@widget'] },
async ({ comfyPage }) => {
await comfyPage.workflow.loadWorkflow('inputs/string_input')
const input = comfyPage.vueNodes
.getWidgetByName('Node With String Input', 'string_input')
.first()

await input.focus()
await input.press('Space')

await expect
.poll(async () => [
await input.inputValue(),
await comfyPage.canvasOps.isReadOnly()
])
.toEqual([' ', false])
}
)

test(
'releasing the pointer during Space-pan ends the node drag',
{ tag: ['@canvas', '@node'] },
async ({ comfyPage, comfyMouse }) => {
await comfyPage.workflow.loadWorkflow('vueNodes/simple-triple')
const node = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
const [nodeRef] = await comfyPage.nodeOps.getNodeRefsByTitle('KSampler')
const headerBox = await node.header.boundingBox()
if (!nodeRef || !headerBox) throw new Error('KSampler is not rendered')
const start = {
x: headerBox.x + headerBox.width / 2,
y: headerBox.y + headerBox.height / 2
}

const positionAfterRelease =
await test.step('Release the pointer while Space-panning', async () => {
await comfyPage.page.mouse.move(start.x, start.y)
await using mouseRelease = await comfyMouse.hold()
await comfyPage.page.mouse.move(start.x + 40, start.y + 40, {
steps: 5
})
await using spaceRelease = await comfyPage.keyboard.hold('Space')
await comfyPage.page.mouse.move(start.x + 80, start.y + 80, {
steps: 5
})
await mouseRelease.disposeAsync()
await spaceRelease.disposeAsync()

return [...(await nodeRef.getProperty<[number, number]>('pos'))]
})

await test.step('Further pointer movement leaves the node in place', async () => {
const headerAfterRelease = await node.header.boundingBox()
if (!headerAfterRelease) throw new Error('KSampler is not rendered')
await comfyPage.page.mouse.move(
headerAfterRelease.x + 5,
headerAfterRelease.y + 5
)
await comfyPage.nextFrame()

await expect
.poll(async () => [
...(await nodeRef.getProperty<[number, number]>('pos'))
])
.toEqual(positionAfterRelease)
})
}
)

test(
'@mobile Can pan with touch',
{ tag: '@screenshot' },
Expand Down
15 changes: 15 additions & 0 deletions src/components/graph/GraphCanvas.vue
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
@pointerdown.capture="forwardPointerDownPanEvent"
@pointerup.capture="forwardPointerUpPanEvent"
@pointermove.capture="forwardPointerMovePanEvent"
@keydown.space="forwardSpaceKeyEvent"
>
<!-- Vue nodes rendered based on graph nodes -->
<LGraphNode
Expand Down Expand Up @@ -619,6 +620,20 @@ function forwardPointerUpPanEvent(e: PointerEvent) {
forwardPanEvent(e, isMiddleButtonEvent)
}

function forwardSpaceKeyEvent(e: KeyboardEvent) {
const target = e.target
if (
!layoutStore.isDraggingVueNodes.value ||
target instanceof HTMLInputElement ||
target instanceof HTMLTextAreaElement ||
target instanceof HTMLButtonElement ||
(target instanceof HTMLElement && target.isContentEditable)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
)
return

comfyApp.canvas?.processKey(e)
}

function forwardPanEvent(
e: PointerEvent,
isMiddleInput: (event: PointerEvent) => boolean
Expand Down
26 changes: 26 additions & 0 deletions src/lib/litegraph/src/LGraphCanvas.linkDragAutoPan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,32 @@ describe('LGraphCanvas link drag auto-pan', () => {
expect(canvas['_autoPan']).not.toBeNull()
})

it('resumes auto-pan after Space panning during a link drag', () => {
canvas.mouse[0] = 400
canvas.mouse[1] = 300
canvas.linkConnector.state.connectingTo = 'output'
canvas.pointer.isDown = true
startLinkDrag()
const autoPan = canvas['_autoPan']
if (!autoPan) throw new Error('Auto-pan controller was not created')
const stop = vi.spyOn(autoPan, 'stop')
const start = vi.spyOn(autoPan, 'start')
const updatePointer = vi.spyOn(autoPan, 'updatePointer')
const keydown = new KeyboardEvent('keydown', { key: ' ' })
const keyup = new KeyboardEvent('keyup', { key: ' ' })
Object.defineProperty(keydown, 'target', { value: canvasElement })
Object.defineProperty(keyup, 'target', { value: canvasElement })

canvas.processKey(keydown)
canvas.mouse[0] = 450
canvas.mouse[1] = 350
canvas.processKey(keyup)

expect(stop).toHaveBeenCalledOnce()
expect(updatePointer).toHaveBeenLastCalledWith(450, 350)
expect(start).toHaveBeenCalledOnce()
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it('keeps graph_mouse consistent with offset after auto-pan', () => {
canvas.mouse[0] = 5
canvas.mouse[1] = 300
Expand Down
8 changes: 8 additions & 0 deletions src/lib/litegraph/src/LGraphCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4008,6 +4008,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
if (e.key === ' ') {
// space
this.read_only = true
this._autoPan?.stop()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (this._previously_dragging_canvas === null) {
this._previously_dragging_canvas = this.dragging_canvas
}
Expand Down Expand Up @@ -4037,6 +4038,13 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
this.dragging_canvas =
(this._previously_dragging_canvas ?? false) && this.pointer.isDown
this._previously_dragging_canvas = null
if (
this.pointer.isDown &&
(this.isDragging || this.linkConnector.isConnecting)
) {
this._autoPan?.updatePointer(this.mouse[0], this.mouse[1])
this._autoPan?.start()
}
}

for (const node of Object.values(this.selected_nodes)) {
Expand Down
12 changes: 6 additions & 6 deletions src/renderer/core/canvas/useCanvasInteractions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,12 @@ export function useCanvasInteractions() {
return
}

// Create new event with same properties
const EventConstructor = event.constructor as
| typeof MouseEvent
| typeof PointerEvent
const newEvent = new EventConstructor(event.type, event)
canvasEl.dispatchEvent(newEvent)
if (event instanceof PointerEvent) {
canvasEl.dispatchEvent(new PointerEvent(event.type, event))
return
}

canvasEl.dispatchEvent(new MouseEvent(event.type, event))
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ export function useNodePointerInteractions(
const canHandlePointer = shouldHandleNodePointerEvents.value
if (!canHandlePointer) {
forwardEventToCanvas(event)
if (hasDraggingStarted || layoutStore.isDraggingVueNodes.value) {
safeDragEnd(event)
}
return
}
const wasDragging = layoutStore.isDraggingVueNodes.value
Expand Down
14 changes: 14 additions & 0 deletions src/renderer/extensions/vueNodes/layout/useNodeDrag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,20 @@ function useNodeDragIndividual() {
if (!dragStartPos || !dragStartMouse) {
return
}
if (canvasStore.isReadOnly) {
autoPan?.stop()
const canvas = canvasStore.getCanvas()
const delta = [event.clientX - lastPointerX, event.clientY - lastPointerY]

canvas.ds.offset[0] += delta[0] / canvas.ds.scale
canvas.ds.offset[1] += delta[1] / canvas.ds.scale
canvas.setDirty(true, true)
lastPointerX = event.clientX
lastPointerY = event.clientY
dragStartMouse.x += delta[0]
dragStartMouse.y += delta[1]
Comment on lines +204 to +210

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that we have to update this many things at once is a pretty big code smell, IMO.
(Not for this PR, just whining)

return
Comment thread
AustinMroz marked this conversation as resolved.
}

// Throttle position updates using requestAnimationFrame for better performance
if (rafId !== null) return // Skip if frame already scheduled
Expand Down
Loading