FCLCollisionDetector does nothing when maxNumContacts == 0

[mkoval]

I am using FCLCollisionDetector to perform a binary check. I set maxNumContacts = 0, since enableContact = false means that I am not computing any contacts. Surprisingly, this causes the collision detector to always return false.

I think we should either change this behavior or print a warning when maxNumContacts == 0.

[jslee02]

I would prefer printing a warning.

If CollisionDetector::collide(~) returns true for binary check when maxNumContacts == 0, the CollisionResult::isCollision() would return false when there is a collision because it returns false when CollisionResult::mContacts is empty. In order to Collision::isCollision() to return for the case, we would need to introduce additional member to CollisionResult for store the binary result.

Also, maxNumContacts == 0 sounds like we don't want to check any contacts to me.

[jslee02]

I actually consider removing binaryCheck option, and regarding maxNumContacts == 1 as binary check.

[mkoval]

Also, maxNumContacts == 0 sounds like we don't want to check any contacts to me.

That is correct. In a binary check you typically don't want to spend time computing the contact point and normal for any contacts (not even one). It is sufficient to know whether contact occurred and, (in some cases) which pair of ShapeFrames was detected to be in contact. I am concerned that requiring maxNumContacts >= 1 will require always computing at least one contact point and normal.

In my opinion, the correct fix is what we discussed in #716: decouple the concept of a "collision between two ShapeFrames" from the concept of a "contact point".

[jslee02]

I am concerned that requiring maxNumContacts >= 1 will require always computing at least one contact point and normal.

We wouldn't need to worry about this case since enableContact == false prevents computing contact information from the collision detector (at least with fcl), but the colliding pairs will be computed. If we pass nullptr for CollisionResult, then we wouldn't get any information but the binary result, but the collision detection engine would compute the colliding pair (but not point, normal, depth) anyways, which is minimum computation to get the binary result. We just don't copy the colliding pairs to CollisionResult in this case.

In my opinion, the correct fix is what we discussed in #716: decouple the concept of a "collision between two ShapeFrames" from the concept of a "contact point".

Yeah, I'm on board with your opinion. I'd save for this for next the release though.

[mkoval]

What is the meaning of maxNumContacts if enableContact == false? My assumption is that setting maxNumContacts == 1 would use FCL to compute the contact position and normal for the first point that is detected to be in collision.

[jslee02]

maxNumContacts and enableContact are completely independent options. If enableContact is false, then the collision detector returns only the information of colliding pairs but don't compute position, normal, and depth.

[mkoval]

I think this is mostly a naming issue - we use "contact" to refer to both the pairwise collision of two ShapeFrames and a contact point/normal. What are the point and normal fields of the Contact struct set to when enableContact == false?

The ideal solution would be to separate the concepts as we discussed in #716. Until then, perhaps we can eliminate some confusion by renaming maxNumContacts?

[jslee02]

What are the point and normal fields of the Contact struct set to when enableContact == false?

They will be left as the default values as assigned when Contact constructed.

Renaming sounds good. Do you have a suggestion? I would prefer to rename enableContact to requestContactInfo rather than renaming maxNumContacts. Maybe I'm a bit biased since I already get used to the current names.