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

use getmetricMin and getMetricMax to get the AABB of the known OcTree… #309

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

simonschmeisser
Copy link

@simonschmeisser simonschmeisser commented Jun 27, 2018

… instead of its max dimension

if only a small part of the Octree is actually used and occupied, this will save lots of NarrowPhase collision checks

Note 1: This has basically been rewritten, will squash or please squash on merge
Note 2: getMetricMin and getMetricMax use lazy evaluation/caching and thus need to be called at least once on a non-const version of the octomap::OcTree


This change is Reviewable

@simonschmeisser
Copy link
Author

@jslee02 @j-rivero any comments? do you have an idea why the unit tests would fail?

@simonschmeisser
Copy link
Author

ping?

@simonschmeisser
Copy link
Author

@Levi-Armstrong you said at ROScon you might know somebody we could ping for this?

@Levi-Armstrong
Copy link
Contributor

@SeanCurtis-TRI Do you have insight into the tests failing? I assume they just need to be updated due to the bounding box changing but have not looked into it.

@simonschmeisser
Copy link
Author

the previous version (first commit) was broken as fcl::OcTree::getRootBV is used not only for broad phase but also to calculate the extend of the child nodes. The entire OcTree was therefore scaled down to the extend of its occupied subspace.

The new approach explicitly adds a new broad phase step that checks for overlap with the occupied part of the octomap. Note that the getMetricMin and getMetricMax use lazy evaluation/caching and thus need to be called at least once on a non-const version of the octomap::OcTree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants