Conversation
Add `estimateSE2(...)`, `estimateSE3(...)`, `estimateSIM2(...)`, `estimateSIM3(...)` for estimating an geometric transformation with rotation and translation (with scaling for SIM) using USAC: as alternative for `estimateAffinePartial2D` and `estimateAffine3D`.
|
@alalek Thank you for your reaction.
|
eaba327 to
30514d4
Compare
savuor
left a comment
There was a problem hiding this comment.
It's fully correct, it works and it should be merged. But:
- Why Partial solver is disabled? I've removed it until it's not in use.
- Weights are supported by the algorithm but never actually used. I think we may extend the interface by supporting weights (maybe in next PR?). We can also extend tests by making some ground truth tests.
- I've optimized the function
solve_weighted_umeyamaa bit: just 1 loop, almost no data copying, plain data types (can be easily vectorized in the future). It's 20% faster on my machine. - I've pushed my changes into your branch, please check
|
@savuor
I tried to cover many functions for SE2/SIM2/SE3/SIM3 by the Partial solver, but I gave up.
I do not understand the design concept of usac module, but I have kept the compatibility with
I appreciate your labors for the optimization. |
| CV_EXPORTS_W cv::Mat estimateSE2(InputArray from, InputArray to, OutputArray inliers = noArray(), | ||
| int method = USAC_DEFAULT, double ransacReprojThreshold = 3, | ||
| size_t maxIters = 2000, double confidence = 0.99, | ||
| size_t refineIters = 10); | ||
|
|
||
| CV_EXPORTS_W cv::Mat estimateSE2(InputArray pts1, InputArray pts2, OutputArray inliers, | ||
| const UsacParams ¶ms); |
There was a problem hiding this comment.
Looks like the function is analog of estimateAffine2D and estimateAffine3D https://docs.opencv.org/4.x/d9/d0c/group__calib3d.html#ga27865b1d26bac9ce91efaee83e94d4dd. They already have overloads with USAC.
There was a problem hiding this comment.
@asmorkalov Thank you for your comment.
My estimateSE2() outputs a rigid transform with 3-DOF (rotation+translation) that is different from the output of estimateAffine2D, which has 6-DOF. Such a solution with some constraints is useful for applications like in-plane registration.
estimateSIM2() outputs a similarity transform with 4-DOF (scaling+rotation+translation), which estimateAffinePartial2D estimates, but there is no implementation with USAC as far as I understood.
I think estimateAffine3D does not have an overload with USAC as I stated:
However, the following functions are not supported and only support legacy methods (RANSAC and LMEDS).
Meanwhile, these functions provided to estimate a constrained geometric transformations consists of rotation, translation, and scaling with appropriate options.
There was a problem hiding this comment.
I propose to extend existing API with USAC then, but not introduce the new one. @vpisarev What is your opinion?
There was a problem hiding this comment.
I propose to extend existing API with USAC then, but not introduce the new one.
I agree.
I was supposed to do that, but I was concerned about backward compatibility.
estimateAffinePartial2D and estimateAffine3D are similar but different in
estimateAffinePartial2Donly supports similarity transform (SIM2) butestimateAffine3Dsupports either rigid or similarity with its argument:scaleestimateAffinePartial2Dsupports RANSAC butestimateAffine3Ddoes NOT.
The main reason for making the new ones is that the function name sounds inappropriate: the estimates are NOT affine transform.
But, I would welcome your suggestions about the interface, of course.
There was a problem hiding this comment.
@asmorkalov
What shall I do for this?
Extending the existing API might lose compatibility, so I think the current form is the second best.
There was a problem hiding this comment.
I propose to extend existing API with USAC then, but not introduce the new one. @vpisarev What is your opinion?
@asmorkalov @vpisarev
1.5 years have past already... Can I have your opinion/decision for the next step?
| */ | ||
| const Mat * points_mat; | ||
| const float * const points; | ||
| float m11, m12, m13, m14, m21, m22, m23, m24, m31, m32, m33, m34; |
There was a problem hiding this comment.
By the way, we have Matx and Vec structs for that
There was a problem hiding this comment.
Yes, I prefer to use them, indeed.
But, I just follow the existing 2D version in favor.
class ReprojectionDistanceAffineImpl : public ReprojectionErrorAffine {
private:
const Mat * points_mat;
const float * const points;
float m11, m12, m13, m21, m22, m23;
Correct the wrong statements in the document.
|
@asmorkalov Are there any actions I can take to get approved? |
For resolve comflicts
|
Hi @asmorkalov, It has been 1.5 years since I submitted a pull request, and it seems to have been forgotten. Could you please review the pull request when you have a moment? Thank you for your time and consideration. |
|
@alalek @asmorkalov Gentle ping – would appreciate your thoughts on whether the current integration is sufficient or if any changes are still needed. Thank you. |
Add
estimateSE2(...),estimateSE3(...),estimateSIM2(...),estimateSIM3(...)for estimating an geometric transformation with rotation and translation (with scaling for SIM) using USAC: as alternative forestimateAffinePartial2DandestimateAffine3D.Problem
Nice feature based on USAC: RANSAC-based universal framework has been introduced for the following functions.
findHomographyfindFundamentalMatfindEssentialMatsolvePnPRansacestimateAffine2DHowever, the following functions are not supported and only support legacy methods (
RANSACandLMEDS).estimateAffinePartial2DestimateAffine3DMeanwhile, these functions provided to estimate a constrained geometric transformations consists of rotation, translation, and scaling with appropriate options.
Proposal
I really needed to estimate for the transformation in the class of Rigid/Similarity transform in 2D and 3D, denoted as$\mathit{SE}(d), \mathit{SIM}(d)$ , where $d=2,3$ .
In order to clarify the class of the transformation of the output, I propose to add the functions of
estimateSE2(...),estimateSE3(...),estimateSIM2(...),estimateSIM3(...)as alternative forestimateAffinePartial2DandestimateAffine3D. These functions takes arguments compatible with the existing functions such asestimateAffine2D.Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.