From e68b583ed84a914025067b8243df569ed6803062 Mon Sep 17 00:00:00 2001
From: Rainer Kartmann <rainer.kartmann@kit.edu>
Date: Tue, 24 Aug 2021 15:03:46 +0200
Subject: [PATCH] Implement edge removal, fix commit

---
 .../LocationGraphEditorWidgetController.cpp   | 150 +++++++++++++-----
 .../LocationGraphEditorWidgetController.h     |   1 +
 .../widgets/EdgeTableWidget.cpp               |  38 +++++
 .../widgets/EdgeTableWidget.h                 |  10 +-
 .../widgets/VertexDataWidget.cpp              |  16 +-
 .../widgets/VertexTableWidget.cpp             |  19 +--
 .../widgets/VertexTableWidget.h               |   5 +-
 .../widgets/graph_scene/ControlWidget.cpp     |   3 +-
 .../LocationGraphEditor/widgets/utils.cpp     |   5 +
 .../LocationGraphEditor/widgets/utils.h       |   9 ++
 10 files changed, 188 insertions(+), 68 deletions(-)

diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.cpp
index 77332863..85663ec8 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.cpp
@@ -53,6 +53,7 @@
 #include <QMouseEvent>
 #include <QObject>
 #include <QPushButton>
+#include <QSignalBlocker>
 
 // std
 #include <sstream>
@@ -150,6 +151,8 @@ namespace armarx::nav::locgrapheditor
 
         connect(view.vertexTable, &VertexTableWidget::newEdgesRequested,
                 this, &This::addEdges);
+        connect(view.edgeTable, &EdgeTableWidget::edgeRemovalRequested,
+                this, &This::removeEdges);
     }
 
 
@@ -210,16 +213,7 @@ namespace armarx::nav::locgrapheditor
     {
         remote.connect(*this);
 
-        if (/* DISABLES CODE */ (true))
-        {
-            widget.sceneGroupBox->setEnabled(true);
-            widget.sceneGroupBox->setTitle("Scenes from graph memory segment");
-        }
-        else
-        {
-            widget.sceneGroupBox->setEnabled(false);
-            widget.sceneGroupBox->setTitle("Scenes from graph memory segment (No graph memory segment available)");
-        }
+        widget.sceneGroupBox->setEnabled(true);
 
         emit connected();
     }
