diff --git a/meshroom/core/graph.py b/meshroom/core/graph.py index 96ba620b67..ae5e88d81f 100644 --- a/meshroom/core/graph.py +++ b/meshroom/core/graph.py @@ -953,6 +953,73 @@ def addEdge(self, srcAttr: Attribute, dstAttr: Attribute) -> tuple[list[Attribut srcAttr.outputLinksChanged.emit() return [edge.src, edge.dst], deletedEdge + @changeTopology + def addListEdges(self, srcAttr: Attribute, dstAttr: Attribute) -> tuple[list[list[Attribute]], list[list[Attribute]]]: + """ + Connect srcAttr to dstAttr, handling ListAttribute specifics: + - List -> Scalar: connect first element of the source list to the destination. + - Scalar -> List: append an element to the destination list, then connect. + - List -> List: if the destination is already linked (as a "view"), decompose the + existing link into individual element links, then append and connect each element + of the source list individually. + - Scalar -> Scalar: delegate to addEdge directly. + + Args: + srcAttr: the source Attribute + dstAttr: the destination Attribute + + Returns: + A tuple containing: + - a list of [src, dst] pairs for every created edge + - a list of [src, dst] pairs for every deleted edge + """ + createdEdges = [] + deletedEdges = [] + + if isinstance(srcAttr, ListAttribute) and not isinstance(dstAttr, ListAttribute): + # List -> Scalar: connect first element of src to dst + connected, deleted = srcAttr.at(0).connectTo(dstAttr) + createdEdges += connected + deletedEdges += deleted + + elif isinstance(dstAttr, ListAttribute) and not isinstance(srcAttr, ListAttribute): + # Scalar -> List: append a new element to dst and connect src to it + with GraphModification(self): + dstAttr.append("") + connected, deleted = srcAttr.connectTo(dstAttr.at(-1)) + createdEdges += connected + deletedEdges += deleted + + elif isinstance(srcAttr, ListAttribute) and isinstance(dstAttr, ListAttribute): + # List -> List + with GraphModification(self): + if dstAttr.isLink: + # Destination is a "view" on another ListAttribute. + # Decompose the existing link into individual element links. + existingEdge = self.edge(dstAttr) + existingSrc = existingEdge.src # the previously connected ListAttribute + deleted = dstAttr.disconnectEdge() + deletedEdges += deleted + for j in range(len(existingSrc)): + dstAttr.append("") + connected, deleted = existingSrc.at(j).connectTo(dstAttr.at(-1)) + createdEdges += connected + deletedEdges += deleted + + # Append and connect each element of src individually + for i in range(len(srcAttr)): + dstAttr.append("") + connected, deleted = srcAttr.at(i).connectTo(dstAttr.at(-1)) + createdEdges += connected + deletedEdges += deleted + else: + # Scalar -> Scalar + connected, deleted = srcAttr.connectTo(dstAttr) + createdEdges += connected + deletedEdges += deleted + + return createdEdges, deletedEdges + @changeTopology def removeEdge(self, dstAttr: Attribute): if not self.edges.get(dstAttr): diff --git a/meshroom/ui/commands.py b/meshroom/ui/commands.py index e43e404689..9ca8f949a6 100755 --- a/meshroom/ui/commands.py +++ b/meshroom/ui/commands.py @@ -487,6 +487,42 @@ def undoImpl(self) -> bool: return True +class AddListEdgesCommand(GraphCommand): + """ + Command to connect two ListAttributes by decomposing them into individual + element-level edges. Handles the case where the destination already has + a list-level link (decomposes it first). + """ + def __init__(self, graph, src, dst, parent=None): + super().__init__(graph, parent) + self.srcAttr = src.fullName + self.dstAttr = dst.fullName + self.createdEdges = [] # List of all edges created + self.deletedEdges = [] # List of all edges deleted + self.initialDstLen = len(dst) # Length of dst before any modification + self.setText(f"Connect Lists '{self.srcAttr}' -> '{self.dstAttr}'") + + def redoImpl(self) -> bool: + srcAttr = self.graph.attribute(self.srcAttr) + dstAttr = self.graph.attribute(self.dstAttr) + self.createdEdges, self.deletedEdges = self.graph.addListEdges(srcAttr, dstAttr) + return True + + def undoImpl(self) -> bool: + dstAttr = self.graph.attribute(self.dstAttr) + # Disconnect all created edges + for edge in self.createdEdges: + edge[1].disconnectEdge() + # Remove all appended elements (restore dst to its initial length) + currentLen = len(dstAttr) + if currentLen > self.initialDstLen: + dstAttr.remove(self.initialDstLen, currentLen - self.initialDstLen) + # Restore all deleted edges + for edge in self.deletedEdges: + edge[0].connectTo(edge[1]) + return True + + class ListAttributeAppendCommand(GraphCommand): def __init__(self, graph, listAttribute, value, parent=None): super().__init__(graph, parent) diff --git a/meshroom/ui/graph.py b/meshroom/ui/graph.py index eaa1a8f8ab..c417a24c33 100644 --- a/meshroom/ui/graph.py +++ b/meshroom/ui/graph.py @@ -1231,11 +1231,19 @@ def clearDataFrom(self, nodes: list[Node]): @Slot(Attribute, Attribute) def addEdge(self, src, dst): if isinstance(src, ListAttribute) and not isinstance(dst, ListAttribute): + # Source is a list, Destination is not a list + # Connect first attribute of the list to the destination self._addEdge(src.at(0), dst) elif isinstance(dst, ListAttribute) and not isinstance(src, ListAttribute): + # Source is not a list, Destination is a list + # Append the source to the destination's list attributes with self.groupedGraphModification(f"Insert and Add Edge on {dst.fullName}"): self.appendAttribute(dst) self._addEdge(src, dst.at(-1)) + elif isinstance(src, ListAttribute) and isinstance(dst, ListAttribute): + # Both Source and Destination attributes are listAttributes + with self.groupedGraphModification(f"Insert and Add Edges on {dst.fullName}"): + self.push(commands.AddListEdgesCommand(self._graph, src, dst)) else: self._addEdge(src, dst) diff --git a/meshroom/ui/qml/GraphEditor/AttributePin.qml b/meshroom/ui/qml/GraphEditor/AttributePin.qml index f3b30c0d56..32d7d017d4 100755 --- a/meshroom/ui/qml/GraphEditor/AttributePin.qml +++ b/meshroom/ui/qml/GraphEditor/AttributePin.qml @@ -130,12 +130,11 @@ RowLayout { || drag.source.objectName != inputDragTarget.objectName // Not an edge connector || !validIncomingConnection // Connection is not allowed || drag.source.nodeItem === inputDragTarget.nodeItem // Connection between attributes of the same node - || drag.source.isList && childrenRepeater.count // Source/target are lists but target already has children || drag.source.connectorType === "input" // Refuse to connect an "input pin" on another one (input attr can be connected to input attr, but not the graphical pin) ) { // Refuse attributes connection drag.accepted = false - } else if (inputDragTarget.attribute.isLink) { // Already connected attribute + } else if (inputDragTarget.attribute.isLink && !(drag.source.isList && root.isList)) { // Already connected attribute (not list-to-list, which is additive) root.edgeAboutToBeRemoved(inputDragTarget.attribute) } inputDropArea.acceptableDrop = drag.accepted @@ -367,12 +366,11 @@ RowLayout { || !validIncomingConnection // Connection is not allowed || drag.source.nodeItem === outputDragTarget.nodeItem // Connection between attributes of the same node || (!drag.source.isList && outputDragTarget.isList) // Connection between a list and a simple attribute - || (drag.source.isList && childrenRepeater.count) // Source/target are lists but target already has children || drag.source.connectorType === "output" // Refuse to connect an output pin on another one ) { // Refuse attributes connection drag.accepted = false - } else if (drag.source.attribute.isLink) { // Already connected attribute + } else if (drag.source.attribute.isLink && !(drag.source.isList && root.isList)) { // Already connected attribute (not list-to-list, which is additive) root.edgeAboutToBeRemoved(drag.source.attribute) } outputDropArea.acceptableDrop = drag.accepted diff --git a/tests/test_listAttribute.py b/tests/test_listAttribute.py index 3b28aa6e94..817aa3832b 100644 --- a/tests/test_listAttribute.py +++ b/tests/test_listAttribute.py @@ -62,3 +62,96 @@ def test_elementAccessUsesLinkParam(self): assert nodeB.listInput.at(0).node == nodeA assert nodeB.listInput.index(nodeB.listInput.at(0)) == 0 + + def test_connectToTwoListAttributesReplacesLink(self): + """ + Test that connecting two different ListAttributes to the same destination + ListAttribute using connectTo replaces the first link with the second one. + """ + graph = Graph("") + + nodeA = graph.addNewNode(NodeWithListAttribute.__name__) + nodeB = graph.addNewNode(NodeWithListAttribute.__name__) + nodeC = graph.addNewNode(NodeWithListAttribute.__name__) + + nodeA.listInput.extend(["A1", "A2", "A3"]) + nodeB.listInput.extend(["B1", "B2"]) + + # First connection: nodeA.listInput -> nodeC.listInput + nodeA.listInput.connectTo(nodeC.listInput) + assert nodeC.listInput.isLink + assert len(nodeC.listInput) == 3 + assert nodeC.listInput.at(0).node == nodeA + + # Second connection replaces the first (connectTo disconnects root) + nodeB.listInput.connectTo(nodeC.listInput) + assert nodeC.listInput.isLink + assert len(nodeC.listInput) == 2 + assert nodeC.listInput.at(0).node == nodeB + assert nodeC.listInput.at(1).node == nodeB + assert not nodeA.listInput.hasAnyOutputLinks + + def test_addListEdgesAccumulatesElements(self): + """ + Test that Graph.addListEdges correctly decomposes an existing list-level + link and accumulates individual element-level links from multiple sources. + """ + graph = Graph("") + + nodeA = graph.addNewNode(NodeWithListAttribute.__name__) + nodeB = graph.addNewNode(NodeWithListAttribute.__name__) + nodeC = graph.addNewNode(NodeWithListAttribute.__name__) + + nodeA.listInput.extend(["A1", "A2"]) + nodeB.listInput.extend(["B1", "B2"]) + + # First addListEdges: nodeA -> nodeC + createdEdges, deletedEdges = graph.addListEdges(nodeA.listInput, nodeC.listInput) + assert len(createdEdges) == 2 + assert len(deletedEdges) == 0 + assert len(nodeC.listInput) == 2 + assert nodeC.listInput.at(0).isLink + assert nodeC.listInput.at(1).isLink + assert nodeC.listInput.at(0).inputLink.node == nodeA + assert nodeC.listInput.at(1).inputLink.node == nodeA + + # Second addListEdges: nodeB -> nodeC (accumulates) + createdEdges, deletedEdges = graph.addListEdges(nodeB.listInput, nodeC.listInput) + assert len(createdEdges) == 2 + assert len(deletedEdges) == 0 + + # All 4 element-level links should be preserved + assert len(nodeC.listInput) == 4 + assert nodeC.listInput.at(0).inputLink.node == nodeA + assert nodeC.listInput.at(1).inputLink.node == nodeA + assert nodeC.listInput.at(2).inputLink.node == nodeB + assert nodeC.listInput.at(3).inputLink.node == nodeB + + def test_addListEdgesDecomposesExistingViewLink(self): + """ + Test that Graph.addListEdges decomposes an existing list-level 'view' link + into individual element links before adding new ones. + """ + graph = Graph("") + + nodeA = graph.addNewNode(NodeWithListAttribute.__name__) + nodeB = graph.addNewNode(NodeWithListAttribute.__name__) + nodeC = graph.addNewNode(NodeWithListAttribute.__name__) + + nodeA.listInput.extend(["A1", "A2"]) + nodeB.listInput.extend(["B1"]) + + # Create a list-level "view" link: nodeA.listInput -> nodeC.listInput + nodeA.listInput.connectTo(nodeC.listInput) + assert nodeC.listInput.isLink + assert len(nodeC.listInput) == 2 + + # addListEdges should decompose the view and add nodeB's elements + createdEdges, deletedEdges = graph.addListEdges(nodeB.listInput, nodeC.listInput) + + # The view link was deleted, individual links from nodeA were re-created, + # plus a new link from nodeB + assert len(nodeC.listInput) == 3 + assert nodeC.listInput.at(0).inputLink.node == nodeA + assert nodeC.listInput.at(1).inputLink.node == nodeA + assert nodeC.listInput.at(2).inputLink.node == nodeB