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

BoundingBox3DType is not generic over frames in which the bounding box is used #142

Closed
m-decoster opened this issue Mar 15, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@m-decoster
Copy link
Contributor

In some cases, we want to define a bounding box in the object's local frame, as this makes it easy to reason about minimal and maximal points that define the BoundingBox3DType.

It is of course possible to do this, but if we then want to use this bounding box in another frame, e.g., to log it in rerun or to add it to a simulation in drake, we need to transform the bounding box points.

We cannot simply transform the two points with transform_points, because minimum and maximum points in one frame may not be the minimum and maximum points in another frame.

One solution is to compute the eight corners of the bounding box, transform these, and recompute the minimum and maximum points. For example:

bounding_box_in_frame_A: BoundingBox3DType = get_bounding_box_in_frame_A()
corners_in_frame_A = compute_corners_of_bounding_box(bounding_box_in_frame_A)
corners_in_frame_B = transform_points(X_B_A, corners_in_frame_A)
bounding_box_in_frame_B = np.min(corners_in_frame_B, axis=0), np.max(corners_in_frame_B, axis=0)

We could create a function that performs these operations. Alternatively, is there some representation of a 3D bounding box that does not have this issue but is still easy to work with?

@m-decoster m-decoster added the enhancement New feature or request label Mar 15, 2024
@m-decoster
Copy link
Contributor Author

I am a proponent of creating a function transform_bounding_box analogous to transform_point_cloud (#141), unless someone thinks another definition for BoundingBox3DType is in order.

@Victorlouisdg
Copy link
Contributor

Thanks for raising this limitation of the BoundingBox3DType. This was indeed something I did not consider/design the datatype for. However, your request of being able to transform bounding boxes is certainly valid and useful for all airo-mono users.

Background and Current Limitations
I used the BoundingBox3DType simply for axis-aligned bounding boxes that I don't transform and keep in their original frame. This worked for my immediate use cases (like in the cloth competition). However, I recognize the need for a more flexible bounding box representation to accommodate transformations.

Possible Solution: Introducing a OrientedBoundingBox3DType
In geometry processing/computer graphics libraries there are often two types of bounding boxes, an axis-aligned one and an oriented bounding box:
image
Here's a potential Python type definition (I haven't thought this through though):

OrientedBoundingBox3DType = Tuple[BoundingBox3DType, HomogeneousMatrixType] 

Considerations and Next Steps

  • BoundingBox3DType: I believe we should keep this datatype as our AABB type, and clarify in its docstring what it is intended for.
  • Open3D: Let's investigate Open3D's OrientedBoundingBox class. It might be wiser to leverage existing, tested functionality rather than reinventing the wheel. However I believe we will likely still want a simple airo-mono data type (analogous to PointCloud), that we can use in our codebase, e.g. for a function that converts it to rerun's Boxes3D type.

@m-decoster
Copy link
Contributor Author

I think we also want to have AABBs and OBBs. Still, both kinds of bounding boxes can be transformed (e.g., AABB: expressed w.r.t. which axes?) in Open3D also: AxisAlignedBoundingBox.

Open3D's OBB class has a constructor that creates OBBs from a center, rotation matrix and extents in X, Y and Z directions. Its AABB class is similar to airo-mono's BoundingBox3DType in that its constructor simply takes a minimum and maximum point.

I see two options if we want to add wrapper functions around transformations for both types.

  1. We could (internally) create an Open3D AABB from a BoundingBox3DType and then use their tested implementation to compute OBBs from this.
  2. We could use a basic implementation that computes the 8 corners from the minimum and maximum corner. This is less battle tested, but does not require conversions into Open3D types.

@tlpss
Copy link
Contributor

tlpss commented Mar 19, 2024

I agree that this is a useful 'tool' to have.
I'm not an expert so I'll leave the details to you :)

However, I think it is important to

  1. make sure there can be no confusion about which type of bbox is being used, so the typing/documentation should reflect this.
  2. leverage existing libraries as much as possible! W/o good reason, I think we should try to use open3D as much as possible. I can agree with some custom types for bounding boxes if they are being used in multiple places across airo-mono, but other than that we should leverage open3D as much as possible imo.

@m-decoster
Copy link
Contributor Author

That is a fair point @tlpss. In airo-mono itself, BoundingBox3dType is currently only used to crop point clouds. I also use it downstream for barista, and I see it's also used in several places in the cloth competition.

I want to raise this as a discussion point, either with @Victorlouisdg directly, or in our upcoming meeting. I'll add it to the agenda in any case.

@m-decoster
Copy link
Contributor Author

leverage existing libraries as much as possible! W/o good reason, I think we should try to use open3D as much as possible. I can agree with some custom types for bounding boxes if they are being used in multiple places across airo-mono, but other than that we should leverage open3D as much as possible imo.

5 months later, I see no other use cases in airo-mono for OBBs, in applications I've been mostly using Open3D directly. My proposal is to close this issue and keep BoundingBox3DType for AABBs only, until and if the need arises to support OBBs in airo-mono.

@m-decoster m-decoster closed this as not planned Won't fix, can't repro, duplicate, stale Aug 13, 2024
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

No branches or pull requests

3 participants