From d3e45c297cd3e55103aaf4cca0e6f3586d8a7cea Mon Sep 17 00:00:00 2001 From: Prateek Machiraju Date: Fri, 19 Mar 2021 16:38:54 -0400 Subject: [PATCH] [wpimath] Make C++ geometry classes immutable (#3249) --- .../src/main/native/cpp/geometry/Pose2d.cpp | 6 --- .../main/native/cpp/geometry/Rotation2d.cpp | 14 ------ .../native/cpp/geometry/Translation2d.cpp | 22 --------- .../cpp/trajectory/TrajectoryGenerator.cpp | 2 +- .../main/native/include/frc/geometry/Pose2d.h | 12 ----- .../native/include/frc/geometry/Rotation2d.h | 24 ---------- .../include/frc/geometry/Translation2d.h | 46 ------------------- .../native/cpp/geometry/Rotation2dTest.cpp | 2 +- 8 files changed, 2 insertions(+), 126 deletions(-) diff --git a/wpimath/src/main/native/cpp/geometry/Pose2d.cpp b/wpimath/src/main/native/cpp/geometry/Pose2d.cpp index 887b43f4f5..d7e57c039d 100644 --- a/wpimath/src/main/native/cpp/geometry/Pose2d.cpp +++ b/wpimath/src/main/native/cpp/geometry/Pose2d.cpp @@ -20,12 +20,6 @@ Pose2d Pose2d::operator+(const Transform2d& other) const { return TransformBy(other); } -Pose2d& Pose2d::operator+=(const Transform2d& other) { - m_translation += other.Translation().RotateBy(m_rotation); - m_rotation += other.Rotation(); - return *this; -} - Transform2d Pose2d::operator-(const Pose2d& other) const { const auto pose = this->RelativeTo(other); return Transform2d(pose.Translation(), pose.Rotation()); diff --git a/wpimath/src/main/native/cpp/geometry/Rotation2d.cpp b/wpimath/src/main/native/cpp/geometry/Rotation2d.cpp index efcc520e4f..37b86f2dc6 100644 --- a/wpimath/src/main/native/cpp/geometry/Rotation2d.cpp +++ b/wpimath/src/main/native/cpp/geometry/Rotation2d.cpp @@ -38,24 +38,10 @@ Rotation2d Rotation2d::operator+(const Rotation2d& other) const { return RotateBy(other); } -Rotation2d& Rotation2d::operator+=(const Rotation2d& other) { - double cos = Cos() * other.Cos() - Sin() * other.Sin(); - double sin = Cos() * other.Sin() + Sin() * other.Cos(); - m_cos = cos; - m_sin = sin; - m_value = units::radian_t(std::atan2(m_sin, m_cos)); - return *this; -} - Rotation2d Rotation2d::operator-(const Rotation2d& other) const { return *this + -other; } -Rotation2d& Rotation2d::operator-=(const Rotation2d& other) { - *this += -other; - return *this; -} - Rotation2d Rotation2d::operator-() const { return Rotation2d(-m_value); } diff --git a/wpimath/src/main/native/cpp/geometry/Translation2d.cpp b/wpimath/src/main/native/cpp/geometry/Translation2d.cpp index 6323b60746..5e066bb40b 100644 --- a/wpimath/src/main/native/cpp/geometry/Translation2d.cpp +++ b/wpimath/src/main/native/cpp/geometry/Translation2d.cpp @@ -33,21 +33,10 @@ Translation2d Translation2d::operator+(const Translation2d& other) const { return {X() + other.X(), Y() + other.Y()}; } -Translation2d& Translation2d::operator+=(const Translation2d& other) { - m_x += other.m_x; - m_y += other.m_y; - return *this; -} - Translation2d Translation2d::operator-(const Translation2d& other) const { return *this + -other; } -Translation2d& Translation2d::operator-=(const Translation2d& other) { - *this += -other; - return *this; -} - Translation2d Translation2d::operator-() const { return {-m_x, -m_y}; } @@ -56,12 +45,6 @@ Translation2d Translation2d::operator*(double scalar) const { return {scalar * m_x, scalar * m_y}; } -Translation2d& Translation2d::operator*=(double scalar) { - m_x *= scalar; - m_y *= scalar; - return *this; -} - Translation2d Translation2d::operator/(double scalar) const { return *this * (1.0 / scalar); } @@ -75,11 +58,6 @@ bool Translation2d::operator!=(const Translation2d& other) const { return !operator==(other); } -Translation2d& Translation2d::operator/=(double scalar) { - *this *= (1.0 / scalar); - return *this; -} - void frc::to_json(wpi::json& json, const Translation2d& translation) { json = wpi::json{{"x", translation.X().to()}, {"y", translation.Y().to()}}; diff --git a/wpimath/src/main/native/cpp/trajectory/TrajectoryGenerator.cpp b/wpimath/src/main/native/cpp/trajectory/TrajectoryGenerator.cpp index 475c7c4257..73d23dcb9d 100644 --- a/wpimath/src/main/native/cpp/trajectory/TrajectoryGenerator.cpp +++ b/wpimath/src/main/native/cpp/trajectory/TrajectoryGenerator.cpp @@ -115,7 +115,7 @@ Trajectory TrajectoryGenerator::GenerateTrajectory( const Transform2d flip{Translation2d(), Rotation2d(180_deg)}; if (config.IsReversed()) { for (auto& waypoint : newWaypoints) { - waypoint += flip; + waypoint = waypoint + flip; } } diff --git a/wpimath/src/main/native/include/frc/geometry/Pose2d.h b/wpimath/src/main/native/include/frc/geometry/Pose2d.h index 357b29e991..310fd8116e 100644 --- a/wpimath/src/main/native/include/frc/geometry/Pose2d.h +++ b/wpimath/src/main/native/include/frc/geometry/Pose2d.h @@ -57,18 +57,6 @@ class Pose2d { */ Pose2d operator+(const Transform2d& other) const; - /** - * Transforms the current pose by the transformation. - * - * This is similar to the + operator, except that it mutates the current - * object. - * - * @param other The transform to transform the pose by. - * - * @return Reference to the new mutated object. - */ - Pose2d& operator+=(const Transform2d& other); - /** * Returns the Transform2d that maps the one pose to another. * diff --git a/wpimath/src/main/native/include/frc/geometry/Rotation2d.h b/wpimath/src/main/native/include/frc/geometry/Rotation2d.h index 1748e60d9a..9f6d1b02a1 100644 --- a/wpimath/src/main/native/include/frc/geometry/Rotation2d.h +++ b/wpimath/src/main/native/include/frc/geometry/Rotation2d.h @@ -59,18 +59,6 @@ class Rotation2d { */ Rotation2d operator+(const Rotation2d& other) const; - /** - * Adds a rotation to the current rotation. - * - * This is similar to the + operator except that it mutates the current - * object. - * - * @param other The rotation to add. - * - * @return The reference to the new mutated object. - */ - Rotation2d& operator+=(const Rotation2d& other); - /** * Subtracts the new rotation from the current rotation and returns the new * rotation. @@ -84,18 +72,6 @@ class Rotation2d { */ Rotation2d operator-(const Rotation2d& other) const; - /** - * Subtracts the new rotation from the current rotation. - * - * This is similar to the - operator except that it mutates the current - * object. - * - * @param other The rotation to subtract. - * - * @return The reference to the new mutated object. - */ - Rotation2d& operator-=(const Rotation2d& other); - /** * Takes the inverse of the current rotation. This is simply the negative of * the current angular value. diff --git a/wpimath/src/main/native/include/frc/geometry/Translation2d.h b/wpimath/src/main/native/include/frc/geometry/Translation2d.h index 57532d2687..f49cd6385a 100644 --- a/wpimath/src/main/native/include/frc/geometry/Translation2d.h +++ b/wpimath/src/main/native/include/frc/geometry/Translation2d.h @@ -110,18 +110,6 @@ class Translation2d { */ Translation2d operator+(const Translation2d& other) const; - /** - * Adds the new translation to the current translation. - * - * This is similar to the + operator, except that the current object is - * mutated. - * - * @param other The translation to add. - * - * @return The reference to the new mutated object. - */ - Translation2d& operator+=(const Translation2d& other); - /** * Subtracts the other translation from the other translation and returns the * difference. @@ -135,18 +123,6 @@ class Translation2d { */ Translation2d operator-(const Translation2d& other) const; - /** - * Subtracts the new translation from the current translation. - * - * This is similar to the - operator, except that the current object is - * mutated. - * - * @param other The translation to subtract. - * - * @return The reference to the new mutated object. - */ - Translation2d& operator-=(const Translation2d& other); - /** * Returns the inverse of the current translation. This is equivalent to * rotating by 180 degrees, flipping the point over both axes, or simply @@ -167,17 +143,6 @@ class Translation2d { */ Translation2d operator*(double scalar) const; - /** - * Multiplies the current translation by a scalar. - * - * This is similar to the * operator, except that current object is mutated. - * - * @param scalar The scalar to multiply by. - * - * @return The reference to the new mutated object. - */ - Translation2d& operator*=(double scalar); - /** * Divides the translation by a scalar and returns the new translation. * @@ -205,17 +170,6 @@ class Translation2d { */ bool operator!=(const Translation2d& other) const; - /* - * Divides the current translation by a scalar. - * - * This is similar to the / operator, except that current object is mutated. - * - * @param scalar The scalar to divide by. - * - * @return The reference to the new mutated object. - */ - Translation2d& operator/=(double scalar); - private: units::meter_t m_x = 0_m; units::meter_t m_y = 0_m; diff --git a/wpimath/src/test/native/cpp/geometry/Rotation2dTest.cpp b/wpimath/src/test/native/cpp/geometry/Rotation2dTest.cpp index 15d683310f..0597cd3218 100644 --- a/wpimath/src/test/native/cpp/geometry/Rotation2dTest.cpp +++ b/wpimath/src/test/native/cpp/geometry/Rotation2dTest.cpp @@ -39,7 +39,7 @@ TEST(Rotation2dTest, RotateByFromZero) { TEST(Rotation2dTest, RotateByNonZero) { auto rot = Rotation2d(90.0_deg); - rot += Rotation2d(30.0_deg); + rot = rot + Rotation2d(30.0_deg); EXPECT_NEAR(rot.Degrees().to(), 120.0, kEpsilon); }