[ui] make Nodes moves undoable

Handle nodes move and auto-layout on Python side by using GraphLayout and MoveNodeCommand.

* Node QML component relies on underlying core.Node object's position
* MoveNodeCommand is called after a Node has been dragged
* partial/full auto-layout for specific graph operations (Duplication, Augmentation...) are now part of those operations
* serialized position in node data allows to properly restore nodes coordinates on undo/redo
* remove all layout/node moving code from QML

meshroom/ui/commands.py

@@ -87,10 +87,11 @@ class GraphCommand(UndoCommand):
 
 
 class AddNodeCommand(GraphCommand):
-    def __init__(self, graph, nodeType, parent=None, **kwargs):
+    def __init__(self, graph, nodeType, position, parent=None, **kwargs):
         super(AddNodeCommand, self).__init__(graph, parent)
         self.nodeType = nodeType
         self.nodeName = None
+        self.position = position
         self.kwargs = kwargs
         # Serialize Attributes as link expressions
         for key, value in self.kwargs.items():
@@ -102,7 +103,7 @@ class AddNodeCommand(GraphCommand):
                          value[idx] = v.asLinkExpr()
 
     def redoImpl(self):
-        node = self.graph.addNewNode(self.nodeType, **self.kwargs)
+        node = self.graph.addNewNode(self.nodeType, position=self.position, **self.kwargs)
         self.nodeName = node.name
         self.setText("Add Node {}".format(self.nodeName))
         return node

meshroom/ui/graph.py

@@ -5,7 +5,7 @@ import os
 from enum import Enum
 from threading import Thread
 
-from PySide2.QtCore import Slot, QJsonValue, QObject, QUrl, Property, Signal
+from PySide2.QtCore import Slot, QJsonValue, QObject, QUrl, Property, Signal, QPoint
 
 from meshroom.common.qt import QObjectListModel
 from meshroom.core.attribute import Attribute, ListAttribute
@@ -199,6 +199,7 @@ class UIGraph(QObject):
         self._computeThread = Thread()
         self._running = self._submitted = False
         self._sortedDFSChunks = QObjectListModel(parent=self)
+        self._layout = GraphLayout(self)
         if filepath:
             self.load(filepath)
 
@@ -346,18 +347,22 @@ class UIGraph(QObject):
         self._modificationCount -= 1
         self._undoStack.endMacro()
 
-    @Slot(str, result=QObject)
+    @Slot(str, QPoint, result=QObject)
-    def addNewNode(self, nodeType, **kwargs):
+    def addNewNode(self, nodeType, position=None, **kwargs):
         """ [Undoable]
         Create a new Node of type 'nodeType' and returns it.
 
         Args:
             nodeType (str): the type of the Node to create.
+            position (QPoint): (optional) the initial position of the node
             **kwargs: optional node attributes values
+
         Returns:
             Node: the created node
         """
-        return self.push(commands.AddNodeCommand(self._graph, nodeType, **kwargs))
+        if isinstance(position, QPoint):
+            position = Position(position.x(), position.y())
+        return self.push(commands.AddNodeCommand(self._graph, nodeType, position=position, **kwargs))
 
     @Slot(Node, QPoint)
     def moveNode(self, node, position):
@@ -415,7 +420,18 @@ class UIGraph(QObject):
         Returns:
             [Nodes]: the list of duplicated nodes
         """
-        return self.push(commands.DuplicateNodeCommand(self._graph, srcNode, duplicateFollowingNodes))
+        title = "Duplicate Nodes from {}" if duplicateFollowingNodes else "Duplicate {}"
+        # enable updates between duplication and layout to get correct depths during layout
+        with self.groupedGraphModification(title.format(srcNode.name), disableUpdates=False):
+            # disable graph updates during duplication
+            with self.groupedGraphModification("Node duplication", disableUpdates=True):
+                duplicates = self.push(commands.DuplicateNodeCommand(self._graph, srcNode, duplicateFollowingNodes))
+            # move nodes below the bounding box formed by the duplicated node(s)
+            bbox = self._layout.boundingBox(duplicates)
+            for n in duplicates:
+                self.moveNode(n, Position(n.x, bbox[3] + self.layout.gridSpacing + n.y))
+
+        return duplicates
 
     @Slot(CompatibilityNode, result=Node)
     def upgradeNode(self, node):
@@ -449,6 +465,7 @@ class UIGraph(QObject):
     graphChanged = Signal()
     graph = Property(Graph, lambda self: self._graph, notify=graphChanged)
     nodes = Property(QObject, lambda self: self._graph.nodes, notify=graphChanged)
+    layout = Property(GraphLayout, lambda self: self._layout, constant=True)
 
     computeStatusChanged = Signal()
     computing = Property(bool, isComputing, notify=computeStatusChanged)

meshroom/ui/qml/GraphEditor/GraphEditor.qml

@@ -14,10 +14,6 @@ Item {
     property bool readOnly: false
     property variant selectedNode: null
 
-    property int nodeWidth: 140
-    property int nodeHeight: 80
-    property int gridSpacing: 15
-    property bool useMinDepth: true
     property var _attributeToDelegate: ({})
 
     // signals
@@ -25,55 +21,32 @@ Item {
     signal workspaceClicked()
     signal nodeDoubleClicked(var node)
 
-    onUseMinDepthChanged: doAutoLayout()
 
     clip: true
 
     SystemPalette { id: activePalette }
 
-    /// Get node delegate based on a node name
+    /// Get node delegate for the given node object
-    function nodeDelegate(nodeName)
+    function nodeDelegate(node)
     {
         for(var i=0; i<nodeRepeater.count; ++i)
         {
-            if(nodeRepeater.itemAt(i).node.name === nodeName)
+            if(nodeRepeater.itemAt(i).node === node)
-                return nodeRepeater.itemAt(i);
+                return nodeRepeater.itemAt(i)
         }
         return undefined
     }
 
-    /// Move the node identified by nodeName to the given position
-    function moveNode(nodeName, posX, posY)
-    {
-        var delegate = nodeDelegate(nodeName)
-        delegate.animatePosition = false
-        delegate.x = posX
-        delegate.y = posY
-        delegate.animatePosition = true
-        selectNode(delegate)
-    }
-
     /// Select node delegate
-    function selectNode(delegate)
+    function selectNode(node)
     {
-        root.selectedNode = delegate.node
+        root.selectedNode = node
-        delegate.forceActiveFocus()
     }
 
     /// Duplicate a node and optionnally all the following ones
     function duplicateNode(node, duplicateFollowingNodes) {
         var nodes = uigraph.duplicateNode(node, duplicateFollowingNodes)
-        var delegates = []
+        selectNode(nodes[0])
-        var from = nodeRepeater.count - nodes.length
-        var to = nodeRepeater.count - 1
-        for(var i=from; i <= to; ++i)
-        {
-            delegates.push(nodeRepeater.itemAt(i))
-        }
-        var srcNodeDelegate = nodeDelegate(node.name)
-        doAutoLayout(from, to, srcNodeDelegate.x, srcNodeDelegate.y + (root.nodeHeight + root.gridSpacing))
-        selectNode(delegates[0])
-        return delegates
     }
 
     MouseArea {
@@ -134,8 +107,8 @@ Item {
             function createNode(nodeType)
             {
                 // add node via the proper command in uigraph
-                var node = uigraph.addNewNode(nodeType)
+                var node = uigraph.addNewNode(nodeType, spawnPosition)
-                moveNode(node.name, spawnPosition.x, spawnPosition.y)
+                selectNode(node)
             }
 
             onVisibleChanged: {
@@ -285,7 +258,6 @@ Item {
 
                 model: root.graph.nodes
                 property bool loaded: count === model.count
-                onLoadedChanged: if(loaded) { doAutoLayout() }
 
                 delegate: Node {
                     id: nodeDelegate
@@ -293,7 +265,7 @@ Item {
                     property bool animatePosition: true
 
                     node: object
-                    width: root.nodeWidth
+                    width: uigraph.layout.nodeWidth
                     readOnly: root.readOnly
                     baseColor: root.selectedNode == node ? Qt.lighter(defaultColor, 1.2) : defaultColor
 
@@ -303,12 +275,11 @@ Item {
                     onPressed: {
                         if(mouse.modifiers & Qt.AltModifier)
                         {
-                            var delegates = duplicateNode(node, true)
+                            duplicateNode(node, true)
-                            selectNode(delegates[0])
                         }
                         else
                         {
-                            selectNode(nodeDelegate)
+                            selectNode(node)
                         }
                         if(mouse.button == Qt.RightButton)
                         {
@@ -319,6 +290,8 @@ Item {
 
                     onDoubleClicked: root.nodeDoubleClicked(node)
 
+                    onMoved: uigraph.moveNode(node, position)
+
                     Keys.onDeletePressed: uigraph.removeNode(node)
 
                     Behavior on x {
@@ -345,13 +318,14 @@ Item {
 
         Button {
             text: "Layout"
-            onClicked: root.doAutoLayout()
+            onClicked: uigraph.layout.reset()
             z: 10
         }
         ComboBox {
             model: ['Min Depth', 'Max Depth']
+            currentIndex: uigraph.layout.depthMode
             onActivated: {
-                useMinDepth = currentIndex == 0
+                uigraph.layout.depthMode = currentIndex
             }
         }
     }
@@ -392,55 +366,4 @@ Item {
         draggable.y = bbox.y*draggable.scale*-1 + (root.height-bbox.height*draggable.scale)*0.5
     }
 
-    /** Basic auto-layout based on node depths
-     * @param {int} from the index of the node to start the layout from (default: 0)
-     * @param {int} to the index of the node end the layout at (default: nodeCount)
-     * @param {real} startX layout origin x coordinate (default: 0)
-     * @param {real} startY layout origin y coordinate (default: 0)
-    */
-    function doAutoLayout(from, to, startX, startY)
-    {
-        // default values
-        from = from === undefined ? 0 : from
-        to = to === undefined ? nodeRepeater.count - 1 : to
-        startX = startX === undefined ? 0 : startX
-        startY = startY === undefined ? 0 : startY
-
-        var count = to - from + 1;
-
-        var depthProperty = useMinDepth ? 'minDepth' : 'depth'
-        var grid = new Array(count)
-
-        for(var i=0; i< count; ++i)
-            grid[i] = new Array(count)
-
-        // retrieve reference depth from start node
-        var zeroDepth = from > 0 ? nodeRepeater.itemAt(from).node[depthProperty] : 0
-
-        for(var i=0; i<count; ++i)
-        {
-            var obj = nodeRepeater.itemAt(from + i);
-            var j=0;
-            while(1)
-            {
-                if(grid[obj.node[depthProperty]-zeroDepth][j] == undefined)
-                {
-                    grid[obj.node[depthProperty]-zeroDepth][j] = obj;
-                    break;
-                }
-                j++;
-            }
-        }
-        for(var x=0; x<count; ++x)
-        {
-            for(var y=0; y<count; ++y)
-            {
-                if(grid[x][y] != undefined)
-                {
-                    grid[x][y].x = startX + x * (root.nodeWidth + root.gridSpacing)
-                    grid[x][y].y = startY + y * (root.nodeHeight + root.gridSpacing)
-                }
-            }
-        }
-    }
 }

meshroom/ui/qml/GraphEditor/Node.qml

@@ -15,6 +15,7 @@ Item {
 
     signal pressed(var mouse)
     signal doubleClicked(var mouse)
+    signal moved(var position)
     signal attributePinCreated(var attribute, var pin)
     signal attributePinDeleted(var attribute, var pin)
 
@@ -23,14 +24,32 @@ Item {
 
     SystemPalette { id: activePalette }
 
+    // initialize position with node coordinates
+    x: root.node.x
+    y: root.node.y
+
+    Connections {
+        target: root.node
+        // update x,y when node position changes
+        onPositionChanged: {
+            root.x = root.node.x
+            root.y = root.node.y
+        }
+    }
+
     MouseArea {
         anchors.fill: parent
         drag.target: parent
-        drag.threshold: 0
+        // small drag threshold to avoid moving the node by mistake
+        drag.threshold: 2
         hoverEnabled: true
         acceptedButtons: Qt.LeftButton | Qt.RightButton
         onPressed: root.pressed(mouse)
         onDoubleClicked: root.doubleClicked(mouse)
+        drag.onActiveChanged: {
+            if(!drag.active)
+                root.moved(Qt.point(root.x, root.y))
+        }
     }
 
     Rectangle {

meshroom/ui/qml/LiveSfmView.qml

@@ -13,8 +13,6 @@ Panel {
     property variant reconstruction
     readonly property variant liveSfmManager: reconstruction.liveSfmManager
 
-    signal requestGraphAutoLayout()
-
     title: "Live Reconstruction"
     icon: Label {
         text: MaterialIcons.linked_camera;

meshroom/ui/qml/WorkspaceView.qml

@@ -24,7 +24,6 @@ Item {
     implicitWidth: 300
     implicitHeight: 400
 
-    signal requestGraphAutoLayout()
 
     // Load a 3D media file in the 3D viewer
     function load3DMedia(filepath)
@@ -79,7 +78,6 @@ Item {
                 reconstruction: root.reconstruction
                 Layout.fillWidth: true
                 Layout.preferredHeight: childrenRect.height
-                onRequestGraphAutoLayout: graphEditor.doAutoLayout()
             }
         }
         Panel {

meshroom/ui/qml/main.qml

@@ -130,7 +130,6 @@ ApplicationWindow {
         nameFilters: ["Meshroom Graphs (*.mg)"]
         onAccepted: {
             _reconstruction.loadUrl(file.toString())
-            graphEditor.doAutoLayout()
         }
     }
 
@@ -209,7 +208,6 @@ ApplicationWindow {
     CompatibilityManager {
         id: compatibilityManager
         uigraph: _reconstruction
-        onUpgradeDone: graphEditor.doAutoLayout()
     }
 
     Action {
@@ -242,7 +240,7 @@ ApplicationWindow {
             title: "File"
             Action {
                 text: "New"
-                onTriggered: ensureSaved(function() { _reconstruction.new(); graphEditor.doAutoLayout() })
+                onTriggered: ensureSaved(function() { _reconstruction.new() })
             }
             Action {
                 text: "Open"
@@ -309,12 +307,6 @@ ApplicationWindow {
 
     Connections {
         target: _reconstruction
-        // Request graph auto-layout when an augmentation step is added for readability
-        onSfmAugmented: graphEditor.doAutoLayout(_reconstruction.graph.nodes.indexOf(arguments[0]),
-                                                 _reconstruction.graph.nodes.indexOf(arguments[1]),
-                                                 0,
-                                                 graphEditor.boundingBox().height + graphEditor.gridSpacing
-                                                 )
 
         // Bind messages to DialogsFactory
         function createDialog(func, message)
@@ -429,7 +421,6 @@ ApplicationWindow {
                 Layout.minimumHeight: 50
                 reconstruction: _reconstruction
                 readOnly: _reconstruction.computing
-                onRequestGraphAutoLayout: graphEditor.doAutoLayout()
             }
         }
 
@@ -489,11 +480,8 @@ ApplicationWindow {
                                 readOnly: _reconstruction.computing
 
                                 onUpgradeRequest: {
-                                    var delegate = graphEditor.nodeDelegate(node.name)
+                                    var n = _reconstruction.upgradeNode(node)
-                                    var posX = delegate.x
+                                    graphEditor.selectNode(n)
-                                    var posY = delegate.y
-                                    _reconstruction.upgradeNode(node)
-                                    graphEditor.moveNode(node.name, posX, posY)
                                 }
                             }
                         }

meshroom/ui/reconstruction.py

@@ -305,10 +305,17 @@ class Reconstruction(UIGraph):
             if len(self._cameraInits[0].viewpoints) == 0:
                 return self._cameraInit, sfm
 
-        with self.groupedGraphModification("SfM Augmentation"):
+        # enable updates between duplication and layout to get correct depths during layout
-            sfm, mvs = multiview.sfmAugmentation(self, self.lastSfmNode(), withMVS=withMVS)
+        with self.groupedGraphModification("SfM Augmentation", disableUpdates=False):
+            # disable graph updates when adding augmentation branch
+            with self.groupedGraphModification("Augmentation", disableUpdates=True):
+                sfm, mvs = multiview.sfmAugmentation(self, self.lastSfmNode(), withMVS=withMVS)
+            first, last = sfm[0], mvs[-1] if mvs else sfm[-1]
+            # use graph current bounding box height to spawn the augmentation branch
+            bb = self.layout.boundingBox()
+            self.layout.autoLayout(first, last, bb[0], bb[3] + self._layout.gridSpacing)
 
-        self.sfmAugmented.emit(sfm[0], mvs[-1] if mvs else sfm[-1])
+        self.sfmAugmented.emit(first, last)
         return sfm[0], sfm[-1]
 
     def allImagePaths(self):