From c96e887abbf38e04a276e3d18ca648c346b46bba Mon Sep 17 00:00:00 2001
From: Rainer Kartmann <rainer.kartmann@kit.edu>
Date: Tue, 24 Aug 2021 18:00:30 +0200
Subject: [PATCH] Add basic changed tracking and message box when loading while
 having changes

---
 source/armarx/navigation/graph/Graph.cpp      |  17 +-
 source/armarx/navigation/graph/Visu.cpp       |  48 ++--
 source/armarx/navigation/graph/Visu.h         |   8 +-
 .../LocationGraphEditor/GuiGraph.cpp          |  16 ++
 .../LocationGraphEditor/GuiGraph.h            |   7 +
 .../LocationGraphEditorWidgetController.cpp   | 220 ++++++++++++++----
 .../LocationGraphEditorWidgetController.h     |  10 +-
 .../widgets/EdgeTableWidget.cpp               |  34 ++-
 .../widgets/VertexDataWidget.cpp              |  62 ++---
 .../widgets/VertexDataWidget.h                |   1 +
 .../widgets/VertexTableWidget.cpp             |  78 +++++--
 .../widgets/VertexTableWidget.h               |   5 +
 .../LocationGraphEditor/widgets/utils.cpp     |   7 +-
 .../LocationGraphEditor/widgets/utils.h       |   6 +-
 14 files changed, 384 insertions(+), 135 deletions(-)

diff --git a/source/armarx/navigation/graph/Graph.cpp b/source/armarx/navigation/graph/Graph.cpp
index f8086dce..48c0f4dd 100644
--- a/source/armarx/navigation/graph/Graph.cpp
+++ b/source/armarx/navigation/graph/Graph.cpp
@@ -22,13 +22,15 @@
 
 #include "Graph.h"
 
+#include <ArmarXCore/core/exceptions/local/ExpressionException.h>
+
 
 namespace armarx::nav::graph
 {
 
     std::string VertexAttribs::getName() const
     {
-        return aron.locationID.entityName;
+        return aron.locationID.providerSegmentName + "/" + aron.locationID.entityName;
     }
 
     Eigen::Matrix4f VertexAttribs::getPose() const
@@ -52,12 +54,18 @@ namespace armarx::nav
         dto = {};
         for (auto vertex : bo.vertices())
         {
-            dto.vertices.push_back(vertex.attrib().aron);
+            auto& v = dto.vertices.emplace_back(vertex.attrib().aron);
+            v.vertexID = static_cast<long>(vertex.objectID());
         }
+        ARMARX_CHECK_EQUAL(dto.vertices.size(), bo.numVertices());
+
         for (auto edge : bo.edges())
         {
-            dto.edges.push_back(edge.attrib().aron);
+            auto& e = dto.edges.emplace_back(edge.attrib().aron);
+            e.sourceVertexID = static_cast<long>(edge.sourceObjectID());
+            e.targetVertexID = static_cast<long>(edge.targetObjectID());
         }
+        ARMARX_CHECK_EQUAL(dto.edges.size(), bo.numEdges());
     }
 
 
@@ -69,11 +77,14 @@ namespace armarx::nav
             auto v = bo.addVertex(semrel::ShapeID(vertex.vertexID));
             v.attrib().aron = vertex;
         }
+        ARMARX_CHECK_EQUAL(bo.numVertices(), dto.vertices.size());
+
         for (const arondto::Edge& edge : dto.edges)
         {
             auto e = bo.addEdge(semrel::ShapeID(edge.sourceVertexID), semrel::ShapeID(edge.targetVertexID));
             e.attrib().aron = edge;
         }
+        ARMARX_CHECK_EQUAL(bo.numEdges(), dto.edges.size());
     }
 
 }
diff --git a/source/armarx/navigation/graph/Visu.cpp b/source/armarx/navigation/graph/Visu.cpp
index 62ede64f..3d98b5d7 100644
--- a/source/armarx/navigation/graph/Visu.cpp
+++ b/source/armarx/navigation/graph/Visu.cpp
@@ -13,7 +13,6 @@
  * You should have received a copy of the GNU General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
- * @package    MemoryX::ArmarXObjects::GraphImportExport
  * @author     Rainer Kartmann ( rainer dot kartmann at kit dot edu )
  * @date       2021
  * @copyright  http://www.gnu.org/licenses/gpl-2.0.txt
