mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-25 08:49:36 +00:00
Fix: Vue-Litegraph conversion bug with Reroutes (#6330)
## Summary The private fields triggered an error in intitializing the linkLayoutSync. Turns out that wasn't necessary anymore. > [!NOTE] > Edit: Doing some more investigation, it looks like the slot sync can also be removed? ## Changes - **What**: Converts JS private fields to typescript private, adds some readonly declarations - **What**: Removes the useLinkLayoutSync usage in useVueNodeLifecycle - **What**: Removes the useSlotLayoutSync usage in useVueNodeLifecycle ## Review Focus Was the sync doing something that wouldn't be caught in normal usage/testing? ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6330-Fix-Vue-Litegraph-conversion-bug-with-Reroutes-2996d73d3650819ebb24e4aa2fc51c65) by [Unito](https://www.unito.io)
This commit is contained in:
Alexander Brown
@@ -1,75 +0,0 @@
|
||||
/**
|
||||
* Slot Registration
|
||||
*
|
||||
* Handles registration of slot layouts with the layout store for hit testing.
|
||||
* This module manages the state mutation side of slot layout management,
|
||||
* while pure calculations are handled separately in SlotCalculations.ts.
|
||||
*/
|
||||
import type { Point } from '@/lib/litegraph/src/interfaces'
|
||||
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import {
|
||||
calculateInputSlotPos,
|
||||
calculateOutputSlotPos
|
||||
} from '@/renderer/core/canvas/litegraph/slotCalculations'
|
||||
import type { SlotPositionContext } from '@/renderer/core/canvas/litegraph/slotCalculations'
|
||||
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
import type { SlotLayout } from '@/renderer/core/layout/types'
|
||||
|
||||
import { getSlotKey } from './slotIdentifier'
|
||||
|
||||
/**
|
||||
* Register slot layout with the layout store for hit testing
|
||||
* @param nodeId The node ID
|
||||
* @param slotIndex The slot index
|
||||
* @param isInput Whether this is an input slot
|
||||
* @param position The slot position in graph coordinates
|
||||
*/
|
||||
function registerSlotLayout(
|
||||
nodeId: string,
|
||||
slotIndex: number,
|
||||
isInput: boolean,
|
||||
position: Point
|
||||
): void {
|
||||
const slotKey = getSlotKey(nodeId, slotIndex, isInput)
|
||||
|
||||
// Calculate bounds for the slot using LiteGraph's standard slot height
|
||||
const slotSize = LiteGraph.NODE_SLOT_HEIGHT
|
||||
const halfSize = slotSize / 2
|
||||
|
||||
const slotLayout: SlotLayout = {
|
||||
nodeId,
|
||||
index: slotIndex,
|
||||
type: isInput ? 'input' : 'output',
|
||||
position: { x: position[0], y: position[1] },
|
||||
bounds: {
|
||||
x: position[0] - halfSize,
|
||||
y: position[1] - halfSize,
|
||||
width: slotSize,
|
||||
height: slotSize
|
||||
}
|
||||
}
|
||||
|
||||
layoutStore.updateSlotLayout(slotKey, slotLayout)
|
||||
}
|
||||
|
||||
/**
|
||||
* Register all slots for a node
|
||||
* @param nodeId The node ID
|
||||
* @param context The slot position context
|
||||
*/
|
||||
export function registerNodeSlots(
|
||||
nodeId: string,
|
||||
context: SlotPositionContext
|
||||
): void {
|
||||
// Register input slots
|
||||
context.inputs.forEach((_, index) => {
|
||||
const position = calculateInputSlotPos(context, index)
|
||||
registerSlotLayout(nodeId, index, true, position)
|
||||
})
|
||||
|
||||
// Register output slots
|
||||
context.outputs.forEach((_, index) => {
|
||||
const position = calculateOutputSlotPos(context, index)
|
||||
registerSlotLayout(nodeId, index, false, position)
|
||||
})
|
||||
}
|
||||
@@ -1,310 +0,0 @@
|
||||
import { tryOnScopeDispose } from '@vueuse/core'
|
||||
import { computed, ref, toValue } from 'vue'
|
||||
|
||||
import type { LGraphCanvas } from '@/lib/litegraph/src/LGraphCanvas'
|
||||
import { LLink } from '@/lib/litegraph/src/LLink'
|
||||
import { Reroute } from '@/lib/litegraph/src/Reroute'
|
||||
import type { Point } from '@/lib/litegraph/src/interfaces'
|
||||
import { LinkDirection } from '@/lib/litegraph/src/types/globalEnums'
|
||||
import { LitegraphLinkAdapter } from '@/renderer/core/canvas/litegraph/litegraphLinkAdapter'
|
||||
import type { LinkRenderContext } from '@/renderer/core/canvas/litegraph/litegraphLinkAdapter'
|
||||
import { getSlotPosition } from '@/renderer/core/canvas/litegraph/slotCalculations'
|
||||
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
import type { LayoutChange } from '@/renderer/core/layout/types'
|
||||
|
||||
export function useLinkLayoutSync() {
|
||||
const canvasRef = ref<LGraphCanvas>()
|
||||
const graphRef = computed(() => canvasRef.value?.graph)
|
||||
const unsubscribeLayoutChange = ref<() => void>()
|
||||
const adapter = new LitegraphLinkAdapter()
|
||||
|
||||
/**
|
||||
* Build link render context from canvas properties
|
||||
*/
|
||||
function buildLinkRenderContext(): LinkRenderContext {
|
||||
const canvas = toValue(canvasRef)
|
||||
if (!canvas) {
|
||||
throw new Error('Canvas not initialized')
|
||||
}
|
||||
|
||||
return {
|
||||
// Canvas settings
|
||||
renderMode: canvas.links_render_mode,
|
||||
connectionWidth: canvas.connections_width,
|
||||
renderBorder: canvas.render_connections_border,
|
||||
lowQuality: canvas.low_quality,
|
||||
highQualityRender: canvas.highquality_render,
|
||||
scale: canvas.ds.scale,
|
||||
linkMarkerShape: canvas.linkMarkerShape,
|
||||
renderConnectionArrows: canvas.render_connection_arrows,
|
||||
|
||||
// State
|
||||
highlightedLinks: new Set(Object.keys(canvas.highlighted_links)),
|
||||
|
||||
// Colors
|
||||
defaultLinkColor: canvas.default_link_color,
|
||||
linkTypeColors: (canvas.constructor as any).link_type_colors || {},
|
||||
|
||||
// Pattern for disabled links
|
||||
disabledPattern: canvas._pattern
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recompute a single link and all its segments
|
||||
*
|
||||
* Note: This logic mirrors LGraphCanvas#renderAllLinkSegments but:
|
||||
* - Works with offscreen context for event-driven updates
|
||||
* - No visibility checks (always computes full geometry)
|
||||
* - No dragging state handling (pure geometry computation)
|
||||
*/
|
||||
function recomputeLinkById(linkId: number): void {
|
||||
const canvas = toValue(canvasRef)
|
||||
const graph = toValue(graphRef)
|
||||
if (!graph || !canvas) return
|
||||
|
||||
const link = graph.links.get(linkId)
|
||||
if (!link || link.id === -1) return // Skip floating/temp links
|
||||
|
||||
// Get source and target nodes
|
||||
const sourceNode = graph.getNodeById(link.origin_id)
|
||||
const targetNode = graph.getNodeById(link.target_id)
|
||||
if (!sourceNode || !targetNode) return
|
||||
|
||||
// Get slots
|
||||
const sourceSlot = sourceNode.outputs?.[link.origin_slot]
|
||||
const targetSlot = targetNode.inputs?.[link.target_slot]
|
||||
if (!sourceSlot || !targetSlot) return
|
||||
|
||||
// Get positions
|
||||
const startPos = getSlotPosition(sourceNode, link.origin_slot, false)
|
||||
const endPos = getSlotPosition(targetNode, link.target_slot, true)
|
||||
|
||||
// Get directions
|
||||
const startDir = sourceSlot.dir || LinkDirection.RIGHT
|
||||
const endDir = targetSlot.dir || LinkDirection.LEFT
|
||||
|
||||
// Get reroutes for this link
|
||||
const reroutes = LLink.getReroutes(graph, link)
|
||||
|
||||
// Build render context
|
||||
const context = buildLinkRenderContext()
|
||||
|
||||
if (reroutes.length > 0) {
|
||||
// Render segmented link with reroutes
|
||||
let segmentStartPos = startPos
|
||||
let segmentStartDir = startDir
|
||||
|
||||
for (let i = 0; i < reroutes.length; i++) {
|
||||
const reroute = reroutes[i]
|
||||
|
||||
// Calculate reroute angle
|
||||
reroute.calculateAngle(Date.now(), graph, [
|
||||
segmentStartPos[0],
|
||||
segmentStartPos[1]
|
||||
])
|
||||
|
||||
// Calculate control points
|
||||
const distance = Math.sqrt(
|
||||
(reroute.pos[0] - segmentStartPos[0]) ** 2 +
|
||||
(reroute.pos[1] - segmentStartPos[1]) ** 2
|
||||
)
|
||||
const dist = Math.min(Reroute.maxSplineOffset, distance * 0.25)
|
||||
|
||||
// Special handling for floating input chain
|
||||
const isFloatingInputChain = !sourceNode && targetNode
|
||||
const startControl: Readonly<Point> = isFloatingInputChain
|
||||
? [0, 0]
|
||||
: [dist * reroute.cos, dist * reroute.sin]
|
||||
|
||||
// Render segment to this reroute
|
||||
adapter.renderLinkDirect(
|
||||
canvas.ctx,
|
||||
segmentStartPos,
|
||||
reroute.pos,
|
||||
link,
|
||||
true, // skip_border
|
||||
0, // flow
|
||||
null, // color
|
||||
segmentStartDir,
|
||||
LinkDirection.CENTER,
|
||||
context,
|
||||
{
|
||||
startControl,
|
||||
endControl: reroute.controlPoint,
|
||||
reroute,
|
||||
disabled: false
|
||||
}
|
||||
)
|
||||
|
||||
// Prepare for next segment
|
||||
segmentStartPos = reroute.pos
|
||||
segmentStartDir = LinkDirection.CENTER
|
||||
}
|
||||
|
||||
// Render final segment from last reroute to target
|
||||
const lastReroute = reroutes[reroutes.length - 1]
|
||||
const finalDistance = Math.sqrt(
|
||||
(endPos[0] - lastReroute.pos[0]) ** 2 +
|
||||
(endPos[1] - lastReroute.pos[1]) ** 2
|
||||
)
|
||||
const finalDist = Math.min(Reroute.maxSplineOffset, finalDistance * 0.25)
|
||||
const finalStartControl: Readonly<Point> = [
|
||||
finalDist * lastReroute.cos,
|
||||
finalDist * lastReroute.sin
|
||||
]
|
||||
|
||||
adapter.renderLinkDirect(
|
||||
canvas.ctx,
|
||||
lastReroute.pos,
|
||||
endPos,
|
||||
link,
|
||||
true, // skip_border
|
||||
0, // flow
|
||||
null, // color
|
||||
LinkDirection.CENTER,
|
||||
endDir,
|
||||
context,
|
||||
{
|
||||
startControl: finalStartControl,
|
||||
disabled: false
|
||||
}
|
||||
)
|
||||
} else {
|
||||
// No reroutes - render direct link
|
||||
adapter.renderLinkDirect(
|
||||
canvas.ctx,
|
||||
startPos,
|
||||
endPos,
|
||||
link,
|
||||
true, // skip_border
|
||||
0, // flow
|
||||
null, // color
|
||||
startDir,
|
||||
endDir,
|
||||
context,
|
||||
{
|
||||
disabled: false
|
||||
}
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recompute all links connected to a node
|
||||
*/
|
||||
function recomputeLinksForNode(nodeId: number): void {
|
||||
const graph = toValue(graphRef)
|
||||
if (!graph) return
|
||||
|
||||
const node = graph.getNodeById(nodeId)
|
||||
if (!node) return
|
||||
|
||||
const linkIds = new Set<number>()
|
||||
|
||||
// Collect output links
|
||||
if (node.outputs) {
|
||||
for (const output of node.outputs) {
|
||||
if (output.links) {
|
||||
for (const linkId of output.links) {
|
||||
linkIds.add(linkId)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Collect input links
|
||||
if (node.inputs) {
|
||||
for (const input of node.inputs) {
|
||||
if (input.link !== null && input.link !== undefined) {
|
||||
linkIds.add(input.link)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Recompute each link
|
||||
for (const linkId of linkIds) {
|
||||
recomputeLinkById(linkId)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recompute all links associated with a reroute
|
||||
*/
|
||||
function recomputeLinksForReroute(rerouteId: number): void {
|
||||
const graph = toValue(graphRef)
|
||||
if (!graph) return
|
||||
|
||||
const reroute = graph.reroutes.get(rerouteId)
|
||||
if (!reroute) return
|
||||
|
||||
// Recompute all links that pass through this reroute
|
||||
for (const linkId of reroute.linkIds) {
|
||||
recomputeLinkById(linkId)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Start link layout sync with event-driven functionality
|
||||
*/
|
||||
function start(canvasInstance: LGraphCanvas): void {
|
||||
canvasRef.value = canvasInstance
|
||||
if (!canvasInstance.graph) return
|
||||
|
||||
// Initial computation for all existing links
|
||||
for (const link of canvasInstance.graph._links.values()) {
|
||||
if (link.id !== -1) {
|
||||
recomputeLinkById(link.id)
|
||||
}
|
||||
}
|
||||
|
||||
// Subscribe to layout store changes
|
||||
unsubscribeLayoutChange.value?.()
|
||||
unsubscribeLayoutChange.value = layoutStore.onChange(
|
||||
(change: LayoutChange) => {
|
||||
switch (change.operation.type) {
|
||||
case 'moveNode':
|
||||
case 'resizeNode':
|
||||
recomputeLinksForNode(parseInt(change.operation.nodeId))
|
||||
break
|
||||
case 'batchUpdateBounds':
|
||||
for (const nodeId of change.operation.nodeIds) {
|
||||
recomputeLinksForNode(parseInt(nodeId))
|
||||
}
|
||||
break
|
||||
case 'createLink':
|
||||
recomputeLinkById(change.operation.linkId)
|
||||
break
|
||||
case 'deleteLink':
|
||||
// No-op - store already cleaned by existing code
|
||||
break
|
||||
case 'createReroute':
|
||||
case 'deleteReroute':
|
||||
// Recompute all affected links
|
||||
if ('linkIds' in change.operation) {
|
||||
for (const linkId of change.operation.linkIds) {
|
||||
recomputeLinkById(linkId)
|
||||
}
|
||||
}
|
||||
break
|
||||
case 'moveReroute':
|
||||
recomputeLinksForReroute(change.operation.rerouteId)
|
||||
break
|
||||
}
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
function stop(): void {
|
||||
unsubscribeLayoutChange.value?.()
|
||||
unsubscribeLayoutChange.value = undefined
|
||||
canvasRef.value = undefined
|
||||
}
|
||||
|
||||
tryOnScopeDispose(stop)
|
||||
|
||||
return {
|
||||
start,
|
||||
stop
|
||||
}
|
||||
}
|
||||
@@ -1,150 +0,0 @@
|
||||
import { tryOnScopeDispose } from '@vueuse/core'
|
||||
import { ref } from 'vue'
|
||||
|
||||
import type { LGraphCanvas } from '@/lib/litegraph/src/LGraphCanvas'
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import type { SlotPositionContext } from '@/renderer/core/canvas/litegraph/slotCalculations'
|
||||
import { registerNodeSlots } from '@/renderer/core/layout/slots/register'
|
||||
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
|
||||
function computeAndRegisterSlots(node: LGraphNode): void {
|
||||
const nodeId = String(node.id)
|
||||
const nodeLayout = layoutStore.getNodeLayoutRef(nodeId).value
|
||||
|
||||
// Fallback to live node values if layout not ready
|
||||
const nodeX = nodeLayout?.position.x ?? node.pos[0]
|
||||
const nodeY = nodeLayout?.position.y ?? node.pos[1]
|
||||
const nodeWidth = nodeLayout?.size.width ?? node.size[0]
|
||||
const nodeHeight = nodeLayout?.size.height ?? node.size[1]
|
||||
|
||||
// Ensure concrete slots & arrange when needed for accurate positions
|
||||
node._setConcreteSlots()
|
||||
const collapsed = node.flags.collapsed ?? false
|
||||
if (!collapsed) {
|
||||
node.arrange()
|
||||
}
|
||||
|
||||
const context: SlotPositionContext = {
|
||||
nodeX,
|
||||
nodeY,
|
||||
nodeWidth,
|
||||
nodeHeight,
|
||||
collapsed,
|
||||
collapsedWidth: node._collapsed_width,
|
||||
slotStartY: node.constructor.slot_start_y,
|
||||
inputs: node.inputs,
|
||||
outputs: node.outputs,
|
||||
widgets: node.widgets
|
||||
}
|
||||
|
||||
registerNodeSlots(nodeId, context)
|
||||
}
|
||||
|
||||
export function useSlotLayoutSync() {
|
||||
const unsubscribeLayoutChange = ref<() => void>()
|
||||
const restoreHandlers = ref<() => void>()
|
||||
|
||||
/**
|
||||
* Attempt to start slot layout sync with full event-driven functionality
|
||||
* @param canvas LiteGraph canvas instance
|
||||
* @returns true if sync was actually started, false if early-returned
|
||||
*/
|
||||
function attemptStart(canvas: LGraphCanvas): boolean {
|
||||
// When Vue nodes are enabled, slot DOM registers exact positions.
|
||||
// Skip calculated registration to avoid conflicts.
|
||||
if (LiteGraph.vueNodesMode) {
|
||||
return false
|
||||
}
|
||||
const graph = canvas?.graph
|
||||
if (!graph) return false
|
||||
|
||||
// Initial registration for all nodes in the current graph
|
||||
for (const node of graph.nodes) {
|
||||
computeAndRegisterSlots(node)
|
||||
}
|
||||
|
||||
// Layout changes → recompute slots for changed nodes
|
||||
unsubscribeLayoutChange.value?.()
|
||||
unsubscribeLayoutChange.value = layoutStore.onChange((change) => {
|
||||
for (const nodeId of change.nodeIds) {
|
||||
const node = graph.getNodeById(parseInt(nodeId))
|
||||
if (node) {
|
||||
computeAndRegisterSlots(node)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
// LiteGraph event hooks
|
||||
const origNodeAdded = graph.onNodeAdded
|
||||
const origNodeRemoved = graph.onNodeRemoved
|
||||
const origTrigger = graph.onTrigger
|
||||
const origAfterChange = graph.onAfterChange
|
||||
|
||||
graph.onNodeAdded = (node: LGraphNode) => {
|
||||
computeAndRegisterSlots(node)
|
||||
if (origNodeAdded) {
|
||||
origNodeAdded.call(graph, node)
|
||||
}
|
||||
}
|
||||
|
||||
graph.onNodeRemoved = (node: LGraphNode) => {
|
||||
layoutStore.deleteNodeSlotLayouts(String(node.id))
|
||||
if (origNodeRemoved) {
|
||||
origNodeRemoved.call(graph, node)
|
||||
}
|
||||
}
|
||||
|
||||
graph.onTrigger = (event) => {
|
||||
if (
|
||||
event.type === 'node:property:changed' &&
|
||||
event.property === 'flags.collapsed'
|
||||
) {
|
||||
const node = graph.getNodeById(parseInt(String(event.nodeId)))
|
||||
if (node) {
|
||||
computeAndRegisterSlots(node)
|
||||
}
|
||||
}
|
||||
|
||||
// Chain to original handler
|
||||
origTrigger?.(event)
|
||||
}
|
||||
|
||||
graph.onAfterChange = (graph: any, node?: any) => {
|
||||
if (node && node.id) {
|
||||
computeAndRegisterSlots(node)
|
||||
}
|
||||
if (origAfterChange) {
|
||||
origAfterChange.call(graph, graph, node)
|
||||
}
|
||||
}
|
||||
|
||||
// Store cleanup function
|
||||
restoreHandlers.value = () => {
|
||||
graph.onNodeAdded = origNodeAdded || undefined
|
||||
graph.onNodeRemoved = origNodeRemoved || undefined
|
||||
// Only restore onTrigger if Vue nodes are not active
|
||||
// Vue node manager sets its own onTrigger handler
|
||||
if (!LiteGraph.vueNodesMode) {
|
||||
graph.onTrigger = origTrigger || undefined
|
||||
}
|
||||
graph.onAfterChange = origAfterChange || undefined
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
function stop(): void {
|
||||
unsubscribeLayoutChange.value?.()
|
||||
unsubscribeLayoutChange.value = undefined
|
||||
restoreHandlers.value?.()
|
||||
restoreHandlers.value = undefined
|
||||
}
|
||||
|
||||
tryOnScopeDispose(stop)
|
||||
|
||||
return {
|
||||
attemptStart,
|
||||
stop
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user