Merge pull request #739 from francesco-romano/cleanup/model_exporter

Model solid shape and exporter cleanup

CHANGELOG.md

@@ -10,8 +10,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - Added the possibility of reusing an already opened figure with the MATLAB iDynTree Visualizer either if the name coincides or by using gcf.
 
+
+### Changed
+- `SolidShapes.h` public API **changes**. API changes are back compatible, but as the **ABI has changed**, this means a re-compilation of the dependent projects is needed. In details:
+    - Added getters and setters to all classes in `SolidShapes.h` (`idyntree-model`). Public attributes are still available for compatibility but are now **deprecated** and will be removed in the next major release of iDynTree (2.x).
+    - Added `Material` class in `SolidShapes.h` (`idyntree-model`). The `material` attribute in `SolidShape` is now deprecated. Please use the `color` property in the new `Material` class to maintain the previous behaviour. Note that the old and new properties are completely orthogonal. Ensure the code is consistent with their uses.
+
 ### Fixed
-- Fixed bug in init() of SimpleLeggedOdometry that used an incorrect initial world frame location if used with an additional frame of a link (https://github.com/robotology/idyntree/pull/698).
+- Fixed bug in init() of `SimpleLeggedOdometry` that used an incorrect initial world frame location if used with an additional frame of a link (https://github.com/robotology/idyntree/pull/698).
 - Fixed bug that prevented to use iDynTree cmake packages directly from the build directory (https://github.com/robotology/idyntree/pull/728).
 
 ## [1.1.0] - 2020-06-08

src/model/include/iDynTree/Model/SolidShapes.h

@@ -15,9 +15,38 @@
 #include <string>
 #include <vector>
 #include <iDynTree/Core/Transform.h>
+#include <iDynTree/Model/Indices.h>
 
+// TODO: Deprecation of public attributes.
+// - Ensure everybody migrated to use getters and setters.
+// - Move public attributes into private section and rename them with member
+//   convention (i.e. m_xxx).
+// - Bonus: change all the getters from `getXXX()` to `xxx()` (this can't be
+//   done now as you can't have a function with the same name as the variable).
 namespace iDynTree
 {
+    class Material {
+    public:
+        explicit Material();
+        explicit Material(const std::string& name);
+
+        std::string name() const;
+
+        bool hasColor() const;
+        Vector4 color() const;
+        void setColor(const Vector4& color);
+
+        bool hasTexture() const;
+        std::string texture() const;
+        void setTexture(const std::string& texture);
+
+    private:
+        Vector4 m_color;
+        bool m_isColorSet;
+        std::string m_texture;
+        std::string m_name;
+    };
+
     class Sphere;
     class Box;
     class Cylinder;
@@ -27,27 +56,57 @@ namespace iDynTree
     class SolidShape
     {
     public:
+        explicit SolidShape();
+
         virtual ~SolidShape()=0;
         virtual SolidShape* clone()=0;
-        std::string name;
+
         /**
-         * True if the name is valid, false otherwise.
+         * Returns the name of the shape.
          */
-        bool nameIsValid{false};
-        Transform link_H_geometry;
+        const std::string& getName() const;
 
         /**
-         * Material of the geometry, encoded as a rgba vector.
+         * Sets the specified name.
          */
-        Vector4 material;
+        void setName(const std::string& name);
 
-        // To correctly wrap this objects in SWIG, we cannot rely on dynamic_cast .
+        /**
+         * Returns if the name is valid.
+         */
+        bool isNameValid() const;
+
+        /**
+         * Returns the homogeneus transformation of the geometry w.r.t. the attached link.
+         */
+        const Transform& getLink_H_geometry() const;
+
+        /**
+         * Sets the homogeneus transformation of the geometry w.r.t. the attached link.
+         */
+        void setLink_H_geometry(const Transform& newTransform);
+
+        /**
+         * Returns if the material is valid, i.e. you can call getMaterial().
+         */
+        bool isMaterialSet() const;
+
+        /**
+         * Returns the current material.
+         */
+        const Material& getMaterial() const;
+
+        /**
+         * Sets the material. isMaterialSet will return true after this call.
+         */
+        void setMaterial(const Material& material);
 
         bool isSphere() const;
         bool isBox() const;
         bool isCylinder() const;
         bool isExternalMesh() const;
 
+        // Utility methods to traverse the SolidShape class hierachy.
         Sphere* asSphere();
         Box *asBox();
         Cylinder* asCylinder();
@@ -57,13 +116,47 @@ namespace iDynTree
         const Box* asBox() const;
         const Cylinder* asCylinder() const;
         const ExternalMesh* asExternalMesh() const;
+
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
+        std::string name;
+        /**
+         * True if the name is valid, false otherwise.
+         */
+        IDYNTREE_DEPRECATED_WITH_MSG("Use isNameValid().")
+        bool nameIsValid;
+
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
+        Transform link_H_geometry;
+
+        /**
+         * Material of the geometry, encoded as a rgba vector.
+         */
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters and the Material class.")
+        Vector4 material;
+
+    private:
+        bool m_isMaterialSet;
+        Material m_material;
     };
 
     class Sphere: public SolidShape
     {
     public:
         virtual ~Sphere();
+
         virtual SolidShape* clone();
+
+        /**
+         * Returns the current radius.
+         */
+        double getRadius() const;
+
+        /**
+         * Sets the new radius.
+         */
+        void setRadius(double radius);
+
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
         double radius;
     };
 
@@ -80,8 +173,42 @@ namespace iDynTree
     public:
         virtual ~Box();
         virtual SolidShape* clone();
+
+        /**
+         * Returns the current x side length.
+         */
+        double getX() const;
+
+        /**
+         * Sets the x side length.
+         */
+        void setX(double x);
+
+        /**
+         * Returns the current y side length.
+         */
+        double getY() const;
+
+        /**
+         * Sets the y side length.
+         */
+        void setY(double y);
+
+        /**
+         * Returns the current z side length.
+         */
+        double getZ() const;
+
+        /**
+         * Sets the z side length.
+         */
+        void setZ(double z);
+
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
         double x;
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
         double y;
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
         double z;
     };
 
@@ -90,7 +217,31 @@ namespace iDynTree
     public:
         virtual ~Cylinder();
         virtual SolidShape* clone();
+
+        /**
+         * Returns the current cylinder length.
+         */
+        double getLength() const;
+
+        /**
+         * Sets the cylinder length.
+         */
+        void setLength(double length);
+
+        /**
+         * Returns the current cylinder radius.
+         */
+        double getRadius() const;
+
+        /**
+         * Sets the cylinder radius.
+         */
+        void setRadius(double radius);
+
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
         double length;
+
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
         double radius;
     };
 
@@ -99,7 +250,31 @@ namespace iDynTree
     public:
         virtual ~ExternalMesh();
         virtual SolidShape* clone();
+
+        /**
+         * Returns the current filename.
+         */
+        const std::string& getFilename() const;
+
+        /**
+         * Sets the filename.
+         */
+        void setFilename(const std::string& filename);
+
+        /**
+         * Returns the current scale.
+         */
+        const iDynTree::Vector3& getScale() const;
+
+        /**
+         * Sets the scale.
+         */
+        void setScale(const iDynTree::Vector3& scale);
+
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
         std::string filename;
+
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use the setter and getters.")
         iDynTree::Vector3 scale;
     };
 
@@ -110,14 +285,21 @@ namespace iDynTree
 
     public:
         ModelSolidShapes();
+        ~ModelSolidShapes();
+
         ModelSolidShapes(const ModelSolidShapes& other);
         ModelSolidShapes& operator=(const ModelSolidShapes& other);
+
         void clear();
-        ~ModelSolidShapes();
         void resize(size_t nrOfLinks);
         void resize(const Model& model);
         bool isConsistent(const Model & model) const;
 
+        std::vector<std::vector<SolidShape *> >& getLinkSolidShapes();
+
+        const std::vector<std::vector<SolidShape *> >& getLinkSolidShapes() const;
+
+        IDYNTREE_DEPRECATED_WITH_MSG("Please use getLinkSolidShapes().")
         /**
          * Storage ot ModelSolidShapes.
          */

src/model/src/Model.cpp

@@ -273,10 +273,10 @@ LinkIndex Model::addLink(const std::string& name, const Link& link)
     }
 
     // Add an empty vector of collision shapes
