From 16c5b9becec444a1bd67ea28508261963b3004bd Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Fri, 17 Dec 2021 11:36:15 -0800 Subject: [PATCH 1/4] Use const references to constexpr storage for static constants Update the required constructors, operators, and related functions to be constexpr. Signed-off-by: Jeremy Nimmer --- Changelog.md | 3 ++ include/ignition/math/Angle.hh | 13 +++-- include/ignition/math/Color.hh | 45 +++++++++++------ include/ignition/math/Matrix3.hh | 48 +++++++++--------- include/ignition/math/Matrix4.hh | 50 +++++++++++-------- include/ignition/math/Pose3.hh | 10 +++- include/ignition/math/Quaternion.hh | 23 ++++++--- include/ignition/math/Vector2.hh | 41 ++++++++++------ include/ignition/math/Vector3.hh | 64 +++++++++++++++--------- include/ignition/math/Vector4.hh | 46 ++++++++++------- src/Angle.cc | 23 +++++---- src/Color.cc | 76 +++++++++-------------------- 12 files changed, 255 insertions(+), 187 deletions(-) diff --git a/Changelog.md b/Changelog.md index 7475b8060..14cbca9af 100644 --- a/Changelog.md +++ b/Changelog.md @@ -42,6 +42,9 @@ 1. Remove virtual from destructors of copyable classes. * [Pull request 293](https://github.com/ignitionrobotics/ign-math/pull/293) +1. Use constexpr for simple static constants. + * [Pull request 283](https://github.com/ignitionrobotics/ign-math/pull/283) + ## Ignition Math 6.x ### Ignition Math 6.x.x diff --git a/include/ignition/math/Angle.hh b/include/ignition/math/Angle.hh index 7ce7d79b3..5395a4dae 100644 --- a/include/ignition/math/Angle.hh +++ b/include/ignition/math/Angle.hh @@ -63,19 +63,19 @@ namespace ignition { /// \brief An angle with a value of zero. /// Equivalent to math::Angle(0). - public: static const Angle Zero; + public: static const Angle &Zero; /// \brief An angle with a value of Pi. /// Equivalent to math::Angle(IGN_PI). - public: static const Angle Pi; + public: static const Angle Π /// \brief An angle with a value of Pi * 0.5. /// Equivalent to math::Angle(IGN_PI * 0.5). - public: static const Angle HalfPi; + public: static const Angle &HalfPi; /// \brief An angle with a value of Pi * 2. /// Equivalent to math::Angle(IGN_PI * 2). - public: static const Angle TwoPi; + public: static const Angle &TwoPi; /// \brief Default constructor that initializes an Angle to zero /// radians/degrees. @@ -91,7 +91,10 @@ namespace ignition // /// \param[in] _radian The radians used to initialize this Angle. // cppcheck-suppress noExplicitConstructor - public: Angle(double _radian); + public: constexpr Angle(double _radian) + : value(_radian) + { + } /// \brief Set the value from an angle in radians. /// \param[in] _radian Radian value. diff --git a/include/ignition/math/Color.hh b/include/ignition/math/Color.hh index ef7f4501c..36b06f566 100644 --- a/include/ignition/math/Color.hh +++ b/include/ignition/math/Color.hh @@ -38,21 +38,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 @@ -71,22 +71,26 @@ namespace ignition public: typedef unsigned int ABGR; /// \brief Constructor - public: Color(); + public: 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: Color(const Color &_clr) = default; /// \brief Destructor - public: ~Color(); + public: ~Color() = default; /// \brief Reset the color to default values to red=0, green=0, /// blue=0, alpha=1. @@ -124,7 +128,7 @@ namespace ignition /// \brief Equal operator /// \param[in] _pt Color to copy /// \return Reference to this color - public: Color &operator=(const Color &_pt); + public: Color &operator=(const Color &_pt) = default; /// \brief Array index operator /// \param[in] _index Color component index(0=red, 1=green, 2=blue, @@ -243,7 +247,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() + { + // These comparisons 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 diff --git a/include/ignition/math/Matrix3.hh b/include/ignition/math/Matrix3.hh index 1c062bb5e..c8806b2a6 100644 --- a/include/ignition/math/Matrix3.hh +++ b/include/ignition/math/Matrix3.hh @@ -95,11 +95,11 @@ namespace ignition { /// \brief A Matrix3 initialized to identity. /// This is equivalent to math::Matrix3(1, 0, 0, 0, 1, 0, 0, 0, 1). - public: static const Matrix3 Identity; + public: static const Matrix3 &Identity; /// \brief A Matrix3 initialized to zero. /// This is equivalent to math::Matrix3(0, 0, 0, 0, 0, 0, 0, 0, 0). - public: static const Matrix3 Zero; + public: static const Matrix3 &Zero; /// \brief Default constructor that initializes the matrix3 to zero. public: Matrix3() @@ -121,19 +121,13 @@ namespace ignition /// \param[in] _v20 Row 2, Col 0 value /// \param[in] _v21 Row 2, Col 1 value /// \param[in] _v22 Row 2, Col 2 value - public: Matrix3(T _v00, T _v01, T _v02, - T _v10, T _v11, T _v12, - T _v20, T _v21, T _v22) + public: constexpr Matrix3(T _v00, T _v01, T _v02, + T _v10, T _v11, T _v12, + T _v20, T _v21, T _v22) + : data{{_v00, _v01, _v02}, + {_v10, _v11, _v12}, + {_v20, _v21, _v22}} { - this->data[0][0] = _v00; - this->data[0][1] = _v01; - this->data[0][2] = _v02; - this->data[1][0] = _v10; - this->data[1][1] = _v11; - this->data[1][2] = _v12; - this->data[2][0] = _v20; - this->data[2][1] = _v21; - this->data[2][2] = _v22; } /// \brief Construct 3x3 rotation Matrix from a quaternion. @@ -645,17 +639,27 @@ namespace ignition private: T data[3][3]; }; + namespace detail { + + template + constexpr Matrix3 gMatrix3Identity( + 1, 0, 0, + 0, 1, 0, + 0, 0, 1); + + template + constexpr Matrix3 gMatrix3Zero( + 0, 0, 0, + 0, 0, 0, + 0, 0, 0); + + } // namespace detail + template - const Matrix3 Matrix3::Identity( - 1, 0, 0, - 0, 1, 0, - 0, 0, 1); + const Matrix3 &Matrix3::Identity = detail::gMatrix3Identity; template - const Matrix3 Matrix3::Zero( - 0, 0, 0, - 0, 0, 0, - 0, 0, 0); + const Matrix3 &Matrix3::Zero = detail::gMatrix3Zero; /// typedef Matrix3 as Matrix3i. typedef Matrix3 Matrix3i; diff --git a/include/ignition/math/Matrix4.hh b/include/ignition/math/Matrix4.hh index 8cba06aa0..f05a4e356 100644 --- a/include/ignition/math/Matrix4.hh +++ b/include/ignition/math/Matrix4.hh @@ -38,10 +38,10 @@ namespace ignition class Matrix4 { /// \brief Identity matrix - public: static const Matrix4 Identity; + public: static const Matrix4 &Identity; /// \brief Zero matrix - public: static const Matrix4 Zero; + public: static const Matrix4 &Zero; /// \brief Constructor public: Matrix4() @@ -70,15 +70,15 @@ namespace ignition /// \param[in] _v31 Row 3, Col 1 value /// \param[in] _v32 Row 3, Col 2 value /// \param[in] _v33 Row 3, Col 3 value - public: Matrix4(T _v00, T _v01, T _v02, T _v03, - T _v10, T _v11, T _v12, T _v13, - T _v20, T _v21, T _v22, T _v23, - T _v30, T _v31, T _v32, T _v33) + public: constexpr Matrix4(T _v00, T _v01, T _v02, T _v03, + T _v10, T _v11, T _v12, T _v13, + T _v20, T _v21, T _v22, T _v23, + T _v30, T _v31, T _v32, T _v33) + : data{{_v00, _v01, _v02, _v03}, + {_v10, _v11, _v12, _v13}, + {_v20, _v21, _v22, _v23}, + {_v30, _v31, _v32, _v33}} { - this->Set(_v00, _v01, _v02, _v03, - _v10, _v11, _v12, _v13, - _v20, _v21, _v22, _v23, - _v30, _v31, _v32, _v33); } /// \brief Construct Matrix4 from a quaternion. @@ -847,19 +847,29 @@ namespace ignition private: T data[4][4]; }; + namespace detail { + + template + constexpr Matrix4 gMatrix4Identity( + 1, 0, 0, 0, + 0, 1, 0, 0, + 0, 0, 1, 0, + 0, 0, 0, 1); + + template + constexpr Matrix4 gMatrix4Zero( + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0); + + } // namespace detail + template - const Matrix4 Matrix4::Identity( - 1, 0, 0, 0, - 0, 1, 0, 0, - 0, 0, 1, 0, - 0, 0, 0, 1); + const Matrix4 &Matrix4::Identity = detail::gMatrix4Identity; template - const Matrix4 Matrix4::Zero( - 0, 0, 0, 0, - 0, 0, 0, 0, - 0, 0, 0, 0, - 0, 0, 0, 0); + const Matrix4 &Matrix4::Zero = detail::gMatrix4Zero; typedef Matrix4 Matrix4i; typedef Matrix4 Matrix4d; diff --git a/include/ignition/math/Pose3.hh b/include/ignition/math/Pose3.hh index 2cce3e4c3..edb8c2742 100644 --- a/include/ignition/math/Pose3.hh +++ b/include/ignition/math/Pose3.hh @@ -74,7 +74,7 @@ namespace ignition { /// \brief A Pose3 initialized to zero. /// This is equivalent to math::Pose3(0, 0, 0, 0, 0, 0). - public: static const Pose3 Zero; + public: static const Pose3 &Zero; /// \brief Default constructor. This initializes the position /// component to zero and the quaternion to identity. @@ -540,7 +540,13 @@ namespace ignition private: Quaternion q; }; - template const Pose3 Pose3::Zero(0, 0, 0, 0, 0, 0); + namespace detail { + + template constexpr Pose3 gPose3Zero{}; + + } // namespace detail + + template const Pose3 &Pose3::Zero = detail::gPose3Zero; /// typedef Pose3 as Pose3d. typedef Pose3 Pose3d; diff --git a/include/ignition/math/Quaternion.hh b/include/ignition/math/Quaternion.hh index 12cc8b960..1d657b45e 100644 --- a/include/ignition/math/Quaternion.hh +++ b/include/ignition/math/Quaternion.hh @@ -81,14 +81,14 @@ namespace ignition { /// \brief A Quaternion initialized to identity. /// This is equivalent to math::Quaternion(1, 0, 0, 0) - public: static const Quaternion Identity; + public: static const Quaternion &Identity; /// \brief A Quaternion initialized to zero. /// This is equivalent to math::Quaternion(0, 0, 0, 0) - public: static const Quaternion Zero; + public: static const Quaternion &Zero; /// \brief Default Constructor - public: Quaternion() + public: constexpr Quaternion() : qw(1), qx(0), qy(0), qz(0) { // quaternion not normalized, because that breaks @@ -102,7 +102,8 @@ namespace ignition /// \param[in] _x X param /// \param[in] _y Y param /// \param[in] _z Z param - public: Quaternion(const T &_w, const T &_x, const T &_y, const T &_z) + public: constexpr Quaternion(const T &_w, const T &_x, const T &_y, + const T &_z) : qw(_w), qx(_x), qy(_y), qz(_z) {} @@ -1253,11 +1254,21 @@ namespace ignition private: T qz; }; + namespace detail { + + template constexpr Quaternion + gQuaternionIdentity(1, 0, 0, 0); + + template constexpr Quaternion + gQuaternionZero(0, 0, 0, 0); + + } // namespace detail + template const Quaternion - Quaternion::Identity(1, 0, 0, 0); + &Quaternion::Identity = detail::gQuaternionIdentity; template const Quaternion - Quaternion::Zero(0, 0, 0, 0); + &Quaternion::Zero = detail::gQuaternionZero; /// typedef Quaternion as Quaterniond typedef Quaternion Quaterniond; diff --git a/include/ignition/math/Vector2.hh b/include/ignition/math/Vector2.hh index b5c729741..9b1df738c 100644 --- a/include/ignition/math/Vector2.hh +++ b/include/ignition/math/Vector2.hh @@ -38,33 +38,31 @@ namespace ignition class Vector2 { /// \brief math::Vector2(0, 0) - public: static const Vector2 Zero; + public: static const Vector2 &Zero; /// \brief math::Vector2(1, 1) - public: static const Vector2 One; + public: static const Vector2 &One; /// \brief math::Vector2(NaN, NaN, NaN) - public: static const Vector2 NaN; + public: static const Vector2 &NaN; /// \brief Default Constructor - public: Vector2() + public: constexpr Vector2() + : data{0, 0} { - this->data[0] = 0; - this->data[1] = 0; } /// \brief Constructor /// \param[in] _x value along x /// \param[in] _y value along y - public: Vector2(const T &_x, const T &_y) + public: constexpr Vector2(const T &_x, const T &_y) + : data{_x, _y} { - this->data[0] = _x; - this->data[1] = _y; } /// \brief Copy constructor /// \param[in] _v the value - public: constexpr Vector2(const Vector2 &_v) = default; + public: Vector2(const Vector2 &_v) = default; /// \brief Destructor public: ~Vector2() = default; @@ -575,16 +573,29 @@ namespace ignition private: T data[2]; }; + namespace detail { + + template + constexpr Vector2 gVector2Zero(0, 0); + + template + constexpr Vector2 gVector2One(1, 1); + + template + constexpr Vector2 gVector2NaN( + std::numeric_limits::quiet_NaN(), + std::numeric_limits::quiet_NaN()); + + } // namespace detail + template - const Vector2 Vector2::Zero(0, 0); + const Vector2 &Vector2::Zero = detail::gVector2Zero; template - const Vector2 Vector2::One(1, 1); + const Vector2 &Vector2::One = detail::gVector2One; template - const Vector2 Vector2::NaN( - std::numeric_limits::quiet_NaN(), - std::numeric_limits::quiet_NaN()); + const Vector2 &Vector2::NaN = detail::gVector2NaN; typedef Vector2 Vector2i; typedef Vector2 Vector2d; diff --git a/include/ignition/math/Vector3.hh b/include/ignition/math/Vector3.hh index eee5fa82b..275be800c 100644 --- a/include/ignition/math/Vector3.hh +++ b/include/ignition/math/Vector3.hh @@ -41,40 +41,36 @@ namespace ignition class Vector3 { /// \brief math::Vector3(0, 0, 0) - public: static const Vector3 Zero; + public: static const Vector3 &Zero; /// \brief math::Vector3(1, 1, 1) - public: static const Vector3 One; + public: static const Vector3 &One; /// \brief math::Vector3(1, 0, 0) - public: static const Vector3 UnitX; + public: static const Vector3 &UnitX; /// \brief math::Vector3(0, 1, 0) - public: static const Vector3 UnitY; + public: static const Vector3 &UnitY; /// \brief math::Vector3(0, 0, 1) - public: static const Vector3 UnitZ; + public: static const Vector3 &UnitZ; /// \brief math::Vector3(NaN, NaN, NaN) - public: static const Vector3 NaN; + public: static const Vector3 &NaN; /// \brief Constructor - public: Vector3() + public: constexpr Vector3() + : data{0, 0, 0} { - this->data[0] = 0; - this->data[1] = 0; - this->data[2] = 0; } /// \brief Constructor /// \param[in] _x value along x /// \param[in] _y value along y /// \param[in] _z value along z - public: Vector3(const T &_x, const T &_y, const T &_z) + public: constexpr Vector3(const T &_x, const T &_y, const T &_z) + : data{_x, _y, _z} { - this->data[0] = _x; - this->data[1] = _y; - this->data[2] = _z; } /// \brief Copy constructor @@ -768,15 +764,37 @@ namespace ignition private: T data[3]; }; - template const Vector3 Vector3::Zero(0, 0, 0); - template const Vector3 Vector3::One(1, 1, 1); - template const Vector3 Vector3::UnitX(1, 0, 0); - template const Vector3 Vector3::UnitY(0, 1, 0); - template const Vector3 Vector3::UnitZ(0, 0, 1); - template const Vector3 Vector3::NaN( - std::numeric_limits::quiet_NaN(), - std::numeric_limits::quiet_NaN(), - std::numeric_limits::quiet_NaN()); + namespace detail { + + template + constexpr Vector3 gVector3Zero(0, 0, 0); + template + constexpr Vector3 gVector3One(1, 1, 1); + template + constexpr Vector3 gVector3UnitX(1, 0, 0); + template + constexpr Vector3 gVector3UnitY(0, 1, 0); + template + constexpr Vector3 gVector3UnitZ(0, 0, 1); + template + constexpr Vector3 gVector3NaN( + std::numeric_limits::quiet_NaN(), + std::numeric_limits::quiet_NaN(), + std::numeric_limits::quiet_NaN()); + } // namespace detail + + template + const Vector3 &Vector3::Zero = detail::gVector3Zero; + template + const Vector3 &Vector3::One = detail::gVector3One; + template + const Vector3 &Vector3::UnitX = detail::gVector3UnitX; + template + const Vector3 &Vector3::UnitY = detail::gVector3UnitY; + template + const Vector3 &Vector3::UnitZ = detail::gVector3UnitZ; + template + const Vector3 &Vector3::NaN = detail::gVector3NaN; typedef Vector3 Vector3i; typedef Vector3 Vector3d; diff --git a/include/ignition/math/Vector4.hh b/include/ignition/math/Vector4.hh index 7244ddbb4..24e8ff1db 100644 --- a/include/ignition/math/Vector4.hh +++ b/include/ignition/math/Vector4.hh @@ -37,18 +37,18 @@ namespace ignition class Vector4 { /// \brief math::Vector4(0, 0, 0, 0) - public: static const Vector4 Zero; + public: static const Vector4 &Zero; /// \brief math::Vector4(1, 1, 1, 1) - public: static const Vector4 One; + public: static const Vector4 &One; /// \brief math::Vector4(NaN, NaN, NaN, NaN) - public: static const Vector4 NaN; + public: static const Vector4 &NaN; /// \brief Constructor - public: Vector4() + public: constexpr Vector4() + : data{0, 0, 0, 0} { - this->data[0] = this->data[1] = this->data[2] = this->data[3] = 0; } /// \brief Constructor with component values @@ -56,12 +56,10 @@ namespace ignition /// \param[in] _y value along y axis /// \param[in] _z value along z axis /// \param[in] _w value along w axis - public: Vector4(const T &_x, const T &_y, const T &_z, const T &_w) + public: constexpr Vector4(const T &_x, const T &_y, const T &_z, + const T &_w) + : data{_x, _y, _z, _w} { - this->data[0] = _x; - this->data[1] = _y; - this->data[2] = _z; - this->data[3] = _w; } /// \brief Copy constructor @@ -725,17 +723,31 @@ namespace ignition private: T data[4]; }; + namespace detail { + + template + constexpr Vector4 gVector4Zero(0, 0, 0, 0); + + template + constexpr Vector4 gVector4One(1, 1, 1, 1); + + template + constexpr Vector4 gVector4NaN( + std::numeric_limits::quiet_NaN(), + std::numeric_limits::quiet_NaN(), + std::numeric_limits::quiet_NaN(), + std::numeric_limits::quiet_NaN()); + + } // namespace detail + template - const Vector4 Vector4::Zero(0, 0, 0, 0); + const Vector4 &Vector4::Zero = detail::gVector4Zero; template - const Vector4 Vector4::One(1, 1, 1, 1); + const Vector4 &Vector4::One = detail::gVector4One; - template const Vector4 Vector4::NaN( - std::numeric_limits::quiet_NaN(), - std::numeric_limits::quiet_NaN(), - std::numeric_limits::quiet_NaN(), - std::numeric_limits::quiet_NaN()); + template + const Vector4 &Vector4::NaN = detail::gVector4NaN; typedef Vector4 Vector4i; typedef Vector4 Vector4d; diff --git a/src/Angle.cc b/src/Angle.cc index 6a7792e60..2a96cf3d8 100644 --- a/src/Angle.cc +++ b/src/Angle.cc @@ -19,16 +19,21 @@ using namespace ignition::math; -const Angle Angle::Zero = Angle(0); -const Angle Angle::Pi = Angle(IGN_PI); -const Angle Angle::HalfPi = Angle(IGN_PI_2); -const Angle Angle::TwoPi = Angle(IGN_PI * 2.0); +namespace { -////////////////////////////////////////////////// -Angle::Angle(const double _radian) -{ - this->value = _radian; -} +// Use constexpr storage for the Color constants, to avoid the C++ static +// initialization order fiasco. +constexpr Angle gZero(0); +constexpr Angle gPi(IGN_PI); +constexpr Angle gHalfPi(IGN_PI_2); +constexpr Angle gTwoPi(IGN_PI * 2.0); + +} // namespace + +const Angle &Angle::Zero = gZero; +const Angle &Angle::Pi = gPi; +const Angle &Angle::HalfPi = gHalfPi; +const Angle &Angle::TwoPi = gTwoPi; ////////////////////////////////////////////////// void Angle::Radian(double _radian) diff --git a/src/Color.cc b/src/Color.cc index e9ffec67f..1b80cb886 100644 --- a/src/Color.cc +++ b/src/Color.cc @@ -22,32 +22,29 @@ 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) = default; - -////////////////////////////////////////////////// -Color::~Color() = default; +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); + +} // namespace + +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; ////////////////////////////////////////////////// void Color::Reset() @@ -366,17 +363,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 { @@ -494,22 +480,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 { From 279d973be4c5798715bec8a869385218d4895a54 Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Mon, 10 Jan 2022 17:55:33 -0800 Subject: [PATCH 2/4] Fix ruby tests Signed-off-by: Steve Peters --- src/ruby/Vector2_TEST.rb | 4 ++-- src/ruby/Vector3_TEST.rb | 4 ++-- src/ruby/Vector4_TEST.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ruby/Vector2_TEST.rb b/src/ruby/Vector2_TEST.rb index cdc350e2c..48f74dd62 100644 --- a/src/ruby/Vector2_TEST.rb +++ b/src/ruby/Vector2_TEST.rb @@ -259,7 +259,7 @@ def test_length end def test_nan - nanVec = Ignition::Math::Vector2d.NaN + nanVec = Ignition::Math::Vector2d.new(Ignition::Math::Vector2d.NaN) assert(!nanVec.IsFinite(), "NaN vector shouldn't be finite") assert(nanVec.X().nan?, "X should be NaN") @@ -269,7 +269,7 @@ def test_nan assert(Ignition::Math::Vector2d.Zero == nanVec, "Corrected vector should equal zero") - nanVecF = Ignition::Math::Vector2f.NaN + nanVecF = Ignition::Math::Vector2f.new(Ignition::Math::Vector2f.NaN) assert(!nanVecF.IsFinite(), "NaN vector shouldn't be finite") assert(nanVecF.X().nan?, "X should be NaN") diff --git a/src/ruby/Vector3_TEST.rb b/src/ruby/Vector3_TEST.rb index 97782e0e6..8368b1e2e 100755 --- a/src/ruby/Vector3_TEST.rb +++ b/src/ruby/Vector3_TEST.rb @@ -442,7 +442,7 @@ def test_finite end def test_nan - nanVec = Ignition::Math::Vector3d.NaN + nanVec = Ignition::Math::Vector3d.new(Ignition::Math::Vector3d.NaN) assert(!nanVec.IsFinite(), "NaN vector shouldn't be finite") assert(nanVec.X().nan?, "X should be NaN") @@ -453,7 +453,7 @@ def test_nan assert(Ignition::Math::Vector3d.Zero == nanVec, "Corrected vector should equal zero") - nanVecF = Ignition::Math::Vector3f.NaN + nanVecF = Ignition::Math::Vector3f.new(Ignition::Math::Vector3f.NaN) assert(!nanVecF.IsFinite(), "NaN vector shouldn't be finite") assert(nanVecF.X().nan?, "X should be NaN") diff --git a/src/ruby/Vector4_TEST.rb b/src/ruby/Vector4_TEST.rb index c594bcc16..2f0e12c90 100644 --- a/src/ruby/Vector4_TEST.rb +++ b/src/ruby/Vector4_TEST.rb @@ -297,7 +297,7 @@ def test_finite end def test_nan - nanVec = Ignition::Math::Vector4d.NaN + nanVec = Ignition::Math::Vector4d.new(Ignition::Math::Vector4d.NaN) assert(!nanVec.IsFinite(), "NaN vector shouldn't be finite") assert(nanVec.X().nan?, "X should be NaN") @@ -309,7 +309,7 @@ def test_nan assert(Ignition::Math::Vector4d.Zero == nanVec, "Corrected vector should equal zero") - nanVecF = Ignition::Math::Vector4f.NaN + nanVecF = Ignition::Math::Vector4f.new(Ignition::Math::Vector4f.NaN) assert(!nanVecF.IsFinite(), "NaN vector shouldn't be finite") assert(nanVecF.X().nan?, "X should be NaN") From cb86347a831063932f63919986824d1ca5b79f17 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Thu, 13 Jan 2022 21:03:30 -0600 Subject: [PATCH 3/4] Revert 279d973 Signed-off-by: Addisu Z. Taddese --- src/ruby/Vector2_TEST.rb | 4 ++-- src/ruby/Vector3_TEST.rb | 4 ++-- src/ruby/Vector4_TEST.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ruby/Vector2_TEST.rb b/src/ruby/Vector2_TEST.rb index 48f74dd62..cdc350e2c 100644 --- a/src/ruby/Vector2_TEST.rb +++ b/src/ruby/Vector2_TEST.rb @@ -259,7 +259,7 @@ def test_length end def test_nan - nanVec = Ignition::Math::Vector2d.new(Ignition::Math::Vector2d.NaN) + nanVec = Ignition::Math::Vector2d.NaN assert(!nanVec.IsFinite(), "NaN vector shouldn't be finite") assert(nanVec.X().nan?, "X should be NaN") @@ -269,7 +269,7 @@ def test_nan assert(Ignition::Math::Vector2d.Zero == nanVec, "Corrected vector should equal zero") - nanVecF = Ignition::Math::Vector2f.new(Ignition::Math::Vector2f.NaN) + nanVecF = Ignition::Math::Vector2f.NaN assert(!nanVecF.IsFinite(), "NaN vector shouldn't be finite") assert(nanVecF.X().nan?, "X should be NaN") diff --git a/src/ruby/Vector3_TEST.rb b/src/ruby/Vector3_TEST.rb index 8368b1e2e..97782e0e6 100755 --- a/src/ruby/Vector3_TEST.rb +++ b/src/ruby/Vector3_TEST.rb @@ -442,7 +442,7 @@ def test_finite end def test_nan - nanVec = Ignition::Math::Vector3d.new(Ignition::Math::Vector3d.NaN) + nanVec = Ignition::Math::Vector3d.NaN assert(!nanVec.IsFinite(), "NaN vector shouldn't be finite") assert(nanVec.X().nan?, "X should be NaN") @@ -453,7 +453,7 @@ def test_nan assert(Ignition::Math::Vector3d.Zero == nanVec, "Corrected vector should equal zero") - nanVecF = Ignition::Math::Vector3f.new(Ignition::Math::Vector3f.NaN) + nanVecF = Ignition::Math::Vector3f.NaN assert(!nanVecF.IsFinite(), "NaN vector shouldn't be finite") assert(nanVecF.X().nan?, "X should be NaN") diff --git a/src/ruby/Vector4_TEST.rb b/src/ruby/Vector4_TEST.rb index 2f0e12c90..c594bcc16 100644 --- a/src/ruby/Vector4_TEST.rb +++ b/src/ruby/Vector4_TEST.rb @@ -297,7 +297,7 @@ def test_finite end def test_nan - nanVec = Ignition::Math::Vector4d.new(Ignition::Math::Vector4d.NaN) + nanVec = Ignition::Math::Vector4d.NaN assert(!nanVec.IsFinite(), "NaN vector shouldn't be finite") assert(nanVec.X().nan?, "X should be NaN") @@ -309,7 +309,7 @@ def test_nan assert(Ignition::Math::Vector4d.Zero == nanVec, "Corrected vector should equal zero") - nanVecF = Ignition::Math::Vector4f.new(Ignition::Math::Vector4f.NaN) + nanVecF = Ignition::Math::Vector4f.NaN assert(!nanVecF.IsFinite(), "NaN vector shouldn't be finite") assert(nanVecF.X().nan?, "X should be NaN") From e0b037c197800c40465e0bc319b1e757770eb2d5 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Thu, 13 Jan 2022 21:04:55 -0600 Subject: [PATCH 4/4] Return copies of static const members This prevents segfaults from happening when users accidentaly try to mutate references to static const members Signed-off-by: Addisu Z. Taddese --- src/python_pybind11/src/Vector2.hh | 9 +++++--- src/python_pybind11/src/Vector3.hh | 18 ++++++++++------ src/python_pybind11/src/Vector4.hh | 11 +++++----- src/ruby/Vector2.i | 20 ++++++++++++++---- src/ruby/Vector3.i | 34 ++++++++++++++++++++++++------ src/ruby/Vector4.i | 19 ++++++++++++++--- 6 files changed, 84 insertions(+), 27 deletions(-) diff --git a/src/python_pybind11/src/Vector2.hh b/src/python_pybind11/src/Vector2.hh index 8f222b6bb..24d08476f 100644 --- a/src/python_pybind11/src/Vector2.hh +++ b/src/python_pybind11/src/Vector2.hh @@ -120,9 +120,12 @@ void defineMathVector2(py::module &m, const std::string &typestr) .def("y", py::overload_cast<>(&Class::Y), "Get the y value.") .def("x", py::overload_cast(&Class::X), "Set the x value.") .def("y", py::overload_cast(&Class::Y), "Set the y value.") - .def_readonly_static("ZERO", &Class::Zero, "math::Vector2(0, 0)") - .def_readonly_static("ONE", &Class::One, "math::Vector2(1, 1)") - .def_readonly_static("NAN", &Class::NaN, "math::Vector3(NaN, NaN)") + .def_readonly_static("ZERO", &Class::Zero, py::return_value_policy::copy, + "math::Vector2(0, 0)") + .def_readonly_static("ONE", &Class::One, py::return_value_policy::copy, + "math::Vector2(1, 1)") + .def_readonly_static("NAN", &Class::NaN, py::return_value_policy::copy, + "math::Vector2(NaN, NaN)") .def("__copy__", [](const Class &self) { return Class(self); }) diff --git a/src/python_pybind11/src/Vector3.hh b/src/python_pybind11/src/Vector3.hh index 344451015..dc7e5c51a 100644 --- a/src/python_pybind11/src/Vector3.hh +++ b/src/python_pybind11/src/Vector3.hh @@ -141,12 +141,18 @@ void defineMathVector3(py::module &m, const std::string &typestr) .def("x", py::overload_cast(&Class::X), "Set the x value.") .def("y", py::overload_cast(&Class::Y), "Set the y value.") .def("z", py::overload_cast(&Class::Z), "Set the z value.") - .def_readonly_static("ZERO", &Class::Zero, "math::Vector3(0, 0, 0)") - .def_readonly_static("ONE", &Class::One, "math::Vector3(1, 1, 1)") - .def_readonly_static("UNIT_X", &Class::UnitX, "math::Vector3(1, 0, 0)") - .def_readonly_static("UNIT_Y", &Class::UnitY, "math::Vector3(0, 1, 0)") - .def_readonly_static("UNIT_Z", &Class::UnitZ, "math::Vector3(0, 0, 1)") - .def_readonly_static("NAN", &Class::NaN, "math::Vector3(NaN, NaN, NaN)") + .def_readonly_static("ZERO", &Class::Zero, py::return_value_policy::copy, + "math::Vector3(0, 0, 0)") + .def_readonly_static("ONE", &Class::One, py::return_value_policy::copy, + "math::Vector3(1, 1, 1)") + .def_readonly_static("UNIT_X", &Class::UnitX, py::return_value_policy::copy, + "math::Vector3(1, 0, 0)") + .def_readonly_static("UNIT_Y", &Class::UnitY, py::return_value_policy::copy, + "math::Vector3(0, 1, 0)") + .def_readonly_static("UNIT_Z", &Class::UnitZ, py::return_value_policy::copy, + "math::Vector3(0, 0, 1)") + .def_readonly_static("NAN", &Class::NaN, py::return_value_policy::copy, + "math::Vector3(NaN, NaN, NaN)") .def("__copy__", [](const Class &self) { return Class(self); }) diff --git a/src/python_pybind11/src/Vector4.hh b/src/python_pybind11/src/Vector4.hh index f2bc02da3..75bf81bbc 100644 --- a/src/python_pybind11/src/Vector4.hh +++ b/src/python_pybind11/src/Vector4.hh @@ -127,11 +127,12 @@ void defineMathVector4(py::module &m, const std::string &typestr) .def("y", py::overload_cast(&Class::Y), "Set the y value.") .def("z", py::overload_cast(&Class::Z), "Set the z value.") .def("w", py::overload_cast(&Class::W), "Set the w value.") - .def_readonly_static("ZERO", &Class::Zero, "math::Vector4(0, 0, 0, 0)") - .def_readonly_static("ONE", &Class::One, "math::Vector4(1, 1, 1, 1)") - .def_readonly_static("NAN", - &Class::NaN, - "math::Vector4(NaN, NaN, NaN, NaN)") + .def_readonly_static("ZERO", &Class::Zero, py::return_value_policy::copy, + "math::Vector4(0, 0, 0, 0)") + .def_readonly_static("ONE", &Class::One, py::return_value_policy::copy, + "math::Vector4(1, 1, 1, 1)") + .def_readonly_static("NAN", &Class::NaN, py::return_value_policy::copy, + "math::Vector4(NaN, NaN, NaN, NaN)") .def("__copy__", [](const Class &self) { return Class(self); }) diff --git a/src/ruby/Vector2.i b/src/ruby/Vector2.i index cad4128ea..180b1b278 100644 --- a/src/ruby/Vector2.i +++ b/src/ruby/Vector2.i @@ -33,10 +33,22 @@ namespace ignition template class Vector2 { - public: static const Vector2 Zero; - public: static const Vector2 One; - public: static const Vector2 NaN; - + // Use %extend to override getters for static member variables so that + // copies of the variables are returned instead of references to the variables. + public: %extend { + static Vector2 Zero() + { + return ignition::math::Vector2::Zero; + } + static Vector2 One() + { + return ignition::math::Vector2::One; + } + static Vector2 NaN() + { + return ignition::math::Vector2::NaN; + } + } public: Vector2(); public: Vector2(const T &_x, const T &_y); public: Vector2(const Vector2 &_v); diff --git a/src/ruby/Vector3.i b/src/ruby/Vector3.i index 43c2801c1..62264bdfd 100644 --- a/src/ruby/Vector3.i +++ b/src/ruby/Vector3.i @@ -33,12 +33,34 @@ namespace ignition template class Vector3 { - public: static const Vector3 Zero; - public: static const Vector3 One; - public: static const Vector3 UnitX; - public: static const Vector3 UnitY; - public: static const Vector3 UnitZ; - public: static const Vector3 NaN; + // Use %extend to override getters for static member variables so that + // copies of the variables are returned instead of references to the variables. + public: %extend { + static Vector3 Zero() + { + return ignition::math::Vector3::Zero; + } + static Vector3 One() + { + return ignition::math::Vector3::One; + } + static Vector3 UnitX() + { + return ignition::math::Vector3::UnitX; + } + static Vector3 UnitY() + { + return ignition::math::Vector3::UnitY; + } + static Vector3 UnitZ() + { + return ignition::math::Vector3::UnitZ; + } + static Vector3 NaN() + { + return ignition::math::Vector3::NaN; + } + } public: Vector3(); public: Vector3(const T &_x, const T &_y, const T &_z); public: Vector3(const Vector3 &_v); diff --git a/src/ruby/Vector4.i b/src/ruby/Vector4.i index a9235cae3..7fbaf1e45 100644 --- a/src/ruby/Vector4.i +++ b/src/ruby/Vector4.i @@ -33,9 +33,22 @@ namespace ignition template class Vector4 { - public: static const Vector4 Zero; - public: static const Vector4 One; - public: static const Vector4 NaN; + // Use %extend to override getters for static member variables so that + // copies of the variables are returned instead of references to the variables. + public: %extend { + static Vector4 Zero() + { + return ignition::math::Vector4::Zero; + } + static Vector4 One() + { + return ignition::math::Vector4::One; + } + static Vector4 NaN() + { + return ignition::math::Vector4::NaN; + } + } public: Vector4(); public: Vector4(const T &_x, const T &_y, const T &_z, const T &_w); public: Vector4(const Vector4 &_v);