From bf5e3f3c772044f9a932761a09f7bc47da9161c1 Mon Sep 17 00:00:00 2001 From: Chenlei Hu Date: Mon, 7 Apr 2025 19:52:58 -0400 Subject: [PATCH] [Bug] Fix layout issue caused by absolute positioned slots (#908) Before: ![image](https://github.com/user-attachments/assets/00bbbf14-a153-4301-b4c4-0c30cb2ebcc7) After: ![image](https://github.com/user-attachments/assets/2247a0a5-2757-478c-b701-9ceab7a133cc) Workflow: [dynamically_added_input.json](https://github.com/user-attachments/files/19640371/dynamically_added_input.json) This PR also fixes the issue that output slot's `pos` property is not relative to node pos. --- src/LGraphNode.ts | 31 +++++++++++++-- test/LGraphNode.test.ts | 87 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 113 insertions(+), 5 deletions(-) diff --git a/src/LGraphNode.ts b/src/LGraphNode.ts index 75d4a59cbf..6426a293ec 100644 --- a/src/LGraphNode.ts +++ b/src/LGraphNode.ts @@ -2871,16 +2871,37 @@ export class LGraphNode implements Positionable, IPinnable, IColorable { // default vertical slots const offset = LiteGraph.NODE_SLOT_HEIGHT * 0.5 + const slotIndex = is_input + ? this.#defaultVerticalInputs.indexOf(this.inputs[slot_number]) + : this.#defaultVerticalOutputs.indexOf(this.outputs[slot_number]) + out[0] = is_input ? nodeX + offset : nodeX + this.size[0] + 1 - offset out[1] = nodeY + - (slot_number + 0.7) * LiteGraph.NODE_SLOT_HEIGHT + + (slotIndex + 0.7) * LiteGraph.NODE_SLOT_HEIGHT + (this.constructor.slot_start_y || 0) return out } + /** + * @internal The inputs that are not positioned with absolute coordinates. + */ + get #defaultVerticalInputs() { + return this.inputs.filter((slot: INodeInputSlot) => !( + slot.pos || + (this.widgets?.length && isWidgetInputSlot(slot)) + )) + } + + /** + * @internal The outputs that are not positioned with absolute coordinates. + */ + get #defaultVerticalOutputs() { + return this.outputs.filter((slot: INodeOutputSlot) => !slot.pos) + } + /** * Gets the position of an input slot, in graph co-ordinates. * @@ -2902,7 +2923,8 @@ export class LGraphNode implements Positionable, IPinnable, IColorable { // default vertical slots const offsetX = LiteGraph.NODE_SLOT_HEIGHT * 0.5 const nodeOffsetY = this.constructor.slot_start_y || 0 - const slotY = (slot + 0.7) * LiteGraph.NODE_SLOT_HEIGHT + const slotIndex = this.#defaultVerticalInputs.indexOf(this.inputs[slot]) + const slotY = (slotIndex + 0.7) * LiteGraph.NODE_SLOT_HEIGHT return [nodeX + offsetX, nodeY + slotY + nodeOffsetY] } @@ -2924,12 +2946,13 @@ export class LGraphNode implements Positionable, IPinnable, IColorable { } const outputPos = outputs?.[slot]?.pos - if (outputPos) return outputPos + if (outputPos) return [nodeX + outputPos[0], nodeY + outputPos[1]] // default vertical slots const offsetX = LiteGraph.NODE_SLOT_HEIGHT * 0.5 const nodeOffsetY = this.constructor.slot_start_y || 0 - const slotY = (slot + 0.7) * LiteGraph.NODE_SLOT_HEIGHT + const slotIndex = this.#defaultVerticalOutputs.indexOf(this.outputs[slot]) + const slotY = (slotIndex + 0.7) * LiteGraph.NODE_SLOT_HEIGHT // TODO: Why +1? return [nodeX + width + 1 - offsetX, nodeY + slotY + nodeOffsetY] diff --git a/test/LGraphNode.test.ts b/test/LGraphNode.test.ts index 1fabdeaf03..21235884ee 100644 --- a/test/LGraphNode.test.ts +++ b/test/LGraphNode.test.ts @@ -1,6 +1,6 @@ import { describe, expect } from "vitest" -import { LGraphNode } from "@/litegraph" +import { LGraphNode, LiteGraph } from "@/litegraph" import { LGraph } from "@/litegraph" import { NodeInputSlot, NodeOutputSlot } from "@/NodeSlot" @@ -314,4 +314,89 @@ describe("LGraphNode", () => { expect(slot?.name).toBe("Input1") }) }) + + describe("LGraphNode slot positioning", () => { + test("should correctly position slots with absolute coordinates", () => { + // Setup + const node = new LGraphNode("test") + node.pos = [100, 100] + + // Add input/output with absolute positions + node.addInput("abs-input", "number") + node.inputs[0].pos = [10, 20] + + node.addOutput("abs-output", "number") + node.outputs[0].pos = [50, 30] + + // Test + const inputPos = node.getInputPos(0) + const outputPos = node.getOutputPos(0) + + // Absolute positions should be relative to node position + expect(inputPos).toEqual([110, 120]) // node.pos + slot.pos + expect(outputPos).toEqual([150, 130]) // node.pos + slot.pos + }) + + test("should correctly position default vertical slots", () => { + // Setup + const node = new LGraphNode("test") + node.pos = [100, 100] + + // Add multiple inputs/outputs without absolute positions + node.addInput("input1", "number") + node.addInput("input2", "number") + node.addOutput("output1", "number") + node.addOutput("output2", "number") + + // Calculate expected positions + const slotOffset = LiteGraph.NODE_SLOT_HEIGHT * 0.5 + const slotSpacing = LiteGraph.NODE_SLOT_HEIGHT + const nodeWidth = node.size[0] + + // Test input positions + expect(node.getInputPos(0)).toEqual([ + 100 + slotOffset, + 100 + (0 + 0.7) * slotSpacing, + ]) + expect(node.getInputPos(1)).toEqual([ + 100 + slotOffset, + 100 + (1 + 0.7) * slotSpacing, + ]) + + // Test output positions + expect(node.getOutputPos(0)).toEqual([ + 100 + nodeWidth + 1 - slotOffset, + 100 + (0 + 0.7) * slotSpacing, + ]) + expect(node.getOutputPos(1)).toEqual([ + 100 + nodeWidth + 1 - slotOffset, + 100 + (1 + 0.7) * slotSpacing, + ]) + }) + + test("should skip absolute positioned slots when calculating vertical positions", () => { + // Setup + const node = new LGraphNode("test") + node.pos = [100, 100] + + // Add mix of absolute and default positioned slots + node.addInput("abs-input", "number") + node.inputs[0].pos = [10, 20] + node.addInput("default-input1", "number") + node.addInput("default-input2", "number") + + const slotOffset = LiteGraph.NODE_SLOT_HEIGHT * 0.5 + const slotSpacing = LiteGraph.NODE_SLOT_HEIGHT + + // Test: default positioned slots should be consecutive, ignoring absolute positioned ones + expect(node.getInputPos(1)).toEqual([ + 100 + slotOffset, + 100 + (0 + 0.7) * slotSpacing, // First default slot starts at index 0 + ]) + expect(node.getInputPos(2)).toEqual([ + 100 + slotOffset, + 100 + (1 + 0.7) * slotSpacing, // Second default slot at index 1 + ]) + }) + }) })