@@ -33,49 +32,58 @@
 namespace armarx::nav::graph
 {
 
-    viz::Pose VertexVisu::Pose::draw(Graph::ConstVertex vertex) const
+    viz::Pose VertexVisu::Pose::draw(const VertexAttribs& attribs) const
     {
-        const arondto::Vertex& aron = vertex.attrib().aron;
-        return viz::Pose(aron.locationID.entityName)
-                .pose(aron.globalRobotPose)
-                .scale(scale);
+        return viz::Pose(attribs.getName()).pose(attribs.getPose()).scale(scale);
     }
 
 
-    viz::Arrow VertexVisu::ForwardArrow::draw(Graph::ConstVertex vertex) const
+    viz::Arrow VertexVisu::ForwardArrow::draw(const VertexAttribs& attribs) const
     {
-        const arondto::Vertex& aron = vertex.attrib().aron;
-        return viz::Arrow(aron.locationID.entityName + " forward")
-                .fromTo(simox::math::position(aron.globalRobotPose),
-                        simox::math::transform_position(aron.globalRobotPose, length * Eigen::Vector3f::UnitY()))
+        return viz::Arrow(attribs.getName() + " forward")
+                .fromTo(simox::math::position(attribs.getPose()),
+                        simox::math::transform_position(attribs.getPose(), length * Eigen::Vector3f::UnitY()))
                 .color(color)
                 .width(width);
     }
 
 
     void VertexVisu::draw(viz::Layer& layer, Graph::ConstVertex vertex) const
+    {
+        draw(layer, vertex.attrib());
+    }
+
+
+    void VertexVisu::draw(viz::Layer& layer, const VertexAttribs& attribs) const
     {
         if (pose.has_value())
         {
-            layer.add(pose->draw(vertex));
+            layer.add(pose->draw(attribs));
         }
         if (forwardArrow.has_value())
         {
-            layer.add(forwardArrow->draw(vertex));
+            layer.add(forwardArrow->draw(attribs));
         }
     }
 
 
     viz::Arrow EdgeVisu::Arrow::draw(Graph::ConstEdge edge) const
     {
-        const auto& sourceAron = edge.source().attrib().aron;
-        const auto& targetAron = edge.target().attrib().aron;
-        return viz::Arrow(sourceAron.locationID.entityName + " -> " +
-                          targetAron.locationID.entityName)
-                  .fromTo(simox::math::position(sourceAron.globalRobotPose),
-                          simox::math::position(targetAron.globalRobotPose))
+        return draw(edge.attrib(), edge.source().attrib(), edge.target().attrib());
+    }
+
+
+    viz::Arrow EdgeVisu::Arrow::draw(
+            const EdgeAttribs& edge,
+            const VertexAttribs& source,
+            const VertexAttribs& target) const
+    {
+        (void) edge;
+        return viz::Arrow(source.getName() + " -> " + target.getName())
+                  .fromTo(simox::math::position(source.getPose()),
+                          simox::math::position(target.getPose()))
                   .width(width)
-                .color(color);
+                  .color(color);
     }
 
 
diff --git a/source/armarx/navigation/graph/Visu.h b/source/armarx/navigation/graph/Visu.h
index 6e2516bc..f38719a9 100644
--- a/source/armarx/navigation/graph/Visu.h
+++ b/source/armarx/navigation/graph/Visu.h
@@ -13,7 +13,6 @@
  * You should have received a copy of the GNU General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
- * @package    MemoryX::ArmarXObjects::GraphImportExport
  * @author     Rainer Kartmann ( rainer dot kartmann at kit dot edu )
  * @date       2021
  * @copyright  http://www.gnu.org/licenses/gpl-2.0.txt
@@ -35,7 +34,6 @@ namespace armarx::viz
     class Layer;
     class Pose;
 }
-
 namespace armarx::nav::graph
 {
 
@@ -45,7 +43,7 @@ namespace armarx::nav::graph
         {
             float scale = 1.0;
 
-            viz::Pose draw(Graph::ConstVertex vertex) const;
+            viz::Pose draw(const VertexAttribs& attribs) const;
         };
         std::optional<Pose> pose = Pose {};
 
@@ -55,12 +53,13 @@ namespace armarx::nav::graph
             float length = 100.0;
             simox::Color color = simox::Color::cyan(220);
 
-            viz::Arrow draw(Graph::ConstVertex vertex) const;
+            viz::Arrow draw(const VertexAttribs& attribs) const;
         };
         std::optional<ForwardArrow> forwardArrow = ForwardArrow {};
 
 
         void draw(viz::Layer& layer, Graph::ConstVertex vertex) const;
+        void draw(viz::Layer& layer, const VertexAttribs& attribs) const;
     };
 
 
@@ -72,6 +71,7 @@ namespace armarx::nav::graph
             simox::Color color = simox::Color::azure(196);
 
             viz::Arrow draw(Graph::ConstEdge edge) const;
