Skip to content
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.

Sign up for GitHub

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

account for rounding in point cloud projection #262

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

account for rounding in point cloud projection #262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97eb5b9
account for rounding in point cloud projection
tom-bu Jul 26, 2021
8187a80
add unit test
tom-bu Jul 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions argoverse/utils/calibration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,8 @@ def determine_valid_cam_coords(uv: np.ndarray, uv_cam: np.ndarray, camera_config
Returns:
Numpy array of shape (N,) with dtype bool
"""
x_valid = np.logical_and(0 <= uv[:, 0], uv[:, 0] < camera_config.img_width)
y_valid = np.logical_and(0 <= uv[:, 1], uv[:, 1] < camera_config.img_height)
x_valid = np.logical_and(0 <= np.round(uv[:, 0]), np.round(uv[:, 0]) < camera_config.img_width)
Copy link
Contributor

@johnwlambert johnwlambert Oct 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tom-bu would it make more sense to consider this as an off-by-one error? If instead we make the change:

x_valid = np.logical_and(0 <= uv[:, 0], uv[:, 0] < camera_config.img_width - 1)
y_valid = np.logical_and(0 <= uv[:, 1], uv[:, 1] < camera_config.img_height - 1)

I think the issue you're describing will only occur if we have a (H,W) = (1920,1200) and suppose we have a (u,v) coordinate such as (1199.7, 1919.7). This uv coordinate would pass the old check, but if we index into the original RGB image a these rounded coordinates, we'll have trouble. I think shifting the valid range by 1 fixes it also, with an even easier check.

Shifting the range like this ignores the case where we have a coordinate like (1199.3, 1919.3) -- which would pass under your fix (e.g. getting the RGB value for a coordinate just barely out of the image) -- but I think that's a very rare case and maybe the code is easier to read with the shift instead.

y_valid = np.logical_and(0 <= np.round(uv[:, 1]), np.round(uv[:, 1]) < camera_config.img_height)
z_valid = uv_cam[2, :] > 0
valid_pts_bool = np.logical_and(np.logical_and(x_valid, y_valid), z_valid)
return valid_pts_bool
Expand Down
36 changes: 36 additions & 0 deletions tests/test_valid_pt_cloud_proj.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-
"""
@author: tom-bu
"""

import numpy as np

from argoverse.utils.calibration import CameraConfig, determine_valid_cam_coords


def test_valid_point_cloud_projections() -> None:
"""Test to ensure determine_valid_cam_coords() returns valid points that are less than the image width and height."""
# create a fake camera config
camera_config = CameraConfig(
extrinsic=np.eye(4), intrinsic=np.eye(3, 4), img_width=1920, img_height=1200, distortion_coeffs=np.zeros(3)
)

# create a test case of projected lidar points in the image space
uv = np.array(
[
[0, 0],
[camera_config.img_width - 0.3, 0],
[0, camera_config.img_height - 0.3],
[camera_config.img_width - 0.3, camera_config.img_height - 0.3],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we could add three more points that should pass regardless

            [camera_config.img_width - 1.3, 0],
            [0, camera_config.img_height - 1.3],
            [camera_config.img_width - 1.3, camera_config.img_height - 1.3],

]
)
uv_cam = np.ones((4, uv.shape[0]))

# use the determine_valid_cam_coords() method
valid_pts_bool = determine_valid_cam_coords(uv, uv_cam, camera_config)

# as done in draw_ground_pts_in_image() in ground_visualization.py
uv = np.round(uv[valid_pts_bool]).astype(np.int32)

assert np.all(uv[:, 0] < camera_config.img_width) and np.all(uv[:, 1] < camera_config.img_height)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should instead check to make sure that only the first uv coordinate is valid -- and the last 3 are invalid. I think that's a bit easier to read.

e.g.

expected_valid_pts_bool = np.array([True, False, False, False])
assert np.allclose(valid_pts_bool, expected_valid_pts_bool)