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

Attaching a depth image to a pinhole camera has unintuitive requirements on their entity paths #2479

Closed
abey79 opened this issue Jun 19, 2023 · 5 comments
Labels
😤 annoying Something in the UI / SDK is annoying to use

Comments

@abey79
Copy link
Member

abey79 commented Jun 19, 2023

Describe the annoyance

Consider this code:

import numpy as np
import rerun as rr

# Create a depth image with Pillow
image = 65535 * np.ones((200, 300), dtype=np.uint16)
image[50:150, 50:150] = 20000
image[130:180, 100:280] = 45000

rr.init("depth_image", spawn=True)

# We need a camera to register the depth image to
focal_length = 200
rr.log_pinhole(
    "camera",
    child_from_parent=np.array(
        (
            (focal_length, 0, image.shape[1] / 2),
            (0, focal_length, image.shape[0] / 2),
            (0, 0, 1),
        ),
    ),
    width=image.shape[1],
    height=image.shape[0],
)

# Log the tensor, assigning names to each dimension
rr.log_depth_image("camera/depth", image, meter=10000.0)

It looks like it should work but it doesn't. Fixing it requires adding a common root node in the pinhole and depth image's entity paths:

  • pinhole: "camera" -> "world/camera"
  • depth image: "camera/depth" -> "world/camera/depth"

This requirement is unintuitive, error prone, and probably under-documented.

@abey79 abey79 added the 😤 annoying Something in the UI / SDK is annoying to use label Jun 19, 2023
@roym899
Copy link
Collaborator

roym899 commented Jun 19, 2023

To add to this, I believe the correct way right now to log a posed RGB(-D) image would require a hierarchy like this
world/camera/ (this one gets the rigid transform, i.e., pose of camera)
world/camera/image (this one gets the pinhole transform)
world/camera/image/rgb (this one gets the rgb image)
world/camera/image/depth (this one gets the depth image)
I guess camera can't have both a rigid transform and a pinhole transform, hence world->camera + camera->image.

I think this makes sense, but it feels a bit under-documented as well right now and maybe a bit over-complicated. Maybe it would be possible to allow a rigid transform and a pinhole transform on the same entity?

@emilk
Copy link
Member

emilk commented Jun 27, 2023

To @abey79's point: I agree this is odd, and it should be relatively easy to fix.

@roym899 this should be pretty doable with something like Transform3D::Product([extrinsics, intrinsics]). It would help reduce the entity path depth, but also carries its own form of complexities.

@Wumpf
Copy link
Member

Wumpf commented Jun 29, 2023

#2568 allows to merge world/camera/ and world/camera/image. Allows as in "this has well defined behavior", one may still find it confusing and prefer the explicit hierachy.

@Wumpf
Copy link
Member

Wumpf commented Jun 29, 2023

Checked this on #2568: It works but the space view creation heuristics won't create the expected space view.
One can do so manually though.
image
In other words, as expected #2568 addresses the deep paths but that's orthogonal to the issue described here!

@Wumpf
Copy link
Member

Wumpf commented Jun 12, 2024

Updating the code to latest:

import numpy as np
import rerun as rr

# Create a depth image with Pillow
image = 65535 * np.ones((200, 300), dtype=np.uint16)
image[50:150, 50:150] = 20000
image[130:180, 100:280] = 45000

rr.init("depth_image", spawn=True)

# We need a camera to register the depth image to
focal_length = 200
rr.log(
    "camera",
    rr.Pinhole(
        image_from_camera=np.array(
            (
                (focal_length, 0, image.shape[1] / 2),
                (0, focal_length, image.shape[0] / 2),
                (0, 0, 1),
            ),
        ),
        width=image.shape[1],
        height=image.shape[0],
    ),
)

# Log the tensor, assigning names to each dimension
rr.log("camera/depth", rr.DepthImage(image, meter=10000.0))

This works nowadays just fine!
image

@Wumpf Wumpf closed this as completed Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😤 annoying Something in the UI / SDK is annoying to use
Projects
None yet
Development

No branches or pull requests

4 participants