From 4078e3ad8b7df08da7fa5032590719c42c37934a Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Sat, 22 Mar 2025 05:47:36 +1100 Subject: [PATCH] Improve reroutes - prep for LinkConnector overhaul (#810) - Fixes TS types - Various bug fixes for reroute / link (re)connect - Adds reroute snap circle when connecting links - Validates reroutes on drop (part of larger work) - Prevent nodes linking to themselves --- src/LGraph.ts | 5 +- src/LGraphCanvas.ts | 9 +-- src/LLink.ts | 20 ++++++ src/Reroute.ts | 2 +- src/canvas/LinkConnector.ts | 123 +++++++++++++++++++++++++++++------- 5 files changed, 127 insertions(+), 32 deletions(-) diff --git a/src/LGraph.ts b/src/LGraph.ts index dd4853465..247d33856 100644 --- a/src/LGraph.ts +++ b/src/LGraph.ts @@ -1393,9 +1393,8 @@ export class LGraph implements LinkNetwork, Serialisable { delete reroute.floating } - if (reroute.floatingLinkIds.size === 0 && reroute.linkIds.size === 0) { - this.removeReroute(reroute.id) - } + const totalLinks = reroute.floatingLinkIds.size + reroute.linkIds.size + if (totalLinks === 0) this.removeReroute(reroute.id) } } diff --git a/src/LGraphCanvas.ts b/src/LGraphCanvas.ts index 46ea6e00b..dba93a1e6 100644 --- a/src/LGraphCanvas.ts +++ b/src/LGraphCanvas.ts @@ -2667,16 +2667,13 @@ export class LGraphCanvas implements ConnectionColorContext { underPointer |= CanvasItem.ResizeSe } } else { + // Reroute const reroute = graph.getRerouteOnPos(e.canvasX, e.canvasY) if (reroute) { underPointer |= CanvasItem.Reroute - if (linkConnector.isConnecting) { - const firstSlot = linkConnector.renderLinks[0]?.fromSlot - const rerouteType = reroute.firstLink?.type ?? reroute.firstFloatingLink?.type - if (firstSlot && rerouteType != null && LiteGraph.isValidConnection(rerouteType, firstSlot.type)) { - this._highlight_pos = reroute.pos - } + if (linkConnector.isConnecting && linkConnector.isRerouteValidDrop(reroute)) { + this._highlight_pos = reroute.pos } } else { this._highlight_pos &&= undefined diff --git a/src/LLink.ts b/src/LLink.ts index 3532cbc09..ac2829456 100644 --- a/src/LLink.ts +++ b/src/LLink.ts @@ -163,6 +163,26 @@ export class LLink implements LinkSegment, Serialisable { } } + /** + * Checks if the specified node id and output index are this link's origin (output side). + * @param nodeId ID of the node to check + * @param outputIndex The array index of the node output + * @returns `true` if the origin matches, otherwise `false`. + */ + hasOrigin(nodeId: NodeId, outputIndex: number): boolean { + return this.origin_id === nodeId && this.origin_slot === outputIndex + } + + /** + * Checks if the specified node id and input index are this link's target (input side). + * @param nodeId ID of the node to check + * @param inputIndex The array index of the node input + * @returns `true` if the target matches, otherwise `false`. + */ + hasTarget(nodeId: NodeId, inputIndex: number): boolean { + return this.target_id === nodeId && this.target_slot === inputIndex + } + /** * Disconnects a link and removes it from the graph, cleaning up any reroutes that are no longer used * @param network The container (LGraph) where reroutes should be updated diff --git a/src/Reroute.ts b/src/Reroute.ts index 21917c3de..fe03b6f12 100644 --- a/src/Reroute.ts +++ b/src/Reroute.ts @@ -203,7 +203,7 @@ export class Reroute implements Positionable, LinkSegment, Serialisable()): Reroute[] | null | undefined { + getReroutes(visited = new Set()): Reroute[] | null { // No parentId - last in the chain if (this.#parentId === undefined) return [this] // Invalid chain - looped diff --git a/src/canvas/LinkConnector.ts b/src/canvas/LinkConnector.ts index 8176987db..85a55ed9f 100644 --- a/src/canvas/LinkConnector.ts +++ b/src/canvas/LinkConnector.ts @@ -1,5 +1,5 @@ import type { RenderLink } from "./RenderLink" -import type { ConnectingLink, ItemLocator, LinkNetwork, LinkSegment } from "@/interfaces" +import type { ConnectingLink, ISlotType, ItemLocator, LinkNetwork, LinkSegment } from "@/interfaces" import type { INodeInputSlot, INodeOutputSlot } from "@/interfaces" import type { LGraphNode } from "@/LGraphNode" import type { Reroute } from "@/Reroute" @@ -7,6 +7,7 @@ import type { CanvasPointerEvent } from "@/types/events" import type { IWidget } from "@/types/widgets" import { LinkConnectorEventMap, LinkConnectorEventTarget } from "@/infrastructure/LinkConnectorEventTarget" +import { LiteGraph } from "@/litegraph" import { LLink } from "@/LLink" import { LinkDirection } from "@/types/globalEnums" @@ -274,7 +275,7 @@ export class LinkConnector { // Get reroute if no node is found const reroute = locator.getRerouteOnPos(canvasX, canvasY) // Drop output->input link on reroute is not impl. - if (reroute) { + if (reroute && this.isRerouteValidDrop(reroute)) { this.dropOnReroute(reroute, event) } else { this.dropOnNothing(event) @@ -325,6 +326,7 @@ export class LinkConnector { const mayContinue = this.events.dispatch("dropped-on-reroute", { reroute, event }) if (mayContinue === false) return + // Connecting to input if (this.state.connectingTo === "input") { const results = reroute.findTargetInputs() if (!results?.length) return @@ -332,37 +334,26 @@ export class LinkConnector { for (const { node: inputNode, input, link: resultLink } of results) { for (const renderLink of this.renderLinks) { if (renderLink.toType !== "input") continue + if (!canConnectInputLinkToReroute(renderLink, input, reroute)) continue if (renderLink instanceof MovingRenderLink) { - const { outputNode, inputSlot, outputSlot, fromReroute } = renderLink - // Link is already connected here - if (inputSlot === input) continue + const { outputNode, outputSlot, fromReroute } = renderLink const newLink = outputNode.connectSlots(outputSlot, inputNode, input, fromReroute?.id) if (newLink) this.events.dispatch("input-moved", renderLink) } else { const { node: outputNode, fromSlot, fromReroute } = renderLink - // Connect to yourself - if (fromReroute?.id === reroute.id) return - // Identical link - if (fromReroute?.id != null && fromReroute.id === reroute.parentId) return const reroutes = reroute.getReroutes() if (reroutes === null) throw new Error("Reroute loop detected.") - if (reroutes && fromReroute?.id != null) { - for (const r of reroutes) { - if (r.id === fromReroute.id) break - if (r.id === reroute.id) { - throw new Error("Cannot connect to reroute that is a parent of the reroute being connected to.") - } - } - } - + // Clean up reroutes if (reroutes) { for (const reroute of reroutes.slice(0, -1).reverse()) { if (reroute.id === fromReroute?.id) break - reroute.remove() + + const totalLinks = reroute.linkIds.size + reroute.floatingLinkIds.size + if (totalLinks === 1) reroute.remove() } } // Set the parentId of the reroute we dropped on, to the reroute we dragged from @@ -377,6 +368,7 @@ export class LinkConnector { return } + // Connecting to output for (const link of this.renderLinks) { if (link.toType !== "output") continue @@ -384,11 +376,10 @@ export class LinkConnector { if (!result) return const { node, output } = result + if (!isValidConnectionToOutput(link, output)) continue if (link instanceof MovingRenderLink) { - const { inputNode, inputSlot, outputSlot, fromReroute } = link - // Link is already connected here - if (outputSlot === output) continue + const { inputNode, inputSlot, fromReroute } = link // Connect the first reroute of the link being dragged to the reroute being dropped on if (fromReroute) { @@ -499,6 +490,8 @@ export class LinkConnector { if (newLink) this.events.dispatch("input-moved", link) } else { const { node: outputNode, fromSlot, fromReroute } = link + if (node === outputNode) continue + const newLink = outputNode.connectSlots(fromSlot, node, input, fromReroute?.id) this.events.dispatch("link-created", newLink) } @@ -519,12 +512,43 @@ export class LinkConnector { this.events.dispatch("output-moved", link) } else { const { node: inputNode, fromSlot, fromReroute } = link + if (inputNode) continue + const newLink = node.connectSlots(output, inputNode, fromSlot, fromReroute?.id) this.events.dispatch("link-created", newLink) } } } + /** + * Checks if a reroute is a valid drop target for any of the links being connected. + * @param reroute The reroute that would be dropped on. + * @returns `true` if any of the current links being connected are valid for the given reroute. + */ + isRerouteValidDrop(reroute: Reroute): boolean { + if (this.state.connectingTo === "input") { + const results = reroute.findTargetInputs() + if (!results?.length) return false + + for (const { input } of results) { + for (const renderLink of this.renderLinks) { + if (renderLink.toType !== "input") continue + if (canConnectInputLinkToReroute(renderLink, input, reroute)) return true + } + } + } else { + const output = reroute.findSourceOutput()?.output + if (!output) return false + + for (const renderLink of this.renderLinks) { + if (renderLink.toType !== "output") continue + if (isValidConnectionToOutput(renderLink, output)) return true + } + } + + return false + } + /** Sets connecting_links, used by some extensions still. */ #setLegacyLinks(fromSlotIsInput: boolean): void { const links = this.renderLinks.map((link) => { @@ -600,3 +624,58 @@ export class LinkConnector { state.draggingExistingLinks = false } } + +function isValidConnectionToOutput(link: ToOutputRenderLink | MovingRenderLink, output: INodeOutputSlot): boolean { + if (link instanceof MovingRenderLink) { + const { inputSlot: { type }, outputSlot } = link + + // Link is already connected here / type mismatch + if (outputSlot === output || !LiteGraph.isValidConnection(type, output.type)) { + return false + } + } else { + const { fromSlot: { type } } = link + if (!LiteGraph.isValidConnection(type, output.type)) return false + } + return true +} + +/** Validates that a single {@link RenderLink} can be dropped on the specified reroute. */ +function canConnectInputLinkToReroute(link: ToInputRenderLink | MovingRenderLink, input: INodeInputSlot, reroute: Reroute): boolean { + if (link instanceof MovingRenderLink) { + const { inputSlot, outputSlot, fromReroute } = link + + // Link is already connected here + if (inputSlot === input || validate(outputSlot.type, reroute, fromReroute)) { + return false + } + } else { + const { fromSlot, fromReroute } = link + + // Connect to yourself + if (fromReroute?.id === reroute.id || validate(fromSlot.type, reroute, fromReroute)) { + return false + } + + // Link would make no change - output to reroute + if ( + reroute.parentId == null && + reroute.firstLink?.hasOrigin(link.node.id, link.fromSlotIndex) + ) { + return false + } + } + return true + + /** Checks connection type & rejects infinite loops. */ + function validate(type: ISlotType, reroute: Reroute, fromReroute?: Reroute): boolean { + return Boolean( + // Link would make no changes + (fromReroute?.id != null && fromReroute.id === reroute.parentId) || + // Type mismatch + !LiteGraph.isValidConnection(type, input.type) || + // Cannot connect from child to parent reroute + fromReroute?.getReroutes()?.includes(reroute), + ) + } +}