+            viz::Arrow draw(const EdgeAttribs& edge, const VertexAttribs& source, const VertexAttribs& target) const;
         };
         std::optional<Arrow> arrow = Arrow {};
 
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/GuiGraph.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/GuiGraph.cpp
index 49361f44..7cd38cc0 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/GuiGraph.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/GuiGraph.cpp
@@ -29,6 +29,22 @@
 namespace armarx::nav::locgrapheditor
 {
 
+    bool GuiGraph::hasChanged() const
+    {
+        if (attrib().edgesChanged)
+        {
+            return true;
+        }
+        for (auto vertex : vertices())
+        {
+            if (vertex.attrib().changed)
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+
     std::optional<GuiGraph::Vertex>
     GuiGraph::getVertexFromTableItem(QTableWidgetItem* item)
     {
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/GuiGraph.h b/source/armarx/navigation/gui-plugins/LocationGraphEditor/GuiGraph.h
index 33f4ddf5..561f4f58 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/GuiGraph.h
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/GuiGraph.h
@@ -51,6 +51,9 @@ namespace armarx::nav::locgrapheditor
 
         /// Whether the node is highlighted.
         bool highlighted = false;
+
+        /// Whether the vertex was changed since loading or committing.
+        bool changed = false;
     };
 
 
@@ -73,6 +76,8 @@ namespace armarx::nav::locgrapheditor
 
     struct GraphData : public nav::graph::GraphAttribs
     {
+        /// Whether the graph structure was changed since loading or committing.
+        bool edgesChanged = false;
     };
 
 
@@ -83,6 +88,8 @@ namespace armarx::nav::locgrapheditor
         using RelationGraph::RelationGraph;
 
 
+        bool hasChanged() const;
+
         std::optional<Vertex> getVertexFromTableItem(QTableWidgetItem* item);
 
         std::map<QTableWidgetItem*, Vertex> getTableItemToVertexMap();
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.cpp
index 85663ec8..3f4a7113 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.cpp
@@ -33,11 +33,12 @@
 #include <Navigation/gui-plugins/LocationGraphEditor/ui_LocationGraphEditorWidget.h>
 
 #include <armarx/navigation/location/constants.h>
-#include <armarx/navigation/graph/constants.h>
+#include <armarx/navigation/location/aron/Location.aron.generated.h>
 #include <armarx/navigation/graph/constants.h>
 #include <armarx/navigation/graph/aron/Graph.aron.generated.h>
 
 #include <RobotAPI/libraries/armem/client/MemoryNameSystem.h>
+#include <RobotAPI/libraries/armem/core/aron_conversions.h>
 #include <RobotAPI/components/ArViz/Client/Client.h>
 
 #include <ArmarXCore/core/exceptions/local/ExpressionException.h>
@@ -50,6 +51,7 @@
 #include <QLabel>
 #include <QLineEdit>
 #include <QMenu>
+#include <QMessageBox>
 #include <QMouseEvent>
 #include <QObject>
 #include <QPushButton>
@@ -128,7 +130,7 @@ namespace armarx::nav::locgrapheditor
         connect(widget.refreshGraphsButton, &QPushButton::pressed, this, &This::updateGraphList);
 
         connect(widget.loadGraphButton, &QPushButton::pressed, this, &This::loadGraph);
-        connect(widget.commitGraphButton, &QPushButton::pressed, this, &This::commitGraph);
+        connect(widget.commitGraphButton, &QPushButton::pressed, this, &This::commit);
 
 
         // Update views
@@ -212,8 +214,12 @@ namespace armarx::nav::locgrapheditor
     void LocationGraphEditorWidgetController::onConnectComponent()
     {
         remote.connect(*this);
-
-        widget.sceneGroupBox->setEnabled(true);
+        {
+            std::stringstream ss;
+            ss << "Navigation Graphs (Entities from Core Segment " << nav::graph::coreSegmentID << ")";
+            widget.sceneGroupBox->setTitle(QString::fromStdString(ss.str()));
+            widget.sceneGroupBox->setEnabled(true);
+        }
 
         emit connected();
     }
@@ -255,11 +261,16 @@ namespace armarx::nav::locgrapheditor
         widget.graphsComboBox->clear();
 
         int i = 0;
-        int previousIndex = -1;
+        int previousIndex = -1;  // To keep selection.
         model.memory.forEachEntity([&](const armem::wm::Entity& entity)
         {
-            QString text = QString::fromStdString(entity.id().str());
-            widget.graphsComboBox->addItem(text);
+            bool hasChanged = (entity.id() == model.graphEntityID
+                               ? model.graph.hasChanged()
+                               : false);
+            QString text = getGraphDisplayName(entity.id(), hasChanged);
+            QString id = QString::fromStdString(entity.id().str());
+
+            widget.graphsComboBox->addItem(text, id);
             if (previousIndex < 0 and text == previousText)
             {
                 previousIndex = i;
@@ -276,9 +287,44 @@ namespace armarx::nav::locgrapheditor
 
     void LocationGraphEditorWidgetController::loadGraph()
     {
-        const armem::MemoryID entityID = armem::MemoryID::fromString(widget.graphsComboBox->currentText().toStdString());
+        if (model.graph.hasChanged())
+        {
+            QMessageBox msgBox;
+            msgBox.setText("The current graph and/or locations have uncommitted changes. ");
+            msgBox.setInformativeText("Do you want to discard them?");
+            QStringList detailLines;
+            if (model.graph.attrib().edgesChanged)
+            {
+                detailLines.append("Graph edges have changed.");
+            }
+            for (auto vertex : model.graph.vertices())
+            {
+                if (vertex.attrib().changed)
+                {
+                    detailLines.append("Location " + QString::fromStdString(vertex.attrib().getName()) + " has changed.");
+                }
+            }
+            msgBox.setDetailedText(detailLines.join("\n"));
+            msgBox.setStandardButtons(QMessageBox::Discard | QMessageBox::Cancel);
+            msgBox.setDefaultButton(QMessageBox::Cancel);
 
-        queryGraphs();  // Refresh local memory.
+            int ret = msgBox.exec();
+            switch (ret)
+            {
+              case QMessageBox::Discard:
+                  // Ok go.
+                  break;
+              case QMessageBox::Cancel:
+                  // Abort loading.
+                  return;
+            }
+        }
+
+        const armem::MemoryID entityID = armem::MemoryID::fromString(
+                    widget.graphsComboBox->currentData().toString().toStdString());
+
+        // Refresh local memory.
+        queryGraphs();
 
         const armem::wm::EntityInstance* instance = model.memory.findLatestInstance(entityID);
         if (instance)
@@ -305,7 +351,99 @@ namespace armarx::nav::locgrapheditor
     }
 
 
-    void LocationGraphEditorWidgetController::commitGraph()
+    void
+    LocationGraphEditorWidgetController::commit()
+    {
+        armem::CommitResult locResults = commitLocations();
+        armem::EntityUpdateResult graphResult = commitGraph();
+
+        {
+            std::stringstream ss;
+            if (locResults.allSuccess())
+            {
+                ss << "Committed " << locResults.results.size() << " location snapshots";
+                if (graphResult.success)
+                {
+                    ss << " and 1 graph snapshot " << graphResult.snapshotID;
+                }
+                else
+                {
+                    ss << " but failed to commit graph: \n" << graphResult.errorMessage;
+                }
+            }
+            else
+            {
+                int numLocs = static_cast<int>(locResults.results.size());
+                int numSuccess = 0;
+                for (const auto& r : locResults.results)
+                {
+                    numSuccess += int(r.success);
+                }
+                int numFailed = numLocs - numSuccess;
+
+                if (graphResult.success)
+                {
+                    ss << "Committed 1 graph snapshot " << graphResult.snapshotID;
+                    ss << " and " << numSuccess << " locations, but failed to commit "
+                       << numFailed << " locations: \n" << locResults.allErrorMessages();
+                }
+                else
+                {
+                    ss << "Failed to commit graph and " << numFailed << " of " << numLocs << " locations: \n";
+                    ss << graphResult.errorMessage << "\n";
+                    ss << locResults.allErrorMessages();
+                }
+            }
+
+            widget.statusLabel->setText(QString::fromStdString(ss.str()));
+        }
+
+        // `changed` flags may have changed
+        emit graphChanged();
+    }
+
+
+    armem::CommitResult
+    LocationGraphEditorWidgetController::commitLocations()
+    {
+        armem::Commit commit;
+
+        for (auto vertex : model.graph.vertices())
+        {
+            if (vertex.attrib().changed)
+            {
+                armem::EntityUpdate& update = commit.add();
+                fromAron(vertex.attrib().aron.locationID, update.entityID);
+                update.timeCreated = armem::Time::now();
+
+                nav::loc::arondto::Location dto;
+                dto.globalRobotPose = vertex.attrib().getPose();
+                update.instancesData = {dto.toAron()};
+            }
+        }
+
+        armem::CommitResult result = remote.locationWriter.commit(commit);
+        auto it = result.results.begin();
+        for (auto vertex : model.graph.vertices())
+        {
+            if (vertex.attrib().changed)
+            {
+                ARMARX_CHECK(it != result.results.end());
+                if (it->success)
+                {
+                    // Only clear dirty flag when update was successful.
+                    vertex.attrib().changed = false;
+                }
+                ++it;
+            }
+        }
+
+        return result;
+    }
+
+
+    armem::EntityUpdateResult
+    LocationGraphEditorWidgetController::commitGraph()
     {
         nav::graph::arondto::Graph dto;
         {
@@ -318,37 +456,19 @@ namespace armarx::nav::locgrapheditor
         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));
-        }
+        return remote.graphWriter.commit(update);
     }
 
 
     void LocationGraphEditorWidgetController::setGraph(graph::Graph& nav)
     {
-#if 0
-        // Store vertex highlighting.
-        std::set<std::string> highlightedVertices;
-        for (auto vertex : model.graph.vertices())
-        {
-            if (vertex.attrib().highlighted)
-            {
-                highlightedVertices.insert(vertex.attrib().getName());
-            }
-        }
-#endif
-
         // Build the gui graph (model).
         {
             QSignalBlocker blocker(this);
 
             clearGraph();
+            model.graph.attrib().edgesChanged = false;
+
             for (auto vertex : nav.vertices())
             {
                 addVertex(vertex);
@@ -357,18 +477,6 @@ namespace armarx::nav::locgrapheditor
             {
                 addEdge(edge);
             }
-
-#if 0
-            // Restore vertex highlighting.
-            for (auto vertex : model.graph.vertices())
-            {
-                if (highlightedVertices.count(vertex.attrib().getName()))
-                {
-                    vertex.attrib().highlighted = true;
-                }
-            }
-#endif
-
         }
 
         // Trigger a view update.
@@ -441,6 +549,8 @@ namespace armarx::nav::locgrapheditor
             }
         }
 
+        model.graph.attrib().edgesChanged = true;
+
         emit graphChanged();
     }
 
@@ -448,7 +558,7 @@ namespace armarx::nav::locgrapheditor
     void LocationGraphEditorWidgetController::removeEdges(
             QList<QTableWidgetItem*> edgeItems)
     {
-        if (/* DISABLES CODE */ (true))
+        if (/* DISABLES CODE */ (false))
         {
             std::stringstream ss;
             ss << "Remoiving edges ...";
@@ -458,7 +568,6 @@ namespace armarx::nav::locgrapheditor
             }
             ARMARX_IMPORTANT << ss.str();
         }
-
         {
             QSignalBlocker blocker(this);
 
@@ -470,6 +579,8 @@ namespace armarx::nav::locgrapheditor
             }
         }
 
+        model.graph.attrib().edgesChanged = true;
+
         emit graphChanged();
     }
 
@@ -484,6 +595,13 @@ namespace armarx::nav::locgrapheditor
         {
             updateEdgeView(edge);
         }
+
+        int index = widget.graphsComboBox->findData(QString::fromStdString(model.graphEntityID.str()));
+        if (index >= 0)
+        {
+            widget.graphsComboBox->setItemText(index, getGraphDisplayName(model.graphEntityID, model.graph.hasChanged()));
+        }
+
         updateArViz();
     }
 
