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.

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

Adds possibility to initialize cameras with their intrinsic matrix #617

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

pascal-roth
Copy link
Collaborator

@pascal-roth pascal-roth commented Jul 2, 2024

Description

This PR adds the possibility of initializing cameras (both Raycaster Cameras and USD Cameras) with the intrinsic matrix instead of using the aperture parameters. The intrinsic matrix is defined in the pattern config and the pinhole cameras spawn config, respectively.

Moreover, it fixes the bug that the vertical aperture is not adjusted for the USD camera case (it will always stay at the default value, even if the horizontal aperture is set). The default is squared pixels; however, it also allows the setting of other values.

Fixes https://github.com/isaac-orbit/IsaacLab/issues/226

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@pascal-roth pascal-roth self-assigned this Jul 2, 2024
@pascal-roth
Copy link
Collaborator Author

@Mayankm96 I remember there were issues when the horizontal and vertical apertures differed for the USD camera case. Do you know if that has been resolved by now? Just from looking at the resulting image, I couldn't see something wrong, but otherwise, I will check again.

Comment on lines +543 to +545
vert_aperture = sensor_prim.GetVerticalApertureAttr().Get()
horiz_aperture_offset = sensor_prim.GetHorizontalApertureOffsetAttr().Get()
vert_aperture_offset = sensor_prim.GetVerticalApertureOffsetAttr().Get()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these now work properly from rendering PoV? I don't know if this issue is resolved from Omniverse side. Also vertical and horizontal aperture should be same for "square" pixels?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know if they work properly. Will implement a test to see if the results between between raycaster and USD camera are the same if I set them

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

apparently it is not resolved

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

offesets are not resolved

@pascal-roth
Copy link
Collaborator Author

I enabled that we can set the vertical aperture, mainly because if we change the horizontal aperture in the USD camera case, the vertical one would always stay at the default value of 15.

Weirdly, the offsets seem to make a difference for both cameras but at a very different scale. I added a test to the raycaster camera that initializes both cameras with the same intrinsic matrix. If the offsets are enabled the values changes as follows:

offsets equal to 0

cam_warp_output
tensor([[[0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         [0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         [0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         ...,
         [3.2779, 3.2779, 3.2779,  ..., 3.2778, 3.2778, 3.2778],
         [3.2735, 3.2735, 3.2735,  ..., 3.2734, 3.2734, 3.2734],
         [3.2692, 3.2692, 3.2692,  ..., 3.2691, 3.2691, 3.2691]]],
       device='cuda:0')
cam_usd_output
tensor([[[0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         [0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         [0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         ...,
         [3.2757, 3.2757, 3.2757,  ..., 3.2756, 3.2756, 3.2756],
         [3.2713, 3.2713, 3.2713,  ..., 3.2712, 3.2712, 3.2712],
         [3.2670, 3.2670, 3.2670,  ..., 3.2669, 3.2669, 3.2669]]],
       device='cuda:0')

offsets unequal 0

cam_warp_output
tensor([[[0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         [0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         [0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         ...,
         [3.2435, 3.2435, 3.2435,  ..., 3.2434, 3.2434, 3.2434],
         [3.2393, 3.2393, 3.2393,  ..., 3.2392, 3.2392, 3.2392],
         [3.2350, 3.2350, 3.2350,  ..., 3.2349, 3.2349, 3.2349]]],
       device='cuda:0')
cam_usd_output
tensor([[[0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         [0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         [0.0000, 0.0000, 0.0000,  ..., 0.0000, 0.0000, 0.0000],
         ...,
         [3.2748, 3.2748, 3.2748,  ..., 3.2747, 3.2747, 3.2747],
         [3.2704, 3.2704, 3.2704,  ..., 3.2703, 3.2703, 3.2703],
         [3.2661, 3.2661, 3.2661,  ..., 3.2660, 3.2660, 3.2660]]],
       device='cuda:0')

@Dhoeller19 and @Mayankm96, do you know how the offsets are handled internally for the USD camera? IMO we handle them correctly for the raycaster case, so that I added warnings now in the USD camera that these values are basically ignored

@Mayankm96 Mayankm96 added the enhancement New feature or request label Jul 4, 2024
@Mayankm96 Mayankm96 requested a review from Dhoeller19 July 5, 2024 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants