Skip to content

Commit c20aad5

Browse files
author
Chenlei Hu
authored
RFC: Litegraph Native Reroute (#6)
* rfc:litegraph_native_reroute * nit * Update and rename 0003-litegraph_native_reroute.md to 0002-litegraph_native_reroute.md
1 parent bbbfcb6 commit c20aad5

File tree

1 file changed

+393
-0
lines changed

1 file changed

+393
-0
lines changed
Lines changed: 393 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,393 @@
1+
# RFC: LiteGraph Native Reroute
2+
3+
- Start Date: 2025-01-12
4+
- Target Major Version: Frontend 1.15
5+
- Implemented PR: https://github.com/Comfy-Org/ComfyUI_frontend/pull/3151
6+
- Reference Issues:
7+
8+
- <https://github.com/Comfy-Org/litegraph.js/pull/301>
9+
- <https://github.com/Comfy-Org/ComfyUI_frontend/pull/1420>
10+
- <https://github.com/Comfy-Org/ComfyUI_frontend/pull/1421>
11+
12+
## Summary
13+
14+
This RFC proposes replacing ComfyUI's current frontend-only reroute node implementation with a native LiteGraph reroute feature. The new implementation will treat reroutes as link metadata rather than full nodes, providing several benefits:
15+
16+
- Simpler workflow JSON representation
17+
- Proper type safety throughout connection chains
18+
- Elimination of special-case handling for reroute nodes
19+
- Reduced complexity in workflow structures and graph traversal
20+
21+
This change requires updates to both the workflow schema and LiteGraph.js library, with a migration path provided for existing workflows.
22+
23+
## Basic example
24+
25+
### New litegraph native reroute node
26+
27+
![new-reroute](https://github.com/user-attachments/assets/dddef61a-f975-4d69-b143-64505b6b9eaa)
28+
29+
![new-reroute-2](https://github.com/user-attachments/assets/c4c90291-38e6-429f-a22d-401848bb82d7)
30+
31+
Representation in workflow json (0.4 Schema):
32+
33+
```json
34+
{
35+
"links": [
36+
[
37+
13,
38+
4,
39+
1,
40+
6,
41+
0,
42+
"CLIP"
43+
]
44+
],
45+
"extra": {
46+
"linkExtensions": [
47+
{
48+
"id": 13,
49+
"parentId": 3
50+
}
51+
],
52+
"reroutes": [
53+
{
54+
"id": 2,
55+
"pos": [
56+
239.8215789794922,
57+
354.64306640625
58+
],
59+
"linkIds": [
60+
13
61+
]
62+
},
63+
{
64+
"id": 3,
65+
"parentId": 2,
66+
"pos": [
67+
309.733154296875,
68+
208.2829132080078
69+
],
70+
"linkIds": [
71+
13
72+
]
73+
}
74+
]
75+
}
76+
}
77+
```
78+
79+
Representation in workflow json (1.0 Schema):
80+
81+
```json
82+
{
83+
"links": [
84+
{
85+
"id": 13,
86+
"origin_id": 4,
87+
"origin_slot": 1,
88+
"target_id": 6,
89+
"target_slot": 0,
90+
"type": "CLIP",
91+
"parentId": 3
92+
}
93+
],
94+
"reroutes": [
95+
{
96+
"id": 2,
97+
"pos": [
98+
239.8215789794922,
99+
354.64306640625
100+
],
101+
"linkIds": [
102+
13
103+
]
104+
},
105+
{
106+
"id": 3,
107+
"parentId": 2,
108+
"pos": [
109+
309.733154296875,
110+
208.2829132080078
111+
],
112+
"linkIds": [
113+
13
114+
]
115+
}
116+
]
117+
}
118+
```
119+
120+
### Old frontend-only reroute node
121+
122+
![old-reroute](https://github.com/user-attachments/assets/03279c0b-cb3d-4668-afa4-3d8304814d67)
123+
124+
Representation in workflow json (0.4 & 1.0 Schema):
125+
126+
```json
127+
{
128+
"links": [
129+
[
130+
11,
131+
4,
132+
1,
133+
10,
134+
0,
135+
"*"
136+
],
137+
[
138+
12,
139+
10,
140+
0,
141+
6,
142+
0,
143+
"CLIP"
144+
]
145+
],
146+
"nodes": [
147+
{
148+
"id": 10,
149+
"type": "Reroute",
150+
"pos": [
151+
245.87435913085938,
152+
185.70533752441406
153+
],
154+
"size": [
155+
75,
156+
26
157+
],
158+
"flags": {},
159+
"order": 1,
160+
"mode": 0,
161+
"inputs": [
162+
{
163+
"name": "",
164+
"type": "*",
165+
"link": 11
166+
}
167+
],
168+
"outputs": [
169+
{
170+
"name": "",
171+
"type": "CLIP",
172+
"links": [
173+
12
174+
],
175+
"slot_index": 0
176+
}
177+
],
178+
"properties": {
179+
"showOutputText": false,
180+
"horizontal": false
181+
}
182+
}
183+
]
184+
}
185+
```
186+
187+
## Motivation
188+
189+
The current frontend-only reroute implementation has several limitations and drawbacks that this proposal aims to address:
190+
191+
1. **Graph Complexity**: The legacy reroute implementation adds unnecessary complexity to the workflow graph by creating additional nodes and links. Each reroute point requires a full node object with inputs/outputs and properties, which bloats the workflow JSON and makes graph traversal more complex.
192+
193+
2. **Type Safety Issues**: The current implementation uses wildcard type matching (`"*"`) for inputs, which bypasses LiteGraph's type checking system. This can lead to type inconsistency issues when connecting nodes, as the reroute node may connect incompatible types without proper validation.
194+
195+
3. **Implementation Overhead**: The legacy reroute implementation has resulted in numerous special-case handling throughout the codebase. Many features require specific patches to handle reroute nodes differently (checking `if (node.name === 'Reroute')`) which increases maintenance burden and makes the codebase more fragile. See <https://cs.comfy.org/search?q=context:global+%22%27Reroute%27%22&patternType=keyword&sm=0> for a list of patches.
196+
197+
By implementing reroutes as a native LiteGraph feature, we can:
198+
199+
- Simplify workflow representations by treating reroutes as link metadata rather than nodes
200+
- Maintain proper type checking through the entire connection chain
201+
- Eliminate the need for special-case handling of reroute nodes in various features
202+
- Reduce the overall complexity of workflow JSON structures
203+
204+
## Detailed design
205+
206+
### Schema Changes
207+
208+
The native reroute implementation introduces a cleaner schema structure that moves reroute information out of the node list and into dedicated sections. The key changes between Schema 0.4 and 1.0 are:
209+
210+
1. **Link Structure**
211+
212+
- 0.4: Links are arrays `[id, origin_id, origin_slot, target_id, target_slot, type]`
213+
- 1.0: Links are objects with named properties:
214+
215+
```
216+
{
217+
"id": number,
218+
"origin_id": number,
219+
"origin_slot": number,
220+
"target_id": number,
221+
"target_slot": number,
222+
"type": string,
223+
"parentId": number // References parent reroute point
224+
}
225+
```
226+
227+
2. **Reroute Structure**
228+
229+
- 0.4: Reroutes are nested under `extra.reroutes`
230+
- 1.0: Reroutes are top-level array under `reroutes`
231+
232+
### Implementation Details
233+
234+
1. **Link Extension**
235+
236+
- Each reroute point creates a virtual extension of the original link
237+
- Links maintain their original type throughout the reroute chain
238+
- Parent-child relationships between reroute points are tracked via `parentId`
239+
240+
2. **Position Management**
241+
242+
- Reroute points store their canvas position as `[x, y]` coordinates
243+
- Multiple reroute points can be chained using the `parentId` reference
244+
- Each reroute point references its associated link(s) via `linkIds`
245+
246+
3. **Type Safety**
247+
248+
- The link type is preserved from source to destination
249+
- No type conversion or wildcard matching occurs at reroute points
250+
- LiteGraph's native type checking remains active throughout the connection
251+
252+
### API Changes
253+
254+
The LiteGraph.js library will be extended with new methods:
255+
256+
```javascript
257+
/**
258+
* Creates a new reroute and adds it to the graph.
259+
* @param pos Position in graph space
260+
* @param before The existing link segment (reroute, link) that will be after this reroute,
261+
* going from the node output to input.
262+
* @returns The newly created reroute - typically ignored.
263+
* Already implemented.
264+
*/
265+
LGraph.prototype.createReroute(pos: Point, before: LinkSegment): Reroute {
266+
...
267+
}
268+
269+
/**
270+
* Removes a reroute from the graph
271+
* @param id ID of reroute to remove
272+
* Already implemented.
273+
*/
274+
LGraph.prototype.removeReroute(id: RerouteId): void {
275+
...
276+
}
277+
278+
// New API endpoint (Refactor needed).
279+
LGraphCanvas.prototype.renderReroutePoints = function() {
280+
// Handles visual rendering of reroute points
281+
}
282+
```
283+
284+
### Migration Path
285+
286+
To ensure a smooth transition, the migration strategy addresses both schema version changes and reroute implementation changes:
287+
288+
#### Schema Migration (0.4 to 1.0)
289+
290+
1. **Automatic Detection**: The system will automatically detect the schema version based on the link format (array vs object)
291+
292+
2. **Link Structure Conversion**:
293+
294+
```javascript
295+
// Old format (0.4)
296+
[13, 4, 1, 6, 0, "CLIP"]
297+
// New format (1.0)
298+
{ "id": 13, "origin_id": 4, "origin_slot": 1, "target_id": 6, "target_slot": 0, "type": "CLIP" }
299+
```
300+
301+
1. **Reroute Location**: Reroutes will be moved from `extra.reroutes` to the top-level `reroutes` array
302+
303+
#### Legacy Reroute Migration
304+
305+
1. **Detection**: During workflow loading, the system will identify legacy reroute nodes by checking:
306+
307+
- Node type === "Reroute"
308+
- Single input/output configuration
309+
- Presence in the `nodes` array
310+
311+
2. **Conversion Process**:
312+
313+
```javascript
314+
// For each legacy reroute node:
315+
{
316+
// Create new native reroute point
317+
const reroute = {
318+
id: legacyNode.id,
319+
pos: legacyNode.pos,
320+
linkIds: [outputLink.id]
321+
};
322+
323+
// Connect original input to final output
324+
const newLink = {
325+
id: generateNewId(),
326+
origin_id: inputLink.origin_id,
327+
origin_slot: inputLink.origin_slot,
328+
target_id: outputLink.target_id,
329+
target_slot: outputLink.target_slot,
330+
type: outputLink.type
331+
};
332+
333+
// Remove old node and links.
334+
// Not fully connected legacy reroutes will be removed.
335+
removeNode(legacyNode);
336+
removeLinks([inputLink.id, outputLink.id]);
337+
}
338+
```
339+
340+
1. **Validation**: After conversion, the system will verify:
341+
342+
- Type consistency is maintained
343+
- All connections are preserved
344+
- Graph topology remains functional
345+
346+
#### Backwards Compatibility
347+
348+
- The system will maintain support for loading legacy reroute nodes throughout the targeted major version release cycle
349+
- Warning messages will be displayed when legacy reroutes are detected
350+
- Documentation will be updated to encourage migration to native reroutes
351+
352+
## Drawbacks
353+
354+
Several important considerations should be weighed before implementing this proposal:
355+
356+
1. **Implementation Complexity**
357+
- Requires significant modifications to the core LiteGraph.js library
358+
- Need to implement new rendering logic for reroute points
359+
- Complex migration logic needed to handle legacy reroute nodes
360+
- Additional testing burden to ensure compatibility across different workflow versions
361+
362+
2. **Breaking Changes**
363+
- Schema changes from 0.4 to 1.0 require all tools in the ecosystem to be updated
364+
- Third-party applications that directly manipulate workflow JSON will need modifications
365+
- Custom node implementations that interact with reroute nodes may break
366+
367+
3. **Performance Considerations**
368+
- Additional overhead in link rendering due to reroute point calculations
369+
- Increased memory usage from storing reroute metadata for each affected link
370+
- Potential impact on workflow loading times during migration
371+
372+
4. **User Experience Impact**
373+
- Users familiar with the current node-based reroute system will need to adapt
374+
- Documentation and tutorials will need significant updates
375+
- Temporary confusion during the transition period as both systems may coexist
376+
377+
5. **Alternative Approaches**
378+
- The current node-based implementation, while not ideal, is functional
379+
- Improvements to the existing system might be less disruptive than a complete redesign
380+
- User-space solutions might be sufficient for some use cases
381+
382+
6. **Maintenance Burden**
383+
- New code paths need long-term maintenance
384+
- Migration logic will need to be maintained for backward compatibility
385+
- Potential for new edge cases and bugs in the rerouting system
386+
387+
## Unresolved questions
388+
389+
1. **Unconnected Reroutes**: The current implementation in the litegraph library only allows reroutes to be created on existing links, which differs from the legacy reroute behavior that allowed unconnected reroutes. We need to determine:
390+
- Whether supporting unconnected reroutes is a necessary feature
391+
- If implemented, how would unconnected reroutes be represented in the schema
392+
- The potential impact on graph validation and type checking
393+
- Use cases where unconnected reroutes provide meaningful functionality

0 commit comments

Comments
 (0)