From e9ee9f2315fc489da3e36ee62f03dccba4519d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Candice=20Bent=C3=A9jac?= Date: Fri, 2 Feb 2024 15:23:44 +0100 Subject: [PATCH] Update `ListAttributes` identically when removing edges or nodes The behaviour regarding `ListAttributes` was different depending on whether an edge or a node was removed: - if an edge was removed, the `ListAttribute` that was the destination of that edge had its corresponding element completely removed; - if a node was removed, the `ListAttribute` that was the destination of one of the node's edges had its corresponding element reset, but not removed. With this behaviour, a user had different UIDs depending on whether a single edge or the whole node had been removed where the UID should always be identical. --- meshroom/core/graph.py | 47 +++++++++++++++++++++++++++++++++++------ meshroom/ui/commands.py | 26 ++++++++++++++++++++--- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/meshroom/core/graph.py b/meshroom/core/graph.py index 265fe8273f..099a8ad658 100644 --- a/meshroom/core/graph.py +++ b/meshroom/core/graph.py @@ -645,18 +645,38 @@ def nodeOutEdges(self, node): @changeTopology def removeNode(self, nodeName): """ - Remove the node identified by 'nodeName' from the graph - and return in and out edges removed by this operation in two dicts {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()} + Remove the node identified by 'nodeName' from the graph. + Returns: + - a dictionary containing the incoming edges removed by this operation: {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()} + - a dictionary containing the outgoing edges removed by this operation: {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()} + - a dictionary containing the values, indices and keys of attributes that were connected to a ListAttribute prior to the removal of all edges: + {dstAttr.getFullNameToNode(), (dstAttr.root.getFullNameToNode(), dstAttr.index, dstAttr.value)} """ node = self.node(nodeName) inEdges = {} outEdges = {} + outListAttributes = {} # Remove all edges arriving to and starting from this node with GraphModification(self): + # Two iterations over the outgoing edges are necessary: + # - the first one is used to collect all the information about the edges while they are all there (overall context) + # - once we have collected all the information, the edges (and perhaps the entries in ListAttributes) can actually be removed for edge in self.nodeOutEdges(node): - self.removeEdge(edge.dst) outEdges[edge.dst.getFullNameToNode()] = edge.src.getFullNameToNode() + + if isinstance(edge.dst.root, ListAttribute): + index = edge.dst.root.index(edge.dst) + outListAttributes[edge.dst.getFullNameToNode()] = (edge.dst.root.getFullNameToNode(), index, edge.dst.value if edge.dst.value else None) + + for edge in self.nodeOutEdges(node): + self.removeEdge(edge.dst) + + # Remove the corresponding attributes from the ListAttributes instead of just emptying their values + if isinstance(edge.dst.root, ListAttribute): + index = edge.dst.root.index(edge.dst) + edge.dst.root.remove(index) + for edge in self.nodeInEdges(node): self.removeEdge(edge.dst) inEdges[edge.dst.getFullNameToNode()] = edge.src.getFullNameToNode() @@ -667,7 +687,7 @@ def removeNode(self, nodeName): self._importedNodes.remove(node) self.update() - return inEdges, outEdges + return inEdges, outEdges, outListAttributes def addNewNode(self, nodeType, name=None, position=None, **kwargs): """ @@ -707,22 +727,35 @@ def upgradeNode(self, nodeName): nodeName (str): the name of the CompatibilityNode to upgrade Returns: - the list of deleted input/output edges + - the upgraded (newly created) node + - a dictionary containing the incoming edges removed by this operation: {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()} + - a dictionary containing the outgoing edges removed by this operation: {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()} + - a dictionary containing the values, indices and keys of attributes that were connected to a ListAttribute prior to the removal of all edges: + {dstAttr.getFullNameToNode(), (dstAttr.root.getFullNameToNode(), dstAttr.index, dstAttr.value)} """ node = self.node(nodeName) if not isinstance(node, CompatibilityNode): raise ValueError("Upgrade is only available on CompatibilityNode instances.") upgradedNode = node.upgrade() with GraphModification(self): - inEdges, outEdges = self.removeNode(nodeName) + inEdges, outEdges, outListAttributes = self.removeNode(nodeName) self.addNode(upgradedNode, nodeName) for dst, src in outEdges.items(): + # Re-create the entries in ListAttributes that were completely removed during the call to "removeNode" + # If they are not re-created first, adding their edges will lead to errors + # 0 = attribute name, 1 = attribute index, 2 = attribute value + if dst in outListAttributes.keys(): + listAttr = self.attribute(outListAttributes[dst][0]) + if isinstance(outListAttributes[dst][2], list): + listAttr[outListAttributes[dst][1]:outListAttributes[dst][1]] = outListAttributes[dst][2] + else: + listAttr.insert(outListAttributes[dst][1], outListAttributes[dst][2]) try: self.addEdge(self.attribute(src), self.attribute(dst)) except (KeyError, ValueError) as e: logging.warning("Failed to restore edge {} -> {}: {}".format(src, dst, str(e))) - return upgradedNode, inEdges, outEdges + return upgradedNode, inEdges, outEdges, outListAttributes def upgradeAllNodes(self): """ Upgrade all upgradable CompatibilityNode instances in the graph. """ diff --git a/meshroom/ui/commands.py b/meshroom/ui/commands.py index 24f48fc41d..ac3295e9a7 100755 --- a/meshroom/ui/commands.py +++ b/meshroom/ui/commands.py @@ -156,10 +156,11 @@ def __init__(self, graph, node, parent=None): self.nodeName = node.getName() self.setText("Remove Node {}".format(self.nodeName)) self.outEdges = {} + self.outListAttributes = {} # maps attribute's key with a tuple containing the name of the list it is connected to and its value def redoImpl(self): - # only keep outEdges since inEdges are serialized in nodeDict - _, self.outEdges = self.graph.removeNode(self.nodeName) + # keep outEdges (inEdges are serialized in nodeDict so unneeded here) and outListAttributes to be able to recreate the deleted elements in ListAttributes + _, self.outEdges, self.outListAttributes = self.graph.removeNode(self.nodeName) return True def undoImpl(self): @@ -169,6 +170,15 @@ def undoImpl(self): assert (node.getName() == self.nodeName) # recreate out edges deleted on node removal for dstAttr, srcAttr in self.outEdges.items(): + # if edges were connected to ListAttributes, recreate their corresponding entry in said ListAttribute + # 0 = attribute name, 1 = attribute index, 2 = attribute value + if dstAttr in self.outListAttributes.keys(): + listAttr = self.graph.attribute(self.outListAttributes[dstAttr][0]) + if isinstance(self.outListAttributes[dstAttr][2], list): + listAttr[self.outListAttributes[dstAttr][1]:self.outListAttributes[dstAttr][1]] = self.outListAttributes[dstAttr][2] + else: + listAttr.insert(self.outListAttributes[dstAttr][1], self.outListAttributes[dstAttr][2]) + self.graph.addEdge(self.graph.attribute(srcAttr), self.graph.attribute(dstAttr)) @@ -410,12 +420,13 @@ def __init__(self, graph, node, parent=None): self.nodeDict = node.toDict() self.nodeName = node.getName() self.outEdges = {} + self.outListAttributes = {} self.setText("Upgrade Node {}".format(self.nodeName)) def redoImpl(self): if not self.graph.node(self.nodeName).canUpgrade: return False - upgradedNode, inEdges, self.outEdges = self.graph.upgradeNode(self.nodeName) + upgradedNode, _, self.outEdges, self.outListAttributes = self.graph.upgradeNode(self.nodeName) return upgradedNode def undoImpl(self): @@ -427,6 +438,15 @@ def undoImpl(self): self.graph.addNode(node, self.nodeName) # recreate out edges for dstAttr, srcAttr in self.outEdges.items(): + # if edges were connected to ListAttributes, recreate their corresponding entry in said ListAttribute + # 0 = attribute name, 1 = attribute index, 2 = attribute value + if dstAttr in self.outListAttributes.keys(): + listAttr = self.graph.attribute(self.outListAttributes[dstAttr][0]) + if isinstance(self.outListAttributes[dstAttr][2], list): + listAttr[self.outListAttributes[dstAttr][1]:self.outListAttributes[dstAttr][1]] = self.outListAttributes[dstAttr][2] + else: + listAttr.insert(self.outListAttributes[dstAttr][1], self.outListAttributes[dstAttr][2]) + self.graph.addEdge(self.graph.attribute(srcAttr), self.graph.attribute(dstAttr))