[wpimath] Use ct_matrix instead of <Eigen/LU> for determinant in headers

This caught a bug in ct_matrix's 3x3 determinant.
calcmogul · Dec 28, 2024 · 5f2c4a4 · 5f2c4a4
1 parent 46d4015
commit 5f2c4a4
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 6 deletions.
diff --git a/wpimath/src/main/native/include/frc/ct_matrix.h b/wpimath/src/main/native/include/frc/ct_matrix.h
@@ -334,7 +334,7 @@ class ct_matrix {
     requires(Rows == 3 && Cols == 3)
   {
     // |a  b  c|
-    // |d  e  f| = aei + bfg + cgh - ceg - bdi - afh
+    // |d  e  f| = aei + bfg + cdh - ceg - bdi - afh
     // |g  h  i|
     Scalar a = (*this)(0, 0);
     Scalar b = (*this)(0, 1);
@@ -345,7 +345,7 @@ class ct_matrix {
     Scalar g = (*this)(2, 0);
     Scalar h = (*this)(2, 1);
     Scalar i = (*this)(2, 2);
-    return a * e * i + b * f * g + c * g * h - c * e * g - b * d * i -
+    return a * e * i + b * f * g + c * d * h - c * e * g - b * d * i -
            a * f * h;
   }
 

diff --git a/wpimath/src/main/native/include/frc/geometry/Rotation2d.h b/wpimath/src/main/native/include/frc/geometry/Rotation2d.h
@@ -8,7 +8,6 @@
 #include <utility>
 
 #include <Eigen/Core>
-#include <Eigen/LU>
 #include <gcem.hpp>
 #include <wpi/StackTrace.h>
 #include <wpi/SymbolExports.h>
@@ -84,7 +83,11 @@ class WPILIB_DLLEXPORT Rotation2d {
       if ((R * R.transpose() - Matrix2d::Identity()).norm() > 1e-9) {
         throw std::domain_error("Rotation matrix isn't orthogonal");
       }
-      if (gcem::abs(R.determinant() - 1.0) > 1e-9) {
+      // HACK: Uses ct_matrix instead of <Eigen/LU> for determinant because
+      //       including <Eigen/LU> doubles compilation times on MSVC, even if
+      //       this constructor is unused. MSVC's frontend inefficiently parses
+      //       large headers; GCC and Clang are largely unaffected.
+      if (gcem::abs(ct_matrix{R}.determinant() - 1.0) > 1e-9) {
         throw std::domain_error(
             "Rotation matrix is orthogonal but not special orthogonal");
       }

diff --git a/wpimath/src/main/native/include/frc/geometry/Rotation3d.h b/wpimath/src/main/native/include/frc/geometry/Rotation3d.h
@@ -8,7 +8,6 @@
 #include <type_traits>
 
 #include <Eigen/Core>
-#include <Eigen/LU>
 #include <fmt/format.h>
 #include <gcem.hpp>
 #include <wpi/SymbolExports.h>
@@ -114,7 +113,11 @@ class WPILIB_DLLEXPORT Rotation3d {
       if ((R * R.transpose() - Matrix3d::Identity()).norm() > 1e-9) {
         throw std::domain_error("Rotation matrix isn't orthogonal");
       }
-      if (gcem::abs(R.determinant() - 1.0) > 1e-9) {
+      // HACK: Uses ct_matrix instead of <Eigen/LU> for determinant because
+      //       including <Eigen/LU> doubles compilation times on MSVC, even if
+      //       this constructor is unused. MSVC's frontend inefficiently parses
+      //       large headers; GCC and Clang are largely unaffected.
+      if (gcem::abs(ct_matrix{R}.determinant() - 1.0) > 1e-9) {
         throw std::domain_error(
             "Rotation matrix is orthogonal but not special orthogonal");
       }