-    m_collisionSolidShapes.linkSolidShapes.push_back(std::vector<SolidShape*>(0));
+    m_collisionSolidShapes.getLinkSolidShapes().push_back(std::vector<SolidShape*>(0));
 
     // Add an empty vector of visual shapes
-    m_visualSolidShapes.linkSolidShapes.push_back(std::vector<SolidShape*>(0));
+    m_visualSolidShapes.getLinkSolidShapes().push_back(std::vector<SolidShape*>(0));
 
 
     return newLinkIndex;
@@ -901,11 +901,6 @@ const ModelSolidShapes& Model::collisionSolidShapes() const
     return m_collisionSolidShapes;
 }
 
-
-
-
-
-
 std::string Model::toString() const
 {
     std::stringstream ss;
@@ -935,9 +930,4 @@ std::string Model::toString() const
 
     return ss.str();
 }
-
-
-
-
-
 }

src/model/src/ModelTransformers.cpp

@@ -297,34 +297,36 @@ void reducedModelAddSolidShapes(const Model& fullModel,
         Transform subModelBase_H_visitedLink     = subModelBase_X_link(visitedLinkIndex);
 
         // Add visual shapes to the new lumped link, i.e. the subModelBase
-        for(int shapeIdx=0; shapeIdx < fullModel.visualSolidShapes().linkSolidShapes[visitedLinkIndex].size(); shapeIdx++)
+        auto& visualSolidShapes = fullModel.visualSolidShapes().getLinkSolidShapes();
+        for(int shapeIdx=0; shapeIdx < visualSolidShapes[visitedLinkIndex].size(); shapeIdx++)
         {
             // Clone the shape
-            SolidShape * copiedShape = fullModel.visualSolidShapes().linkSolidShapes[visitedLinkIndex][shapeIdx]->clone();
+            SolidShape * copiedShape = visualSolidShapes[visitedLinkIndex][shapeIdx]->clone();
 
             // Update shape transform from the old link to the new link
-            Transform visitedLink_H_shape = fullModel.visualSolidShapes().linkSolidShapes[visitedLinkIndex][shapeIdx]->link_H_geometry;
-            copiedShape->link_H_geometry = subModelBase_H_visitedLink*visitedLink_H_shape;
+            Transform visitedLink_H_shape = visualSolidShapes[visitedLinkIndex][shapeIdx]->getLink_H_geometry();
+            copiedShape->setLink_H_geometry(subModelBase_H_visitedLink * visitedLink_H_shape);
 
             // Ownership of the new pointer is transfered to the ModelSolidShapes class
             // (it will be eventually deleted by the close() method
-            reducedModel.visualSolidShapes().linkSolidShapes[subModelBaseIndexInReducedModel].push_back(copiedShape);
+            reducedModel.visualSolidShapes().getLinkSolidShapes()[subModelBaseIndexInReducedModel].push_back(copiedShape);
         }
 
         // Add collision shapes to the new lumped link, i.e. the subModelBase
-        for(int shapeIdx=0; shapeIdx < fullModel.collisionSolidShapes().linkSolidShapes[visitedLinkIndex].size(); shapeIdx++)
+        auto& collisionSolidShapes = fullModel.collisionSolidShapes().getLinkSolidShapes();
+        for(int shapeIdx=0; shapeIdx < collisionSolidShapes[visitedLinkIndex].size(); shapeIdx++)
         {
             // Clone the shape : ownership of the new pointer is transfered to the ModelSolidShapes class
             // (it will be eventually deleted by the close() method
-            SolidShape * copiedShape = fullModel.collisionSolidShapes().linkSolidShapes[visitedLinkIndex][shapeIdx]->clone();
+            SolidShape * copiedShape = collisionSolidShapes[visitedLinkIndex][shapeIdx]->clone();
 
             // Update shape transform from the old link to the new link
-            Transform visitedLink_H_shape = fullModel.collisionSolidShapes().linkSolidShapes[visitedLinkIndex][shapeIdx]->link_H_geometry;
-            copiedShape->link_H_geometry = subModelBase_H_visitedLink*visitedLink_H_shape;
+            Transform visitedLink_H_shape = collisionSolidShapes[visitedLinkIndex][shapeIdx]->getLink_H_geometry();
+            copiedShape->setLink_H_geometry(subModelBase_H_visitedLink*visitedLink_H_shape);
 
             // Ownership of the new pointer is transfered to the ModelSolidShapes class
             // (it will be eventually deleted by the close() method
-            reducedModel.collisionSolidShapes().linkSolidShapes[subModelBaseIndexInReducedModel].push_back(copiedShape);
+            reducedModel.collisionSolidShapes().getLinkSolidShapes()[subModelBaseIndexInReducedModel].push_back(copiedShape);
         }
     }