Refactor action lib interfaces

g2o/apps/g2o_viewer/abstract_properties_widget.cpp

@@ -21,6 +21,7 @@
 #include <QLineEdit>
 #include <cassert>
 #include <iostream>
+#include <memory>
 
 #include "g2o/stuff/property.h"
 
@@ -87,12 +88,14 @@ void AbstractPropertiesWidget::updateDisplayedProperties() {
 
 void AbstractPropertiesWidget::applyProperties() {
   assert(tableWidget->rowCount() == (int)propNames_.size());
-  g2o::PropertyMap* properties = propertyMap();
+  const g2o::PropertyMap* properties = propertyMap();
   if (!properties) return;
   for (int r = 0; r < tableWidget->rowCount(); ++r) {
     const std::string& propName = propNames_[r];
+    // HACK cast away the const
     std::shared_ptr<g2o::BaseProperty> baseProp =
-        properties->getProperty<g2o::BaseProperty>(propName);
+        std::const_pointer_cast<g2o::BaseProperty>(
+            properties->getProperty<g2o::BaseProperty>(propName));
     if (!baseProp) continue;
 
     if (dynamic_cast<g2o::Property<bool>*>(baseProp.get())) {

g2o/apps/g2o_viewer/abstract_properties_widget.h

@@ -20,7 +20,6 @@
 #define G2O_ABSTRACT_PROPERTIES_WINDOW_H
 
 #include <QDialog>
-#include <memory>
 #include <string>
 #include <vector>
 
@@ -42,7 +41,7 @@ class G2O_VIEWER_API AbstractPropertiesWidget
   explicit AbstractPropertiesWidget(QWidget* parent = nullptr);
   ~AbstractPropertiesWidget() override = default;
 
-  virtual g2o::PropertyMap* propertyMap() = 0;
+  virtual const g2o::PropertyMap* propertyMap() = 0;
 
   void on_btnApply_clicked();
   void on_btnOK_clicked();

g2o/apps/g2o_viewer/g2o_qglviewer.cpp

@@ -51,6 +51,16 @@ namespace g2o {
 
 namespace {
 
+void applyAction(HyperGraph& graph, HyperGraphElementAction& action,
+                 HyperGraphElementAction::Parameters& parameters) {
+  for (auto& it : graph.vertices()) {
+    (action)(*it.second, parameters);
+  }
+  for (const auto& it : graph.edges()) {
+    (action)(*it, parameters);
+  }
+}
+
 /**
  * \brief helper for setting up a camera for qglviewer
  */
@@ -80,7 +90,6 @@ class StandardCamera : public qglviewer::Camera {
 G2oQGLViewer::G2oQGLViewer(QWidget* parent)
     : QGLViewer(parent), drawActions_(nullptr) {
   setAxisIsDrawn(false);
-  drawActionParameters_ = std::make_shared<DrawAction::Parameters>();
 }
 
 G2oQGLViewer::~G2oQGLViewer() { glDeleteLists(drawList_, 1); }

g2o/apps/g2o_viewer/g2o_qglviewer.h

@@ -49,7 +49,7 @@ class G2O_VIEWER_API G2oQGLViewer : public QGLViewer {
   [[nodiscard]] bool updateDisplay() const { return updateDisplay_; }
   void setUpdateDisplay(bool updateDisplay);
 
-  [[nodiscard]] std::shared_ptr<DrawAction::Parameters> parameters() const {
+  [[nodiscard]] const DrawAction::Parameters& parameters() const {
     return drawActionParameters_;
   }
 
@@ -59,7 +59,7 @@ class G2O_VIEWER_API G2oQGLViewer : public QGLViewer {
   HyperGraphElementAction::HyperGraphElementActionPtr drawActions_;
   GLuint drawList_ = 0;
   bool updateDisplay_ = true;
-  std::shared_ptr<DrawAction::Parameters> drawActionParameters_;
+  DrawAction::Parameters drawActionParameters_;
 };
 
 }  // namespace g2o

g2o/apps/g2o_viewer/gui_hyper_graph_action.cpp

@@ -25,16 +25,15 @@
 
 namespace g2o {
 
-bool GuiHyperGraphAction::operator()(
-    const HyperGraph& graph,
-    const std::shared_ptr<HyperGraphAction::Parameters>& parameters) {
+bool GuiHyperGraphAction::operator()(const HyperGraph& graph,
+                                     HyperGraphAction::Parameters& parameters) {
   (void)graph;
   if (viewer) {
     viewer->setUpdateDisplay(true);
     viewer->update();
 
     if (dumpScreenshots) {
-      auto p = std::dynamic_pointer_cast<ParametersIteration>(parameters);
+      const auto* p = dynamic_cast<ParametersIteration*>(&parameters);
       if (p) {
         viewer->setSnapshotFormat(QString("PNG"));
         viewer->setSnapshotQuality(-1);

g2o/apps/g2o_viewer/gui_hyper_graph_action.h

@@ -36,8 +36,7 @@ class G2O_VIEWER_API GuiHyperGraphAction : public HyperGraphAction {
    * iteration
    */
   bool operator()(const HyperGraph& graph,
-                  const std::shared_ptr<HyperGraphAction::Parameters>&
-                      parameters = nullptr) override;
+                  HyperGraphAction::Parameters& parameters) override;
 
   G2oQGLViewer* viewer = nullptr;  ///< the viewer which visualizes the graph
   bool dumpScreenshots = false;

g2o/apps/g2o_viewer/properties_widget.cpp

@@ -32,8 +32,7 @@ void PropertiesWidget::setSolver(
   updateDisplayedProperties();
 }
 
-g2o::PropertyMap* PropertiesWidget::propertyMap() {
+const g2o::PropertyMap* PropertiesWidget::propertyMap() {
   if (!solver_) return nullptr;
-  // HACK cast away the constness of the property map
-  return const_cast<g2o::PropertyMap*>(&solver_->properties());
+  return &solver_->properties();
 }

g2o/apps/g2o_viewer/properties_widget.h

@@ -33,7 +33,7 @@ class G2O_VIEWER_API PropertiesWidget : public AbstractPropertiesWidget {
   explicit PropertiesWidget(QWidget* parent = nullptr);
   ~PropertiesWidget() override = default;
 
-  g2o::PropertyMap* propertyMap() override;
+  const g2o::PropertyMap* propertyMap() override;
 
   void setSolver(std::shared_ptr<g2o::OptimizationAlgorithm> solver);
 

g2o/apps/g2o_viewer/viewer_properties_widget.cpp

@@ -18,7 +18,7 @@
 
 #include "viewer_properties_widget.h"
 
-#include "g2o/stuff/property.h"
+#include "g2o/core/hyper_graph_action.h"
 #include "g2o_qglviewer.h"
 
 #ifdef __GNUC__
@@ -80,7 +80,9 @@ std::string ViewerPropertiesWidget::humanReadablePropName(
   return demangleName(propertyName);
 }
 
-g2o::PropertyMap* ViewerPropertiesWidget::propertyMap() {
+const g2o::PropertyMap* ViewerPropertiesWidget::propertyMap() {
   if (!viewer_) return nullptr;
-  return viewer_->parameters().get();
+  const auto* params =
+      dynamic_cast<const g2o::DrawAction::Parameters*>(&viewer_->parameters());
+  return params;
 }

g2o/apps/g2o_viewer/viewer_properties_widget.h

@@ -35,7 +35,7 @@ class G2O_VIEWER_API ViewerPropertiesWidget : public AbstractPropertiesWidget {
   ~ViewerPropertiesWidget() override = default;
 
   void setViewer(g2o::G2oQGLViewer* viewer);
-  g2o::PropertyMap* propertyMap() override;
+  const g2o::PropertyMap* propertyMap() override;
 
  protected:
   g2o::G2oQGLViewer* viewer_ = nullptr;

g2o/core/hyper_graph_action.cpp

@@ -39,42 +39,24 @@ namespace g2o {
 std::unique_ptr<HyperGraphActionLibrary>
     HyperGraphActionLibrary::actionLibInstance_;
 
-HyperGraphAction::Parameters::~Parameters() = default;
-
 HyperGraphAction::ParametersIteration::ParametersIteration(int iter)
     : HyperGraphAction::Parameters(), iteration(iter) {}
 
-HyperGraphAction::~HyperGraphAction() = default;
-
-bool HyperGraphAction::operator()(const HyperGraph&,
-                                  const std::shared_ptr<Parameters>&) {
-  return false;
-}
-
-HyperGraphElementAction::Parameters::~Parameters() = default;
-
-HyperGraphElementAction::HyperGraphElementAction(std::string typeName)
-    : typeName_(std::move(typeName)) {}
+HyperGraphElementAction::HyperGraphElementAction(std::string typeName,
+                                                 std::string name)
+    : typeName_(std::move(typeName)), name_(std::move(name)) {}
 
 void HyperGraphElementAction::setTypeName(std::string typeName) {
   typeName_ = std::move(typeName);
 }
 
-bool HyperGraphElementAction::operator()(HyperGraph::HyperGraphElement&,
-                                         const std::shared_ptr<Parameters>&) {
-  return false;
-}
-
-HyperGraphElementAction::~HyperGraphElementAction() = default;
-
 HyperGraphElementActionCollection::HyperGraphElementActionCollection(
     const std::string& name) {
   name_ = name;
 }
 
 bool HyperGraphElementActionCollection::operator()(
-    HyperGraph::HyperGraphElement& element,
-    const std::shared_ptr<Parameters>& parameters) {
+    HyperGraph::HyperGraphElement& element, Parameters& parameters) {
   auto it = actionMap_.find(typeid(element).name());
   if (it == actionMap_.end()) return false;
   HyperGraphElementAction* action = it->second.get();
@@ -83,9 +65,8 @@ bool HyperGraphElementActionCollection::operator()(
 
 bool HyperGraphElementActionCollection::registerAction(
     const HyperGraphElementAction::HyperGraphElementActionPtr& action) {
-#ifdef G2O_DEBUG_ACTIONLIB
-  G2O_DEBUG("{} {}", action->name(), action->typeName());
-#endif
+  G2O_TRACE("Register action name: {} type: {}", action->name(),
+            action->typeName());
   if (action->name() != name()) {
     G2O_ERROR(
         "invalid attempt to register an action in a collection with a "
@@ -143,9 +124,7 @@ bool HyperGraphActionLibrary::registerAction(
     }
     return collection->registerAction(action);
   }
-#ifdef G2O_DEBUG_ACTIONLIB
-  G2O_DEBUG("creating collection for {}", action->name());
-#endif
+  G2O_TRACE("creating collection for {}", action->name());
   collection =
       std::make_shared<HyperGraphElementActionCollection>(action->name());
   actionMap_.insert(std::make_pair(action->name(), collection));
@@ -185,23 +164,20 @@ DrawAction::Parameters::Parameters() = default;
 DrawAction::DrawAction(const std::string& typeName_)
     : HyperGraphElementAction(typeName_) {
   name_ = "draw";
-  // previousParams_.reset(reinterpret_cast<Parameters*>(0x42));
-  refreshPropertyPtrs(nullptr);
-  cacheDrawActions_ = nullptr;
 }
 
-bool DrawAction::refreshPropertyPtrs(
-    const std::shared_ptr<HyperGraphElementAction::Parameters>& params) {
-  if (previousParams_ == params) return false;
-  auto p = std::dynamic_pointer_cast<DrawAction::Parameters>(params);
+DrawAction::Parameters* DrawAction::refreshPropertyPtrs(
+    HyperGraphElementAction::Parameters& params) {
+  if (previousParams_ == &params) return nullptr;
+  auto* p = dynamic_cast<DrawAction::Parameters*>(&params);
   if (!p) {
     previousParams_ = nullptr;
     show_ = nullptr;
   } else {
-    previousParams_ = p;
+    previousParams_ = &p;
     show_ = p->makeProperty<BoolProperty>(typeName_ + "::SHOW", true);
   }
-  return true;
+  return p;
 }
 
 void DrawAction::initializeDrawActionsCache() {
@@ -211,39 +187,20 @@ void DrawAction::initializeDrawActionsCache() {
   }
 }
 
-void DrawAction::drawCache(
-    const CacheContainer& caches,
-    const std::shared_ptr<HyperGraphElementAction::Parameters>& params) {
+void DrawAction::drawCache(const CacheContainer& caches,
+                           HyperGraphElementAction::Parameters& params) {
   for (const auto& cache : caches) {
     Cache* c = cache.second.get();
     (*cacheDrawActions_)(*c, params);
   }
 }
 
-void DrawAction::drawUserData(
-    const HyperGraph::DataContainer::DataVector& data,
-    const std::shared_ptr<HyperGraphElementAction::Parameters>& params) {
+void DrawAction::drawUserData(const HyperGraph::DataContainer::DataVector& data,
+                              HyperGraphElementAction::Parameters& params) {
   if (!cacheDrawActions_) return;
   for (const auto& d : data) {
     (*cacheDrawActions_)(*d, params);
   }
 }
 
-void applyAction(
-    HyperGraph& graph, HyperGraphElementAction& action,
-    const std::shared_ptr<HyperGraphElementAction::Parameters>& parameters,
-    const std::string& typeName) {
-  for (auto& it : graph.vertices()) {
-    auto& aux = *it.second;
-    if (typeName.empty() || typeid(aux).name() == typeName) {
-      (action)(*it.second, parameters);
-    }
-  }
-  for (const auto& it : graph.edges()) {
-    auto& aux = *it;
-    if (typeName.empty() || typeid(aux).name() == typeName)
-      (action)(aux, parameters);
-  }
-}
-
 }  // namespace g2o