@@ -257,23 +251,41 @@ namespace armarx::nav::locgrapheditor
 
     void LocationGraphEditorWidgetController::updateGraphList()
     {
+        QString previousText = widget.graphsComboBox->currentText();
         widget.graphsComboBox->clear();
 
-        bool enable = false;
-        model.memory.forEachEntity([this, &enable](const armem::wm::Entity& entity)
+        int i = 0;
+        int previousIndex = -1;
+        model.memory.forEachEntity([&](const armem::wm::Entity& entity)
         {
-            widget.graphsComboBox->addItem(QString::fromStdString(entity.id().str()));
-            enable = true;
+            QString text = QString::fromStdString(entity.id().str());
+            widget.graphsComboBox->addItem(text);
+            if (previousIndex < 0 and text == previousText)
+            {
+                previousIndex = i;
+            }
+            ++i;
         });
-        widget.loadGraphButton->setEnabled(enable);
+        if (previousIndex >= 0)
+        {
+            widget.graphsComboBox->setCurrentIndex(previousIndex);
+        }
+        widget.loadGraphButton->setEnabled(widget.graphsComboBox->count() > 0);
     }
 
 
     void LocationGraphEditorWidgetController::loadGraph()
     {
         const armem::MemoryID entityID = armem::MemoryID::fromString(widget.graphsComboBox->currentText().toStdString());
+
+        queryGraphs();  // Refresh local memory.
+
         const armem::wm::EntityInstance* instance = model.memory.findLatestInstance(entityID);
-        if (not instance)
+        if (instance)
+        {
+            widget.statusLabel->setText(QString::fromStdString("Loaded snapshot " + instance->id().getEntitySnapshotID().str()));
+        }
+        else
         {
             std::stringstream ss;
             ss << "No latest instance of entity " << entityID << " in memory.";
@@ -288,13 +300,33 @@ namespace armarx::nav::locgrapheditor
         }
 
         model.graphEntityID = entityID;
+        ARMARX_VERBOSE << "Loading graph " << nav.str();
         setGraph(nav);
     }
 
 
     void LocationGraphEditorWidgetController::commitGraph()
     {
+        nav::graph::arondto::Graph dto;
+        {
+            nav::graph::Graph nav = fromGuiGraph(model.graph);
+            toAron(dto, nav);
+        }
 
+        armem::EntityUpdate update;
+        update.entityID = model.graphEntityID;
+        update.timeCreated = armem::Time::now();
+        update.instancesData = {dto.toAron()};
+
+        armem::EntityUpdateResult result = remote.graphWriter.commit(update);
+        if (result.success)
+        {
+            widget.statusLabel->setText(QString::fromStdString("Committed snapshot " + result.snapshotID.str()));
+        }
+        else
+        {
+            widget.statusLabel->setText(QString::fromStdString(result.errorMessage));
+        }
     }
 
 
@@ -313,31 +345,31 @@ namespace armarx::nav::locgrapheditor
 #endif
 
         // Build the gui graph (model).
-        const bool initialBlocked = this->signalsBlocked();
-        blockSignals(true);
-
-        clearGraph();
-        for (auto vertex : nav.vertices())
-        {
-            addVertex(vertex);
-        }
-        for (auto edge : nav.edges())
         {
-            addEdge(edge);
-        }
+            QSignalBlocker blocker(this);
+
+            clearGraph();
+            for (auto vertex : nav.vertices())
+            {
+                addVertex(vertex);
+            }
+            for (auto edge : nav.edges())
+            {
+                addEdge(edge);
+            }
 
 #if 0
-        // Restore vertex highlighting.
-        for (auto vertex : model.graph.vertices())
-        {
-            if (highlightedVertices.count(vertex.attrib().getName()))
+            // Restore vertex highlighting.
+            for (auto vertex : model.graph.vertices())
             {
-                vertex.attrib().highlighted = true;
+                if (highlightedVertices.count(vertex.attrib().getName()))
+                {
+                    vertex.attrib().highlighted = true;
+                }
             }
-        }
 #endif
 
-        blockSignals(initialBlocked);
+        }
 
         // Trigger a view update.
         emit graphChanged();
@@ -413,6 +445,35 @@ namespace armarx::nav::locgrapheditor
     }
 
 
