Replies: 2 comments
-
|
Hi @rjoomen, I think it's a bad idea to use virtualization for the support functions. The reason why coal is fast is that it efficiently dispatches the support function. The tag system in coal is ugly, I agree. But the support function calls need to happen as fast as possible, as they are called many times during collision detection. Currently, when the GJK solver is created, it automatically resolves the exact support function of the minkowski difference it needs to run. When the solver runs, it never goes through the virtual table of the shapes. |
Beta Was this translation helpful? Give feedback.
-
|
That makes sense, although, as you can see above, question is how much this actually will hurt performance. I started this discussion just to pose an alternative solution to the GEOM_CUSTOM PR. I'd be happy with either solution, as long as Coal is extensible. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
This is a design brief, created for discussion as an alternative option to PR #822.
The current dispatch is manual virtual dispatch
Coal's GJK support function dispatch uses
NODE_TYPEswitches, template specializations, and function pointers to route each shape to its support function body. This is virtual dispatch — implemented with templates and switches instead of a vtable.The call chain from GJK to the actual support body is 4 levels deep:
The function pointer is resolved once per collision pair in
MinkowskiDiff::set()via two nested switches (makeGetSupportFunction0→makeGetSupportFunction1), producing ~400 template instantiations (10 shapes x 10 x 2 identity x 2 SupportOptions).This manual dispatch has three structural problems:
1. Per-shape logic is separated from the shapes. Each shape's support function lives in
support_functions.cppas a template specialization ofgetShapeSupport<T>, not on the shape class itself. To understand how Box computes its support point, you readsupport_functions.cpp, notBox.2. Per-shape configuration is embedded in dispatch machinery.
makeGetSupportFunction0/1doesn't just dispatch — it also handles per-shape setup that only those shapes know about:swept_sphere_radius += radius) — duplicated in both functionstruefor exactly one shape type3. The shape list is duplicated in 6 switch sites across 4 files.
makeGetSupportFunction0,makeGetSupportFunction1,getSupport,getSupportSet,getNormalizeSupportDirection,makeSupportSetFunction— totaling ~350 lines of boilerplate. Adding a shape means editing all six and matching the existing patterns for radius promotion, data initialization, and traits.Proposed design: use actual virtual dispatch
Replace the manual dispatch with virtual methods on
ShapeBase. Each shape owns its support logic, its inflation semantics, and its setup requirements. The dispatch machinery disappears.Virtual methods on
ShapeBasegetShapeSupportis pure virtual — every concreteShapeBasemust provide a support function. The compiler catches missing overrides. Shapes that should never reach GJK (Plane, Halfspace) implement an override that throws — these shapes have analytical specializations for every pair, so a GJK call indicates a bug.What moves where
Each built-in shape moves its
getShapeSupport<NoSweptSphere>template specialization body into agetShapeSupportoverride markedfinal. Per-shape configuration moves from the dispatch machinery onto the shapes:support_functions.cpp)makeGetSupportFunction0/1)halfSidesupport.setZero()(point core)getInflationRadius→radius + getSweptSphereRadius()v / sqrt(v . dir)(normalizes internally)±halfLengthon Z (segment core)getInflationRadius→radius + getSweptSphereRadius()±halfLengthon Z + radialinitSupportData(visited array),needNesterovNormalizeHeuristicConvexBaseTpldeserves a note: currently the dispatch machinery selects betweenSmallConvexandLargeConvextag types based on vertex count. With virtual dispatch,ConvexBaseTpl::getShapeSupportmakes this decision internally. The tag types become a private implementation detail.Marking overrides
finalallows the compiler to devirtualize when the concrete type is statically known.Simplified MinkowskiDiff
The ~250 lines of dispatch machinery in
minkowski_difference.cppreduce to:Similarly, contact patch dispatch simplifies from a switch + function pointer (
makeSupportSetFunction) to a direct virtual call (shape->getShapeSupportSet(...)).Removed from
minkowski_difference.cpp:GetSupportFunctiontypedef,getSupportFuncpointer,getSupportTpl,getSupportFuncTpl,makeGetSupportFunction0,makeGetSupportFunction1,getNormalizeSupportDirection,getNormalizeSupportDirectionFromShapes.Consequence for function matrices
MinkowskiDiff::set()no longer needs concrete types — it takesShapeBase*and calls virtuals. This affects all three function matrices:Collision and distance matrices. (CollisionFunctionMatrix/DistanceFunctionMatrix) Each has 121 explicit shape-shape entries (11 x 11) using
ShapeShapeCollide<S0, S1>/ShapeShapeDistance<S0, S1>— the template types exist solely to thread concrete types through toMinkowskiDiff::set(). Of the 55 unique pairs, 26 have analytical specializations (Sphere-Sphere, Sphere-Capsule, *-Plane, *-Halfspace, etc.) that bypass GJK and need concrete types. The remaining 29 go through GJK and can use a generic<ShapeBase, ShapeBase>fallback. Each matrix shrinks from 121 explicit entries to ~26.Contact patch matrix. (ContactPatchFunctionMatrix) The contact patch solver calls
makeSupportSetFunction, which is one of the 6 switch sites eliminated by this refactoring. With virtualgetShapeSupportSet, the solver doesn't need concrete types at all — a single generic entry covers all shape-shape pairs.In all three matrices, adding a shape no longer requires adding matrix entries — GJK collision, distance, and contact patches work automatically via the virtual interface. This is a follow-up change enabled by the refactoring.
What the current design does well
The current approach has genuine performance advantages that the refactoring trades away:
One indirect call per iteration, not two. The pair function pointer (
getSupportFuncTpl<Box, Sphere, false, NoSS>) dispatches both shapes in a single indirect call. Inside, the twogetShapeSupportcalls are direct (concrete type baked in via template parameter). Virtual dispatch replaces this with two indirect calls. The total call count drops (2 vs 3), but the indirect count rises (2 vs 1) — indirect calls have slightly higher latency due to branch prediction pipeline effects.Identity transform compiled out.
TransformIsIdentityis a template parameter that selects between two function pointer instantiations atset()time. The identity instantiation contains no rotation/translation instructions at all — nooR1.transpose() * dir, nooR1 * supp1 + ot1. Virtual dispatch replaces this with a runtimeif (identity)branch insidesupport(). The branch is well-predicted, but the non-identity code (matrix multiply + transform) is still compiled into the function body and occupies instruction cache.Inlinable under LTO. The concrete type is statically known throughout the call chain (
getSupportFuncTpl<Box, ...>→getShapeSupport(const Box*, ...)). Under LTO, the linker can inline across TUs because the types are explicit. With virtual dispatch throughShapeBase*, LTO can only inline if it proves the dynamic type — harder even withfinal, sinceMinkowskiDiffstoresShapeBase*that could point to any concrete shape.These are real tradeoffs. They're small — GJK iterations are dominated by the support body math (dot products, comparisons, hill-climbing), not call overhead — but should be benchmarked rather than assumed negligible.
Performance summary
final+ type proofRisk areas
getInflationRadius()must reproduce the current accounting exactly. Wrong values silently produce incorrect collision results.initSupportDataoverride causes silent hill-climbing failure. Add a debug assert inConvexBaseTpl::getShapeSupportthatdata.visitedis correctly sized for large meshes.dirinternally. The formulas are moved unchanged from the existing specializations, but should be verified.isIdentity()/isZero()behavior.Files to modify
include/coal/shape/geometric_shapes.hShapeBase. Addfinaloverrides to each built-in shape. Add#include "coal/narrowphase/support_data.h".src/narrowphase/support_functions.cppgetShapeSupport<T>body intoT::getShapeSupport. SimplifygetSupportfree function from 40-line switch to convenience wrapper (creates localShapeSupportData, calls virtual). Remove per-shape template specializations.include/coal/narrowphase/support_functions.hgetShapeSupportdeclarations. KeepgetSupportfree function.include/coal/narrowphase/minkowski_difference.hGetSupportFunctiontypedef andgetSupportFuncpointer. Makedata[2]mutable. Addidentitymember. Replacesupport()with virtual calls.src/narrowphase/minkowski_difference.cppset()to ~20 lines.src/contact_patch/contact_patch_solver.cppmakeSupportSetFunctionand related templates. CallgetShapeSupportSetdirectly.include/coal/shape/geometric_shapes_traits.hNeedNesterovNormalizeHeuristicandIsStrictlyConvexreplaced by virtuals. Three other traits (NeedNormalizedDir,IsInflatable,HasInflatedSupportFunction) are dead code.All existing GJK/EPA/contact-patch tests must pass unchanged.
Follow-up: virtual
computeAABBcomputeBV<AABB, ShapeBase>has the same structural problem as the support dispatch: it's a template specialization that custom shapes can't override. After the fork's change to useaabb_localinstead of support queries, it also has a precondition (computeLocalAABB()must have been called first) that makes it unsuitable for bootstrapping a shape's own AABB.With virtual dispatch on
ShapeBase, this becomes a virtual method with a default implementation:Built-in shapes like
Cylindercan override with their closed-form (R.cwiseAbs() * (radius, radius, halfLength)). Custom shapes override to compute from their own geometry. The precondition on the default is still there, but shapes that need to bootstrap (likeDeformedCylinder) simply override instead of being forced throughcomputeShapeSupport.This also unifies
computeBV<AABB, T>(currently one specialization per shape) withCollisionObject::computeAABB()(which already uses theaabb_localformula). Both would delegate to the same virtual.Not in scope for the initial refactoring — the support dispatch is the priority. But it's a natural follow-up that completes the move from template specializations to virtual methods.
All existing GJK/EPA/contact-patch tests must pass unchanged.
API impact
Top-level API unchanged.
coal::collide(),coal::distance(),coal::computeContactPatch(), and all request/result types are unaffected.ShapeBasegains new virtuals. Most notably a pure virtualgetShapeSupport. This does not changeShapeBase's instantiability — it is already abstract (inherits 4 pure virtuals fromCollisionGeometry:clone,computeLocalAABB,getNodeType,isEqual). ExternalShapeBasesubclasses must add agetShapeSupportoverride; the remaining new virtuals (getInflationRadius,initSupportData,needNesterovNormalizeHeuristic,isStrictlyConvex,getShapeSupportSet) have defaults.MinkowskiDiffinternals change, public methods don't.GetSupportFunctiontypedef andgetSupportFuncpointer are removed.support(),support0(),support1()keep their signatures — Python bindings are preserved.MinkowskiDifflives incoal::details, signaling internal use.getSupportfree function unchanged. Signature preserved, implementation simplified. Also incoal::details.Removed from public headers:
getShapeSupport<T>declarations fromsupport_functions.hContactPatchSolver::SupportSetFunctiontypedef andmakeSupportSetFunctiongeometric_shapes_traits.h(entire file)All removed items are in
coal::detailsor internal headers. Code that depends on them is reaching into implementation details.Implementing a custom shape outside of Coal
A custom shape subclasses
ShapeBase, overrides virtuals, and works with Coal's collision/distance pipeline without modifying any Coal source files.Minimal example
What to override
getShapeSupportgetNodeTypeGEOM_CUSTOM.clonecomputeLocalAABBisEqualgetInflationRadiusgetSweptSphereRadius().initSupportDataneedNesterovNormalizeHeuristicfalse.isStrictlyConvexfalse.getShapeSupportSetContrast with the current GEOM_CUSTOM fork
The current fork (13 commits on top of
14b40842f) adds custom shape support by boltingGEOM_CUSTOMonto the existing dispatch machinery. The virtual dispatch refactoring makes this machinery unnecessary.sqrt+ 3 div) → virtual calldircontract for custom shapesgetShapeSupportSetfor multi-point patchesinner->getShapeSupport()directlyBoth approaches require significant modifications to Coal. The difference is structural: the current fork adds a second-class extension point alongside the existing dispatch machinery, leaving the machinery itself intact. The virtual dispatch refactoring replaces the dispatch machinery, making extensibility a consequence of the cleaner internal structure.
What works automatically
Once the shape class exists, it participates in Coal's full pipeline without registration:
MinkowskiDiff::set()calls the virtuals; no matrix entry needed for GJK pairs.getShapeSupportSetis called directly; default produces a single contact point.computeLocalAABB().getShapeSupport(e.g., a swept volume wrapping a Box).Beta Was this translation helpful? Give feedback.
All reactions