@@ -710,4 +828,16 @@ namespace armarx::nav::locgrapheditor
         view.vertexData->setVertex(vertex);
     }
 
+
+    QString LocationGraphEditorWidgetController::getGraphDisplayName(
+            const armem::MemoryID& entityID, bool changed) const
+    {
+        QString name = QString::fromStdString(entityID.providerSegmentName + "/" + entityID.entityName);
+        if (changed)
+        {
+            name += "*";
+        }
+        return name;
+    }
+
 }
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.h b/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.h
index ff1177db..e4de7136 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.h
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/LocationGraphEditorWidgetController.h
@@ -138,7 +138,6 @@ namespace armarx::nav::locgrapheditor
         void queryGraphs();
         void updateGraphList();
         void loadGraph();
-        void commitGraph();
 
         void setGraph(graph::Graph& nav);
         GuiGraph::Vertex addVertex(graph::Graph::ConstVertex vertex);
@@ -157,6 +156,10 @@ namespace armarx::nav::locgrapheditor
         void removeEdges(QList<QTableWidgetItem*> edgeItems);
 
 
+        void commit();
+        armem::CommitResult commitLocations();
+        armem::EntityUpdateResult commitGraph();
+
         // View & Tables
 
         void updateGraphView();
@@ -175,6 +178,11 @@ namespace armarx::nav::locgrapheditor
         void updateEdgeHighlighting();
 
 
