Fix initial guess for Cal3Bundler calibrate
#775
Conversation
|
Nice work, thanks Varun |
gtsam/geometry/Cal3Bundler.cpp
Outdated
| double px = pi.x(), py = pi.y(), rr = px * px + py * py; | ||
| double g = (1 + k1_ * rr + k2_ * rr * rr); | ||
| Point2 pn = invKPi / g; |
There was a problem hiding this comment.
invKPi is in normalized coordinates but g is in pixel (unnormalized image) coordinates here. Shouldnt px and py be initialized to x and y respectively?
There was a problem hiding this comment.
If you change that, it becomes as though writing the first iteration of the loop outside the loop.
So can the loop can be changed to a do-while? (I am not a fan of do-while or anything, but maybe this is a valid use case? )
There was a problem hiding this comment.
I did think about using a do-while yesterday. Let me update.
There was a problem hiding this comment.
I believe g should be initialized using the pixel coordinates since we capture the radial distortions that way. If g was initialized to x and y (aka the intrinsic coordinates), we wouldn't see any effect of the radial distortion in the inverse process.
There was a problem hiding this comment.
But for rr we need the radius from the center of the image, so we should subtract the principal point right?
I think Pi = K * g * Pn where g is computed using Pn (was able to find a couple quick references: slide 30 here and slide 13 here). This is how it is implemented inside the for loop. While initializing, Pn is unknown, so we can only do as much as computing g using invKPi. Please correct me if I am wrong.
There was a problem hiding this comment.
Great references. I agree that g should be defined using the intrinsic coordinates and not the image coordinates. I like your suggestion to use invKPi since that makes sense to me as per your references. I'll update the PR later tonight.
There was a problem hiding this comment.
@akshay-krishnan could you explain this point a bit more? I didn't quite follow
There was a problem hiding this comment.
John, could you check the references? They explain the pinhole camera model with distortion parameters.
In this model,
Pi = K * Pn_distorted where Pn_distorted = g * Pn_undistorted.
And g is a nonlinear function of Pn_undistorted.
In calibrate() we are trying to find Pn_undistorted by optimization. The initial guess for Pn_undistorted = (invK * Pi) / g (using the above). But since g is a function of Pn_undistorted itself, we need an initial guess for g as well. The best we can do is initialize g using Pn_distorted ( = invK * Pi).
I hope that makes sense.
|
both, please test the hell out of this :-) Also, please don't merge until @akshay-krishnan and you are on the same wavelength. |
|
@akshay-krishnan I added all the tests from your branch |
gtsam/geometry/Cal3Bundler.cpp
Outdated
| // pixels around boundary | ||
| Point2 pn(x, y); | ||
| Point2 pn; | ||
| double px = pi.x(), py = pi.y(); |
There was a problem hiding this comment.
Per our discussion in the other thread, this should be x, y right? I was basically suggesting the following:
double px = (pi.x() - u0_) / fx_, py = (pi.y() - v0_) / fx_;
const Point2 invKPi(px, py);
Point2 pn;
Then the do-while as you have below.
It might be worth adding another unit test for a Cal3Bundler K(5, 0, 0, 2, 2); i.e non-zero u0_, v0_.
There was a problem hiding this comment.
Ah good catch. Sorry I missed that.
|
|
||
| /* ************************************************************************* */ | ||
| TEST(Cal3Bundler, DcalibratePrincipalPoint) { | ||
| Cal3Bundler K(2, 0, 0, 2, 2); |
There was a problem hiding this comment.
There’s also a global static variable by name K, just FYI in case you missed that. I don’t think it should cause any issues though.
There was a problem hiding this comment.
Yeah I know about that. The local scope will take precedence so it's all good.
This PR fixes #770.
I simply incorporated the radial distortion from the iteration loop to the initial guess and the test passed. This makes sense since when we tweak the radial distortion parameters a bit for finite difference, we want to be able to see that effect in the output.