diff --git a/src/layout/selected-groups.ts b/src/layout/selected-groups.ts index 38b7fa8..feb3f77 100644 --- a/src/layout/selected-groups.ts +++ b/src/layout/selected-groups.ts @@ -528,7 +528,7 @@ function layoutGroupContents( } // Process nested groups (bottom-up: deepest first) - // Sort by containment depth + // In default mode, reposition direct nested groups below member content const sortedNested = [...nestedGroups].sort((a, b) => { // Count how many other nested groups contain each let depthA = 0; @@ -540,13 +540,52 @@ function layoutGroupContents( return depthB - depthA; // Deepest first }); - for (const nestedGroup of sortedNested) { - // Get direct members of nested group + // Identify direct nested groups (not contained by other nested groups) + const directNestedGroups = sortedNested.filter((g) => { + for (const other of nestedGroups) { + if (other !== g && groupContainsGroup(other, g)) return false; + } + return true; + }); + + // Sort direct groups by original position for stable ordering + directNestedGroups.sort((a, b) => { + const yDiff = a.pos[1] - b.pos[1]; + return yDiff !== 0 ? yDiff : a.pos[0] - b.pos[0]; + }); + + for (const nestedGroup of directNestedGroups) { + // Skip if already processed (by token mode) + if (processedGroups.has(nestedGroup)) { + // Still include bounds + minX = Math.min(minX, nestedGroup.pos[0]); + minY = Math.min(minY, nestedGroup.pos[1]); + maxX = Math.max(maxX, nestedGroup.pos[0] + nestedGroup.size[0]); + maxY = Math.max(maxY, nestedGroup.pos[1] + nestedGroup.size[1]); + continue; + } + + // Get nested group's members and children const nestedChildren = collectNestedGroups(nestedGroup, nestedGroups); const nestedMembers = collectDirectMembers(nestedGroup, allNodes, nestedChildren); - // Recursively layout nested group - const nestedBounds = layoutGroupContents( + // In default mode, reposition nested group below current content + if (layoutMode.type === "default") { + // Position nested group's left edge at startX (same as members) for consistent alignment + // This prevents the group from shifting leftward on each layout run + const targetX = startX; + const targetY = contentEndY + config.verticalGap; + const deltaX = targetX - nestedGroup.pos[0]; + const deltaY = targetY - nestedGroup.pos[1]; + + if (deltaX !== 0 || deltaY !== 0) { + translateGroupWithMembers(nestedGroup, deltaX, deltaY, nestedMembers, allNodes, nestedGroups); + debugLog(` Repositioned nested group "${nestedGroup.title}" to [${targetX}, ${targetY}]`); + } + } + + // Recursively layout nested group contents + layoutGroupContents( nestedGroup, nestedMembers, nestedChildren, @@ -555,24 +594,17 @@ function layoutGroupContents( processedGroups ); - if (nestedBounds) { - // Update bounds to include nested group - minX = Math.min(minX, nestedBounds.minX - config.groupPadding); - minY = Math.min(minY, nestedBounds.minY - config.groupPadding - GROUP_TITLE_HEIGHT); - maxX = Math.max(maxX, nestedBounds.maxX + config.groupPadding); - maxY = Math.max(maxY, nestedBounds.maxY + config.groupPadding); - } else { - // Include nested group's current bounds - const nx = nestedGroup.pos[0]; - const ny = nestedGroup.pos[1]; - const nw = nestedGroup.size[0]; - const nh = nestedGroup.size[1]; + // Resize nested group to fit its organized contents + resizeGroupToFitMembers(nestedGroup, nestedMembers, allNodes, config); - minX = Math.min(minX, nx); - minY = Math.min(minY, ny); - maxX = Math.max(maxX, nx + nw); - maxY = Math.max(maxY, ny + nh); - } + // Update contentEndY for next nested group + contentEndY = Math.max(contentEndY, nestedGroup.pos[1] + nestedGroup.size[1]); + + // Update bounds to include nested group + minX = Math.min(minX, nestedGroup.pos[0]); + minY = Math.min(minY, nestedGroup.pos[1]); + maxX = Math.max(maxX, nestedGroup.pos[0] + nestedGroup.size[0]); + maxY = Math.max(maxY, nestedGroup.pos[1] + nestedGroup.size[1]); } // Resize group to fit contents diff --git a/tests/fixtures/group-test-simple.json b/tests/fixtures/group-test-simple.json new file mode 100644 index 0000000..a1a99cf --- /dev/null +++ b/tests/fixtures/group-test-simple.json @@ -0,0 +1,188 @@ +{ + "id": "bbe5e07d-ae25-42f3-b43f-63e9274165a1", + "revision": 0, + "last_node_id": 6, + "last_link_id": 4, + "nodes": [ + { + "id": 3, + "type": "PreviewImage", + "pos": [ + 550, + 570 + ], + "size": [ + 660, + 1000 + ], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [ + { + "name": "images", + "type": "IMAGE", + "link": 3 + } + ], + "outputs": [], + "title": "Preview Image 1", + "properties": { + "cnr_id": "comfy-core", + "ver": "0.8.2", + "Node name for S&R": "PreviewImage" + }, + "widgets_values": [] + }, + { + "id": 4, + "type": "PreviewImage", + "pos": [ + 1310, + 570 + ], + "size": [ + 660, + 1000 + ], + "flags": {}, + "order": 3, + "mode": 0, + "inputs": [ + { + "name": "images", + "type": "IMAGE", + "link": 4 + } + ], + "outputs": [], + "title": "Preview Image 2", + "properties": { + "cnr_id": "comfy-core", + "ver": "0.8.2", + "Node name for S&R": "PreviewImage" + }, + "widgets_values": [] + }, + { + "id": 5, + "type": "GetNode", + "pos": [ + 140, + 150 + ], + "size": [ + 210, + 60 + ], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "name": "*", + "type": "*", + "links": [ + 3 + ] + } + ], + "title": "GetNode 1", + "properties": {}, + "widgets_values": [ + "" + ] + }, + { + "id": 6, + "type": "GetNode", + "pos": [ + 450, + 150 + ], + "size": [ + 210, + 58 + ], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [], + "outputs": [ + { + "name": "*", + "type": "*", + "links": [ + 4 + ] + } + ], + "title": "GetNode 2", + "properties": {}, + "widgets_values": [ + "" + ] + } + ], + "links": [ + [ + 3, + 5, + 0, + 3, + 0, + "IMAGE" + ], + [ + 4, + 6, + 0, + 4, + 0, + "IMAGE" + ] + ], + "groups": [ + { + "id": 1, + "title": "Group Wrapper", + "bounding": [ + 80, + -10, + 2030, + 1670 + ], + "color": "#3f789e", + "font_size": 22, + "flags": {} + }, + { + "id": 2, + "title": "Inner Group", + "bounding": [ + 110, + 70, + 580, + 170 + ], + "color": "#3f789e", + "font_size": 22, + "flags": {} + } + ], + "config": {}, + "extra": { + "workflowRendererVersion": "LG", + "ue_links": [], + "ds": { + "scale": 0.47082251883518567, + "offset": [ + 1126.5268822743458, + 397.1123544821961 + ] + }, + "frontendVersion": "1.37.9" + }, + "version": 0.4 +} diff --git a/tests/integration/selected-groups.test.ts b/tests/integration/selected-groups.test.ts index a235c1b..f6f5403 100644 --- a/tests/integration/selected-groups.test.ts +++ b/tests/integration/selected-groups.test.ts @@ -718,6 +718,123 @@ describe("layoutSelectedGroups", () => { }); }); + describe("nested group + disconnected nodes overlap regression (group-test-simple.json)", () => { + it("positions nested groups below disconnected nodes (no token)", () => { + const graph = loadFixture("group-test-simple.json"); + layoutSelectedGroups(graph, new Set([1])); // "Group Wrapper" + + const innerGroup = graph._groups.find((g) => g.id === 2)!; + const node3 = graph._nodes.find((n) => n.id === 3)!; + const node4 = graph._nodes.find((n) => n.id === 4)!; + + // Inner group should be BELOW the preview images + const node3Bottom = node3.pos[1] + node3.size[1]; + const node4Bottom = node4.pos[1] + node4.size[1]; + const membersBottom = Math.max(node3Bottom, node4Bottom); + expect(innerGroup.pos[1]).toBeGreaterThan(membersBottom); + }); + + it("maintains no overlaps after default layout", () => { + const graph = loadFixture("group-test-simple.json"); + layoutSelectedGroups(graph, new Set([1])); + assertNodesInsideGroups(graph); + assertFiniteCoordinates(graph); + }); + + it("is idempotent with nested groups in default mode", () => { + const graph = loadFixture("group-test-simple.json"); + layoutSelectedGroups(graph, new Set([1])); + const positions1 = capturePositions(graph); + + layoutSelectedGroups(graph, new Set([1])); + const positions2 = capturePositions(graph); + + // Compare positions with small tolerance + for (const [id, pos1] of positions1) { + const pos2 = positions2.get(id); + if (pos2) { + expect(Math.abs(pos1.x - pos2.x)).toBeLessThan(5); + expect(Math.abs(pos1.y - pos2.y)).toBeLessThan(5); + } + } + }); + + it("multiple sibling nested groups stack vertically", () => { + const graph: LGraph = { + _nodes: [ + // Disconnected node in parent + { + id: 1, + type: "Note", + title: "Parent Node", + pos: [100, 100] as [number, number], + size: [200, 100] as [number, number], + inputs: [], + outputs: [], + }, + // Node in first nested group + { + id: 2, + type: "Note", + title: "Nested 1 Node", + pos: [100, 300] as [number, number], + size: [200, 100] as [number, number], + inputs: [], + outputs: [], + }, + // Node in second nested group + { + id: 3, + type: "Note", + title: "Nested 2 Node", + pos: [100, 500] as [number, number], + size: [200, 100] as [number, number], + inputs: [], + outputs: [], + }, + ], + _groups: [ + { + id: 1, + title: "Parent Group", + pos: [50, 50] as [number, number], + size: [400, 600] as [number, number], + }, + { + id: 2, + title: "Nested 1", + pos: [80, 250] as [number, number], + size: [300, 180] as [number, number], + }, + { + id: 3, + title: "Nested 2", + pos: [80, 450] as [number, number], + size: [300, 180] as [number, number], + }, + ], + links: new Map(), + }; + + layoutSelectedGroups(graph, new Set([1])); + + const nested1 = graph._groups.find((g) => g.id === 2)!; + const nested2 = graph._groups.find((g) => g.id === 3)!; + const parentNode = graph._nodes.find((n) => n.id === 1)!; + + // Both nested groups should be below the parent node + const parentNodeBottom = parentNode.pos[1] + parentNode.size[1]; + expect(nested1.pos[1]).toBeGreaterThan(parentNodeBottom); + expect(nested2.pos[1]).toBeGreaterThan(parentNodeBottom); + + // Nested groups should not overlap each other + const nested1Bottom = nested1.pos[1] + nested1.size[1]; + expect(nested2.pos[1]).toBeGreaterThanOrEqual(nested1Bottom); + + assertFiniteCoordinates(graph); + }); + }); + describe("nested group position update regression (group-test.json)", () => { it("nested group position updates when parent has token layout", () => { const graph = loadFixture("group-test.json");