1.Fixed broken placement of nodes in view, 2.Warning instead of crash upon attempt to move a node which doesnt exist.

Author: joern274

plugins/gui/include/gui/graph_widget/contexts/graph_context.h

@@ -325,9 +325,10 @@ namespace hal
         }
 
         /**
-		 * Move node to antother grid location
-		 */
-        void moveNodeAction(const QPoint& from, const QPoint& to);
+         * Update placement of nodes already existing in view
+         * @param plc - the placement hash (Node -> QPoint)
+         */
+        void updatePlacement(const GridPlacement& plc);
 
         /**
          * Returns whether the scene is in an updating process (i.e. layouter process) or not.

plugins/gui/include/gui/graph_widget/layouters/graph_layouter.h

@@ -284,6 +284,8 @@ namespace hal
 
         void dumpNodePositions(const QPoint& search) const;
 
+        void updatePlacement(const GridPlacement& plc);
+
         void setNodePosition(const Node& n, const QPoint& p);
         void swapNodePositions(const Node& n1, const Node& n2);
         void removeNodeFromMaps(const Node& n);

plugins/gui/include/gui/gui_def.h

@@ -144,7 +144,13 @@ namespace hal
         GridPlacement(const QHash<hal::Node,QPoint>& data) : QHash<hal::Node,QPoint>(data) {;}
 
         void setGatePosition(u32 gateId, std::pair<int,int>p, bool swap = false) {
-            QPoint pos = QPoint(gatePosition(gateId)->first, gatePosition(gateId)->second); //position of current gate to move
+            std::pair<int,int>* posPtr = gatePosition(gateId);
+            if (!posPtr)
+            {
+                log_warning("gui", "Gate id {} cannot be moved, not found in current placement", gateId);
+                return;
+            }
+            QPoint pos = QPoint(posPtr->first, posPtr->second); //position of current gate to move
             hal::Node nd = key(QPoint(p.first, p.second)); //find the node in the destination
 
             if(!nd.isNull() && !swap) //if the destination placement is not available
@@ -159,8 +165,15 @@ namespace hal
             else
                 operator[](hal::Node(gateId,hal::Node::Gate)) = QPoint(p.first,p.second);
         }
+
         void setModulePosition(u32 moduleId, std::pair<int,int>p, bool swap = false){
-            QPoint pos = QPoint(modulePosition(moduleId)->first, modulePosition(moduleId)->second);
+            std::pair<int,int>* posPtr = modulePosition(moduleId);
+            if (!posPtr)
+            {
+                log_warning("gui", "Module id {} cannot be moved, not found in current placement", moduleId);
+                return;
+            }
+            QPoint pos = QPoint(posPtr->first, posPtr->second); //position of current module to move
             hal::Node nd = key(QPoint(p.first, p.second));
 
             if(!nd.isNull() && !swap)
@@ -174,11 +187,13 @@ namespace hal
             }
             else
                 operator[](hal::Node(moduleId,hal::Node::Module)) = QPoint(p.first,p.second);};
+
         std::pair<int,int>* gatePosition(u32 gateId) const
         {
             auto it = constFind(hal::Node(gateId,hal::Node::Gate));
             return (it == constEnd() ? nullptr : new std::pair<int,int>(it->x(),it->y()));
         }
+
         std::pair<int,int>* modulePosition(u32 moduleId) const
         {
             auto it = constFind(hal::Node(moduleId,hal::Node::Module));

plugins/gui/src/graph_widget/contexts/graph_context.cpp

@@ -533,12 +533,9 @@ namespace hal
         if (mParentWidget) mParentWidget->storeViewport();
     }
 
-    void GraphContext::moveNodeAction(const QPoint& from, const QPoint& to)
+    void GraphContext::updatePlacement(const GridPlacement& plc)
     {
-        const QMap<QPoint,Node> nodeMap = mLayouter->positionToNodeMap();
-        auto it = nodeMap.find(from);
-        if (it==nodeMap.constEnd()) return;
-        mLayouter->setNodePosition(it.value(),to);
+        mLayouter->updatePlacement(plc);
         scheduleSceneUpdate();
     }
 

plugins/gui/src/graph_widget/layouters/graph_layouter.cpp

@@ -107,6 +107,29 @@ namespace hal
         return mPositionToNodeMap;
     }
 
+    void GraphLayouter::updatePlacement(const GridPlacement& plc)
+    {
+        // Keep track which positions are occupied by new nodes, possible values:
+        // -1 : position no longer occupied
+        //  1 : position which was not occupied before now occupied
+        //  0 : old position taken by new node
+        QHash<QPoint,int> positionOccupied;
+
+        for (auto it = plc.constBegin(); it != plc.constEnd(); ++it)
+        {
+            auto jt = mNodeToPositionMap.find(it.key());
+            if (jt == mNodeToPositionMap.end()) continue;
+            --positionOccupied[jt.value()];  // old value from mNodePositionMap
+            ++positionOccupied[it.value()];  // new value from plc
+            jt.value() = it.value();
+            mPositionToNodeMap[it.value()] = it.key();
+        }
+
+        for (auto rpit = positionOccupied.begin(); rpit != positionOccupied.end(); ++rpit)
+            if (rpit.value() < 0)
+                mPositionToNodeMap.remove(rpit.key());
+    }
+
     void GraphLayouter::setNodePosition(const Node& n, const QPoint& p)
     {
         if (mNodeToPositionMap.contains(n))

plugins/gui/src/user_action/action_move_node.cpp

@@ -160,18 +160,7 @@ namespace hal
         }
 
         LayoutLocker llock;
-        ctx->clear();
-
-        QSet<u32> modIds;
-        QSet<u32> gateIds;
-
-        for (Node node : mGridPlacement.keys())
-        {
-            if(node.isModule()) modIds.insert(node.id());
-            else gateIds.insert(node.id());
-        }
-
-        ctx->add(modIds, gateIds, PlacementHint(mGridPlacement));
+        ctx->updatePlacement(mGridPlacement);
         ctx->scheduleSceneUpdate();
 
         return UserAction::exec();