Skip to content

Commit

Permalink
WIP Use constexpr for simple static constants
Browse files Browse the repository at this point in the history 
At the moment, only Color has been refactored with this pattern
while we discuss the implications.

Signed-off-by: Jeremy Nimmer <[email protected]>
  • Loading branch information
@jwnimmer-tri
jwnimmer-tri committed Nov 23, 2021
1 parent 12f5f7f commit f303991
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 70 deletions.
45 changes: 30 additions & 15 deletions include/ignition/math/Color.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,21 @@ namespace ignition
class IGNITION_MATH_VISIBLE Color
{
/// \brief (1, 1, 1)
public: static const Color White;
public: static const Color& White;
/// \brief (0, 0, 0)
public: static const Color Black;
public: static const Color& Black;
/// \brief (1, 0, 0)
public: static const Color Red;
public: static const Color& Red;
/// \brief (0, 1, 0)
public: static const Color Green;
public: static const Color& Green;
/// \brief (0, 0, 1)
public: static const Color Blue;
public: static const Color& Blue;
/// \brief (1, 1, 0)
public: static const Color Yellow;
public: static const Color& Yellow;
/// \brief (1, 0, 1)
public: static const Color Magenta;
public: static const Color& Magenta;
/// \brief (0, 1, 1)
public: static const Color Cyan;
public: static const Color& Cyan;

/// \typedef RGBA
/// \brief A RGBA packed value as an unsigned int
Expand All @@ -70,22 +70,26 @@ namespace ignition
public: typedef unsigned int ABGR;

/// \brief Constructor
public: Color();
public: constexpr Color() = default;

/// \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] _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);
public: constexpr Color(const Color &_clr) = default;

/// \brief Destructor
public: virtual ~Color();
public: ~Color() = default;

/// \brief Reset the color to default values to red=0, green=0,
/// blue=0, alpha=1.
Expand Down Expand Up @@ -123,7 +127,7 @@ namespace ignition
/// \brief Equal operator
/// \param[in] _pt Color to copy
/// \return Reference to this color
public: Color &operator=(const Color &_pt);
public: constexpr Color &operator=(const Color &_pt) = default;

/// \brief Array index operator
/// \param[in] _index Color component index(0=red, 1=green, 2=blue,
Expand Down Expand Up @@ -242,7 +246,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
Expand Down
75 changes: 20 additions & 55 deletions src/Color.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,38 +22,30 @@
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);
namespace {

//////////////////////////////////////////////////
Color::Color()
{
}
// 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);

//////////////////////////////////////////////////
Color::Color(const float _r, const float _g, const float _b, const float _a)
: r(_r), g(_g), b(_b), a(_a)
{
this->Clamp();
}
} // namespace

//////////////////////////////////////////////////
Color::Color(const Color &_pt)
: r(_pt.r), g(_pt.g), b(_pt.b), a(_pt.a)
{
this->Clamp();
}
const Color& Color::White = gWhite;
const Color& Color::Black = gBlack;
const Color& Color::Red = gRed;
const Color& Color::Green = gGreen;
const Color& Color::Blue = gBlue;
const Color& Color::Yellow = gYellow;
const Color& Color::Magenta = gMagenta;
const Color& Color::Cyan = gCyan;

//////////////////////////////////////////////////
Color::~Color()
{
}

//////////////////////////////////////////////////
void Color::Reset()
Expand Down Expand Up @@ -372,17 +364,6 @@ void Color::SetFromABGR(const Color::ABGR _v)
this->r = (val32 & 0xFF) / 255.0f;
}

//////////////////////////////////////////////////
Color &Color::operator=(const Color &_clr)
{
this->r = _clr.r;
this->g = _clr.g;
this->b = _clr.b;
this->a = _clr.a;

return *this;
}

//////////////////////////////////////////////////
Color Color::operator+(const Color &_pt) const
{
Expand Down Expand Up @@ -500,22 +481,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
{
Expand Down

0 comments on commit f303991

Please sign in to comment.