Clamp values between [0, 1]

Signed-off-by: Addisu Z. Taddese <[email protected]>

include/gz/math/Color.hh

@@ -100,10 +100,34 @@ namespace gz::math
     /// \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: constexpr Color(const float _r, const float _g, const float _b,
-                            const float _a = 1.0)
+    /// \note If there are values outside the range [0, 1], they will be
+    /// clamped and an error message will be printed
+    public: Color(const float _r, const float _g, const float _b,
+                  const float _a = 1.0)
     : r(_r), g(_g), b(_b), a(_a)
     {
+      this->Clamp();
+    }
+
+    /// \brief Static function to create colors that are not clamped.
+    /// \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] _a Alpha value (0=transparent, 1=opaque)
+    /// \note This is mainly intended to initialize constexpr values
+    /// such as Color::gRed. The Color::Color constructor is recommended
+    /// for regular use.
+    public: static constexpr Color UnclampedColor(const float _r,
+                                                  const float _g,
+                                                  const float _b,
+                                                  const float _a)
+    {
+      auto clr = Color();
+      clr.r = _r;
+      clr.g = _g;
+      clr.b = _b;
+      clr.a = _a;
+      return clr;
     }
 
     /// \brief Reset the color to default values to red=0, green=0,
@@ -255,6 +279,9 @@ namespace gz::math
     /// \return True if the this color does not equal _pt
     public: bool operator!=(const Color &_pt) const;
 
+    /// \brief Clamp the color values to valid ranges
+    private: void Clamp();
+
     /// \brief Stream insertion operator
     /// \param[in] _out the output stream
     /// \param[in] _color the color

src/Color.cc

@@ -16,6 +16,7 @@
  */
 #include <cmath>
 #include <algorithm>
+#include <iostream>
 
 #include "gz/math/Color.hh"
 
@@ -26,14 +27,14 @@ namespace {
 
 // Use constexpr storage for the Color constants, to avoid the C++ static
 // initialization order fiasco.
-constexpr Color gWhite = Color(1, 1, 1, 1);
-constexpr Color gBlack = Color(0, 0, 0, 1);
-constexpr Color gRed = Color(1, 0, 0, 1);
-constexpr Color gGreen = Color(0, 1, 0, 1);
-constexpr Color gBlue = Color(0, 0, 1, 1);
-constexpr Color gYellow = Color(1, 1, 0, 1);
-constexpr Color gMagenta = Color(1, 0, 1, 1);
-constexpr Color gCyan = Color(0, 1, 1, 1);
+constexpr Color gWhite = Color::UnclampedColor(1, 1, 1, 1);
+constexpr Color gBlack = Color::UnclampedColor(0, 0, 0, 1);
+constexpr Color gRed = Color::UnclampedColor(1, 0, 0, 1);
+constexpr Color gGreen = Color::UnclampedColor(0, 1, 0, 1);
+constexpr Color gBlue = Color::UnclampedColor(0, 0, 1, 1);
+constexpr Color gYellow = Color::UnclampedColor(1, 1, 0, 1);
+constexpr Color gMagenta = Color::UnclampedColor(1, 0, 1, 1);
+constexpr Color gCyan = Color::UnclampedColor(0, 1, 1, 1);
 
 }  // namespace
 
@@ -60,6 +61,8 @@ void Color::Set(const float _r, const float _g, const float _b, const float _a)
   this->g = _g;
   this->b = _b;
   this->a = _a;
+
+  this->Clamp();
 }
 
 //////////////////////////////////////////////////
@@ -122,6 +125,8 @@ void Color::SetFromHSV(const float _h, const float _s, const float _v)
       this->b = q;
       break;
   }
+
+  this->Clamp();
 }
 
 //////////////////////////////////////////////////
@@ -179,6 +184,10 @@ void Color::SetFromYUV(const float _y, const float _u, const float _v)
   this->r = _y + 1.140f*_v;
   this->g = _y - 0.395f*_u - 0.581f*_v;
   this->b = _y + 2.032f*_u;
+  this->r = this->r > 1 ? this->r / 255.0 : this->r;
+  this->g = this->g > 1 ? this->g / 255.0 : this->g;
+  this->b = this->b > 1 ? this->b / 255.0 : this->b;
+  this->Clamp();
 }
 
 //////////////////////////////////////////////////
@@ -380,6 +389,8 @@ const Color &Color::operator+=(const Color &_pt)
   this->b += _pt.b;
   this->a += _pt.a;
 
+  this->Clamp();
+
   return *this;
 }
 
@@ -404,6 +415,8 @@ const Color &Color::operator-=(const Color &_pt)
   this->b -= _pt.b;
   this->a -= _pt.a;
 
