👩‍🌾 Prevent -0 with out stream operator (#206)

Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Steve Peters <[email protected]>

Changelog.md

@@ -21,6 +21,9 @@
 1. Updated Temperature class documentation, script interface, and added examples.
     * [BitBucket pull request 339](https://osrf-migration.github.io/ignition-gh-pages/#!/ignitionrobotics/ign-math/pull-requests/339)
 
+1. Prevent -0 with out stream operator
+    * [Pull request 206](https://github.com/ignitionrobotics/ign-math/pull/206)
+
 ## Ignition Math 6.x
 
 ### Ignition Math 6.x.x

Migration.md

@@ -53,6 +53,11 @@ release will remove the deprecated code.
     + ***Deprecation:*** public: void W(T)
     + ***Replacement:*** public: void SetW(T)
 
+### Modifications
+
+1. The out stream operator is guaranteed to return always plain 0 and not to
+   return -0, 0.0 or other instances of zero value.
+
 ## Ignition Math 4.X to 5.X
 
 ### Additions

include/ignition/math/Color.hh

@@ -74,8 +74,8 @@ namespace ignition
 
       /// \brief Constructor
       /// \param[in] _r Red value (range 0 to 1)
-      /// \param[in] _g Green value (range 0 to 1
-      /// \param[in] _b Blue value (range 0 to 1
+      /// \param[in] _g Green value (range 0 to 1)
+      /// \param[in] _b Blue value (range 0 to 1)
       /// \param[in] _a Alpha value (0=transparent, 1=opaque)
       public: Color(const float _r, const float _g, const float _b,
                   const float _a = 1.0);
@@ -126,11 +126,19 @@ namespace ignition
       public: Color &operator=(const Color &_pt);
 
       /// \brief Array index operator
-      /// \param[in] _index Color component index(0=red, 1=green, 2=blue)
+      /// \param[in] _index Color component index(0=red, 1=green, 2=blue,
+      /// 3=alpha)
       /// \return r, g, b, or a when _index is 0, 1, 2 or 3. A NAN_F value is
       /// returned if the _index is invalid
       public: float operator[](const unsigned int _index);
 
+      /// \brief Array index operator, const version
+      /// \param[in] _index Color component index(0=red, 1=green, 2=blue,
+      /// 3=alpha)
+      /// \return r, g, b, or a when _index is 0, 1, 2 or 3. A NAN_F value is
+      /// returned if the _index is invalid
+      public: float operator[](const unsigned int _index) const;
+
       /// \brief Get as uint32 RGBA packed value
       /// \return the color
       public: RGBA AsRGBA() const;
@@ -238,12 +246,18 @@ namespace ignition
 
       /// \brief Stream insertion operator
       /// \param[in] _out the output stream
-      /// \param[in] _pt the color
+      /// \param[in] _color the color
       /// \return the output stream
       public: friend std::ostream &operator<<(std::ostream &_out,
-                                              const Color &_pt)
+                                              const Color &_color)
       {
-        _out << _pt.r << " " << _pt.g << " " << _pt.b << " " << _pt.a;
+        for (auto i : {0, 1, 2, 3})
+        {
+          if (i > 0)
+            _out << " ";
+
+          appendToStream(_out, _color[i], 6);
+        }
         return _out;
       }
 

include/ignition/math/Helpers.hh

@@ -462,6 +462,33 @@ namespace ignition
       sort2(_a, _b);
     }
 
+    /// \brief Append a number to a stream. Makes sure "-0" is returned as "0".
+    /// \param[out] _out Output stream.
+    /// \param[in] _number Number to append.
+    /// \param[in] _precision Precision for floating point numbers.
+    template<typename T>
+    inline void appendToStream(std::ostream &_out, T _number, int _precision)
+    {
+      if (std::fpclassify(_number) == FP_ZERO)
+      {
+        _out << 0;
+      }
+      else
+      {
+        _out << precision(_number, _precision);
+      }
+    }
+
+    /// \brief Append a number to a stream, specialized for int.
+    /// \param[out] _out Output stream.
+    /// \param[in] _number Number to append.
+    /// \param[in] _precision Not used for int.
+    template<>
+    inline void appendToStream(std::ostream &_out, int _number, int)
+    {
+      _out << _number;
+    }
+
     /// \brief Is the parameter a power of 2?
     /// \param[in] _x The number to check.
     /// \return True if _x is a power of 2, false otherwise.

include/ignition/math/Matrix3.hh

@@ -610,15 +610,16 @@ namespace ignition
       public: friend std::ostream &operator<<(
                   std::ostream &_out, const ignition::math::Matrix3<T> &_m)
       {
-        _out << precision(_m(0, 0), 6) << " "
-             << precision(_m(0, 1), 6) << " "
-             << precision(_m(0, 2), 6) << " "
-             << precision(_m(1, 0), 6) << " "
-             << precision(_m(1, 1), 6) << " "
-             << precision(_m(1, 2), 6) << " "
-             << precision(_m(2, 0), 6) << " "
-             << precision(_m(2, 1), 6) << " "
-             << precision(_m(2, 2), 6);
+        for (auto i : {0, 1, 2})
+        {
+          for (auto j : {0, 1, 2})
+          {
+            if (!(i == 0 && j == 0))
+              _out << " ";
+
+            appendToStream(_out, _m(i, j), 6);
+          }
+        }
 
         return _out;
       }

include/ignition/math/Matrix4.hh

@@ -759,22 +759,16 @@ namespace ignition
       public: friend std::ostream &operator<<(
                   std::ostream &_out, const ignition::math::Matrix4<T> &_m)
       {
-        _out << precision(_m(0, 0), 6) << " "
-             << precision(_m(0, 1), 6) << " "
-             << precision(_m(0, 2), 6) << " "
-             << precision(_m(0, 3), 6) << " "
-             << precision(_m(1, 0), 6) << " "
-             << precision(_m(1, 1), 6) << " "
-             << precision(_m(1, 2), 6) << " "
-             << precision(_m(1, 3), 6) << " "
-             << precision(_m(2, 0), 6) << " "
-             << precision(_m(2, 1), 6) << " "
-             << precision(_m(2, 2), 6) << " "
-             << precision(_m(2, 3), 6) << " "
-             << precision(_m(3, 0), 6) << " "
-             << precision(_m(3, 1), 6) << " "
-             << precision(_m(3, 2), 6) << " "
-             << precision(_m(3, 3), 6);
+        for (auto i : {0, 1, 2, 3})
+        {
+          for (auto j : {0, 1, 2, 3})
+          {
+            if (!(i == 0 && j == 0))
+              _out << " ";
+
+            appendToStream(_out, _m(i, j), 6);
+          }
+        }
 
         return _out;
       }

include/ignition/math/Quaternion.hh

@@ -1220,8 +1220,7 @@ namespace ignition
                   const ignition::math::Quaternion<T> &_q)
       {
         Vector3<T> v(_q.Euler());
-        _out << precision(v.X(), 6) << " " << precision(v.Y(), 6) << " "
-             << precision(v.Z(), 6);
+        _out << v;
         return _out;
       }
 