+    private:
+
+        QString getGraphDisplayName(const armem::MemoryID& entityID, bool changed = false) const;
+
+
     private:
 
         /// Widget Form
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.cpp
index 21aba203..adecbca4 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/EdgeTableWidget.cpp
@@ -59,13 +59,21 @@ namespace armarx::nav::locgrapheditor
             const graph::VertexAttribs& sourceAttrib,
             const graph::VertexAttribs& targetAttrib)
     {
-        int row = rowCount();
-        setRowCount(row + 1);
+        QTableWidgetItem* result = nullptr;
 
-        setItem(row, 0, new QTableWidgetItem {QString::fromStdString(sourceAttrib.getName())});
-        setItem(row, 1, new QTableWidgetItem {QString::fromStdString(targetAttrib.getName())});
+        setSortingEnabled(false);
+        {
+            int row = rowCount();
+            setRowCount(row + 1);
+
+            setItem(row, 0, new QTableWidgetItem {QString::fromStdString(sourceAttrib.getName())});
+            setItem(row, 1, new QTableWidgetItem {QString::fromStdString(targetAttrib.getName())});
+
+            result = item(row, 0);
+        }
+        setSortingEnabled(true);
 
-        return item(row, 0);
+        return result;
     }
 
 
@@ -76,15 +84,19 @@ namespace armarx::nav::locgrapheditor
         QFont font;
         font.setBold(edge.attrib().highlighted);
 
