Add GEOM_CUSTOM node type for user-defined shapes#822
Conversation
1eaf84a to
1d7a213
Compare
|
Awesome, thank you @rjoomen ! |
|
Hey @rjoomen thanks a lot for this PR. There will probably be a bit of efficiency loss for custom shapes (because of the virtualization, which can become expensive when calling the support function for example) but I think this feature is very cool for experimentation. I asked @jorisv to have a look into it. Otherwise this looks good to me. |
1d7a213 to
d1ed6d5
Compare
|
@lmontaut, @jorisv Friendly ping. We (as in: the Tesseract Robotics project repo, docs) would really like to add Coal as a collision checker. Compared to the currently used Bullet and FCL it has increased performance, and it is actively maintained, whereas Bullet and FCL are not anymore. (I do admit this GEOM_CUSTOM solution seems somewhat clunky, but it follows from the current design of the Coal dispatch internals. I've also iterated with Coal over a design brief for a different solution that would do away with the enormous CollisionFunctionMatrix, ContactPatchFunctionMatrix and DistanceFunctionMatrix altogether, but this might be too invasive a change.) |
|
|
||
| /// @brief Delegate to the underlying shape via shape_traits lookup. | ||
| bool needNesterovNormalizeHeuristic() const override { | ||
| return details::getNormalizeSupportDirection(&shape_); |
There was a problem hiding this comment.
I'm not sure to understand this. details::getNormalizeSupportDirection calls needNesterovNormalizeHeuristic in the GEOM_CUSTOM case. Isn't this an infinitely self calling situation?
| /// @param[out] support the computed support point. | ||
| /// @param[in,out] hint warm-start hint (used mainly for convex shapes). | ||
| /// @param[in,out] data temporary data for support computation. | ||
| virtual void computeShapeSupport(const Vec3s& dir, Vec3s& support, int& hint, |
There was a problem hiding this comment.
I am really not a fan of having the computeShapeSupport method in ShapeBase. And the fact that the default behavior is throwing, regardless of the shape.
All the shapes in coal have an implementation of the support function. This function lets the reader think that this is not the case. It should at least call getShapeSupport for shapes which are not GEOM_CUSTOM.
|
Hi @rjoomen, sorry for the delay. The team has been super focused on the release of Pinocchio 4 so @jorisv and I haven't had the time to properly look into this PR. First thing, we need to make sure the CI is green for this to merge, as this library is used in various environments. Second, apart from the comments I left, I think this is a good fix for shape customization in coal for now. We can see later how to improve on that. That being said, I would really appreciate if we could wait for @jorisv input on this. There are many ways we could go about shape customization in coal and I want to make sure we are taking a maintainable road. |
0130fdb to
b5a9afc
Compare
|
Update: CI is fixed, including the upstream ROS issues. |
3570595 to
be8dc8f
Compare
468f937 to
bfbe555
Compare
Add virtual methods to ShapeBase that allow custom shapes to participate in GJK/EPA collision and distance computations without modifying the Coal library source code: - ShapeBase::computeShapeSupport() - virtual support function - ShapeBase::needNesterovNormalizeHeuristic() - virtual Nesterov flag - getShapeSupport/getShapeSupportSet overloads for ShapeBase* using virtual dispatch - All switch-case dispatch functions (getSupport, getSupportSet, makeGetSupportFunction0/1, getNormalizeSupportDirection) now fall through to virtual dispatch for unrecognized shape types instead of throwing https://claude.ai/code/session_01KWVrFzxsXcQKYLx1ttWfDd
- Add GEOM_CUSTOM to NODE_TYPE enum as the default type for user-defined shapes that subclass ShapeBase outside the Coal library - Override getNodeType() in ShapeBase to return GEOM_CUSTOM (vs BV_UNKNOWN) - Fix get_node_type_name() array: correct GEOM_CONVEX16/32 names that were wrong due to the split of the old single GEOM_CONVEX entry, and add GEOM_CUSTOM - Register ShapeShapeDistance<ShapeBase, X> entries in the distance matrix for all GJK-compatible built-in shape types, enabling distance() API to work with custom shapes without modification - Register ShapeShapeCollide<ShapeBase, X> entries in the collision matrix similarly for collide() API support - Register ShapeShapeContactPatch<ShapeBase, X> entries in the contact patch matrix for contact patch support - Update ContactPatchSolver::makeSupportSetFunction to handle GEOM_CUSTOM via virtual dispatch instead of throwing - Add explicit GEOM_CUSTOM cases to GJK support function dispatch switches in support_functions.cpp and minkowski_difference.cpp - Remove redundant swept_sphere_radius assignments in makeGetSupportFunction default cases (already set at top of function) - Fix dir.normalized() -> dir in getShapeSupport(ShapeBase*,...) since direction is already unit-length per Coal convention - Expose GEOM_CUSTOM in Python bindings (Boost.Python and nanobind) - Add test/custom_shape.cpp demonstrating CastHullShape-style CCD Co-authored-by: rjoomen <1153434+rjoomen@users.noreply.github.com>
The DefaultGJK variant passes dir=ray (the closest simplex point, NOT unit-length) to the support function. Built-in shapes are invariant to direction magnitude, but custom shapes implementing computeShapeSupport assume unit-length direction (e.g. support = radius * dir). Normalize dir before calling computeShapeSupport so all GJK variants work correctly with custom shapes. Also restore dir_normalized in the WithSweptSphere branch (regressed in f77e55f). Update doc comment to make the unit-length guarantee explicit. Co-authored-by: rjoomen <1153434+rjoomen@users.noreply.github.com>
…V types Register GEOM_CUSTOM entries in collision and distance function matrices for all 8 BVH mesh types, both heightfield types, and octree (both directions). Add computeBV<AABB, ShapeBase> specialization using support queries for exact tight AABBs, and getBoundVertices(ShapeBase) overload so the generic computeBV template handles OBB/RSS/OBBRSS/kIOS/KDOP. Fix redundant dir.normalized() call in getShapeSupportLog swept sphere path — reuse the already-computed dir_normalized variable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cover the three untested GEOM_CUSTOM code paths: contact patch func matrix dispatch, ContactPatchSolver::makeSupportSetFunction, and heightfield distance (documents upstream not-implemented limitation). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tom shapes Remove incorrect needNesterovNormalizeHeuristic() overrides from test custom shapes (CustomSphere, CastSphere, DeformedCylinder). These smooth shapes should return false, matching Coal's built-in Sphere/Cylinder shape_traits. Expose getNormalizeSupportDirection as public API so wrapper shapes like CastHullShape can delegate the Nesterov heuristic lookup to the underlying shape via shape_traits, the single source of truth. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Store a Sphere object and delegate computeShapeSupport via getSupport<WithSweptSphere>, computeLocalAABB via support-function probing, and needNesterovNormalizeHeuristic via getNormalizeSupportDirection. Make all members private. Clean up comments: remove Tesseract-specific references, fix stale math in test comments, correct claim that GEOM_CUSTOM is the default (it is opt-in). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace 6 virtual computeShapeSupport calls with the |R|*half-extents formula on the precomputed aabb_local. This is consistent with how CollisionObject::computeAABB() and per-shape computeBV specializations already work, and avoids expensive hill-climbing for ConvexBase shapes. The change introduces a precondition: computeLocalAABB() must have been called before computeBV<AABB, ShapeBase>. This is documented in the header. Callers bootstrapping their own aabb_local must compute from geometric parameters directly (closed-form or support function). Other changes: - Transform3s::inverse() marked const - CastSphere uses computeBV + AABB merge instead of support queries - DeformedCylinder uses closed-form per-disk extent formula - test_computeBV_AABB_ShapeBase uses enclosure checks (conservative AABB) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Custom shapes (GEOM_CUSTOM) that delegate to an inner shape's support function were forced to use getSupport(shape, dir, hint) which creates a fresh stack-local ShapeSupportData every call. This caused getShapeSupportLog's visited vector to be reallocated on every support query instead of being reused across calls. Add a getSupport(shape, dir, hint, support_data) overload that accepts caller-provided ShapeSupportData, enabling buffer reuse. The original 3-arg overload now delegates to the new one with a local ShapeSupportData. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Doxygen embeds explicit-specialization template args inside <name> (e.g. computeBV<AABB, ShapeBase>), so the generated static_cast<...>(&coal::computeBV<AABB, ShapeBase>) failed to compile inside namespace doxygen — the bare type names aren't visible there. The static_cast target type already pins the specialization, so emit &coal::computeBV and let function-pointer deduction pick it. Operator overloads embed '<' in the name and are passed through. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bfbe555 to
3d1b84e
Compare
In order to support continuous collision checking in Trajopt (currently done using Bullet), Coal needs support for custom shapes. For Bullet, this was trivial, as btConvexShape can be overridden. Now, using the new GEOM_CUSTOM, this is easy using Coal as well.
As you might notice, a lot of this was created with the help of Claude, for completeness here is the Claude PR message:
Summary
Motivation
Adding a new primitive shape to Coal currently requires modifying the NODE_TYPE enum, registering 63+ dispatch matrix entries, editing the Minkowski difference support dispatch, and adding serialization — all inside Coal's source tree. This was discussed in #792.
GEOM_CUSTOM reduces this to three steps for external users:
Everything else (GJK, EPA, contact patches, broadphase) works automatically via virtual dispatch.
Design
Entirely opt-in. ShapeBase does not default to GEOM_CUSTOM — it inherits BV_UNKNOWN from CollisionGeometry. Custom shapes must explicitly override getNodeType(). Built-in shapes are unaffected; all existing NODE_TYPE values and dispatch paths are unchanged.
Virtual dispatch only for GEOM_CUSTOM. The dispatch matrices route GEOM_CUSTOM pairs through ShapeShapeCollide<ShapeBase, T> (and equivalents for distance/contact patch), which call computeShapeSupport() via the virtual method. Built-in shapes continue to use their template-specialized fast paths.
No specialized fast-paths. GEOM_CUSTOM always goes through GJK/EPA. For shapes where closed-form solutions exist (e.g. sphere-sphere), implementing them as a built-in NODE_TYPE will still be faster.
getNormalizeSupportDirection exposed publicly so that wrapper shapes (e.g. swept-volume shapes for continuous collision detection) can query the Nesterov heuristic for their underlying shape through shape_traits, the single source of truth.
API additions
// collision_object.h
enum NODE_TYPE { ..., GEOM_CUSTOM, NODE_COUNT };
// geometric_shapes.h — on ShapeBase
virtual void computeShapeSupport(const Vec3s& dir, Vec3s& support,
int& hint, details::ShapeSupportData& data) const;
virtual bool needNesterovNormalizeHeuristic() const;
// minkowski_difference.h
COAL_DLLAPI bool getNormalizeSupportDirection(const ShapeBase* shape);
Tests
Two new test files with 21 test cases:
custom_shape.cpp (17 tests) — CustomSphere (leaf custom shape) and CastSphere (swept-volume wrapper, demonstrating continuous collision detection):
custom_shape_deformed_cylinder.cpp (4 tests) — DeformedCylinder (convex hull of two oriented disks, from #792):
Test plan