Fix reflection in umeyama. #1820
Conversation
RonaldEnsing
force-pushed
the
fix-reflection-umeyama
branch
from
32b94f8 to
12738dd
Compare
|
Guys, I'm tempted to revert this PR. From the links @RonaldEnsing provided, umeyama was fixed for Eigen >= 3.3 only. Meaning that every user which uses this function which doesn't meet that version number, will have the output from a buggy umeyama implementation. A good example of that is our own unit tests which broke after this merge, because the Eigen version available on Travis is only 3.0.5. I think the way to address this is have a preprocessor conditional which uses the Eigen one if the detected version is 3.3 or above. |
|
Very good point. Since the old PCL implementation has a bug, should we additionally incorporate #1711? |
Definitely |
|
I can craft a PR with reversion of this, #1711, and conditional compilation. |
|
That would be awesome, thank you! |
This reverts commit 12738dd following the discussion in PointCloudLibrary#1820.
This reverts commit 12738dd following the discussion in PointCloudLibrary#1820.
The umeyama function contains a bug that allows a possible reflection in the transformation matrix. This is fixed in Eigen (https://bitbucket.org/eigen/eigen/src/98d31e9b40fab63fcb17b060bea142444239064f/Eigen/src/Geometry/Umeyama.h?fileviewer=file-view-default). This PR replaces the pcl version of umeyama with the Eigen version of umeyama.
Related to issue #1707.
More information:
http://www.pcl-users.org/ICP-allows-reflections-in-resulting-transformation-td4043877.html
and
http://eigen.tuxfamily.org/bz/show_bug.cgi?id=774