+  this->Clamp();
+
   return *this;
 }
 
@@ -428,6 +441,8 @@ const Color &Color::operator/=(const Color &_pt)
   this->b /= _pt.b;
   this->a /= _pt.a;
 
+  this->Clamp();
+
   return *this;
 }
 
@@ -452,6 +467,8 @@ const Color &Color::operator*=(const Color &_pt)
   this->b *= _pt.b;
   this->a *= _pt.a;
 
+  this->Clamp();
+
   return *this;
 }
 
@@ -539,3 +556,24 @@ void Color::A(const float _a)
 {
   this->a = _a;
 }
+
+//////////////////////////////////////////////////
+void Color::Clamp()
+{
+  bool clamped = false;
+  // These comparisons are carefully written to handle NaNs correctly.
+  if (!(this->r >= 0)) { this->r = 0; clamped=true;}
+  if (!(this->g >= 0)) { this->g = 0; clamped=true;}
+  if (!(this->b >= 0)) { this->b = 0; clamped=true;}
+  if (!(this->a >= 0)) { this->a = 0; clamped=true;}
+  if (this->r > 1) { this->r = 1.0; clamped=true;}
+  if (this->g > 1) { this->g = 1.0; clamped=true;}
+  if (this->b > 1) { this->b = 1.0; clamped=true;}
+  if (this->a > 1) { this->a = 1.0; clamped=true;}
+
+  if (clamped)
+  {
+    // TODO(azeey) Use spdlog when we have it's available.
+    std::cerr << "Color values were clamped\n";
+  }
+}

src/Color_TEST.cc

@@ -247,7 +247,7 @@ TEST(Color, Color)
 
   clr = math::Color(1.0f, 0.0f, 0.5f, 1.0f) +
     math::Color(0.1f, 0.3f, 0.4f, 1.0f);
-  EXPECT_TRUE(math::equal(0.00431373f, clr.R()));
+  EXPECT_TRUE(math::equal(1.0f, clr.R())) << clr;
   EXPECT_TRUE(math::equal(0.3f, clr.G()));
   EXPECT_TRUE(math::equal(0.9f, clr.B()));
   EXPECT_TRUE(math::equal(1.0f, clr.A()));
@@ -422,8 +422,8 @@ TEST(Color, HSV)
   EXPECT_NEAR(hsv.Z(), 0.3f, 1e-3);
 
   clr.SetFromHSV(60, 10, 5);
-  EXPECT_NEAR(clr.R(), 0.0196078f, 1e-3);
-  EXPECT_NEAR(clr.G(), 0.0196078f, 1e-3);
+  EXPECT_NEAR(clr.R(), 1.0, 1e-3);
+  EXPECT_NEAR(clr.G(), 1.0, 1e-3);
   EXPECT_NEAR(clr.B(), 0.0f, 1e-3);
   EXPECT_NEAR(clr.A(), 1.0, 1e-3);
 

src/python_pybind11/test/Color_TEST.py

@@ -216,10 +216,10 @@ def test_color(self):
         self.assertAlmostEqual(0.9064, clr.b(), delta=1e-3)
         self.assertAlmostEqual(0.04, clr.a())
 
-        self.assertTrue(clr.yuv() == Vector3f(0.104985, 0.95227, 0.429305))
+        self.assertTrue(clr.yuv() == Vector3f(0.104985, 0.95227, 0.429305), msg=f"{clr}")
 
         clr = Color(1.0, 0.0, 0.5, 1.0) + Color(0.1, 0.3, 0.4, 1.0)
-        self.assertAlmostEqual(0.00431373, clr.r(), delta=1e-4)
+        self.assertAlmostEqual(1.0, clr.r(), delta=1e-4)
         self.assertAlmostEqual(0.3, clr.g(), delta=1e-4)
         self.assertAlmostEqual(0.9, clr.b(), delta=1e-4)
         self.assertAlmostEqual(1.0, clr.a(), delta=1e-4)
@@ -318,8 +318,8 @@ def test_HSV(self):
         self.assertAlmostEqual(hsv.z(), 0.3, delta=1e-3)
 
         clr.set_from_hsv(60, 10, 5)
-        self.assertAlmostEqual(clr.r(), 0.0196078, delta=1e-3)
-        self.assertAlmostEqual(clr.g(), 0.0196078, delta=1e-3)
+        self.assertAlmostEqual(clr.r(), 1.0, delta=1e-3)
+        self.assertAlmostEqual(clr.g(), 1.0, delta=1e-3)
         self.assertAlmostEqual(clr.b(), 0.0, delta=1e-3)
         self.assertAlmostEqual(clr.a(), 1.0, delta=1e-3)