Fix misc point cloud ogre helper bounds issues #1644
Conversation
The PointCloudRenderable returns an incorrect bounding radius when the furthest corner of the box is not the minimum or maximum corner. Use Ogre::Math::boundingRadiusFromAABB to return the correct value.
getSquaredViewDepth was not applying scene node transforms to the camera position, resulting in incorrect values of squared depth. Instead, find the distance in world coordinates from the bounding box to the camera.
Instead of calculating the bounding radius of the entire cloud as we go, calculate it once at the end when updating the bounding box by calling Ogre::Math::boundingRadiusFromAABB on the bounding box.
Add the size of the point visual into the bounding box. For small or zero sized points this makes little difference, but for large points (such as visualizing an octomap with large compressed regions), it is important to add the bounds to keep the points visible at the very edges of the cloud.
|
One of these commits (the getSquaredViewDepth) may fix #1186 |
c-andy-martin
force-pushed
the
fix-misc-point-cloud-bounds
branch
from
c0315f3
to
cfb3f81
Compare
There was a problem hiding this comment.
Thanks a lot for this contribution. While I agree that the original implementation is erroneous, I don't yet agree with your solution approach, which seems to overestimate the bounding box radius (see below). I started an alternative PR: #1656.
Also, I wasn't able to reproduce the actual issue. However, given a few issue reports, I agree that it exists. Could you, please, provide a simple example (ideally a cloud with a few points only that are generated programmatically) illustrating the issue?
| aabb.merge(p.position + point_size_offset); | ||
| aabb.merge(p.position - point_size_offset); |
There was a problem hiding this comment.
While I agree that the point size should be considered, I suggest doing so once in the end.
Also, you should adapt the bounding box(es) as soon as the point size changed via setDimensions().
| vDist = cam->getDerivedPosition() - vMid; | ||
|
|
||
| return vDist.squaredLength(); | ||
| return getWorldBoundingBox().squaredDistance(cam->getDerivedPosition()); |
There was a problem hiding this comment.
The previous code attempted to compute the (squared) distance of the cam to the center of the bbox.
However, the new code computes the closest (squared) distance of the cam to the bbox!
| @@ -571,6 +572,7 @@ void PointCloud::addPoints(Point* points, uint32_t num_points) | |||
| op->vertexData->vertexCount = current_vertex_count - op->vertexData->vertexStart; | |||
| rend->setBoundingBox(aabb); | |||
| bounding_box_.merge(aabb); | |||
| bounding_radius_ = Ogre::Math::boundingRadiusFromAABB(bounding_box_); | |||
There was a problem hiding this comment.
I'm afraid boundingRadiusFromAABB() is wrong as it computes the radius of the sphere around the origin that includes the bbox. However, here we need the radius of the local bounding sphere, don't we?
Only later they introduced boundingRadiusFromAABBCentered() to compute the latter.
There were a few circumstances where the point cloud ogre helper calculated incorrect bounds. These circumstances were mostly noticed when using the octomap_rviz_plugins to visualize octomaps where the maps would disappear when viewed from certain angles or zoom levels, or when using a non-unity alpha the different prune depths of the tree would render out-of-depth-order due to the bug in getSquaredViewDepth. Sorry I don't have any visuals right now to post with the PR as these commits were made on our branch of rviz a couple years ago and I'm just now getting a bit of time to send the fixes upstream.