Add VoxelGridShape #1076
Add VoxelGridShape #1076
Conversation
…shape # Conflicts: # .ci/docker/ubuntu-artful # .ci/docker/ubuntu-bionic
There was a problem hiding this comment.
Voxel occupancy maps are super valuable, and I'm totally on board with supporting that type of shape in DART. But I do have a high-level concern about this PR:
This is adding a dependency to the core DART library for a new feature that many users won't care about. In an ideal world, we'd be able to make this an optional component library so that only users who care about voxel grids will get this dependency. However, our current relationship structure between shapes and collision detection wrappers seems to make this impossible.
It would be nice if we could restructure that relationship somehow. For example, have a dart-shapes library that contains the default dependency-free shapes, and then dart-shapes-voxel could be a component that adds the VoxelGridShape class and has an octomap dependency.
We'd have to make drastic changes to the implementations of the collision detection wrappers, so that shapes can inform the collision detector about how to convert its data from the DART representation to the underlying collision detector's representation. That would have the additional benefit of allowing us to support user-defined shape types without the user needing to make any changes to the DART codebase.
I don't think it would be feasible to tackle that kind of redesign in this PR, but we could consider it for 7.0.
If this VoxelGridShape feature is important for some work that you're doing, then I wouldn't want to block it over dependency concerns. I'm okay with merging this, but I'd recommend that we consider flipping around our shape design in the near future.
| const dart::collision::fcl::Transform3& transform1, | ||
| const dart::collision::fcl::Transform3& transform2, | ||
| int evalContactPosition(const fcl::Contact& fclContact, | ||
| const::fcl::BVHModel<fcl::OBBRSS>& mesh1, |
There was a problem hiding this comment.
I'm guessing the lack of a space between const and :: here was a copy/replace accident?
|
@mxgrey I agree with you. As you mentioned, making the voxel shape as an optional component requires many changes, which I also think it would be better to be in DART 7. To reduce the impact in dependencies and API changes, I added OctoMap dependency optional so that Let me create issue(s) for your suggestions. |
Codecov Report
@@ Coverage Diff @@
## release-6.6 #1076 +/- ##
===============================================
- Coverage 56.52% 56.52% -0.01%
===============================================
Files 316 316
Lines 24396 24398 +2
===============================================
+ Hits 13790 13791 +1
- Misses 10606 10607 +1
|
|
The macOS failure is due to that Octomap 1.9.0 returns different results from Octomap (>= 1.8.1). I'll investigate on that but not in this PR. In the meantime, the support of Octomap (>= 1.9.0) will be disabled. |
This PR adds
VoxelGridShapeclass and its collision detection feature using fcl.VoxelGridShapeusesoctomapfor the underlying voxel grid representation.This is a minimal implementation for rapid test for voxel grid collision detection. There are many rooms for improvements, which will be handled subsequent PRs in the future.
Before creating a pull request
clang-formatBefore merging a pull request
Fix test failure on macOS(disabled for Octomap (>= 1.9.0))CHANGELOG.md