From 01362d5ff15bfe530a6b3b86e365e41570e1bf46 Mon Sep 17 00:00:00 2001 From: Rizumu Ayaka Date: Tue, 27 Jan 2026 09:34:49 +0700 Subject: [PATCH] fix: `recomputeInsideNodes` does not support nested group processing (#8275) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit related https://github.com/Comfy-Org/ComfyUI_frontend/pull/8274 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8275-fix-recomputeInsideNodes-does-not-support-nested-group-processing-2f16d73d3650815e9b44dbd182f8dd74) by [Unito](https://www.unito.io) --- src/lib/litegraph/src/LGraphGroup.test.ts | 70 ++++++++++++++++++++++- src/lib/litegraph/src/LGraphGroup.ts | 25 +++++++- 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/src/lib/litegraph/src/LGraphGroup.test.ts b/src/lib/litegraph/src/LGraphGroup.test.ts index 589e5a958..64b437b6a 100644 --- a/src/lib/litegraph/src/LGraphGroup.test.ts +++ b/src/lib/litegraph/src/LGraphGroup.test.ts @@ -1,6 +1,6 @@ import { describe, expect } from 'vitest' -import { LGraphGroup } from '@/lib/litegraph/src/litegraph' +import { LGraph, LGraphGroup } from '@/lib/litegraph/src/litegraph' import { test } from './__fixtures__/testExtensions' @@ -9,4 +9,72 @@ describe('LGraphGroup', () => { const link = new LGraphGroup('title', 929) expect(link.serialize()).toMatchSnapshot('Basic') }) + + describe('recomputeInsideNodes', () => { + test('uses visited set to avoid redundant computation', () => { + const graph = new LGraph() + + // Create 4 nested groups: outer -> mid1 -> mid2 -> inner + const outer = new LGraphGroup('outer') + outer.pos = [0, 0] + outer.size = [400, 400] + graph.add(outer) + + const mid1 = new LGraphGroup('mid1') + mid1.pos = [10, 10] + mid1.size = [300, 300] + graph.add(mid1) + + const mid2 = new LGraphGroup('mid2') + mid2.pos = [20, 20] + mid2.size = [200, 200] + graph.add(mid2) + + const inner = new LGraphGroup('inner') + inner.pos = [30, 30] + inner.size = [100, 100] + graph.add(inner) + + // Track the visited set to verify each group is only fully processed once + const visited = new Set() + outer.recomputeInsideNodes(100, visited) + + // All nested groups should be in the visited set + expect(visited.has(outer.id)).toBe(true) + expect(visited.has(mid1.id)).toBe(true) + expect(visited.has(mid2.id)).toBe(true) + expect(visited.has(inner.id)).toBe(true) + expect(visited.size).toBe(4) + + // Verify children relationships are correct + expect(outer.children.has(mid1)).toBe(true) + expect(outer.children.has(mid2)).toBe(true) + expect(outer.children.has(inner)).toBe(true) + expect(mid1.children.has(mid2)).toBe(true) + expect(mid1.children.has(inner)).toBe(true) + expect(mid2.children.has(inner)).toBe(true) + }) + + test('respects maxDepth limit', () => { + const graph = new LGraph() + + const outer = new LGraphGroup('outer') + outer.pos = [0, 0] + outer.size = [300, 300] + graph.add(outer) + + const inner = new LGraphGroup('inner') + inner.pos = [10, 10] + inner.size = [100, 100] + graph.add(inner) + + // With maxDepth=1, inner group is added as child but not processed + outer.recomputeInsideNodes(1) + + // outer should have inner as a child + expect(outer.children.has(inner)).toBe(true) + // inner should not have computed its own children (it was never processed) + expect(inner.children.size).toBe(0) + }) + }) }) diff --git a/src/lib/litegraph/src/LGraphGroup.ts b/src/lib/litegraph/src/LGraphGroup.ts index 9e14cf277..ee3197d43 100644 --- a/src/lib/litegraph/src/LGraphGroup.ts +++ b/src/lib/litegraph/src/LGraphGroup.ts @@ -241,8 +241,21 @@ export class LGraphGroup implements Positionable, IPinnable, IColorable { return this.pinned ? false : snapPoint(this.pos, snapTo) } - recomputeInsideNodes(): void { + /** + * Recomputes which items (nodes, reroutes, nested groups) are inside this group. + * Recursively processes nested groups to ensure their children are also computed. + * @param maxDepth Maximum recursion depth for nested groups. Use 1 to skip nested group computation. + * @param visited Set of already visited group IDs to prevent redundant computation. + */ + recomputeInsideNodes( + maxDepth: number = 100, + visited: Set = new Set() + ): void { if (!this.graph) throw new NullGraphError() + if (maxDepth <= 0 || visited.has(this.id)) return + + visited.add(this.id) + const { nodes, reroutes, groups } = this.graph const children = this._children this._nodes.length = 0 @@ -261,10 +274,16 @@ export class LGraphGroup implements Positionable, IPinnable, IColorable { if (isPointInRect(reroute.pos, this._bounding)) children.add(reroute) } - // Move groups we wholly contain + // Move groups we wholly contain and recursively compute their children + const containedGroups: LGraphGroup[] = [] for (const group of groups) { - if (containsRect(this._bounding, group._bounding)) children.add(group) + if (group !== this && containsRect(this._bounding, group._bounding)) { + children.add(group) + containedGroups.push(group) + } } + for (const group of containedGroups) + group.recomputeInsideNodes(maxDepth - 1, visited) groups.sort((a, b) => { if (a === this) {