IsaacLab icon indicating copy to clipboard operation
IsaacLab copied to clipboard

Adds possibility to initialize cameras with their intrinsic matrix

Open pascal-roth opened this issue 1 year ago • 3 comments

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

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

pascal-roth avatar Jul 02 '24 10:07 pascal-roth

@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.

pascal-roth avatar Jul 02 '24 10:07 pascal-roth

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

pascal-roth avatar Jul 03 '24 12:07 pascal-roth

@Mayankm96 and @Dhoeller19 ping again about that issue

pascal-roth avatar Aug 09 '24 15:08 pascal-roth

@Mayankm96 all issues resolved, can you give it a last check?

pascal-roth avatar Aug 16 '24 20:08 pascal-roth

The tiled camera seems to handle the intrinsics wrong. We are running the same function in the background to assign the values to the USDCamera, but the results do not match. I added a test for it in the test_tiled_camera.py. @kellyguo11 can you talk to the tiled rendering about that?

Result of the USD Camera cam_test_usd

Result of the Tiled USD Camera cam_test_tiled

Difference between both cam_test_diff

pascal-roth avatar Aug 28 '24 10:08 pascal-roth

also would be great to get some reviews here, as more PR depends on this one @Mayankm96 @jsmith-bdai and @Dhoeller19

pascal-roth avatar Aug 28 '24 14:08 pascal-roth

Thanks for adding the tiled rendering test! Perhaps it may be due to that tiled rendering and multi-render product rendering are using different renderers at the moment. Let's test again with the new tiled rendering.

Also, looks like test_spawn_sensors.py test is failing

kellyguo11 avatar Aug 28 '24 16:08 kellyguo11

You're correct, the test was failing. Fixed it now.

pascal-roth avatar Aug 28 '24 16:08 pascal-roth