[ui] Graph: remove selectedNodes model

Expose `getSelectedNode` that relies on the QItemSelectionModel
for imperative code in QML that still requires to access the
selected node instances.

Author: yann-lty

meshroom/ui/graph.py

@@ -7,7 +7,7 @@
 from enum import Enum
 from threading import Thread, Event, Lock
 from multiprocessing.pool import ThreadPool
-from typing import Iterator
+from typing import Iterator, Optional, Union
 
 from PySide6.QtCore import (
     Slot,
@@ -369,9 +369,7 @@ def __init__(self, undoStack, taskManager, parent=None):
         self._sortedDFSChunks = QObjectListModel(parent=self)
         self._layout = GraphLayout(self)
         self._selectedNode = None
-        self._selectedNodes = QObjectListModel(parent=self)
         self._nodeSelection = QItemSelectionModel(self._graph.nodes, parent=self)
-        self._nodeSelection.selectionChanged.connect(self.onNodeSelectionChanged)
         self._hoveredNode = None
 
         self.submitLabel = "{projectName}"
@@ -516,9 +514,10 @@ def updateLockedUndoStack(self):
         else:
             self._undoStack.unlock()
 
-    @Slot(QObjectListModel)
+    @Slot()
     @Slot(Node)
-    def execute(self, nodes=None):
+    @Slot(list)
+    def execute(self, nodes: Optional[Union[list[Node], Node]] = None):
         nodes = [nodes] if not isinstance(nodes, Iterable) and nodes else nodes
         self._taskManager.compute(self._graph, nodes)
         self.updateLockedUndoStack()  # explicitly call the update while it is already computing
@@ -554,9 +553,10 @@ def cancelNodeComputation(self, node):
                 n.clearSubmittedChunks()
                 self._taskManager.removeNode(n, displayList=True, processList=True)
 
-    @Slot(QObjectListModel)
+    @Slot()
     @Slot(Node)
-    def submit(self, nodes=None):
+    @Slot(list)
+    def submit(self, nodes: Optional[Union[list[Node], Node]] = None):
         """ Submit the graph to the default Submitter.
         If a node is specified, submit this node and its uncomputed predecessors.
         Otherwise, submit the whole
@@ -696,16 +696,14 @@ def removeNodes(self, nodes: list[Node]):
             for node in nodes:
                 self.push(commands.RemoveNodeCommand(self._graph, node))
 
-    @Slot(QObject)
-    def removeNodesFrom(self, nodes):
+    @Slot(list)
+    def removeNodesFrom(self, nodes: list[Node]):
         """
-        Remove all nodes starting from 'startNode' to graph leaves.
+        Remove all nodes starting from 'nodes' to graph leaves.
 
         Args:
-            startNode (Node): the node to start from.
+            nodes: the nodes to start from.
         """
-        if isinstance(nodes, Node):
-            nodes = [nodes]
         with self.groupedGraphModification("Remove Nodes From Selected Nodes"):
             nodesToRemove, _ = self._graph.dfsOnDiscover(startNodes=nodes, reverse=True, dependenciesOnly=True)
             # filter out nodes that will be removed more than once
@@ -714,17 +712,17 @@ def removeNodesFrom(self, nodes):
             # can be re-created in correct order on redo.
             self.removeNodes(list(reversed(uniqueNodesToRemove)))
 
-    @Slot(QObject, result="QVariantList")
-    def duplicateNodes(self, nodes):
+    @Slot(list, result=list)
+    def duplicateNodes(self, nodes: list[Node]) -> list[Node]:
         """
         Duplicate 'nodes'.
 
         Args:
-            nodes (list[Node]): the nodes to duplicate
+            nodes: the nodes to duplicate.
+
         Returns:
-            list[Node]: the list of duplicated nodes
+            The list of duplicated nodes.
         """
-        nodes = self.filterNodes(nodes)
         nPositions = [(n.x, n.y) for n in self._graph.nodes]
         # enable updates between duplication and layout to get correct depths during layout
         with self.groupedGraphModification("Duplicate Selected Nodes", disableUpdates=False):
@@ -747,18 +745,16 @@ def duplicateNodes(self, nodes):
 
         return duplicates
 
-    @Slot(QObject, result="QVariantList")
-    def duplicateNodesFrom(self, nodes):
+    @Slot(list, result=list)
+    def duplicateNodesFrom(self, nodes: list[Node]) -> list[Node]:
         """
         Duplicate all nodes starting from 'nodes' to graph leaves.
 
         Args:
-            nodes (list[Node]): the nodes to start from.
+            node: The nodes to start from.
         Returns:
-            list[Node]: the list of duplicated nodes
+            The list of duplicated nodes.
         """
-        if isinstance(nodes, Node):
-            nodes = [nodes]
         with self.groupedGraphModification("Duplicate Nodes From Selected Nodes"):
             nodesToDuplicate, _ = self._graph.dfsOnDiscover(startNodes=nodes, reverse=True, dependenciesOnly=True)
             # filter out nodes that will be duplicated more than once
@@ -789,7 +785,7 @@ def expandForLoop(self, currentEdge):
             dst = currentEdge.dst
 
             for i in range(1, len(listAttribute)):
-                duplicates = self.duplicateNodesFrom(dst.node)
+                duplicates = self.duplicateNodesFrom([dst.node])
                 newNode = duplicates[0]
                 previousEdge = self.graph.edge(newNode.attribute(dst.name))
                 self.replaceEdge(previousEdge, listAttribute.at(i), previousEdge.dst)
@@ -809,7 +805,7 @@ def collapseForLoop(self, currentEdge):
                     continue
                 occurence = allSrc.index(listAttribute.at(i)) if listAttribute.at(i) in allSrc else -1
                 if occurence != -1:
-                    self.removeNodesFrom(self.graph.edges.at(occurence).dst.node)
+                    self.removeNodesFrom([self.graph.edges.at(occurence).dst.node])
                     # update the edges from allSrc
                     allSrc = [e.src for e in self._graph.edges.values()]
 
@@ -954,11 +950,6 @@ def removeImagesFromAllGroups(self):
         with self.groupedGraphModification("Remove Images From All CameraInit Nodes"):
             self.push(commands.RemoveImagesCommand(self._graph, list(self.cameraInits)))
 
-    def onNodeSelectionChanged(self, selected, deselected):
-        # Update internal cache of selected Node instances.
-        self._selectedNodes.setObjectList(list(self.iterSelectedNodes()))
-        self.selectedNodesChanged.emit()
-
     @Slot(list)
     @Slot(list, int)
     def selectNodes(self, nodes, command=QItemSelectionModel.SelectionFlag.ClearAndSelect):
@@ -1014,6 +1005,11 @@ def iterSelectedNodes(self) -> Iterator[Node]:
         for idx in self._nodeSelection.selectedRows():
             yield self._graph.nodes.at(idx.row())
 
+    @Slot(result=list)
+    def getSelectedNodes(self) -> list[Node]:
+        """Return the list of selected Node instances."""
+        return list(self.iterSelectedNodes())
+
     @Slot(Node, result=bool)
     def isSelected(self, node: Node) -> bool:
         """Whether `node` is part of the current selection."""
@@ -1046,19 +1042,17 @@ def setSelectedNodesColor(self, color: str):
     @Slot(result=str)
     def getSelectedNodesContent(self) -> str:
         """
-        Return the content of the currently selected nodes in a string, formatted to JSON.
-        If no node is currently selected, an empty string is returned.
+        Serialize the current node selection and return it as JSON formatted string.
+
+        Returns an empty string if the selection is empty.
         """
-        if self._selectedNodes:
-            d = self._graph.toDict()
-            selection = {}
-            for node in self._selectedNodes:
-                selection[node.name] = d[node.name]
-            return json.dumps(selection, indent=4)
-        return ''
-
-    @Slot(str, QPoint, bool, result="QVariantList")
-    def pasteNodes(self, clipboardContent, position=None, centerPosition=False):
+        if not self._nodeSelection.hasSelection():
+            return ""
+        serializedSelection = {node.name: node.toDict() for node in self.iterSelectedNodes()}
+        return json.dumps(serializedSelection, indent=4)
+
+    @Slot(str, QPoint, bool, result=list)
+    def pasteNodes(self, clipboardContent, position=None, centerPosition=False) -> list[Node]:
         """
         Parse the content of the clipboard to see whether it contains
         valid node descriptions. If that is the case, the nodes described
@@ -1195,10 +1189,6 @@ def pasteNodes(self, clipboardContent, position=None, centerPosition=False):
     # Current main selected node
     selectedNode = makeProperty(QObject, "_selectedNode", selectedNodeChanged, resetOnDestroy=True)
 
-    selectedNodesChanged = Signal()
-    # Currently selected nodes
-    selectedNodes = makeProperty(QObject, "_selectedNodes", selectedNodesChanged, resetOnDestroy=True)
-
     nodeSelection = makeProperty(QObject, "_nodeSelection")
 
     hoveredNodeChanged = Signal()

meshroom/ui/qml/GraphEditor/GraphEditor.qml

@@ -63,11 +63,10 @@ Item {
     function duplicateNode(duplicateFollowingNodes) {
         var nodes
         if (duplicateFollowingNodes) {
-            nodes = uigraph.duplicateNodesFrom(uigraph.selectedNodes)
+            nodes = uigraph.duplicateNodesFrom(uigraph.getSelectedNodes())
         } else {
-            nodes = uigraph.duplicateNodes(uigraph.selectedNodes)
+            nodes = uigraph.duplicateNodes(uigraph.getSelectedNodes())
         }
-        uigraph.clearNodeSelection()
         uigraph.selectedNode = nodes[0]
         uigraph.selectNodes(nodes)
     }
@@ -100,7 +99,6 @@ Item {
         var copiedContent = Clipboard.getText()
         var nodes = uigraph.pasteNodes(copiedContent, finalPosition, centerPosition)
         if (nodes.length > 0) {
-            uigraph.clearNodeSelection()
             uigraph.selectedNode = nodes[0]
             uigraph.selectNodes(nodes)
         }
@@ -116,7 +114,7 @@ Item {
             fit()
         } else if (event.key === Qt.Key_Delete) {
             if (event.modifiers === Qt.AltModifier) {
-                uigraph.removeNodesFrom(uigraph.selectedNodes)
+                uigraph.removeNodesFrom(uigraph.getSelectedNodes())
             } else {
                 uigraph.removeSelectedNodes()
             }
@@ -637,11 +635,11 @@ Item {
                             nodeMenuLoader.showDataDeletionDialog(
                                 false, 
                                 function(request, uigraph) {
-                                    request(uigraph.selectedNodes);
+                                    request(uigraph.getSelectedNodes());
                                 }.bind(null, computeRequest, uigraph)
                             );
                         } else {
-                            computeRequest(uigraph.selectedNodes);
+                            computeRequest(uigraph.getSelectedNodes());
                         }
                     }
                 }
@@ -658,11 +656,11 @@ Item {
                             nodeMenuLoader.showDataDeletionDialog(
                                 false, 
                                 function(request, uigraph) {
-                                    request(uigraph.selectedNodes);
+                                    request(uigraph.getSelectedNodes());
                                 }.bind(null, submitRequest, uigraph)
                             );
                         } else {
-                            submitRequest(uigraph.selectedNodes);
+                            submitRequest(uigraph.getSelectedNodes());
                         }
                     }
                 }
@@ -747,7 +745,7 @@ Item {
                         }
                         text: MaterialIcons.fast_forward
                         onClicked: {
-                            uigraph.removeNodesFrom(uigraph.selectedNodes)
+                            uigraph.removeNodesFrom(uigraph.getSelectedNodes())
                             nodeMenu.close()
                         }
                     }
@@ -807,13 +805,13 @@ Item {
                     modal: false
                     header.visible: false
 
-                    text: "Delete Data of '" + node.label + "'" + (uigraph.selectedNodes.count > 1 ? " and other selected Nodes" : "") + (deleteFollowing ?  " and following Nodes?" : "?")
+                    text: "Delete Data of '" + node.label + "'" + (uigraph.nodeSelection.selectedIndexes.length > 1 ? " and other selected Nodes" : "") + (deleteFollowing ?  " and following Nodes?" : "?")
                     helperText: "Warning: This operation cannot be undone."
                     standardButtons: Dialog.Yes | Dialog.Cancel
 
                     onAccepted: {
                         if (deleteFollowing)
-                            uigraph.clearDataFrom(uigraph.selectedNodes);
+                            uigraph.clearDataFrom(uigraph.getSelectedNodes());
                         else
                             uigraph.clearSelectedNodesData();
                         dataDeleted();