-        int row = this->row(edge.attrib().tableWidgetItem);
-        for (int col = 0; col < 2; ++col)
+        setSortingEnabled(false);
         {
-            auto* item = this->item(row, col);
-            ARMARX_CHECK_NOT_NULL(item);
+            int row = this->row(edge.attrib().tableWidgetItem);
+            for (int col = 0; col < 2; ++col)
+            {
+                auto* item = this->item(row, col);
+                ARMARX_CHECK_NOT_NULL(item);
 
-            item->setData(Qt::BackgroundRole, bgColor);
-            item->setFont(font);
+                item->setData(Qt::BackgroundRole, bgColor);
+                item->setFont(font);
+            }
         }
+        setSortingEnabled(true);
     }
 
 
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.cpp
index 7119c030..2aceb2bf 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.cpp
@@ -48,6 +48,8 @@ namespace armarx::nav::locgrapheditor
     {
         setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Minimum);
 
+        name = new QLineEdit(this);
+        name->setReadOnly(true);
         locationID = new QLineEdit(this);
         locationID->setReadOnly(true);
 
@@ -81,7 +83,8 @@ namespace armarx::nav::locgrapheditor
         QFormLayout* layout = new QFormLayout();
         this->setLayout(layout);
 
-        layout->addRow("Location ID", locationID);
+        layout->addRow("Name", name);
+        layout->addRow("Entity ID", locationID);
         layout->addRow("Frame", frame);
         layout->addRow("X", x);
         layout->addRow("Y", y);
@@ -142,30 +145,6 @@ namespace armarx::nav::locgrapheditor
     }
 
 
-    void VertexDataWidget::_setFromVertex(const GuiGraph::Vertex& vertex)
-    {
-        const VertexData& attrib = vertex.attrib();
-        const Eigen::Matrix4d pose = attrib.getPose().cast<qreal>();
-
-        locationID->setText(QString::fromStdString(aron::fromAron<armem::MemoryID>(attrib.aron.locationID).str()));
-        frame->setText("<WIP>");
-        _setXyz(simox::math::position(pose));
-        _setRpyRad(simox::math::mat4f_to_rpy(pose));
-    }
-
-
-    void VertexDataWidget::_getToVertex(GuiGraph::Vertex& vertex)
-    {
-        VertexData& attrib = vertex.attrib();
-
-        // locationID is read-only
-        // frame->setText("<WIP>");  // WIP
-
-        Eigen::Matrix4d pose = simox::math::pose(xyz(), simox::math::rpy_to_mat3f(rpyRad()));
-        attrib.setPose(pose.cast<float>());
-    }
-
-
     Eigen::Vector3d VertexDataWidget::xyz() const
     {
         return { x->value(), y->value(), z->value() };
@@ -200,6 +179,32 @@ namespace armarx::nav::locgrapheditor
     }
 
 
+    void VertexDataWidget::_setFromVertex(const GuiGraph::Vertex& vertex)
+    {
+        const VertexData& attrib = vertex.attrib();
+        const Eigen::Matrix4d pose = attrib.getPose().cast<qreal>();
+
+        name->setText(QString::fromStdString(attrib.getName()));
+        locationID->setText(QString::fromStdString(aron::fromAron<armem::MemoryID>(attrib.aron.locationID).str()));
+        frame->setText("<WIP>");
+        _setXyz(simox::math::position(pose));
+        _setRpyRad(simox::math::mat4f_to_rpy(pose));
+    }
+
+
+    void VertexDataWidget::_getToVertex(GuiGraph::Vertex& vertex)
+    {
+        VertexData& attrib = vertex.attrib();
+
+        // name is read-only
+        // locationID is read-only
+        // frame->setText("<WIP>");  // WIP
+
+        Eigen::Matrix4d pose = simox::math::pose(xyz(), simox::math::rpy_to_mat3f(rpyRad()));
+        attrib.setPose(pose.cast<float>());
+    }
+
+
     void VertexDataWidget::_updateAngleUnit()
     {
         std::function<double(double)> convertValue;
@@ -230,10 +235,10 @@ namespace armarx::nav::locgrapheditor
             };
         }
 
