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

Jump to bottom

bugfix: handle multiple camera types in a single batch #3415

Merged
merged 3 commits into from
Sep 10, 2024
Merged

bugfix: handle multiple camera types in a single batch #3415

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c2ad294
bugfix: handle multiple camera types in a single batch
decrispell Sep 5, 2024
4a00448
Merge branch 'main' into mixedcam_bugfix
decrispell Sep 5, 2024
4da56ac
Merge branch 'main' into mixedcam_bugfix
decrispell Sep 10, 2024
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
18 changes: 9 additions & 9 deletions nerfstudio/cameras/cameras.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,15 +778,15 @@ def _compute_rays_for_vr180(

return vr180_origins, directions_stack

for cam in cam_types:
if CameraType.PERSPECTIVE.value in cam_types:
Copy link
Collaborator

Choose a reason for hiding this comment

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

As an alternative fix, would it make sense to:

  • Remove the for cam in cam_types
  • De-dent all of the contents
  • Change all of the if to elif?

I went through this file history and it seems like that's how these conditions were originally set up:

nerfstudio/nerfstudio/cameras/cameras.py

Lines 646 to 681 in d413ca4

if CameraType.PERSPECTIVE.value in cam_types:
mask = (self.camera_type[true_indices] == CameraType.PERSPECTIVE.value).squeeze(-1) # (num_rays)
mask = torch.stack([mask, mask, mask], dim=0)
directions_stack[..., 0][mask] = torch.masked_select(coord_stack[..., 0], mask).float()
directions_stack[..., 1][mask] = torch.masked_select(coord_stack[..., 1], mask).float()
directions_stack[..., 2][mask] = -1.0
if CameraType.FISHEYE.value in cam_types:
mask = (self.camera_type[true_indices] == CameraType.FISHEYE.value).squeeze(-1) # (num_rays)
mask = torch.stack([mask, mask, mask], dim=0)
theta = torch.sqrt(torch.sum(coord_stack**2, dim=-1))
theta = torch.clip(theta, 0.0, math.pi)
sin_theta = torch.sin(theta)
directions_stack[..., 0][mask] = torch.masked_select(coord_stack[..., 0] * sin_theta / theta, mask).float()
directions_stack[..., 1][mask] = torch.masked_select(coord_stack[..., 1] * sin_theta / theta, mask).float()
directions_stack[..., 2][mask] = -torch.masked_select(torch.cos(theta), mask).float()
if CameraType.EQUIRECTANGULAR.value in cam_types:
mask = (self.camera_type[true_indices] == CameraType.EQUIRECTANGULAR.value).squeeze(-1) # (num_rays)
mask = torch.stack([mask, mask, mask], dim=0)
# For equirect, fx = fy = height = width/2
# Then coord[..., 0] goes from -1 to 1 and coord[..., 1] goes from -1/2 to 1/2
theta = -torch.pi * coord_stack[..., 0] # minus sign for right-handed
phi = torch.pi * (0.5 - coord_stack[..., 1])
# use spherical in local camera coordinates (+y up, x=0 and z<0 is theta=0)
directions_stack[..., 0][mask] = torch.masked_select(-torch.sin(theta) * torch.sin(phi), mask).float()
directions_stack[..., 1][mask] = torch.masked_select(torch.cos(phi), mask).float()
directions_stack[..., 2][mask] = torch.masked_select(-torch.cos(theta) * torch.sin(phi), mask).float()
for value in cam_types:
if value not in [CameraType.PERSPECTIVE.value, CameraType.FISHEYE.value, CameraType.EQUIRECTANGULAR.value]:
raise ValueError(f"Camera type {value} not supported.")

Which makes sense to me. The current fix also seems fine though!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's actually the way I originally wrote the fix, but decided to fix the loop instead since it seemed more in line with the intentions of the original author.

I don't have a strong opinion on which way is better, but I don't have a way to test anything other than fisheye and perspective cases so I figured the smaller change was safer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

makes sense, thanks @decrispell!

for cam_type in cam_types:
if CameraType.PERSPECTIVE.value == cam_type:
mask = (self.camera_type[true_indices] == CameraType.PERSPECTIVE.value).squeeze(-1) # (num_rays)
mask = torch.stack([mask, mask, mask], dim=0)
directions_stack[..., 0][mask] = torch.masked_select(coord_stack[..., 0], mask).float()
directions_stack[..., 1][mask] = torch.masked_select(coord_stack[..., 1], mask).float()
directions_stack[..., 2][mask] = -1.0

elif CameraType.FISHEYE.value in cam_types:
elif CameraType.FISHEYE.value == cam_type:
mask = (self.camera_type[true_indices] == CameraType.FISHEYE.value).squeeze(-1) # (num_rays)
mask = torch.stack([mask, mask, mask], dim=0)

Expand All @@ -803,7 +803,7 @@ def _compute_rays_for_vr180(
).float()
directions_stack[..., 2][mask] = -torch.masked_select(torch.cos(theta), mask).float()

elif CameraType.EQUIRECTANGULAR.value in cam_types:
elif CameraType.EQUIRECTANGULAR.value == cam_type:
mask = (self.camera_type[true_indices] == CameraType.EQUIRECTANGULAR.value).squeeze(-1) # (num_rays)
mask = torch.stack([mask, mask, mask], dim=0)

Expand All @@ -816,22 +816,22 @@ def _compute_rays_for_vr180(
directions_stack[..., 1][mask] = torch.masked_select(torch.cos(phi), mask).float()
directions_stack[..., 2][mask] = torch.masked_select(-torch.cos(theta) * torch.sin(phi), mask).float()

elif CameraType.OMNIDIRECTIONALSTEREO_L.value in cam_types:
elif CameraType.OMNIDIRECTIONALSTEREO_L.value == cam_type:
ods_origins_circle, directions_stack = _compute_rays_for_omnidirectional_stereo("left")
# assign final camera origins
c2w[..., :3, 3] = ods_origins_circle

elif CameraType.OMNIDIRECTIONALSTEREO_R.value in cam_types:
elif CameraType.OMNIDIRECTIONALSTEREO_R.value == cam_type:
ods_origins_circle, directions_stack = _compute_rays_for_omnidirectional_stereo("right")
# assign final camera origins
c2w[..., :3, 3] = ods_origins_circle

elif CameraType.VR180_L.value in cam_types:
elif CameraType.VR180_L.value == cam_type:
vr180_origins, directions_stack = _compute_rays_for_vr180("left")
# assign final camera origins
c2w[..., :3, 3] = vr180_origins

elif CameraType.VR180_R.value in cam_types:
elif CameraType.VR180_R.value == cam_type:
vr180_origins, directions_stack = _compute_rays_for_vr180("right")
# assign final camera origins
c2w[..., :3, 3] = vr180_origins
Expand Down Expand Up @@ -880,7 +880,7 @@ def _compute_rays_for_vr180(
directions_stack[coord_mask] = camera_utils.fisheye624_unproject(masked_coords, camera_params)

else:
raise ValueError(f"Camera type {cam} not supported.")
raise ValueError(f"Camera type {cam_type} not supported.")

assert directions_stack.shape == (3,) + num_rays_shape + (3,)

Expand Down
Loading