-
Notifications
You must be signed in to change notification settings - Fork 767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pose2::Align #1150
Pose2::Align #1150
Conversation
for (size_t j=0; j < a.cols(); j++) { | ||
ab_pairs.emplace_back(a.col(j), b.col(j)); | ||
} | ||
return Pose2::Align(ab_pairs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would go in the other direction, but okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aTb corresponds to (a,b) pairs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant we compute everything with matrices and use the Pose2Pairs version to call the matrix version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I agree with that! Other PR. Will merge now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with 1 comment.
/** | ||
* @deprecated Use static constructor (with reversed pairs!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason we've reversed the pairs? This seems API breaking since the new method is semantically exactly the same as the old one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're bringing 2D in line with 3D
Pose2::Align
constructors with (a, b) conventionalign
in Pose2.h