include/ignition/math/Vector2.hh

@@ -531,13 +531,19 @@ namespace ignition
       }
 
       /// \brief Stream extraction operator
-      /// \param[in] _out output stream
+      /// \param[out] _out output stream
       /// \param[in] _pt Vector2 to output
       /// \return The stream
       public: friend std::ostream
       &operator<<(std::ostream &_out, const Vector2<T> &_pt)
       {
-        _out << _pt[0] << " " << _pt[1];
+        for (auto i : {0, 1})
+        {
+          if (i > 0)
+            _out << " ";
+
+          appendToStream(_out, _pt[i], 6);
+        }
         return _out;
       }
 

include/ignition/math/Vector3.hh

@@ -741,8 +741,14 @@ namespace ignition
       public: friend std::ostream &operator<<(
                   std::ostream &_out, const ignition::math::Vector3<T> &_pt)
       {
-        _out << precision(_pt[0], 6) << " " << precision(_pt[1], 6) << " "
-          << precision(_pt[2], 6);
+        for (auto i : {0, 1, 2})
+        {
+          if (i > 0)
+            _out << " ";
+
+          appendToStream(_out, _pt[i], 6);
+        }
+
         return _out;
       }
 

include/ignition/math/Vector4.hh

@@ -700,7 +700,13 @@ namespace ignition
       public: friend std::ostream &operator<<(
                   std::ostream &_out, const ignition::math::Vector4<T> &_pt)
       {
-        _out << _pt[0] << " " << _pt[1] << " " << _pt[2] << " " << _pt[3];
+        for (auto i : {0, 1, 2, 3})
+        {
+          if (i > 0)
+            _out << " ";
+
+          appendToStream(_out, _pt[i], 6);
+        }
         return _out;
       }
 

src/Color.cc

@@ -195,9 +195,15 @@ void Color::SetFromYUV(const float _y, const float _u, const float _v)
 }
 
 //////////////////////////////////////////////////