+    void LocationGraphEditorWidgetController::removeEdges(
+            QList<QTableWidgetItem*> edgeItems)
+    {
+        if (/* DISABLES CODE */ (true))
+        {
+            std::stringstream ss;
+            ss << "Remoiving edges ...";
+            for (const auto& edgeItem : edgeItems)
+            {
+                ss << "\n- " << edgeItem->text();
+            }
+            ARMARX_IMPORTANT << ss.str();
+        }
+
+        {
+            QSignalBlocker blocker(this);
+
+            const auto itemToEdgeMap = model.graph.getTableItemToEdgeMap();
+            for (const auto& edgeItem : edgeItems)
+            {
+                GuiGraph::Edge edge = itemToEdgeMap.at(edgeItem);
+                removeEdge(edge);
+            }
+        }
+
+        emit graphChanged();
+    }
+
+
     void LocationGraphEditorWidgetController::updateGraphView()
     {
         for (auto vertex : model.graph.vertices())
@@ -484,13 +545,11 @@ namespace armarx::nav::locgrapheditor
 
     void LocationGraphEditorWidgetController::clearGraph()
     {
-        const bool initialBlocked = this->signalsBlocked();
-        blockSignals(true);
-
-        clearEdges();
-        clearVertices();
-
-        blockSignals(initialBlocked);
+        {
+            QSignalBlocker blocker(this);
+            clearEdges();
+            clearVertices();
+        }
 
         // Clear data structure
         ARMARX_CHECK_EQUAL(model.graph.numEdges(), 0);
@@ -539,12 +598,21 @@ namespace armarx::nav::locgrapheditor
 
     void LocationGraphEditorWidgetController::removeEdge(GuiGraph::Edge& edge)
     {
+        ARMARX_CHECK(model.graph.hasEdge(edge.sourceDescriptor(), edge.targetDescriptor()))
+                << "Cannot remove edge that does not exist.";
+
         // Remove view elements
         view.graph->scene()->removeEdge(edge.attrib().graphicsItem);
+
+        const int rows = view.edgeTable->rowCount();
         view.edgeTable->removeEdge(edge);
+        ARMARX_CHECK_EQUAL(view.edgeTable->rowCount(), rows - 1);
 
         // Remove from model
         model.graph.removeEdge(edge);
+
+        ARMARX_CHECK(not model.graph.hasEdge(edge.sourceDescriptor(), edge.targetDescriptor()))
+                << edge.sourceDescriptor() << " -> " << edge.targetDescriptor();
     }
 
 
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.h b/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.h
index 825ea418..ff1177db 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.h
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.h
@@ -154,6 +154,7 @@ namespace armarx::nav::locgrapheditor
         void removeVertex(GuiGraph::Vertex& vertex);
 
         void addEdges(QList<QPair<QTableWidgetItem*, QTableWidgetItem*>> vertexItems);
+        void removeEdges(QList<QTableWidgetItem*> edgeItems);
 
 
         // View & Tables
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.cpp
index 081091c5..21aba203 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.cpp
@@ -22,7 +22,9 @@
 #include "EdgeTableWidget.h"
 #include "utils.h"
 
+#include <QAction>
 #include <QHeaderView>
+#include <QMenu>
 
 
 namespace armarx::nav::locgrapheditor
@@ -47,6 +49,8 @@ namespace armarx::nav::locgrapheditor
         setStyleSheet(styleSheet);
 
         setContextMenuPolicy(Qt::CustomContextMenu);
+        connect(this, &This::customContextMenuRequested,
+                this, &This::makeContextMenu);
     }
 
 
@@ -98,4 +102,38 @@ namespace armarx::nav::locgrapheditor
     }
 
 
+    void EdgeTableWidget::makeContextMenu(QPoint pos)
+    {
+        QList<QTableWidgetItem*> items = selectedEdgeItems();
+
+        QMenu menu;
+        if (items.size() == 0)
+        {
+            QAction* action = menu.addAction("No edges selected");
+            action->setEnabled(false);
+        }
+        else
+        {
+            QString desc;
+            if (items.size() == 1)
+            {
+                desc = "edge '" + items[0]->text() + "' " + utils::arrowRight
+                        + " '" + item(row(items[0]), 1)->text() + "'";
+            }
+            else
+            {
+                desc = QString::number(items.size()) + " edges";
+            }
+
+            menu.addSection("Selected " + desc);
+            connect(menu.addAction("Remove " + desc),
+                    &QAction::triggered, [this, &items]()
+            {
+                emit edgeRemovalRequested(items);
+            });
+        }
+
+        menu.exec(mapToGlobal(pos));
+    }
+
 }
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.h b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.h
index 0cd2cbee..2e745a1e 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.h
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.h
@@ -25,6 +25,7 @@
 #include <armarx/navigation/gui-plugins/LocationGraphEditor/widgets/default_colors.h>
 
 #include <QColor>
+#include <QList>
 #include <QTableWidget>
 
 
@@ -63,7 +64,14 @@ namespace armarx::nav::locgrapheditor
         QList<QTableWidgetItem*> selectedEdgeItems();
 
 
