Undistortion should not occur for already-undistorted images

[liorfritz]

This mostly concerns public datasets, like Mip-NeRF 360, which already comes undistorted.

The current logic undistorts even pinhole cameras. To avoid clashing with other logic in the code, we can just skip the undistortion if all distortion parameters are zero.
Unnecessary undistortion smoothes the images, and may incorrectly create the impression that image comparison metrics (PSNR, SSIM, LPIPS) are better. The PR in #2709 shows improvements in the implementation through multiple modifications, that are complementary to this change.

Here's a comparison of what it does to an image from the training set of 'garden',

On the garden dataset, without the undistortion the metrics are affected as follows (keep in mind that the change applies to the evaluation images as well, hence the metrics are lower),

images (training and evaluation)	PSNR	SSIM	LPIPS
unnecessarily undistorted	28.58	0.885	0.073
original	27.29	0.858	0.084

While the metrics are lower after the change, they are aligned with other repositories (because the evaluation images are unnecessarily undistorted as well). Due to the added details, there are also more gaussians (5.1M gaussians without undistortion, compared to 3.8M with the unnecesary undistortion).

---

This change creates misalignment between how images are downscaled in nerfstudio compared to other repositories (and in Mip-NeRF 360). Hence, the rounding of the downscale factor in cameras.py. Without it, in 'garden', the original resolution of 5187x3361 downscaled to 4, results in 1296x840. While in the supplied images in the data, the resolution is 1297x840. This occurs because 5187/4=1296.75. The change in cameras.py makes it necessary to apply the same logic to all downscaling mechanisms in the code. This shouldn't change much for most users (e.g., HD resolution).

[jb-ye]

Wow, this is an interesting finding.

[jb-ye]

This change wouldn't be compatible with other features (mask, depth_image). I would suggest to check np.any(distortion_params) and if False, return the original K and image without triggering cv2 functions. Something like below should work

if np.any(distortion_params):
    newK, roi = cv2.getOptimalNewCameraMatrix(K, distortion_params, (image.shape[1], image.shape[0]), 0)
    image = cv2.undistort(image, K, distortion_params, None, newK)  # type: ignore
else:
    newK = K
    roi = 0, 0, image.width, image.height

[jb-ye]

BTW, I found the following lines are problematic, roi was calculated for undistorted images, not the original image/mask. Should be deleted maybe.

                if "mask" in data:
                    data["mask"] = data["mask"][y : y + h, x : x + w]
                if "depth_image" in data:
                    data["depth_image"] = data["depth_image"][y : y + h, x : x + w]

[liorfritz]

Great catch. Fixed.
Per the other comment - I removed the lines for the mask, because indeed it occurs twice in the code. I left the depth as is.

[liorfritz]

Per the great comments from @jb-ye, we removed the problematic logic of image downscaling. Now, cameras.py receives the width/height of the image in either case, so that it works even with the odd resolution of Mip-NeRF 360.

In addition, just to be clear, this PR leads to better results. Here are the results on the 'garden' sample, evaluated on the original evaluation set (i.e., without duplicated undistortion), comparing training with the original way to this PR.

Training Code	PSNR	SSIM	LPIPS
Original	27.08	0.833	0.142
This PR	27.29	0.858	0.084

[devernay]

what causes the blurriness exactly?

• newK returned by getOptimalNewCameraMatrix is equal to K
• the undistorted image is the same as the input image.
• but roi is (0,0,width-1,height-1) !

So it seems like the bug is actually in the ROI returned by cv2.getOptimalNewCameraMatrix

import cv2
import numpy as np
from matplotlib import pyplot as plt

# zero distortion
distortion_params = np.array(
    [
        0,#k1
        0,#k2
        0,#p1
        0,#p2
        0,#k3
        0,#k4,
        0,#k5,
        0,#k6
    ]
)

# https://docs.opencv.org/4.x/d9/d0c/group__calib3d.html#ga7a6c4e032c97f03ba747966e6ad862b1
width=1920
height=1080
ffactor=1.6
# intrinsics with a non-centered principal point
K = np.array([[width*ffactor, 0, (width-1)*0.5+0.05],
              [0,width*ffactor, (height-1)*0.5-0.04],
              [0,0,1]])

newK, roi = cv2.getOptimalNewCameraMatrix(K, distortion_params, (width, height), 0)

print(np.linalg.norm(K-newK))

# create a checkerboard image
img = np.fromfunction(lambda i, j,k: 255*((i//64+j//64)%2), (height,width,3), dtype=np.uint8)

plt.imshow(img)
plt.title('checkerboard')
plt.show()

imgu = cv2.undistort(img, K, distortion_params, None, newK)

print(np.linalg.norm(img-imgu))
print(roi)

Outputs:

4.547473508864641e-13
0.0
(0, 0, 1919, 1079)

What is different between this code snippet and what happens in nerfstudio?

[liorfritz]

I am getting a different newK with Mip-NeRF 360:

print(K)
print(newK)
print(roi)


[[961.2247    0.      648.375  ]
 [  0.      963.08905 420.125  ]
 [  0.        0.        1.     ]]
[[960.4835    0.      647.8751 ]
 [  0.      961.9425  419.62485]
 [  0.        0.        1.     ]]
(0, 0, 1296, 839)

[devernay]

OK so maybe there are two problems.
Are the distortion parameters 0? Can you provide a standalone test code like the one above?
I submitted an OpenCV issue for the wrong ROI (opencv/opencv#24831)

[devernay]

Ok, so the bug newK != K is present in OpenCV 4.6 and 4.7, bug is fixed in 4.8 (which is the version I used above).
Here's the OpenCV fix: opencv/opencv#23305

I suggest we:

• skip the undistortion if dist params are 0 (this PR) to avoid issues with past, present (roi bug) and future versions of OpenCV
• think about upgrading OpenCV in nerfstudio (4.6 is 1.5yr old, latest release is 4.9.0)

[kerrj]

Thanks for tracking that down! Bumping opencv to 4.9 sounds good to me, would this also fix #2683 or is that a separate problem?

[jb-ye]

Thanks for tracking that down! Bumping opencv to 4.9 sounds good to me, would this also fix #2683 or is that a separate problem?

I think the ROI bug is not yet fixed even in the latest version, sadly. But as long as the input images are undistorted, the colmap loader should work as expected after this PR.

[f-dy]

would this also fix #2683 or is that a separate problem?

#2683 is not the right fix. This is an upstream opencv issue (opencv/opencv#24831).

[liorfritz]

Right. If the ROI issue with opencv is fixed, then #2683 will break.
I'm thinking as follows,

1. This PR makes the code compatible with older versions of opencv, and the one used in nerfstudio currently (4.6)
2. Update opencv to 4.8
3. Keep a close eye on opencv/opencv#24831 and update opencv when the ROI bug is fixed

[kerrj]

thanks for the catch!