-
Notifications
You must be signed in to change notification settings - Fork 498
Fix Camera Distortion rounding #3114
Fix Camera Distortion rounding #3114
Conversation
Attention: @scpeters @jacobperron @audrow And thanks to @mogumbo for helping explain compositor interpolation and suggesting the half step-size fix |
gazebo/rendering/Distortion.cc
Outdated
unsigned int imageSize = | ||
this->dataPtr->distortionTexWidth * this->dataPtr->distortionTexHeight; | ||
double colStepSize = 1.0 / this->dataPtr->distortionTexWidth; | ||
double rowStepSize = 1.0 / this->dataPtr->distortionTexHeight; | ||
|
||
// Half step-size vector to add to the value being placed in distortion map. | ||
// Necessary for compositor to correctly interpolate pixel values. | ||
const auto halfStepVec = |
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.
"step" is pretty generic. I would call this "halfTexelSize".
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.
Changed it, thanks
this->dataPtr->distortionTexWidth = texSide - 1; | ||
this->dataPtr->distortionTexHeight = texSide - 1; | ||
this->dataPtr->distortionTexWidth = texSide; | ||
this->dataPtr->distortionTexHeight = texSide; |
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.
looks like this was changed from texSide + 1
to texSide - 1
when pincushion model was added in 8ef2d42. I don't exactly remember why. But I do remember why I had originally set it to texSide + 1
. This is mainly because later on I wanted the normalized coordinates to cover from edge to edge, i.e. [0, 0] to [1, 1], in the for loop below. With the new rounding logic and half texel pos, setting it to just texSide
looks like it may be fine.
some thoughts about interpolation: we have this non-ideal interpolation logic that need to be done because the distorted image typically do not have all pixels filled. I think one way around this could be to reverse the mapping logic. Right now we have:
This can probably change to:
|
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.
changes look good to me.
This PR makes a few small but necessary fixes to the distortion code.
The biggest issue is that there is a half-pixel shift applied to the entire image when distortion is active. You can see an example of that here, where k1 = -0.000005 and the rest of the distortion coefficients are 0. k1 is just large enough to have distortion enabled but not large enough to have any visible effects. But the top-side and left-side of the images have a dark-gray bar.
No distortion:

k1 = -0.000005:

k1 = -0.000005 (zoomed in on top-left corner):

k1 = -0.000005 (zoomed in on left side in the middle of the checkerboard pattern):

Note in the last image what appears to be an interpolation between the black & white squares (which does not exist in the non-distorted image). I believe this same interpolation is what is causing the gray bars on the top and left sides of the image, and I believe the cause is that the nested loop that populates the distortion map takes step sizes 0/IMAGE_SIZE, 1/IMAGE_SIZE, 2/IMAGE_SIZE, etc. When the distortion compositor gets this it tries to sample the pixels at the centers, which means the values in the distortion map should actually use step sizes 0.5/IMAGE_SIZE, 1.5/IMAGE_SIZE, 2.5/IMAGE_SIZE, etc.
Here is the resulting distorted image with k1 = -0.000005 with this change:

There is no interpolation smoothing the entire image, and no visible distortion as we expect.
Another fix is rounding the distortion results. For a 2048x2048 image, k1 = -0.00075 should produce just enough distortion to distort the corners inwards diagonally by 1-pixel if the resulting pixel locations post-distortion are rounded to the nearest integer, rather than rounding down (as is the current implementation prior to this fix). Here is a sample confirming this:

(Note that to generate this image I manually removed the distortion map interpolation on lines 344-419. This doesn't affect the way the image gets distorted but it does make the test easier to see.)
The last fix is changing the distortion map size from texSide-1 by texSide-1 to just texSide by texSide. I don't know why there was the -1 before and it resulted in a distortion map that was not large enough to fill all the pixels of the image.