* Umeyama has now similar performance for RowMajor and ColMajor layouts.

* Fixed a bug in umeyama for fixed size matrices.
* Fixed the umeyama unit test for fixed size matrices.
* Added XprHelper::ei_plain_matrix_type_row_major.

3 files changed, 27 insertions, 13 deletions

diff --git a/Eigen/src/Core/util/XprHelper.h b/Eigen/src/Core/util/XprHelper.h
index f1cf93081..7b139b0c1 100644
--- a/Eigen/src/Core/util/XprHelper.h
+++ b/Eigen/src/Core/util/XprHelper.h
@@ -153,6 +153,19 @@ template<typename T> struct ei_plain_matrix_type_column_major
           > type;
 };
 
+/* ei_plain_matrix_type_row_major : same as ei_plain_matrix_type but guaranteed to be row-major
+ */
+template<typename T> struct ei_plain_matrix_type_row_major
+{
+  typedef Matrix<typename ei_traits<T>::Scalar,
+                ei_traits<T>::RowsAtCompileTime,
+                ei_traits<T>::ColsAtCompileTime,
+                AutoAlign | RowMajor,
+                ei_traits<T>::MaxRowsAtCompileTime,
+                ei_traits<T>::MaxColsAtCompileTime
+          > type;
+};
+
 template<typename T> struct ei_must_nest_by_value { enum { ret = false }; };
 template<typename T> struct ei_must_nest_by_value<NestByValue<T> > { enum { ret = true }; };
 
diff --git a/Eigen/src/Geometry/Umeyama.h b/Eigen/src/Geometry/Umeyama.h
index 6eb1f58fd..b55445797 100644
--- a/Eigen/src/Geometry/Umeyama.h
+++ b/Eigen/src/Geometry/Umeyama.h
@@ -46,21 +46,18 @@ namespace
   {    
     enum {
       MinRowsAtCompileTime = EIGEN_ENUM_MIN(MatrixType::RowsAtCompileTime, OtherMatrixType::RowsAtCompileTime),
-      MinMaxRowsAtCompileTime = EIGEN_ENUM_MIN(MatrixType::MaxRowsAtCompileTime, OtherMatrixType::MaxRowsAtCompileTime),
 
       // When possible we want to choose some small fixed size value since the result
       // is likely to fit on the stack.
-      HomogeneousDimension = EIGEN_ENUM_MIN(MinRowsAtCompileTime+1, Dynamic),
-      MaxRowsAtCompileTime = EIGEN_ENUM_MIN(MinMaxRowsAtCompileTime+1, Dynamic),
-      MaxColsAtCompileTime = EIGEN_ENUM_MIN(MatrixType::MaxColsAtCompileTime, OtherMatrixType::MaxColsAtCompileTime)
+      HomogeneousDimension = EIGEN_ENUM_MIN(MinRowsAtCompileTime+1, Dynamic)
     };
 
     typedef Matrix<typename ei_traits<MatrixType>::Scalar,
       HomogeneousDimension,
       HomogeneousDimension,
       AutoAlign | (ei_traits<MatrixType>::Flags & RowMajorBit ? RowMajor : ColMajor),
-      MaxRowsAtCompileTime, 
-      MaxColsAtCompileTime
+      HomogeneousDimension, 
+      HomogeneousDimension
     > type;
   };
 }
@@ -87,9 +84,9 @@ namespace
 * of the input point sets \f$ \mathbf{x} \f$ and \f$ \mathbf{y} \f$ where 
 * \f$d\f$ is corresponding to the dimension (which is typically small).
 * The analysis is involving the SVD having a complexity of \f$O(d^3)\f$
-* though the actual bottleneck usually lies in the computation of the covariance
-* matrix which has an asymptotic lower bound of \f$O(dm)\f$ when the input point 
-* sets have dimension \f$d \times m\f$.
+* though the actual computational effort lies in the covariance
+* matrix computation which has an asymptotic lower bound of \f$O(dm)\f$ when 
+* the input point sets have dimension \f$d \times m\f$.
 *
 * Currently the method is working only for floating point matrices.
 *
@@ -120,7 +117,8 @@ umeyama(const MatrixBase<Derived>& src, const MatrixBase<OtherDerived>& dst, boo
   enum { Dimension = EIGEN_ENUM_MIN(Derived::RowsAtCompileTime, OtherDerived::RowsAtCompileTime) };
 
   typedef Matrix<Scalar, Dimension, 1> VectorType;
-  typedef Matrix<Scalar, Dimension, Dimension> MatrixType;
+  typedef typename ei_plain_matrix_type<Derived>::type MatrixType;
+  typedef typename ei_plain_matrix_type_row_major<Derived>::type RowMajorMatrixType;
 
   const int m = src.rows(); // dimension
   const int n = src.cols(); // number of measurements
@@ -133,8 +131,8 @@ umeyama(const MatrixBase<Derived>& src, const MatrixBase<OtherDerived>& dst, boo
   const VectorType dst_mean = dst.rowwise().sum() * one_over_n;
 
   // demeaning of src and dst points
-  MatrixType src_demean(m,n);
-  MatrixType dst_demean(m,n);
+  RowMajorMatrixType src_demean(m,n);
+  RowMajorMatrixType dst_demean(m,n);
   for (int i=0; i<n; ++i)
   {
     src_demean.col(i) = src.col(i) - src_mean;
diff --git a/test/umeyama.cpp b/test/umeyama.cpp
index 216f4a72e..94198ae44 100644
--- a/test/umeyama.cpp
+++ b/test/umeyama.cpp
@@ -168,7 +168,10 @@ void run_fixed_size_test(int num_elements)
 
   MatrixX dst = (cR_t*src).lazy();
 
-  HomMatrix cR_t_umeyama = umeyama(src.block(0,0,dim,num_elements), dst.block(0,0,dim,num_elements));
+  Block<MatrixX, Dimension, Dynamic> src_block(src,0,0,dim,num_elements);
+  Block<MatrixX, Dimension, Dynamic> dst_block(dst,0,0,dim,num_elements);
+
+  HomMatrix cR_t_umeyama = umeyama(src_block, dst_block);
 
   const Scalar error = ( cR_t_umeyama*src - dst ).cwise().square().sum();