ros-perception / vision_msgs

Algorithm-agnostic computer vision message types for ROS.
Apache License 2.0
155 stars 74 forks source link

Clarify size in BoundingBox2D, BoundingBox3D. #27

Closed mistermult closed 3 years ago

mistermult commented 5 years ago

The code says that it is:

# The size of the bounding box, in meters, surrounding the object's center
#   pose.
geometry_msgs/Vector3 size

# The size (in pixels) of the bounding box surrounding the object relative
#   to the pose of its center.
float64 size_x
float64 size_y

Is this size_x/size[0] the distance to the center (halfsize) or the extent in x-direction?

mintar commented 5 years ago

It's the size, i.e., the extent in x-/y-direction. I agree that the comment could be clearer - what does the bounding box center have to do with its size?

mistermult commented 5 years ago

I guess these are oriented bounding boxes and the orientation is saved in the center. That means if you rotate by pi/2 around z, the extent in x/y direction is interchanged :)

SteveMacenski commented 3 years ago

Now(-ish) would be a good junction to clarify this. Would something like The absolute size (in pixels)... fix this issue? I think that makes it clear its the total size, not the half size. I can submit a couple of patches to close this ticket out before the ros-perception/vision_msgs discourse announcement post.

Kukanani commented 3 years ago

absolute size or total size would seem to clear it up for me. We should also probably clarify that relative to the pose of its center includes orientation (i.e. the x and y axes are expressed in the frame defined by the the center field).

SteveMacenski commented 3 years ago

I don't think we need to change the name of the field in the message, I think we just need to add a comment to clarify what the value means.