Geometry: Reset last row when building Affine/Isometry Transform from matrix

libeigen/eigen!3412

Co-authored-by: Amirishetty Sai Vignesh <amirishettysaivignesh1@gmail.com>

CHANGELOG.md

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ea3c0aa..932fe6f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,7 @@
 - `BDCSVD` with QR-only `Options`, such as `DisableQRDecomposition`, dropped runtime unitary requests for inputs below the switch size, so `matrixU()` asserted, or returned an empty `U` and `V` under `NDEBUG` [!3212]
 - `JacobiSVD` with `FullPivHouseholderQRPreconditioner` accepted `ComputeThinV` and returned a non-orthogonal `V` for wide inputs; it now fails to compile, or asserts when requested at runtime [!3212]
 - `HouseholderQR` (and the decompositions and `SplineFitting` built on it) and `JacobiSVD` did not compile with `AutoDiffScalar` [#3192, !3265]
+- Constructing or assigning an `Affine` or `Isometry` `Transform` from a (Dim+1)x(Dim+1) matrix copied its last row verbatim; the last row is now set to `[0 ... 0 1]`, as for Dim x Dim and Dim x (Dim+1) inputs [#2715, !3412]
 
 Breaking changes:
 - The default `epsfcn` for central mode differences was changed to `cbrt(NumTraits<Scalar>::epsilon())` (used to be `sqrt(NumTraits<Scalar>::epsilon())`). The new default should reduce numerical error in most cases.

Eigen/src/Geometry/Transform.h

diff --git a/Eigen/src/Geometry/Transform.h b/Eigen/src/Geometry/Transform.h
index 1896f20..4d1133a 100644
--- a/Eigen/src/Geometry/Transform.h
+++ b/Eigen/src/Geometry/Transform.h
@@ -296,7 +296,8 @@
 
   using take_affine_part = internal::transform_take_affine_part<Transform>;
 
-  /** Constructs and initializes a transformation from a Dim^2 or a (Dim+1)^2 matrix. */
+  /** Constructs and initializes a transformation from a Dim^2 or a (Dim+1)^2 matrix.
+   * If Mode==Affine or Mode==Isometry, then the last row is set to [0 ... 0 1] */
   template <typename OtherDerived>
   EIGEN_DEVICE_FUNC inline explicit Transform(const EigenBase<OtherDerived>& other) {
     EIGEN_STATIC_ASSERT(
@@ -307,7 +308,8 @@
     internal::transform_construct_from_matrix<OtherDerived, Mode, Options, Dim, HDim>::run(this, other.derived());
   }
 
-  /** Set \c *this from a Dim^2 or (Dim+1)^2 matrix. */
+  /** Set \c *this from a Dim^2 or (Dim+1)^2 matrix.
+   * If Mode==Affine or Mode==Isometry, then the last row is set to [0 ... 0 1] */
   template <typename OtherDerived>
   EIGEN_DEVICE_FUNC inline Transform& operator=(const EigenBase<OtherDerived>& other) {
     EIGEN_STATIC_ASSERT(
@@ -1362,6 +1364,9 @@
   static EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void run(
       Transform<typename Other::Scalar, Dim, Mode, Options>* transform, const Other& other) {
     transform->matrix() = other;
+    // Affine and Isometry transforms require the last row to be [0 ... 0 1]; Projective keeps it as given.
+    transform_make_affine<(int(Mode) == Affine || int(Mode) == Isometry) ? Affine : AffineCompact>::run(
+        transform->matrix());
   }
 };
 

test/geo_transformations.cpp

diff --git a/test/geo_transformations.cpp b/test/geo_transformations.cpp
index d762eaa..b00bae6 100644
--- a/test/geo_transformations.cpp
+++ b/test/geo_transformations.cpp
@@ -951,12 +951,34 @@
   VERIFY((s0 - s1).norm() < c * eps);
 }
 
+// Part of #2715: constructing or assigning from a (Dim+1)^2 matrix must respect Mode.
+template <typename Scalar, int Mode, int Options>
+void transform_from_full_matrix() {
+  using Transform3 = Transform<Scalar, 3, Mode, Options>;
+  using Matrix4 = Matrix<Scalar, 4, 4>;
+  Matrix4 m = Matrix4::Random();
+  Transform3 t1(m);
+  Transform3 t2;
+  t2 = m;
+  Matrix4 expected = m;
+  if (Mode != int(Projective)) expected.row(3) << 0, 0, 0, 1;
+  // AffineCompact stores only the top three rows.
+  const Index r = t1.matrix().rows();
+  VERIFY_IS_EQUAL(t1.matrix(), expected.topRows(r));
+  VERIFY_IS_EQUAL(t2.matrix(), expected.topRows(r));
+}
+
 EIGEN_DECLARE_TEST(geo_transformations) {
   for (int i = 0; i < g_repeat; i++) {
     CALL_SUBTEST_1((transformations<double, Affine, AutoAlign>()));
     CALL_SUBTEST_1((non_projective_only<double, Affine, AutoAlign>()));
     CALL_SUBTEST_1((transformations_computed_scaling_continuity<double, Affine, AutoAlign>()));
     CALL_SUBTEST_1(custom_scalar_test<>());
+    CALL_SUBTEST_1((transform_from_full_matrix<double, Affine, AutoAlign>()));
+    CALL_SUBTEST_1((transform_from_full_matrix<double, Isometry, AutoAlign>()));
+    CALL_SUBTEST_1((transform_from_full_matrix<double, AffineCompact, AutoAlign>()));
+    CALL_SUBTEST_1((transform_from_full_matrix<double, Projective, AutoAlign>()));
+    CALL_SUBTEST_1((transform_from_full_matrix<float, Affine, RowMajor>()));
 
     CALL_SUBTEST_2((transformations<float, AffineCompact, AutoAlign>()));
     CALL_SUBTEST_2((non_projective_only<float, AffineCompact, AutoAlign>()));