Correct way to use Eigen::Transpose<const Matrix3>?

I have a rotation class which can use either Quaternions or Rotation matrices to represent rotations. I have no issues when the transpose function is defined like below:

Matrix3 Rot3::transpose() const {
  // quaternion_ is a class variable of type `Quaternion`.
  return quaternion_.toRotationMatrix().transpose();
}

The moment I switch to the recommended version using Eigen::Transpose, my unit tests fail (and I get NaNs)

Eigen::Transpose<const Matrix3> Rot3::transpose() const {
  // quaternion_ is a class variable of type `Quaternion`.
  return quaternion_.toRotationMatrix().eval().transpose();
}

I need to use the .eval() in that weird way else the compiler complains with an obscure error message. My guess is that my use of the Eigen::Transpose declaration is not aligning with what I am returning. Any help or suggestions as to why this method behaves so strangely, and any recommendations for the correct way to do this?

Solution

Eigen::Transpose is simply a wrapper class around an existing matrix. It keeps a reference to the matrix and relays the call to access the elements while reversing the indices. The goal of this class is to be able to use the transposed matrix without having to actually copy the matrix itself.

In other words this is a very simplified version of how the Transpose class is defined :

struct Transpose<const Matrix3> {

   const Matrix3& m_matrix;

   const float& coeffRef(int row, int col) const { 
       return m_matrix.coeffRef(col,row);
   }
}

You can see the actual source here.

The critical part is that the Transpose class stores a reference to the given matrix.

Now what happens in your second version of the transpose function ?

you create a temporary Matrix3 with quaternion.toRotationMatrix().eval()
you then create a Transpose wrapper around this temporary matrix , storing a reference to this matrix.

When you return from the function the Transpose that you return has a reference to an object that has gone out of scope. This is called a dangling reference, which leads to undefined behavior (the reason you see NaNs is most likely that the memory where your temporary matrix lived has been overwritten by other data). The code is equivalent to this:

Eigen::Transpose<const Matrix3> Rot3::transpose() const {
  // Equivalent to your second version
  Matrix3 temp = quaternion_.toRotationMatrix().eval();
  Transpose<const Matrix3> result = temp.transpose(); // Holds ref to temp -> Leads to UB
  return result; 
}

Note that this doesn't happen in the first version because your return type is Matrix3. In this case the Transpose object you create is converted to a new Matrix3 object to be returned, which does a copy of the coefficients. Here is an equivalent version of your first function.

Matrix3 Rot3::transpose() const {
  // Equivalent to your first version
  Matrix3 temp = quaternion_.toRotationMatrix().eval();
  Matrix3 result = temp.transpose(); // Does a full copy of the transpose of `temp`
  return result; 
}

If you still want to keep using Eigen::Transpose as your return type (maybe you really want to avoid the copy operation), you need to store the rotation matrix in a permanent location (e.g. as a cached Matrix3 member variable in your class) to avoid that dangling reference problem.

Another option is to pass a pre-existing matrix to be filled e.g.

void Rot3::setToTranspose() (Matrix3& result)
{
     result = quaternion_.toRotationMatrix().transpose();
}