mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 19:09:52 +00:00
Fix vue node slot position calculation (#7877)
## Summary Fix slot position for Vue nodes using LiteGraph calculation Fixes #7446 ## Changes - Updated getInput/OutputPos to call getSlotPosition which handles positions for both LiteGraph & Vue nodes correctly - Add regression test ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7877-Fix-vue-node-slot-position-calculation-2e16d73d365081219afef0e4ebfb0620) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com>
This commit is contained in:
@@ -3,7 +3,7 @@ import { test as base, expect } from '@playwright/test'
|
||||
import dotenv from 'dotenv'
|
||||
import * as fs from 'fs'
|
||||
|
||||
import type { LGraphNode } from '../../src/lib/litegraph/src/litegraph'
|
||||
import type { LGraphNode, LGraph } from '../../src/lib/litegraph/src/litegraph'
|
||||
import type { NodeId } from '../../src/platform/workflow/validation/schemas/workflowSchema'
|
||||
import type { KeyCombo } from '../../src/schemas/keyBindingSchema'
|
||||
import type { useWorkspaceStore } from '../../src/stores/workspaceStore'
|
||||
@@ -1591,14 +1591,29 @@ export class ComfyPage {
|
||||
return window['app'].graph.nodes
|
||||
})
|
||||
}
|
||||
async getNodeRefsByType(type: string): Promise<NodeReference[]> {
|
||||
async waitForGraphNodes(count: number) {
|
||||
await this.page.waitForFunction((count) => {
|
||||
return window['app']?.canvas.graph?.nodes?.length === count
|
||||
}, count)
|
||||
}
|
||||
async getNodeRefsByType(
|
||||
type: string,
|
||||
includeSubgraph: boolean = false
|
||||
): Promise<NodeReference[]> {
|
||||
return Promise.all(
|
||||
(
|
||||
await this.page.evaluate((type) => {
|
||||
return window['app'].graph.nodes
|
||||
.filter((n: LGraphNode) => n.type === type)
|
||||
.map((n: LGraphNode) => n.id)
|
||||
}, type)
|
||||
await this.page.evaluate(
|
||||
({ type, includeSubgraph }) => {
|
||||
const graph = (
|
||||
includeSubgraph ? window['app'].canvas.graph : window['app'].graph
|
||||
) as LGraph
|
||||
const nodes = graph.nodes
|
||||
return nodes
|
||||
.filter((n: LGraphNode) => n.type === type)
|
||||
.map((n: LGraphNode) => n.id)
|
||||
},
|
||||
{ type, includeSubgraph }
|
||||
)
|
||||
).map((id: NodeId) => this.getNodeRefById(id))
|
||||
)
|
||||
}
|
||||
|
||||
@@ -163,4 +163,14 @@ export class VueNodeHelpers {
|
||||
incrementButton: widget.getByTestId('increment')
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Enter the subgraph of a node.
|
||||
* @param nodeId - The ID of the node to enter the subgraph of. If not provided, the first matched subgraph will be entered.
|
||||
*/
|
||||
async enterSubgraph(nodeId?: string): Promise<void> {
|
||||
const locator = nodeId ? this.getNodeLocator(nodeId) : this.page
|
||||
const editButton = locator.getByTestId('subgraph-enter-button')
|
||||
await editButton.click()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -102,7 +102,7 @@ test.describe('Vue Node Link Interaction', () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
|
||||
await comfyPage.setSetting('Comfy.VueNodes.Enabled', true)
|
||||
await comfyPage.setup()
|
||||
// await comfyPage.setup()
|
||||
await comfyPage.loadWorkflow('vueNodes/simple-triple')
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
await fitToViewInstant(comfyPage)
|
||||
@@ -993,4 +993,51 @@ test.describe('Vue Node Link Interaction', () => {
|
||||
expect(linked).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
test('Dragging from subgraph input connects to correct slot', async ({
|
||||
comfyPage,
|
||||
comfyMouse
|
||||
}) => {
|
||||
// Setup workflow with a KSampler node
|
||||
await comfyPage.executeCommand('Comfy.NewBlankWorkflow')
|
||||
await comfyPage.waitForGraphNodes(0)
|
||||
await comfyPage.executeCommand('Workspace.SearchBox.Toggle')
|
||||
await comfyPage.nextFrame()
|
||||
await comfyPage.searchBox.fillAndSelectFirstNode('KSampler')
|
||||
await comfyPage.waitForGraphNodes(1)
|
||||
|
||||
// Convert the KSampler node to a subgraph
|
||||
let ksamplerNode = (await comfyPage.getNodeRefsByType('KSampler'))?.[0]
|
||||
await comfyPage.vueNodes.selectNode(String(ksamplerNode.id))
|
||||
await comfyPage.executeCommand('Comfy.Graph.ConvertToSubgraph')
|
||||
|
||||
// Enter the subgraph
|
||||
await comfyPage.vueNodes.enterSubgraph()
|
||||
await fitToViewInstant(comfyPage)
|
||||
|
||||
// Get the KSampler node inside the subgraph
|
||||
ksamplerNode = (await comfyPage.getNodeRefsByType('KSampler', true))?.[0]
|
||||
const positiveInput = await ksamplerNode.getInput(1)
|
||||
const negativeInput = await ksamplerNode.getInput(2)
|
||||
|
||||
const positiveInputPos = await getSlotCenter(
|
||||
comfyPage.page,
|
||||
ksamplerNode.id,
|
||||
1,
|
||||
true
|
||||
)
|
||||
|
||||
const sourceSlot = await comfyPage.getSubgraphInputSlot()
|
||||
const calculatedSourcePos = await sourceSlot.getOpenSlotPosition()
|
||||
|
||||
await comfyMouse.move(calculatedSourcePos)
|
||||
await comfyMouse.drag(positiveInputPos)
|
||||
await comfyMouse.drop()
|
||||
|
||||
// Verify connection went to the correct slot
|
||||
const positiveLinks = await positiveInput.getLinkCount()
|
||||
const negativeLinks = await negativeInput.getLinkCount()
|
||||
expect(positiveLinks).toBe(1)
|
||||
expect(negativeLinks).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
Binary file not shown.
|
Before Width: | Height: | Size: 60 KiB After Width: | Height: | Size: 60 KiB |
Binary file not shown.
|
Before Width: | Height: | Size: 61 KiB After Width: | Height: | Size: 61 KiB |
@@ -1,8 +1,6 @@
|
||||
import { LGraphNodeProperties } from '@/lib/litegraph/src/LGraphNodeProperties'
|
||||
import {
|
||||
calculateInputSlotPos,
|
||||
calculateInputSlotPosFromSlot,
|
||||
calculateOutputSlotPos,
|
||||
getSlotPosition
|
||||
} from '@/renderer/core/canvas/litegraph/slotCalculations'
|
||||
import type { SlotPositionContext } from '@/renderer/core/canvas/litegraph/slotCalculations'
|
||||
@@ -3349,7 +3347,7 @@ export class LGraphNode
|
||||
* @returns Position of the input slot
|
||||
*/
|
||||
getInputPos(slot: number): Point {
|
||||
return calculateInputSlotPos(this.#getSlotPositionContext(), slot)
|
||||
return getSlotPosition(this, slot, true)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -3369,10 +3367,7 @@ export class LGraphNode
|
||||
* @returns Position of the output slot
|
||||
*/
|
||||
getOutputPos(outputSlotIndex: number): Point {
|
||||
return calculateOutputSlotPos(
|
||||
this.#getSlotPositionContext(),
|
||||
outputSlotIndex
|
||||
)
|
||||
return getSlotPosition(this, outputSlotIndex, false)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -45,7 +45,7 @@ export interface SlotPositionContext {
|
||||
* @param slot The input slot index
|
||||
* @returns Position of the input slot center in graph coordinates
|
||||
*/
|
||||
export function calculateInputSlotPos(
|
||||
function calculateInputSlotPos(
|
||||
context: SlotPositionContext,
|
||||
slot: number
|
||||
): Point {
|
||||
@@ -93,7 +93,7 @@ export function calculateInputSlotPosFromSlot(
|
||||
* @param slot The output slot index
|
||||
* @returns Position of the output slot center in graph coordinates
|
||||
*/
|
||||
export function calculateOutputSlotPos(
|
||||
function calculateOutputSlotPos(
|
||||
context: SlotPositionContext,
|
||||
slot: number
|
||||
): Point {
|
||||
@@ -138,38 +138,41 @@ export function getSlotPosition(
|
||||
slotIndex: number,
|
||||
isInput: boolean
|
||||
): Point {
|
||||
// Try to get precise position from slot layout (DOM-registered)
|
||||
const slotKey = getSlotKey(String(node.id), slotIndex, isInput)
|
||||
const slotLayout = layoutStore.getSlotLayout(slotKey)
|
||||
if (slotLayout) {
|
||||
return [slotLayout.position.x, slotLayout.position.y]
|
||||
}
|
||||
|
||||
// Fallback: derive position from node layout tree and slot model
|
||||
const nodeLayout = layoutStore.getNodeLayoutRef(String(node.id)).value
|
||||
|
||||
if (nodeLayout) {
|
||||
// Create context from layout tree data
|
||||
const context: SlotPositionContext = {
|
||||
nodeX: nodeLayout.position.x,
|
||||
nodeY: nodeLayout.position.y,
|
||||
nodeWidth: nodeLayout.size.width,
|
||||
nodeHeight: nodeLayout.size.height,
|
||||
collapsed: node.flags.collapsed || false,
|
||||
collapsedWidth: node._collapsed_width,
|
||||
slotStartY: node.constructor.slot_start_y,
|
||||
inputs: node.inputs,
|
||||
outputs: node.outputs,
|
||||
widgets: node.widgets
|
||||
// Only use DOM-registered slot positions when Vue nodes mode is enabled
|
||||
if (LiteGraph.vueNodesMode) {
|
||||
// Try to get precise position from slot layout (DOM-registered)
|
||||
const slotKey = getSlotKey(String(node.id), slotIndex, isInput)
|
||||
const slotLayout = layoutStore.getSlotLayout(slotKey)
|
||||
if (slotLayout) {
|
||||
return [slotLayout.position.x, slotLayout.position.y]
|
||||
}
|
||||
|
||||
// Use helper to calculate position
|
||||
return isInput
|
||||
? calculateInputSlotPos(context, slotIndex)
|
||||
: calculateOutputSlotPos(context, slotIndex)
|
||||
// Fallback: derive position from node layout tree and slot model
|
||||
const nodeLayout = layoutStore.getNodeLayoutRef(String(node.id)).value
|
||||
|
||||
if (nodeLayout) {
|
||||
// Create context from layout tree data
|
||||
const context: SlotPositionContext = {
|
||||
nodeX: nodeLayout.position.x,
|
||||
nodeY: nodeLayout.position.y,
|
||||
nodeWidth: nodeLayout.size.width,
|
||||
nodeHeight: nodeLayout.size.height,
|
||||
collapsed: node.flags.collapsed || false,
|
||||
collapsedWidth: node._collapsed_width,
|
||||
slotStartY: node.constructor.slot_start_y,
|
||||
inputs: node.inputs,
|
||||
outputs: node.outputs,
|
||||
widgets: node.widgets
|
||||
}
|
||||
|
||||
// Use helper to calculate position
|
||||
return isInput
|
||||
? calculateInputSlotPos(context, slotIndex)
|
||||
: calculateOutputSlotPos(context, slotIndex)
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback: calculate directly from node properties if layout not available
|
||||
// Fallback: calculate directly from node properties (legacy litegraph behavior)
|
||||
const context: SlotPositionContext = {
|
||||
nodeX: node.pos[0],
|
||||
nodeY: node.pos[1],
|
||||
|
||||
Reference in New Issue
Block a user