Enable specifying extra margins for objects in DynamicAABBTreeArray broadphase manager.

[torresl-google]

Before this change, if a user wanted to do collision checking with a security margin, they couldn't use a broadphase manager because the broadphase may discard pairs of objects whose AABB's do not overlap.

This change allows users to specify margins on CollisionObjects that they add to DynamicAABBTreeArrayCollisionManager. The broadphase will operate on these objects as if they had larger AABBs, but the objects themselves are unchanged.

This change enables the following useful patterns:

• Add objects with a margin m to the broadphase, then use a CollisionCallback that checks for narrow phase collisions with a security margin equal to m. Now we can do collide() queries and catch all collisions within the security margin.
• Create two separate broadphase managers, where both have objects with margin m. When checking for collisions between the two managers, you can set the narrow phase security margin as high as 2*m and detect all collisions.

I don't consider this PR complete at all! I wanted to use it to start a conversation:

• Is this feature useful to other people?
• Does my implementation actually work as expected?
• Should it be generalized to other broadphase managers?

[lmontaut]

Hi @torresl-google ! You're right that the broadphase managers in coal don't deal with the security margin of the various narrowphases by default.

For reference, here is how we deal with it in pinocchio: see here, inside the update method of pinocchio's broadphase manager (which inherits from coal).
What update does: for a given object in the scene, it looks at all the collision pairs it is involved in and retrieves the maximum security margin of all these pairs. It then inflates the AABB of the object by this maximum security margin.

You could do something similar in your application:

1. update the objects' AABBs so that they take into account the security margins as you would like (e.g. what is done in pinocchio's broadphase manager update).
2. run the broadphase using coal's default CollisionCallBackCollect (this simply stores the pairs of objects that the broadphase has considered as overlapping) or using something more tailored to your application (see the example of pinocchio's CollisionCallBackCollect).
3. run the narrowphase with the "right" security margin for each pair stored in CollisionCallBackCollect.

Getting back to Coal: I think the value of the broadphase managers in Coal is the implementation of various methods that construct the different data structures and run the broadphase, given a set of objects and their AABBs. Coal proposes default callbacks, but it is quite flexible to suit various specific needs (again, cf pinocchio).

TLDR: I would suggest users to inherit from coal's broadphase manager (to benefit from the core algorithms) and implement their own callback in their application. I am not convinced it's the job of coal to handle specific use cases.

@torresl-google what do you think? Am I missing something?

[torresl-google]

Thanks for the thoughtful response, @lmontaut! I had not thought of using inheritance to implement my own broadphase manager with margin awareness.

I do have one concern about the solution you proposed from Pinocchio: in that update function, the CollisionObject's AABB is directly modified to include the required margin. This should indeed make the broadphase correctly identify possible overlaps that include security margins. But if I understand correctly, when we call collide() with request.security_margin > 0, we use AABB::overlap(), which also includes the security margins (see code). This means that, if we inflate the CollisionObject AABBs for the broadphase like Pinocchio, then the object-object AABB checks will be redundantly expanded again, which will cause us to run the narrowphase checks more often than necessary.

Does the above sound correct? This is why I chose to store the margin-expanded AABBs separately in the broadphase manager without modifying the CollisionObject AABBs.

I tried to implement a margin-aware broadphase manager that inherits from DynamicAABBTreeArrayCollisionManager and does not modify the CollisionObject AABBs (to avoid the above issue), but I found this difficult because the members table and dtree of DynamicAABBTreeArrayCollisionManager are private; it would be much easier if those were protected.

[lmontaut]

Hey @torresl-google! Can I ask which signature of collide you are talking about?

Short version:

I think I understand the confusion: you are refering to an overlap signature which is not called by the broadphase.
It is called internally by BVHs during the narrowphase (a BVH is a specific kind of shape supported by coal: it is a collection of internal AABBs).
However, when it comes to BVHs:

• the global AABB of the BVH itself is used for broadphase -> you would typically modify this in your case if you are using BVHs
• internal AABBs of BVHs are used for the narrowphase -> even if you modify the "global" AABB, these would not be affected by the exapansion of the global AABB, so you would not "count" the security margin twice
• In short: internal AABBs of a BVH and its global AABB are two different things

More details:

The broadphase manager's collide methods don't internally use any security margin, they only rely on the bounding boxes of the collision objects (see the overlap method here which is used in the dynamic aabb tree broadphase manager for example).

The narrowphase runs directly on the objects' geometries. The narrowphase does not look at their AABBs.

The signature of overlap you are referring to is only used in the narrowphase for collisions involving BVHs.
In this case, overlap is called on the internal AABBs of a given BVH, not on the global AABB of the BVH itself (these are two different things).

Does this make sense? I hope I am not missing something, otherwise please let me know!

[torresl-google]

Thank you, @lmontaut !

Sorry, my question was very unclear but I believe you still answered it! My question was specifically about calling collide(obj1, obj2, request, result) where request.security_margin > 0 and obj1 and obj2 have AABBs that have been manually expanded.

If I understand you correctly, the above collide function never uses the "global" AABB from CollisionObject::getAABB(); only the broadphase uses this global AABB. If this is correct, then it should be safe to manually expand CollisionObject::getAABB() when adding objects to the broadphase.

Is the above correct? If so, then I think I can do everything I need by deriving a new class that inherits from DynamicAABBTreeCollisionManager and we can close this PR.

[lmontaut]

@torresl-google yes this is correct!
The "global" AABB is only used for broadphase. You can safely expand it, it won't be used for the narrowphase.

@torresl-google I'll let you close this PR :)