4

I am currently looking into Eigen::Isometry3f, defined as typedef Transform<float,3,Isometry> Isometry3f;.

Therewith i cannot, for example, assign an Affine3f to that Isometry3f, which is good to keep the isometry intact. (The reason is, that Mode is checked in the assignment operator of Transform.)

I can however - via the Transform::operator(...), which shortcuts to Transform::m_matrix(...) - do

Eigen::Isometry3f iso;
iso.setIdentity();
iso(1, 1) = 2; //works (but should not ?!)

and thus destroy the isometry.

Q1: Shouldn't Transform::operator(...) be disallowed or at least issue a warning? If you really want to mess up you could still use Transform.matrix()(1,1) = 2 ...

Q2: Are there other pitfalls where i could accidentally destroy my isometry?

Q3: If there are other pitfalls: what is the intention of Mode==Isometry? Is it not to ensure closedness/safety?

Chiel
  • 6,006
  • 2
  • 32
  • 57
Thomas
  • 725
  • 4
  • 14

1 Answers1

4

The main purpose of Mode==Isometry is to improve the speed of some operations, like inversion, or extraction of rotation part. It essentially says "I, the user, guaranty to Eigen that the underlying matrix represent an isometry". So it is the responsibility of the user to no shoot itself. You can also break an initial isometry by replacing the linear part with a bad matrix:

iso.linear() = Matrix3f::Random();

Checking for isometry is not cheap at all, so adding checks everywhere would break the initial purpose. Perhaps, adding a bool Transform::checkIsometry() would help tracking issues in user code, but this is out-of the scope of SO.

ggael
  • 28,425
  • 2
  • 65
  • 71
  • It seems to me there are some places where Eigen takes responsibility to preserve Isometries, like forbidding scale, shear and assignment or concatenation with non-isometries. Why not take the next step and forbid matrix access through transform objects of type isometry? Alternative: make it absolutely clear in the docs that this is a hint since the docs of `Transform` only say `_Mode` is "the type of transformation" and do not mention its internal use as a - possibly incorrect - hint. Maybe then `TransformTraits` enum should not be used as both storage mode indicator and hint for inverse etc. – Thomas Sep 13 '17 at 09:29
  • Making operator() and linear() const for Isometry would be painful for the user and this would break existing code. In contrast, calling scale/shear and the likes is a non sense for isometries. But I do agree that the doc could be clearer and warn about such pitfalls. – ggael Sep 13 '17 at 21:32