-        QList<QSignalBlocker>> blockers;
+        std::vector<QSignalBlocker> blockers;
         for (QDoubleSpinBox* angle : _angleSpinBoxes())
         {
-            blockers.append(QSignalBlocker(angle));
+            blockers.emplace_back(angle);
 
             angle->setSuffix(suffix);
             angle->setMinimum(min);
@@ -252,9 +257,10 @@ namespace armarx::nav::locgrapheditor
 
     void VertexDataWidget::_updateVertexAttribs()
     {
-        if (_vertex.has_value())
+        if (not signalsBlocked() and _vertex.has_value())
         {
             _getToVertex(_vertex.value());
+            _vertex->attrib().changed = true;
             emit vertexDataChanged();
         }
     }
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.h b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.h
index cb14f0c5..e16c824e 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.h
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexDataWidget.h
@@ -89,6 +89,7 @@ namespace armarx::nav::locgrapheditor
         std::optional<GuiGraph::Vertex> _vertex;
 
 
+        QLineEdit* name = nullptr;
         QLineEdit* locationID = nullptr;
         QLineEdit* frame = nullptr;
 
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.cpp
index 2d1e965e..908ea14f 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.cpp
@@ -22,6 +22,9 @@
 #include "VertexTableWidget.h"
 #include "utils.h"
 
+#include <ArmarXCore/core/exceptions/local/ExpressionException.h>
+#include <ArmarXCore/core/logging/Logging.h>
+
 #include <QAction>
 #include <QHeaderView>
 #include <QMenu>
@@ -58,20 +61,36 @@ namespace armarx::nav::locgrapheditor
     {
         (void) vertex;
 
-        int row = rowCount();
-        setRowCount(row + 1);
+        // We need to disable sorting to prevent the new row from being moved away.
+        setSortingEnabled(false);
 
-        // Just fill with vanilla items, they will get values in the update.
-        for (int col = 0; col < 4; ++col)
-        {
-            setItem(row, col, new QTableWidgetItem {});
-        }
-        for (int col = 1; col <= 3; ++col)
+        QTableWidgetItem* result = nullptr;
         {
-            item(row, col)->setTextAlignment(Qt::AlignRight | Qt::AlignVCenter);
+            int row = rowCount();
+            setRowCount(row + 1);
+
+            for (int col = 0; col < 4; ++col)
+            {
+                // Just fill with vanilla items, they will get values in the update.
+                QTableWidgetItem* item = new QTableWidgetItem {};
+                setItem(row, col, item);
+
+                if (col >= 1)
+                {
+                    item->setTextAlignment(Qt::AlignRight | Qt::AlignVCenter);
+                }
+                if (col == 0)
+                {
+                    result = item;
+                }
+            }
         }
 
-        return item(row, 0);
+        // Enable sorting - `row` could now be moved away.
+        setSortingEnabled(true);
+
+        ARMARX_CHECK_NOT_NULL(result);
+        return result;
     }
 
 
@@ -82,12 +101,24 @@ namespace armarx::nav::locgrapheditor
         char format = 'f';
         const int precision = 2;
 
+        // Changing the values may trigger a re-sort.
+        setSortingEnabled(false);
+
+
         int row = this->row(vertex.attrib().tableWidgetItem);
-        item(row, 0)->setText(QString::fromStdString(vertex.attrib().getName()));
+
+        QString displayName = QString::fromStdString(vertex.attrib().getName());
+        if (vertex.attrib().changed)
+        {
+            displayName += "*";
+        }
+        item(row, 0)->setText(displayName);
+        item(row, 0)->setData(Qt::UserRole, QString::fromStdString(vertex.attrib().getName()));
         item(row, 1)->setText(QString::number(pose(0, 3), format, precision));
         item(row, 2)->setText(QString::number(pose(1, 3), format, precision));
         item(row, 3)->setText(QString::number(getYawAngleDegree(pose), format, precision));
 
+        setSortingEnabled(true);
 
         QColor bgColor = vertex.attrib().highlighted ? bgColorSelected : bgColorDefault;
         QFont font;
@@ -110,8 +141,13 @@ namespace armarx::nav::locgrapheditor
         {
             setCurrentItem(nullptr);
         }
-        this->removeRow(row(vertex.attrib().tableWidgetItem));
+
+        const int numRows = rowCount();
+        int row = this->row(vertex.attrib().tableWidgetItem);
+        removeRow(row);
         vertex.attrib().tableWidgetItem = nullptr;
+
+        ARMARX_CHECK_EQUAL(rowCount(), numRows - 1);
     }
 
 
