Fix non-const Color::operator[] to return mutable reference

include/gz/math/Color.hh

@@ -168,7 +168,7 @@ namespace gz::math
     /// 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);
+    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,
@@ -177,6 +177,14 @@ namespace gz::math
     /// returned if the _index is invalid
     public: float operator[](const unsigned int _index) const;
 
+    /// \brief Returns the Color component indexed by _index (accessible also
+    /// through const object)
+    /// \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
+    private: const float &GetRGBAfromIndex(const unsigned int _index) const;
+
     /// \brief Get as uint32 RGBA packed value
     /// \return the color
     public: RGBA AsRGBA() const;

src/Color.cc

@@ -191,13 +191,21 @@ 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)
 {
-  return (*static_cast<const Color *>(this))[_index];
+    return const_cast<float&>(
+            static_cast<const Color *>(this)->GetRGBAfromIndex(_index)
+            );
 }
 
 //////////////////////////////////////////////////
 float Color::operator[](const unsigned int _index) const
+{
+  return this->GetRGBAfromIndex(_index);
+}
+
+//////////////////////////////////////////////////
+const float &Color::GetRGBAfromIndex(const unsigned int _index) const
 {
   switch (_index)
   {
@@ -213,7 +221,13 @@ float Color::operator[](const unsigned int _index) const
       break;
   }
 
-  return NAN_F;
+  // This allows referencing a NAN_F value.
+  static float invalidParam;
+  // Prevents overwriting of invalidParam with non NAN_F
+  // value through wrong index.
+  invalidParam = NAN_F;
+
+  return invalidParam;
 }
 
 //////////////////////////////////////////////////

src/Color_TEST.cc

@@ -186,20 +186,36 @@ TEST(Color, Color)
   EXPECT_FLOAT_EQ(1.0f, clr.A());
 
   clr.SetFromHSV(300, 0.5, 1.0);
-  EXPECT_FLOAT_EQ(1.0f, clr[0]);
-  EXPECT_FLOAT_EQ(0.5f, clr[1]);
-  EXPECT_FLOAT_EQ(1.0f, clr[2]);
-  EXPECT_FLOAT_EQ(1.0f, clr[3]);
-  EXPECT_TRUE(std::isnan(clr[4]));
+  EXPECT_FLOAT_EQ(1.0f, clr.R());
+  EXPECT_FLOAT_EQ(0.5f, clr.G());
+  EXPECT_FLOAT_EQ(1.0f, clr.B());
+  EXPECT_FLOAT_EQ(1.0f, clr.A());
 
   clr.R() = 0.1f;
   clr.G() = 0.2f;
   clr.B() = 0.3f;
   clr.A() = 0.4f;
-  EXPECT_FLOAT_EQ(0.1f, clr[0]);
-  EXPECT_FLOAT_EQ(0.2f, clr[1]);
-  EXPECT_FLOAT_EQ(0.3f, clr[2]);
-  EXPECT_FLOAT_EQ(0.4f, clr[3]);
+  EXPECT_FLOAT_EQ(0.1f, clr.R());
+  EXPECT_FLOAT_EQ(0.2f, clr.G());
+  EXPECT_FLOAT_EQ(0.3f, clr.B());
+  EXPECT_FLOAT_EQ(0.4f, clr.A());
+
+  EXPECT_FLOAT_EQ(clr.R(), clr[0]);
+  EXPECT_FLOAT_EQ(clr.G(), clr[1]);
+  EXPECT_FLOAT_EQ(clr.B(), clr[2]);
+  EXPECT_FLOAT_EQ(clr.A(), clr[3]);
+  EXPECT_TRUE(std::isnan(clr[4]));
+
+  clr[0] = 0.5f;
+  clr[1] = 0.6f;
+  clr[2] = 0.7f;
+  clr[3] = 0.8f;
+  clr[4] = 0.9f;
+  EXPECT_FLOAT_EQ(0.5f, clr[0]);
+  EXPECT_FLOAT_EQ(0.6f, clr[1]);
+  EXPECT_FLOAT_EQ(0.7f, clr[2]);
+  EXPECT_FLOAT_EQ(0.8f, clr[3]);
+  EXPECT_TRUE(std::isnan(clr[4]));
 
   clr.Set(0.1f, 0.2f, 0.3f, 0.4f);
   clr = clr + 0.2f;
@@ -328,6 +344,12 @@ TEST(Color, ConstAndSet)
   EXPECT_FLOAT_EQ(clr.B(), 0.3f);
   EXPECT_FLOAT_EQ(clr.A(), 0.4f);
 
+  EXPECT_FLOAT_EQ(clr.R(), clr[0]);
+  EXPECT_FLOAT_EQ(clr.G(), clr[1]);
+  EXPECT_FLOAT_EQ(clr.B(), clr[2]);
+  EXPECT_FLOAT_EQ(clr.A(), clr[3]);
+  EXPECT_TRUE(std::isnan(clr[4]));
+
   math::Color clr2;
   clr2.R(0.4f);
   clr2.G(0.3f);

src/python_pybind11/test/Color_TEST.py

@@ -172,10 +172,15 @@ def test_color(self):
         self.assertAlmostEqual(1.0, clr.a())
 
         clr.set_from_hsv(300, 0.5, 1.0)
-        self.assertAlmostEqual(1.0, clr[0])
-        self.assertAlmostEqual(0.5, clr[1])
-        self.assertAlmostEqual(1.0, clr[2])
-        self.assertAlmostEqual(1.0, clr[3])
+        self.assertAlmostEqual(1.0, clr.r())
+        self.assertAlmostEqual(0.5, clr.g())
+        self.assertAlmostEqual(1.0, clr.b())
+        self.assertAlmostEqual(1.0, clr.a())
+
+        self.assertAlmostEqual(clr.r(), clr[0])
+        self.assertAlmostEqual(clr.g(), clr[1])
+        self.assertAlmostEqual(clr.b(), clr[2])
+        self.assertAlmostEqual(clr.a(), clr[3])
         self.assertTrue(math.isnan(clr[4]))
 
         clr.set(0.1, 0.2, 0.3, 0.4)