mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
[backport cloud/1.42] fix: _removeDuplicateLinks incorrectly removes valid link when slot indices shift (#10376)
Backport of #10289 to `cloud/1.42` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10376-backport-cloud-1-42-fix-_removeDuplicateLinks-incorrectly-removes-valid-link-when-slo-32a6d73d365081dda82bcc70bbd938e5) by [Unito](https://www.unito.io) Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com>
This commit is contained in:
@@ -627,6 +627,87 @@ describe('_removeDuplicateLinks', () => {
|
||||
expect(graph._links.has(dupLink.id)).toBe(false)
|
||||
})
|
||||
|
||||
it('keeps the valid link when input.link is at a shifted slot index', () => {
|
||||
LiteGraph.registerNodeType('test/DupTestNode', TestNode)
|
||||
const graph = new LGraph()
|
||||
|
||||
const source = LiteGraph.createNode('test/DupTestNode', 'Source')!
|
||||
const target = LiteGraph.createNode('test/DupTestNode', 'Target')!
|
||||
graph.add(source)
|
||||
graph.add(target)
|
||||
|
||||
// Connect source:0 -> target:0, establishing input.link on target
|
||||
source.connect(0, target, 0)
|
||||
const validLinkId = target.inputs[0].link!
|
||||
expect(graph._links.has(validLinkId)).toBe(true)
|
||||
|
||||
// Simulate widget-to-input conversion shifting the slot: insert a new
|
||||
// input BEFORE the connected one, moving it from index 0 to index 1.
|
||||
target.addInput('extra_widget', 'number')
|
||||
const connectedInput = target.inputs[0]
|
||||
target.inputs[0] = target.inputs[1]
|
||||
target.inputs[1] = connectedInput
|
||||
// Now target.inputs[1].link === validLinkId, but target.inputs[0].link is null
|
||||
|
||||
// Add a duplicate link with the same connection tuple (target_slot=0
|
||||
// in the LLink, matching the original slot before the shift).
|
||||
const dupLink = new LLink(
|
||||
++graph.state.lastLinkId,
|
||||
'number',
|
||||
source.id,
|
||||
0,
|
||||
target.id,
|
||||
0
|
||||
)
|
||||
graph._links.set(dupLink.id, dupLink)
|
||||
source.outputs[0].links!.push(dupLink.id)
|
||||
|
||||
expect(graph._links.size).toBe(2)
|
||||
|
||||
graph._removeDuplicateLinks()
|
||||
|
||||
// The valid link (referenced by an actual input) must survive
|
||||
expect(graph._links.size).toBe(1)
|
||||
expect(graph._links.has(validLinkId)).toBe(true)
|
||||
expect(graph._links.has(dupLink.id)).toBe(false)
|
||||
expect(target.inputs[1].link).toBe(validLinkId)
|
||||
})
|
||||
|
||||
it('repairs input.link when it points to a removed duplicate', () => {
|
||||
LiteGraph.registerNodeType('test/DupTestNode', TestNode)
|
||||
const graph = new LGraph()
|
||||
|
||||
const source = LiteGraph.createNode('test/DupTestNode', 'Source')!
|
||||
const target = LiteGraph.createNode('test/DupTestNode', 'Target')!
|
||||
graph.add(source)
|
||||
graph.add(target)
|
||||
|
||||
source.connect(0, target, 0)
|
||||
|
||||
// Create a duplicate link
|
||||
const dupLink = new LLink(
|
||||
++graph.state.lastLinkId,
|
||||
'number',
|
||||
source.id,
|
||||
0,
|
||||
target.id,
|
||||
0
|
||||
)
|
||||
graph._links.set(dupLink.id, dupLink)
|
||||
source.outputs[0].links!.push(dupLink.id)
|
||||
|
||||
// Point input.link to the duplicate (simulating corrupted state)
|
||||
target.inputs[0].link = dupLink.id
|
||||
|
||||
graph._removeDuplicateLinks()
|
||||
|
||||
expect(graph._links.size).toBe(1)
|
||||
// input.link must point to whichever link survived
|
||||
const survivingId = graph._links.keys().next().value!
|
||||
expect(target.inputs[0].link).toBe(survivingId)
|
||||
expect(graph._links.has(target.inputs[0].link!)).toBe(true)
|
||||
})
|
||||
|
||||
it('is a no-op when no duplicates exist', () => {
|
||||
registerTestNodes()
|
||||
const graph = new LGraph()
|
||||
|
||||
@@ -1628,42 +1628,66 @@ export class LGraph
|
||||
* output.links and the graph's _links map.
|
||||
*/
|
||||
_removeDuplicateLinks(): void {
|
||||
const seen = new Map<string, LinkId>()
|
||||
const toRemove: LinkId[] = []
|
||||
|
||||
// Group all link IDs by their connection tuple.
|
||||
const groups = new Map<string, LinkId[]>()
|
||||
for (const [id, link] of this._links) {
|
||||
const key = LGraph._linkTupleKey(link)
|
||||
if (seen.has(key)) {
|
||||
const existingId = seen.get(key)!
|
||||
// Keep the link that the input side references
|
||||
const node = this.getNodeById(link.target_id)
|
||||
const input = node?.inputs?.[link.target_slot]
|
||||
if (input?.link === id) {
|
||||
toRemove.push(existingId)
|
||||
seen.set(key, id)
|
||||
} else {
|
||||
toRemove.push(id)
|
||||
}
|
||||
} else {
|
||||
seen.set(key, id)
|
||||
let group = groups.get(key)
|
||||
if (!group) {
|
||||
group = []
|
||||
groups.set(key, group)
|
||||
}
|
||||
group.push(id)
|
||||
}
|
||||
|
||||
for (const id of toRemove) {
|
||||
const link = this._links.get(id)
|
||||
if (!link) continue
|
||||
for (const [, ids] of groups) {
|
||||
if (ids.length <= 1) continue
|
||||
|
||||
// Remove from origin node's output.links array
|
||||
const originNode = this.getNodeById(link.origin_id)
|
||||
if (originNode) {
|
||||
const output = originNode.outputs?.[link.origin_slot]
|
||||
if (output?.links) {
|
||||
const idx = output.links.indexOf(id)
|
||||
if (idx !== -1) output.links.splice(idx, 1)
|
||||
const sampleLink = this._links.get(ids[0])!
|
||||
const node = this.getNodeById(sampleLink.target_id)
|
||||
|
||||
// Find which link ID is actually referenced by any input on the target
|
||||
// node. Cannot rely on target_slot index because widget-to-input
|
||||
// conversions during configure() can shift slot indices.
|
||||
let keepId: LinkId | undefined
|
||||
if (node) {
|
||||
for (const input of node.inputs ?? []) {
|
||||
const match = ids.find((id) => input.link === id)
|
||||
if (match != null) {
|
||||
keepId = match
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
keepId ??= ids[0]
|
||||
|
||||
this._links.delete(id)
|
||||
for (const id of ids) {
|
||||
if (id === keepId) continue
|
||||
|
||||
const link = this._links.get(id)
|
||||
if (!link) continue
|
||||
|
||||
// Remove from origin node's output.links array
|
||||
const originNode = this.getNodeById(link.origin_id)
|
||||
if (originNode) {
|
||||
const output = originNode.outputs?.[link.origin_slot]
|
||||
if (output?.links) {
|
||||
const idx = output.links.indexOf(id)
|
||||
if (idx !== -1) output.links.splice(idx, 1)
|
||||
}
|
||||
}
|
||||
|
||||
this._links.delete(id)
|
||||
}
|
||||
|
||||
// Ensure input.link points to the surviving link
|
||||
if (node) {
|
||||
for (const input of node.inputs ?? []) {
|
||||
if (ids.includes(input.link as LinkId) && input.link !== keepId) {
|
||||
input.link = keepId
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user