@@ -122,6 +158,12 @@ namespace armarx::nav::locgrapheditor
     }
 
 
+    QString VertexTableWidget::_nameOf(QTableWidgetItem* item)
+    {
+        return item->data(Qt::UserRole).toString();
+    }
+
+
     void VertexTableWidget::makeContextMenu(QPoint pos)
     {
         QList<QTableWidgetItem*> items = selectedVertexItems();
@@ -141,22 +183,22 @@ namespace armarx::nav::locgrapheditor
             // Generate actions for connecting these two nodes.
             std::sort(items.begin(), items.end(), [](QTableWidgetItem* first, QTableWidgetItem* second)
             {
-                return first->text() < second->text();
+                return _nameOf(first) < _nameOf(second);
             });
             QTableWidgetItem* first = items[0];
             QTableWidgetItem* second = items[1];
 
-            connect(menu.addAction("Add edge '" + first->text() + "' " + utils::arrowRight + " '" + second->text() + "'"),
+            connect(menu.addAction("Add edge '" + _nameOf(first) + "' " + utils::arrowRight + " '" + _nameOf(second) + "'"),
                     &QAction::triggered, [this, first, second]()
             {
                 emit newEdgesRequested({{first, second}});
             });
-            connect(menu.addAction("Add edge '" + first->text() + "' " + utils::arrowLeft + " '" + second->text() + "'"),
+            connect(menu.addAction("Add edge '" + _nameOf(first) + "' " + utils::arrowLeft + " '" + _nameOf(second) + "'"),
                     &QAction::triggered, [this, first, second]()
             {
                 emit newEdgesRequested({{second, first}});
             });
-            connect(menu.addAction("Add edges '" + first->text() + "' " + utils::arrowBoth + " '" + second->text() + "'"),
+            connect(menu.addAction("Add edges '" + _nameOf(first) + "' " + utils::arrowBoth + " '" + _nameOf(second) + "'"),
                     &QAction::triggered, [this, first, second]()
             {
                 emit newEdgesRequested({{first, second}, {second, first}});
@@ -170,7 +212,7 @@ namespace armarx::nav::locgrapheditor
                     ? "edge"
                     : "edges";
             QString desc = items.size() == 1
-                    ? "'" + items[0]->text() + "'"
+                    ? "'" + _nameOf(items[0]) + "'"
                     : QString::number(items.size()) + " locations";
 
             if (items.size() == 1)
@@ -199,7 +241,7 @@ namespace armarx::nav::locgrapheditor
                     QTableWidgetItem* action = this->item(row, 0);
                     if (items.count(action) == 0)  // Do no generate self-edges
                     {
-                        QAction* a = submenu->addAction(action->text());
+                        QAction* a = submenu->addAction(_nameOf(action));
                         connect(a, &QAction::triggered,
                                 [this, &items, action, appendFunc]()
                         {
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.h b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.h
index de609ce0..e782867f 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.h
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/VertexTableWidget.h
@@ -64,6 +64,11 @@ namespace armarx::nav::locgrapheditor
         void makeContextMenu(QPoint pos);
 
 
+    private:
+
+        static QString _nameOf(QTableWidgetItem* item);
+
+
     public:
 
         QColor bgColorDefault = default_colors::tableBackgroundDefault;
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.cpp b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.cpp
index e61b79b9..1a101fff 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.cpp
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.cpp
@@ -29,9 +29,12 @@
 namespace armarx::nav::locgrapheditor
 {
 
-    const QString utils::arrowLeft = QStringLiteral("←");
+    // <- https://unicode-table.com/de/2190/
+    const QString utils::arrowLeft = QStringLiteral("\u2190");
+    // -> https://unicode-table.com/de/2192/
     const QString utils::arrowRight = QStringLiteral("\u2192");
-    const QString utils::arrowBoth = QStringLiteral("⇄");
+    // <-> https://unicode-table.com/de/21C4/
+    const QString utils::arrowBoth = QStringLiteral("\u21C4");
 
 
     QList<QTableWidgetItem*>
diff --git a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.h b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.h
index 44eb497a..ad866a66 100644
--- a/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.h
+++ b/source/armarx/navigation/gui-plugins/LocationGraphEditor/widgets/utils.h
@@ -33,11 +33,11 @@ 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