account for rounding in point cloud projection#262
account for rounding in point cloud projection#262tom-bu wants to merge 2 commits intoargoverse:masterfrom
Conversation
|
Thanks for the PR, @tom-bu. Would you mind adding a small unit test to your PR that fails without your change, and passes with your change? |
|
Hi John, I have added a unit test. Let me know if it's what you're looking for. Thanks. |
| """ | ||
| 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) |
There was a problem hiding this comment.
@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.
| # 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) |
There was a problem hiding this comment.
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)| [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], |
There was a problem hiding this comment.
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],2 participants
The current code still considers a uv point that rounds up to the img width/height as valid. This causes errors when you try to run draw_ground_pts_in_image() in the ground_visualization.py.