-float Color::operator[](const unsigned int index)
+float Color::operator[](const unsigned int _index)
 {
-  switch (index)
+  return (*static_cast<const Color *>(this))[_index];
+}
+
+//////////////////////////////////////////////////
+float Color::operator[](const unsigned int _index) const
+{
+  switch (_index)
   {
     case 0:
       return this->r;

src/Color_TEST.cc

@@ -330,10 +330,10 @@ TEST(Color, ConstAndSet)
 /////////////////////////////////////////////////
 TEST(Color, OperatorStreamOut)
 {
-  math::Color c(0.1f, 0.2f, 0.3f, 0.5f);
+  math::Color c(0.1f, 0.2f, 0.3f, 0.0f);
   std::ostringstream stream;
   stream << c;
-  EXPECT_EQ(stream.str(), "0.1 0.2 0.3 0.5");
+  EXPECT_EQ(stream.str(), "0.1 0.2 0.3 0");
 }
 
 /////////////////////////////////////////////////

src/Helpers_TEST.cc

@@ -933,3 +933,27 @@ TEST(HelpersTest, roundUpMultiple)
   EXPECT_EQ(0, math::roundUpMultiple(0, -2));
   EXPECT_EQ(-2, math::roundUpMultiple(-2, -2));
 }
+
+/////////////////////////////////////////////////
+TEST(HelpersTest, AppendToStream)
+{
+  std::ostringstream out;
+
+  math::appendToStream(out, 0.12345678, 3);
+  EXPECT_EQ(out.str(), "0.123");
+
+  out << " ";
+
+  math::appendToStream(out, 0.0f, 5);
+  EXPECT_EQ(out.str(), "0.123 0");
+
+  out << " ";
+
+  math::appendToStream(out, 456, 3);
+  EXPECT_EQ(out.str(), "0.123 0 456");
+
+  out << " ";
+
+  math::appendToStream(out, 0, 3);
+  EXPECT_EQ(out.str(), "0.123 0 456 0");
+}

src/Matrix3_TEST.cc

@@ -161,12 +161,12 @@ TEST(Matrix3dTest, OperatorMul)
 TEST(Matrix3dTest, OperatorStreamOut)
 {
   math::Matrix3d matA(1, 2, 3,
-                      4, 5, 6,
+                      4, 0, 6,
                       7, 8, 9);
 
   std::ostringstream stream;
   stream << matA;
-  EXPECT_EQ(stream.str(), "1 2 3 4 5 6 7 8 9");
+  EXPECT_EQ(stream.str(), "1 2 3 4 0 6 7 8 9");
 }
 
 /////////////////////////////////////////////////

src/Matrix4_TEST.cc

@@ -548,11 +548,11 @@ TEST(Matrix4dTest, OperatorStreamOut)
   math::Matrix4d matA(1, 2, 3, 4,
                       5, 6, 7, 8,
                       9, 10, 11, 12,
-                      13, 14, 15, 16);
+                      13, 14, 15, 0);
 
   std::ostringstream stream;
   stream << matA;
-  EXPECT_EQ(stream.str(), "1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16");
+  EXPECT_EQ(stream.str(), "1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 0");
 }
 
 /////////////////////////////////////////////////

src/Pose_TEST.cc

@@ -187,3 +187,55 @@ TEST(PoseTest, OperatorStreamOut)
   EXPECT_EQ(stream.str(), "0.1 1.2 2.3 0 0.1 1");
 }
 
+/////////////////////////////////////////////////
+TEST(PoseTest, OperatorStreamOutZero)
+{
+  math::Pose3d p(0.1, 1.2, 2.3, 0, 0, 0);
+  std::ostringstream stream;
+  stream << p;
+  EXPECT_EQ(stream.str(), "0.1 1.2 2.3 0 0 0");
+}
+
+/////////////////////////////////////////////////
+TEST(PoseTest, MutablePose)
+{
+  math::Pose3d pose(0, 1, 2, 0, 0, 0);
+
+  EXPECT_TRUE(pose.Pos() == math::Vector3d(0, 1, 2));
+  EXPECT_TRUE(pose.Rot() == math::Quaterniond(0, 0, 0));
+
+  pose.Pos() = math::Vector3d(10, 20, 30);
+  pose.Rot() = math::Quaterniond(1, 2, 1);
+
+  EXPECT_TRUE(pose.Pos() == math::Vector3d(10, 20, 30));
+  EXPECT_TRUE(pose.Rot() == math::Quaterniond(1, 2, 1));
+}
+
+/////////////////////////////////////////////////
+TEST(PoseTest, ConstPoseElements)
+{
+  const math::Pose3d pose(0, 1, 2, 1, 1, 2);
+  EXPECT_DOUBLE_EQ(pose.X(), 0);
+  EXPECT_DOUBLE_EQ(pose.Y(), 1);
+  EXPECT_DOUBLE_EQ(pose.Z(), 2);
+  EXPECT_DOUBLE_EQ(pose.Roll(), 1);
+  EXPECT_DOUBLE_EQ(pose.Pitch(), 1);
+  EXPECT_DOUBLE_EQ(pose.Yaw(), 2);
+}
+
+/////////////////////////////////////////////////
+TEST(PoseTest, SetPoseElements)
+{
+  math::Pose3d pose(1, 2, 3, 1.57, 1, 2);
+  EXPECT_DOUBLE_EQ(pose.X(), 1);
+  EXPECT_DOUBLE_EQ(pose.Y(), 2);
+  EXPECT_DOUBLE_EQ(pose.Z(), 3);
+
+  pose.SetX(10);
+  pose.SetY(12);
+  pose.SetZ(13);
+
+  EXPECT_DOUBLE_EQ(pose.X(), 10);
+  EXPECT_DOUBLE_EQ(pose.Y(), 12);
+  EXPECT_DOUBLE_EQ(pose.Z(), 13);
+}