-    private slots:
+    signals:
+
+        void edgeRemovalRequested(QList<QTableWidgetItem*> edgeItems);
+
+
+    public slots:
+
+        void makeContextMenu(QPoint pos);
 
 
     public:
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.cpp
index ba63ea0f..7119c030 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.cpp
@@ -30,7 +30,9 @@
 #include <QFormLayout>
 #include <QHBoxLayout>
 #include <QLineEdit>
+#include <QList>
 #include <QRadioButton>
+#include <QSignalBlocker>
 
 #include <SimoxUtility/math/convert/deg_to_rad.h>
 #include <SimoxUtility/math/convert/mat4f_to_rpy.h>
@@ -125,13 +127,11 @@ namespace armarx::nav::locgrapheditor
 
     void VertexDataWidget::setVertex(GuiGraph::Vertex vertex)
     {
-        blockSignals(true);
+        QSignalBlocker blocker(this);
 
         _vertex = vertex;
         _setFromVertex(vertex);
         setEnabled(true);
-
-        blockSignals(false);
     }
 
 
@@ -229,9 +229,11 @@ namespace armarx::nav::locgrapheditor
                 return simox::math::rad_to_deg(rad);
             };
         }
+
+        QList<QSignalBlocker>> blockers;
         for (QDoubleSpinBox* angle : _angleSpinBoxes())
         {
-            angle->blockSignals(true);
+            blockers.append(QSignalBlocker(angle));
 
             angle->setSuffix(suffix);
             angle->setMinimum(min);
@@ -243,10 +245,8 @@ namespace armarx::nav::locgrapheditor
         {
             _setRpyRad(simox::math::mat4f_to_rpy(_vertex->attrib().getPose().cast<qreal>()));
         }
-        for (QDoubleSpinBox* angle : _angleSpinBoxes())
-        {
-            angle->blockSignals(false);
-        }
+
+        // blockers will disable blocking for all spin boxes on destruction.
     }
 
 
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.cpp
index 6722f828..2d1e965e 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.cpp
@@ -133,13 +133,6 @@ namespace armarx::nav::locgrapheditor
             action->setEnabled(false);
         }
 
