Subgraph io fixes (#12281)

Fixes 3 different bugs when making links to and from subgraph IO from
vue nodes
- When dragging a link from a node to a subgraph IO, there is no
feedback if a slot is not a valid connection target or if a slot is
actively hovered
- When a link is made from a subgraph IO to a node, the reactivity is
not triggered on the node to indicate a change of link state.
- When dragging a link from a subgraph IO to a node, the link would not
snap to the valid connection targets on nodes
- The fix for this one is not as thorough as I would like. It only
allows connections to the slot, not connections to the hovered widget.
We have two deeply disconnected linking systems and properly reconciling
them would be a multi-week project.

Resolves FE-561

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12281-Subgraph-io-fixes-3606d73d365081089f7ef19331c6d70a)
by [Unito](https://www.unito.io)

Author: AustinMroz

browser_tests/fixtures/helpers/SubgraphHelper.ts

@@ -11,6 +11,7 @@ import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/w
 import type { ComfyPage } from '@e2e/fixtures/ComfyPage'
 import { SubgraphEditor } from '@e2e/fixtures/components/SubgraphEditor'
 import { TestIds } from '@e2e/fixtures/selectors'
+import type { Position, Size } from '@e2e/fixtures/types'
 import type { NodeReference } from '@e2e/fixtures/utils/litegraphUtils'
 import { SubgraphSlotReference } from '@e2e/fixtures/utils/litegraphUtils'
 
@@ -241,6 +242,17 @@ export class SubgraphHelper {
     return new SubgraphSlotReference('output', slotName || '', this.comfyPage)
   }
 
+  async getInputBounds(): Promise<Position & Size> {
+    return await this.comfyPage.page.evaluate(() => {
+      const graph = app!.canvas.graph as Subgraph
+      const inputNode = graph.inputNode
+      const [x, y] = app!.canvas.ds.convertOffsetToCanvas(inputNode.pos)
+      const width = inputNode.size[0] * app!.canvas.ds.scale
+      const height = inputNode.size[1] * app!.canvas.ds.scale
+      return { x, y, width, height }
+    })
+  }
+
   /**
    * Connect a regular node output to a subgraph input.
    * This creates a new input slot on the subgraph if targetInputName is not provided.

browser_tests/tests/subgraph/subgraphSlots.spec.ts

@@ -632,3 +632,72 @@ test.describe('Subgraph Slots', { tag: ['@slow', '@subgraph'] }, () => {
     })
   })
 })
+
+test(
+  'link interactions',
+  { tag: ['@vue-nodes', '@subgraph'] },
+  async ({ comfyPage }) => {
+    await comfyPage.workflow.loadWorkflow('subgraphs/basic-subgraph')
+    await comfyPage.vueNodes.enterSubgraph('2')
+
+    const ksampler = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
+    const seedSlot = ksampler.getSlot('seed')
+    const seedIOSlot = await comfyPage.subgraph.getInputSlot('seed')
+
+    await test.step('Make second INT typed connection', async () => {
+      const toPos = await seedIOSlot.getOpenSlotPosition()
+      await seedSlot.dragTo(comfyPage.canvas, { targetPosition: toPos })
+      const isConnected = () => comfyPage.vueNodes.isSlotConnected(seedSlot)
+      await expect.poll(isConnected).toBe(true)
+    })
+
+    const stepsSlot = ksampler.getSlot('steps')
+
+    await test.step('Node -> I/O hover effect', async () => {
+      await stepsSlot.hover()
+      await stepsSlot.click({ trial: true })
+      await comfyPage.page.mouse.down()
+      await comfyPage.canvas.hover({ position: await seedIOSlot.getPosition() })
+
+      const rawClip = await comfyPage.subgraph.getInputBounds()
+      const absolutePos = await comfyPage.canvasOps.toAbsolute(rawClip)
+      const clip = { ...rawClip, ...absolutePos }
+      await expect(comfyPage.page).toHaveScreenshot('vue-io-highlight.png', {
+        clip
+      })
+
+      //cancel link operation
+      await stepsSlot.hover()
+      await comfyPage.page.mouse.up()
+    })
+
+    await ksampler.title.hover()
+
+    const slotParent = stepsSlot.locator('../..')
+    await expect(slotParent, 'unconnected slot is hidden').toHaveCSS(
+      'opacity',
+      '0'
+    )
+
+    await test.step('Connect I/O to node with snap', async () => {
+      const hasSnap = () =>
+        comfyPage.page.evaluate(() => !!app!.canvas._highlight_pos)
+      expect(await hasSnap()).toBe(false)
+
+      const emptySlotPos = await seedIOSlot.getOpenSlotPosition()
+      await comfyPage.canvas.hover({ position: emptySlotPos })
+      await comfyPage.page.mouse.down()
+      await stepsSlot.hover()
+      await expect.poll(hasSnap).toBe(true)
+      await comfyPage.page.mouse.up()
+
+      //move hover off the slot
+      await ksampler.title.hover()
+    })
+
+    await expect(slotParent, 'connected slot is visible').not.toHaveCSS(
+      'opacity',
+      '0'
+    )
+  }
+)

browser_tests/tests/subgraph/subgraphSlots.spec.ts-snapshots/vue-io-highlight-chromium-linux.png

@@ -2,6 +2,7 @@ import { toString } from 'es-toolkit/compat'
 import { toValue } from 'vue'
 
 import { MovingInputLink } from '@/lib/litegraph/src/canvas/MovingInputLink'
+import type { RenderLink } from '@/lib/litegraph/src/canvas/RenderLink'
 import { AutoPanController } from '@/renderer/core/canvas/useAutoPan'
 import { LitegraphLinkAdapter } from '@/renderer/core/canvas/litegraph/litegraphLinkAdapter'
 import type { LinkRenderContext } from '@/renderer/core/canvas/litegraph/litegraphLinkAdapter'
@@ -3306,11 +3307,15 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
         if (result != null) this.dirty_canvas = result
       }
     }
+    const firstLink: RenderLink | undefined = linkConnector.renderLinks.at(0)
+    const isSubgraphIOLink =
+      linkConnector.isConnecting && firstLink?.isIoNodeLink
 
     // get node over
-    const node = LiteGraph.vueNodesMode
-      ? null
-      : graph.getNodeOnPos(x, y, this.visible_nodes)
+    const node =
+      LiteGraph.vueNodesMode && !isSubgraphIOLink
+        ? null
+        : graph.getNodeOnPos(x, y, this.visible_nodes)
 
     const dragRect = this.dragging_rectangle
     if (dragRect) {
@@ -3401,8 +3406,6 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
 
           // Check if link is over anything it could connect to - record position of valid target for snap / highlight
           if (linkConnector.isConnecting) {
-            const firstLink = linkConnector.renderLinks.at(0)
-
             // Default: nothing highlighted
             let highlightPos: Point | undefined
             let highlightInput: INodeInputSlot | undefined
@@ -3453,7 +3456,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
                   highlightInput = node.inputs[inputId]
                 }
 
-                if (highlightInput) {
+                if (highlightInput && !LiteGraph.vueNodesMode) {
                   const widget = node.getWidgetFromSlot(highlightInput)
                   if (widget) linkConnector.overWidget = widget
                 }

src/lib/litegraph/src/LGraphCanvas.ts

@@ -43,6 +43,8 @@ export interface RenderLink {
   /** The reroute that the link is being connected from. */
   readonly fromReroute?: Reroute
 
+  readonly isIoNodeLink?: boolean
+
   /**
    * Capability checks used for hit-testing and validation during drag.
    * Implementations should return `false` when a connection is not possible

src/lib/litegraph/src/canvas/RenderLink.ts

@@ -24,6 +24,7 @@ export class ToInputFromIoNodeLink implements RenderLink {
   readonly fromPos: Point
   fromDirection: LinkDirection = LinkDirection.RIGHT
   readonly existingLink?: LLink
+  readonly isIoNodeLink = true
 
   constructor(
     readonly network: LinkNetwork,

src/lib/litegraph/src/canvas/ToInputFromIoNodeLink.ts

@@ -23,6 +23,7 @@ export class ToOutputFromIoNodeLink implements RenderLink {
   readonly fromPos: Point
   readonly fromSlotIndex: SlotIndex
   fromDirection: LinkDirection = LinkDirection.LEFT
+  readonly isIoNodeLink = true
 
   constructor(
     readonly network: LinkNetwork,

src/lib/litegraph/src/canvas/ToOutputFromIoNodeLink.ts

@@ -136,6 +136,13 @@ export class SubgraphInput extends SubgraphSlot {
     }
     subgraph.incrementVersion()
 
+    subgraph.trigger('node:slot-links:changed', {
+      nodeId: node.id,
+      slotType: NodeSlotType.INPUT,
+      slotIndex: inputIndex,
+      connected: true,
+      linkId: link.id
+    })
     node.onConnectionsChange?.(NodeSlotType.INPUT, inputIndex, true, link, slot)
 
     subgraph.afterChange()
@@ -239,11 +246,8 @@ export class SubgraphInput extends SubgraphSlot {
   override isValidTarget(
     fromSlot: INodeInputSlot | INodeOutputSlot | SubgraphInput | SubgraphOutput
   ): boolean {
-    if (isNodeSlot(fromSlot)) {
-      return (
-        'link' in fromSlot &&
-        LiteGraph.isValidConnection(this.type, fromSlot.type)
-      )
+    if (isNodeSlot(fromSlot) && 'link' in fromSlot) {
+      return LiteGraph.isValidConnection(this.type, fromSlot.type)
     }
 
     if (isSubgraphOutput(fromSlot)) {

src/lib/litegraph/src/subgraph/SubgraphInput.ts

@@ -226,6 +226,13 @@ export class SubgraphInputNode
         link,
         subgraphInput
       )
+      subgraph.trigger('node:slot-links:changed', {
+        nodeId: node.id,
+        slotType: NodeSlotType.INPUT,
+        slotIndex: slotIndex,
+        connected: false,
+        linkId: link.id
+      })
     }
   }
 

src/lib/litegraph/src/subgraph/SubgraphInputNode.ts

@@ -140,11 +140,8 @@ export class SubgraphOutput extends SubgraphSlot {
   override isValidTarget(
     fromSlot: INodeInputSlot | INodeOutputSlot | SubgraphInput | SubgraphOutput
   ): boolean {
-    if (isNodeSlot(fromSlot)) {
-      return (
-        'links' in fromSlot &&
-        LiteGraph.isValidConnection(fromSlot.type, this.type)
-      )
+    if (isNodeSlot(fromSlot) && 'links' in fromSlot) {
+      return LiteGraph.isValidConnection(fromSlot.type, this.type)
     }
 
     if (isSubgraphInput(fromSlot)) {