Skip to content

Commit 7972550

Browse files
christian-byrnegithub-actions
christian-byrne
and
github-actions
authored
[fix] Fix link deletion from middle button when connected to reroute nodes deletes wrong link (#4928)
Co-authored-by: github-actions <[email protected]>
1 parent c7baf3c commit 7972550

File tree

4 files changed

+294
-2
lines changed

4 files changed

+294
-2
lines changed

‎browser_tests/assets/reroute/single-native-reroute-default-workflow.json‎

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
{
2+
"id": "dec788c2-9829-4a5d-a1ee-d6f0a678b42a",
3+
"revision": 0,
4+
"last_node_id": 9,
5+
"last_link_id": 9,
6+
"nodes": [
7+
{
8+
"id": 7,
9+
"type": "CLIPTextEncode",
10+
"pos": [413, 389],
11+
"size": [425.27801513671875, 180.6060791015625],
12+
"flags": {},
13+
"order": 3,
14+
"mode": 0,
15+
"inputs": [
16+
{
17+
"name": "clip",
18+
"type": "CLIP",
19+
"link": 5
20+
}
21+
],
22+
"outputs": [
23+
{
24+
"name": "CONDITIONING",
25+
"type": "CONDITIONING",
26+
"slot_index": 0,
27+
"links": [6]
28+
}
29+
],
30+
"properties": {
31+
"Node name for S&R": "CLIPTextEncode"
32+
},
33+
"widgets_values": ["text, watermark"]
34+
},
35+
{
36+
"id": 6,
37+
"type": "CLIPTextEncode",
38+
"pos": [415, 186],
39+
"size": [422.84503173828125, 164.31304931640625],
40+
"flags": {},
41+
"order": 2,
42+
"mode": 0,
43+
"inputs": [
44+
{
45+
"name": "clip",
46+
"type": "CLIP",
47+
"link": 3
48+
}
49+
],
50+
"outputs": [
51+
{
52+
"name": "CONDITIONING",
53+
"type": "CONDITIONING",
54+
"slot_index": 0,
55+
"links": [4]
56+
}
57+
],
58+
"properties": {
59+
"Node name for S&R": "CLIPTextEncode"
60+
},
61+
"widgets_values": [
62+
"beautiful scenery nature glass bottle landscape, , purple galaxy bottle,"
63+
]
64+
},
65+
{
66+
"id": 5,
67+
"type": "EmptyLatentImage",
68+
"pos": [473, 609],
69+
"size": [315, 106],
70+
"flags": {},
71+
"order": 0,
72+
"mode": 0,
73+
"inputs": [],
74+
"outputs": [
75+
{
76+
"name": "LATENT",
77+
"type": "LATENT",
78+
"slot_index": 0,
79+
"links": [2]
80+
}
81+
],
82+
"properties": {
83+
"Node name for S&R": "EmptyLatentImage"
84+
},
85+
"widgets_values": [512, 512, 1]
86+
},
87+
{
88+
"id": 3,
89+
"type": "KSampler",
90+
"pos": [863, 186],
91+
"size": [315, 262],
92+
"flags": {},
93+
"order": 4,
94+
"mode": 0,
95+
"inputs": [
96+
{
97+
"name": "model",
98+
"type": "MODEL",
99+
"link": 1
100+
},
101+
{
102+
"name": "positive",
103+
"type": "CONDITIONING",
104+
"link": 4
105+
},
106+
{
107+
"name": "negative",
108+
"type": "CONDITIONING",
109+
"link": 6
110+
},
111+
{
112+
"name": "latent_image",
113+
"type": "LATENT",
114+
"link": 2
115+
}
116+
],
117+
"outputs": [
118+
{
119+
"name": "LATENT",
120+
"type": "LATENT",
121+
"slot_index": 0,
122+
"links": [7]
123+
}
124+
],
125+
"properties": {
126+
"Node name for S&R": "KSampler"
127+
},
128+
"widgets_values": [
129+
156680208700286,
130+
"randomize",
131+
20,
132+
8,
133+
"euler",
134+
"normal",
135+
1
136+
]
137+
},
138+
{
139+
"id": 8,
140+
"type": "VAEDecode",
141+
"pos": [1209, 188],
142+
"size": [210, 46],
143+
"flags": {},
144+
"order": 5,
145+
"mode": 0,
146+
"inputs": [
147+
{
148+
"name": "samples",
149+
"type": "LATENT",
150+
"link": 7
151+
},
152+
{
153+
"name": "vae",
154+
"type": "VAE",
155+
"link": 8
156+
}
157+
],
158+
"outputs": [
159+
{
160+
"name": "IMAGE",
161+
"type": "IMAGE",
162+
"slot_index": 0,
163+
"links": [9]
164+
}
165+
],
166+
"properties": {
167+
"Node name for S&R": "VAEDecode"
168+
},
169+
"widgets_values": []
170+
},
171+
{
172+
"id": 9,
173+
"type": "SaveImage",
174+
"pos": [1451, 189],
175+
"size": [210, 58],
176+
"flags": {},
177+
"order": 6,
178+
"mode": 0,
179+
"inputs": [
180+
{
181+
"name": "images",
182+
"type": "IMAGE",
183+
"link": 9
184+
}
185+
],
186+
"outputs": [],
187+
"properties": {},
188+
"widgets_values": ["ComfyUI"]
189+
},
190+
{
191+
"id": 4,
192+
"type": "CheckpointLoaderSimple",
193+
"pos": [26, 474],
194+
"size": [315, 98],
195+
"flags": {},
196+
"order": 1,
197+
"mode": 0,
198+
"inputs": [],
199+
"outputs": [
200+
{
201+
"name": "MODEL",
202+
"type": "MODEL",
203+
"slot_index": 0,
204+
"links": [1]
205+
},
206+
{
207+
"name": "CLIP",
208+
"type": "CLIP",
209+
"slot_index": 1,
210+
"links": [3, 5]
211+
},
212+
{
213+
"name": "VAE",
214+
"type": "VAE",
215+
"slot_index": 2,
216+
"links": [8]
217+
}
218+
],
219+
"properties": {
220+
"Node name for S&R": "CheckpointLoaderSimple"
221+
},
222+
"widgets_values": ["v1-5-pruned-emaonly-fp16.safetensors"]
223+
}
224+
],
225+
"links": [
226+
[1, 4, 0, 3, 0, "MODEL"],
227+
[2, 5, 0, 3, 3, "LATENT"],
228+
[3, 4, 1, 6, 0, "CLIP"],
229+
[4, 6, 0, 3, 1, "CONDITIONING"],
230+
[5, 4, 1, 7, 0, "CLIP"],
231+
[6, 7, 0, 3, 2, "CONDITIONING"],
232+
[7, 3, 0, 8, 0, "LATENT"],
233+
[8, 4, 2, 8, 1, "VAE"],
234+
[9, 8, 0, 9, 0, "IMAGE"]
235+
],
236+
"groups": [],
237+
"config": {},
238+
"extra": {
239+
"ds": {
240+
"offset": [0, 0],
241+
"scale": 1
242+
},
243+
"reroutes": [
244+
{
245+
"id": 1,
246+
"pos": [372.66668701171875, 415.33331298828125],
247+
"linkIds": [3]
248+
}
249+
],
250+
"linkExtensions": [
251+
{
252+
"id": 3,
253+
"parentId": 1
254+
}
255+
],
256+
"frontendVersion": "1.26.1"
257+
},
258+
"version": 0.4
259+
}

‎browser_tests/tests/rerouteNode.spec.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,29 @@ test.describe('LiteGraph Native Reroute Node', () => {
100100
'native_reroute_context_menu.png'
101101
)
102102
})
103+
104+
test('Can delete link that is connected to two reroutes', async ({
105+
comfyPage
106+
}) => {
107+
// https://github.com/Comfy-Org/ComfyUI_frontend/issues/4695
108+
await comfyPage.loadWorkflow(
109+
'reroute/single-native-reroute-default-workflow'
110+
)
111+
112+
// To find the clickable midpoint button, we use the hardcoded value from the browser logs
113+
// since the link is a bezier curve and not a straight line.
114+
const middlePoint = { x: 359.4188232421875, y: 468.7716979980469 }
115+
116+
// Click the middle point of the link to open the context menu.
117+
await comfyPage.page.mouse.click(middlePoint.x, middlePoint.y)
118+
119+
// Click the "Delete" context menu option.
120+
await comfyPage.page
121+
.locator('.litecontextmenu .litemenu-entry', { hasText: 'Delete' })
122+
.click()
123+
124+
await expect(comfyPage.canvas).toHaveScreenshot(
125+
'native_reroute_delete_from_midpoint_context_menu.png'
126+
)
127+
})
103128
})

‎browser_tests/tests/rerouteNode.spec.ts-snapshots/native-reroute-delete-from-midpoint-context-menu-chromium-linux.png‎

92 KB
Loading

‎src/lib/litegraph/src/LGraphCanvas.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6234,9 +6234,17 @@ export class LGraphCanvas
62346234
break
62356235
}
62366236

6237-
case 'Delete':
6238-
graph.removeLink(segment.id)
6237+
case 'Delete': {
6238+
// segment can be a Reroute object, in which case segment.id is the reroute id
6239+
const linkId =
6240+
segment instanceof Reroute
6241+
? segment.linkIds.values().next().value
6242+
: segment.id
6243+
if (linkId !== undefined) {
6244+
graph.removeLink(linkId)
6245+
}
62396246
break
6247+
}
62406248
default:
62416249
}
62426250
}

0 commit comments

Comments
 (0)