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

Add MultiSphereShape #770

Merged
merged 5 commits into from
Sep 28, 2016
Merged

Add MultiSphereShape #770

merged 5 commits into from
Sep 28, 2016

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Sep 24, 2016

This pull request adds new shape class MultiSphereShape inspired by bullet's btMultiSphereShape. A multi-sphere shape is a convex hull of a collection of spheres.

Collision Detection

Only BulletCollisionDetector support this shape.

Visualization

Both OpenGL and OSG merely draw the sub-spheres instead of the actual convex hull. This should be addressed in the future.

Parsing

This shape is supported by SKEL file format. Here is part of an example skel file that for a MultiSphereShape with two sub-spheres:

<visualization_shape>
    <transformation>0 0 0 0 0 0</transformation>
    <geometry>
        <multi_sphere>
            <sphere>
                <radius>0.05</radius>
                <position>-0.075 0.0 0.0</position>
            </sphere>
            <sphere>
                <radius>0.075</radius>
                <position>0.075 0.0 0.0</position>
            </sphere>
        </multi_sphere>
    </geometry>
    <color>0.6 0.6 0.8</color>
</visualization_shape>

This figure shows all the shapes supported by DART (from left: box, sphere, ellipsoid, cylinder, capsule, cone, multi-sphere (two spheres), mesh):
image


This change is Reviewable

@jslee02 jslee02 added this to the DART 6.1.0 milestone Sep 24, 2016
@psigen
Copy link
Collaborator

psigen commented Sep 25, 2016

Is there a good reason to support this as its own shape type if only bullet supports it? Does anyone want this shape as a first class primitive over just using a convex hull mesh?

I am worried that this creates excess bloat for other dynamics implementations that want to cover all DART shape types, while not being an especially common or efficiency-gaining primitive.

@jslee02
Copy link
Member Author

jslee02 commented Sep 25, 2016

There was a request for multi-sphere shape type for human body modeling. Human bodies could be tightly approximated using multi-sphere shapes (I expect most of each body part can be modeled with two-sphered shapes). Since primitive shapes (generally) perform faster collision calculate than mesh, we could take the advantages in this case.

It's true that this shape is supported by only one collision detector, bullet. However, the situation is that none of the collision detectors satisfies all the features required by DART, anyways. For example, bullet's missing distance query, and fcl's missing primitive shape collisions. We should pick the proper collision detector depending on the demand.

I am worried that this creates excess bloat for other dynamics implementations that want to cover all DAR shape types.

If you refer to the dynamics implementation within DART, then there is nothing to do for the dynamics computation per new shape type additions. Required changes for new shape type are collision detection, parsing, and rendering, but not dynamics. If you mean the consumer project of DART, then the project could simply ignore the not interesting shapes. That said, the project wouldn't want to use the shapes, and wouldn't need to code for the shapes as well.

@mkoval
Copy link
Collaborator

mkoval commented Sep 26, 2016

Approximating a convex hull of spheres as a convex hull of meshes has potentially serious performance implications: it can take a lot of vertices to accurately approximate a sphere. It is also may be much slower to collision check.

There are a lot of types of primitive geometry, so I don't see any way to avoid a proliferation of Shape classes. I don't see any issue with this as long as: (1) they are only used by expert users who are concerned about performance and (2) classes that do not support these shapes print a reasonable error message.

@jslee02 What is the current behavior if you pass an unsupported shape, e.g. into a collision detector?

I expect most of each body part can be modeled with two-sphered shapes.

Isn't that equivalent to a capsule? 😉

@jslee02
Copy link
Member Author

jslee02 commented Sep 26, 2016

For an unsupported shape, bullet and fcl collision detectors print warning message and use predefined shape (a sphere with radius 0.1), while DARTCollisionDetector ignores (don't do anything) the shape (might need to make this behavior be consistent with bullet and fcl collision detectors).

Isn't that equivalent to a capsule?

Yeah, I thought that too at first. I then realized that a two-sphered convex hull has one more level of flexibility than a capsule. It allows having different radii for the head and base (almost) hemispheres.

@psigen
Copy link
Collaborator

psigen commented Sep 26, 2016

Thanks for the explanation! I suppose if this has a specifically useful application (such as humanoid modeling) then it is probably worth doing.

We do still have to drag this into any extensions that we write, including third-party addons like viewers (like aikido_rviz), import/exporters, and any geometry operators that might branch on shape type. So this cost is passed on not just to core developers, but also users that are maintaining dependent libraries.

While it is easy to say that people who don't need these shapes can just throw up their hands and return warnings/errors, the reality is that this makes it harder to build up a community of contributed code and I think that's something we probably want to avoid.

Also note that we can always go down the rabbit-hole specializing all sorts of shapes for efficiency. For example, we could also specialize extruded prisms, torii, etc. @mkoval brings up a good point that regular curved surfaces are a particularly nasty case for meshing and the performance gains here are sizeable, I just want to make sure that the relatively high API cost of adding any new shape is justified by relatively common use cases.

Does this mean that CapsuleShape is no longer necessary? It seems like it is a special subclass of this shape where the two spheres are equal, right?

@jslee02
Copy link
Member Author

jslee02 commented Sep 26, 2016

Does this mean that CapsuleShape is no longer necessary? It seems like it is a special subclass of this shape where the two spheres are equal, right?

It's like the relation of SphereShape and EllipsoidShape. In the geometry view, a capsule and a two-sphered multi-sphere shape with the same radii are identical. However, for a multi-sphere shape, the collision detector should consider the case if the multi-sphere shape has different radii and also even the case it has more spheres. That means the underlying collision checking algorithm should consider the cases as well that makes the algorithm less efficient than an algorithm only for a capsule.

@jslee02 jslee02 merged commit ca0b837 into master Sep 28, 2016
@jslee02 jslee02 deleted the multi_sphere_shape branch September 28, 2016 13:10
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.

3 participants