-        // <- https://unicode-table.com/de/2190/
-        const QString arrowLeft = QStringLiteral("←");
-        // -> https://unicode-table.com/de/2192/
-        const QString arrowRight = QStringLiteral("\u2192");
-        // <-> https://unicode-table.com/de/21C4/
-        const QString arrowBoth = QStringLiteral("⇄");
-
         // Partners selected
         if (items.size() == 2)
         {
@@ -153,17 +146,17 @@ namespace armarx::nav::locgrapheditor
             QTableWidgetItem* first = items[0];
             QTableWidgetItem* second = items[1];
 
-            connect(menu.addAction("Add edge '" + first->text() + "' " + arrowRight + " '" + second->text() + "'"),
+            connect(menu.addAction("Add edge '" + first->text() + "' " + utils::arrowRight + " '" + second->text() + "'"),
                     &QAction::triggered, [this, first, second]()
             {
                 emit newEdgesRequested({{first, second}});
             });
-            connect(menu.addAction("Add edge '" + first->text() + "' " + arrowLeft + " '" + second->text() + "'"),
+            connect(menu.addAction("Add edge '" + first->text() + "' " + utils::arrowLeft + " '" + second->text() + "'"),
                     &QAction::triggered, [this, first, second]()
             {
                 emit newEdgesRequested({{second, first}});
             });
-            connect(menu.addAction("Add edges '" + first->text() + "' " + arrowBoth + " '" + second->text() + "'"),
+            connect(menu.addAction("Add edges '" + first->text() + "' " + utils::arrowBoth + " '" + second->text() + "'"),
                     &QAction::triggered, [this, first, second]()
             {
                 emit newEdgesRequested({{first, second}, {second, first}});
@@ -221,17 +214,17 @@ namespace armarx::nav::locgrapheditor
                 }
             };
 
-            addBulkActions(menu.addMenu("Add " + edges + " " + desc + " " + arrowRight + " ..."),
+            addBulkActions(menu.addMenu("Add " + edges + " " + desc + " " + utils::arrowRight + " ..."),
                            [](ListOfEdges& edges, Item* selected, Item* action)
             {
                 edges.append({selected, action});
             });
-            addBulkActions(menu.addMenu("Add " + edges + " " + desc + " " + arrowLeft + " ..."),
+            addBulkActions(menu.addMenu("Add " + edges + " " + desc + " " + utils::arrowLeft + " ..."),
                            [](ListOfEdges& edges, Item* selected, Item* action)
             {
                 edges.append({action, selected});
             });
-            addBulkActions(menu.addMenu("Add " + edges + " " + desc + " " + arrowBoth + " ..."),
+            addBulkActions(menu.addMenu("Add " + edges + " " + desc + " " + utils::arrowBoth + " ..."),
                            [](ListOfEdges& edges, Item* selected, Item* action)
             {
                 edges.append({selected, action});
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.h b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.h
index 379c224d..de609ce0 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.h
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.h
@@ -25,6 +25,8 @@
 #include <armarx/navigation/gui-plugins/LocationGraphEditor/widgets/default_colors.h>
 
 #include <QColor>
+#include <QList>
+#include <QPair>
 #include <QTableWidget>
 
 
@@ -62,9 +64,6 @@ namespace armarx::nav::locgrapheditor
         void makeContextMenu(QPoint pos);
 
 
-    private slots:
-
-
     public:
 
         QColor bgColorDefault = default_colors::tableBackgroundDefault;
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/graph_scene/ControlWidget.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/graph_scene/ControlWidget.cpp
index 5549b56f..d6dcf9b7 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/graph_scene/ControlWidget.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/graph_scene/ControlWidget.cpp
@@ -1,6 +1,6 @@
 #include "ControlWidget.h"
 
-#include <ArmarXCore/core/logging/Logging.h>
+// #include <ArmarXCore/core/logging/Logging.h>
 
 #include <QDoubleSpinBox>
 #include <QFrame>
@@ -67,7 +67,6 @@ namespace armarx::nav::locgrapheditor::graph_scene
         connect(_angle.turnClockwise, &QPushButton::pressed, [this]()
         {
             qreal newangle = std::fmod(angle() + _angle.rotateStepSize, 360.0);
-            ARMARX_IMPORTANT << VAROUT(newangle);
             setAngle(newangle);
         });
         connect(_angle.turnCounterClockwise, &QPushButton::pressed, [this]()
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.cpp
index c81e5509..e61b79b9 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.cpp
@@ -29,6 +29,11 @@
 namespace armarx::nav::locgrapheditor
 {
 
+    const QString utils::arrowLeft = QStringLiteral("←");
+    const QString utils::arrowRight = QStringLiteral("\u2192");
+    const QString utils::arrowBoth = QStringLiteral("⇄");
+
+
     QList<QTableWidgetItem*>
     utils::getSelectedItemsOfColumn(QTableWidget* widget, int column)
     {
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.h b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.h
index 8c344f01..44eb497a 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.h
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.h
@@ -32,4 +32,13 @@ namespace armarx::nav::locgrapheditor::utils
 
     QList<QTableWidgetItem*> getSelectedItemsOfColumn(QTableWidget* widget, int column);
 
+
+    // <- https://unicode-table.com/de/2190/
+    extern const QString arrowLeft;
+    // -> https://unicode-table.com/de/2192/
+    extern const QString arrowRight;
+    // <-> https://unicode-table.com/de/21C4/
+    extern const QString arrowBoth;
+
+
 }
-- 
GitLab