WIP Use constexpr for simple static constants

At the moment, only Color has been refactored with this pattern while we discuss the implications. Signed-off-by: Jeremy Nimmer <[email protected]>
gazebosim · Nov 22, 2021 · 85edfaf · 85edfaf
1 parent 5201e53
commit 85edfaf
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 43 deletions.
diff --git a/include/ignition/math/Color.hh b/include/ignition/math/Color.hh
@@ -74,18 +74,22 @@ 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);
+      public: constexpr 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 Copy Constructor
       /// \param[in] _clr Color to copy
       public: Color(const Color &_clr);
 
       /// \brief Destructor
-      public: virtual ~Color();
+      public: ~Color() = default;
 
       /// \brief Reset the color to default values to red=0, green=0,
       /// blue=0, alpha=1.
@@ -234,7 +238,18 @@ namespace ignition
       public: bool operator!=(const Color &_pt) const;
 
       /// \brief Clamp the color values to valid ranges
-      private: void Clamp();
+      private: constexpr void Clamp()
+      {
+        // The comparisons here are carefully written to handle NaNs correctly.
+        if (!(this->r >= 0)) { this->r = 0; }
+        if (!(this->g >= 0)) { this->g = 0; }
+        if (!(this->b >= 0)) { this->b = 0; }
+        if (!(this->a >= 0)) { this->a = 0; }
+        if (this->r > 1) { this->r = this->r/255.0f; }
+        if (this->g > 1) { this->g = this->g/255.0f; }
+        if (this->b > 1) { this->b = this->b/255.0f; }
+        if (this->a > 1) { this->a = 1; }
+      }
 
       /// \brief Stream insertion operator
       /// \param[in] _out the output stream
@@ -332,6 +347,15 @@ namespace ignition
       /// \brief Alpha value
       private: float a = 1;
     };
+
+    inline constexpr const Color Color::White{1, 1, 1};
+    inline constexpr const Color Color::Black{0, 0, 0};
+    inline constexpr const Color Color::Red{1, 0, 0};
+    inline constexpr const Color Color::Green{0, 1, 0};
+    inline constexpr const Color Color::Blue{0, 0, 1};
+    inline constexpr const Color Color::Yellow{1, 1, 0};
+    inline constexpr const Color Color::Magenta{1, 0, 1};
+    inline constexpr const Color Color::Cyan{0, 1, 1};
     }
   }
 }

diff --git a/src/Color.cc b/src/Color.cc
@@ -22,39 +22,18 @@
 using namespace ignition;
 using namespace math;
 
-const Color Color::White = Color(1, 1, 1, 1);
-const Color Color::Black = Color(0, 0, 0, 1);
-const Color Color::Red = Color(1, 0, 0, 1);
-const Color Color::Green = Color(0, 1, 0, 1);
-const Color Color::Blue = Color(0, 0, 1, 1);
-const Color Color::Yellow = Color(1, 1, 0, 1);
-const Color Color::Magenta = Color(1, 0, 1, 1);
-const Color Color::Cyan = Color(0, 1, 1, 1);
-
 //////////////////////////////////////////////////
 Color::Color()
 {
 }
 
-//////////////////////////////////////////////////
-Color::Color(const float _r, const float _g, const float _b, const float _a)
-: r(_r), g(_g), b(_b), a(_a)
-{
-  this->Clamp();
-}
-
 //////////////////////////////////////////////////
 Color::Color(const Color &_pt)
 : r(_pt.r), g(_pt.g), b(_pt.b), a(_pt.a)
 {
   this->Clamp();
 }
 
-//////////////////////////////////////////////////
-Color::~Color()
-{
-}
-
 //////////////////////////////////////////////////
 void Color::Reset()
 {
@@ -494,22 +473,6 @@ bool Color::operator!=(const Color &_pt) const
   return !(*this == _pt);
 }
 
-//////////////////////////////////////////////////
-void Color::Clamp()
-{
-  this->r = this->r < 0 || isnan(this->r) ? 0: this->r;
-  this->r = this->r > 1 ? this->r/255.0f: this->r;
-
-  this->g = this->g < 0 || isnan(this->g) ? 0: this->g;
-  this->g = this->g > 1 ? this->g/255.0f: this->g;
-
-  this->b = this->b < 0 || isnan(this->b) ? 0: this->b;
-  this->b = this->b > 1 ? this->b/255.0f: this->b;
-
-  this->a = this->a < 0 || isnan(this->a) ? 0: this->a;
-  this->a = this->a > 1 ? 1.0f: this->a;
-}
-
 //////////////////////////////////////////////////
 float Color::R() const
 {