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

Interpretation of contact "position" needs to be unified. #258

Open
SeanCurtis-TRI opened this issue Feb 21, 2018 · 3 comments
Open

Interpretation of contact "position" needs to be unified. #258

SeanCurtis-TRI opened this issue Feb 21, 2018 · 3 comments

Comments

@SeanCurtis-TRI
Copy link
Contributor

In the documetation of fcl::Contact, the pos field is simply described as:
"Contact position, in world space".

However, understanding the interpretation of this contact point w.r.t. the normal, the penetration depth, and the colliding geometries is a bit incoherent.

Sometimes, the contact position is the mid-point between the points on the colliding bodies that most deeply penetrate each other (e.g., halfspace-* collisions), other times, it lies completely on one surface (e.g., box-box, plane-, triangle-), and other times it is a point that lies somewhere on the line segment connecting the two most deeply-penetrating points (e.g., sphere-sphere). This inconsistency makes it nearly impossible to ascribe any physical interpretation on the contact position. We have several options:

  1. Continue the pattern in halfspace-* collision where it always lies at the mid-point,
  2. Follow the pattern of plane-* where it always lies on one particular geometry. In this case, it should always like, by convention, either on object 1 or object 2, and its corresponding point should be pos + normal * penetration_depth so that the pair of points can be consistently constructed, regardless of object ordering.
  3. Rather than producing a single point, we produce a pair of points (the two most deeply penetrating points). For contact points p1 and p2, normal n and penetration depth d, we would assert that (|p2 - p1|.dot(n) = d).

All three of these representations are interchangeable; from one you can reach either of the other two. No matter what, we should consider the sphere-sphere behavior incorrect.

Of the three, I have a slight bias for the first option. But, no matter what, the contact data needs to be unified so that code making collision queries can reason about the data without having to have special case code which depends on the types of geometries colliding.

SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Feb 21, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
@sherm1
Copy link
Member

sherm1 commented Feb 22, 2018

For better physical interpretation, where I think we want to get to eventually with these "one point" contact queries is:

  1. Calculate the centroid C of the overlap volume, and normal 𝗻. Project C along 𝗻 in both directions to get points p₁ and p₂ on the surfaces of objects 1 and 2, resp. Then define penetration depth d≜|p₂−p₁|.

I don't think we know how to do that yet for most primitives, but in the meanwhile option 3 above seems closest to me, using the result of option 1 (midpoint) as a stand-in for the centroid. Is that feasible?

@SeanCurtis-TRI
Copy link
Contributor Author

I would argue that 1-3 are equal approximations of 4. They are all interchangeable. Given one, you can get either of the other two. As such, it is impossible to favor one over another for mathematical reasons. Option 4 is definitely a superior mathematical model (in how it defines C). But once C and n are defined, you can then apply 1-3 as the encoding of C and n arbitrarily.

That said, in the absence of infrastructure for 4 and given that options 1-3 are all mathematically equivalent, I would suggest a different metric for preferring one -- minimize churn to interface. Option 1 & 2 don't change the structure of the Contact class -- option 3, would. So, generally, I favor option 1 or 2 (with a slight bias for two based on the litany of examples based on halfspace-* collisions).

SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Feb 22, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Feb 22, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Feb 22, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Feb 22, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Feb 22, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
@sherm1
Copy link
Member

sherm1 commented Feb 23, 2018

@SeanCurtis-TRI and I discussed f2f. We agreed that this would be the best approach for this query:

  • This query returns a point p, unit normal 𝗻, and depth d.
  • For a good physical response, the caller may need to know the line segment between "contact points" p₁ and p₂, one on each body. That allows, for example, adjusting the actual contact point based on the relative stiffness of the two materials in contact, by sliding along the line segment.
  • We propose to always define point p as the midpoint of that line segment, with p₁=p−(d/2)𝗻 and p₂=p+(d/2)𝗻.
  • The chosen location of p and direction of 𝗻 will vary for different geometries and algorithms and can be improved over time but would always retain the same interpretation for the caller.

The idea would be to standardize on this representation as the meaning of the result for this particular query, regardless of the types of geometry in contact. Does anyone object to that standardization?

SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 1, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 1, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 1, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 1, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 1, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 1, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 1, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 2, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 2, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
SeanCurtis-TRI added a commit to SeanCurtis-TRI/fcl that referenced this issue Mar 5, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue flexible-collision-library#258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
sherm1 pushed a commit that referenced this issue Mar 5, 2018
This does several things:

1. Modifies the "correctness" of box-box intersection:
  a. Addresses the interpretation of the contact position (as per
     issue #258); the contact position lies between the two surfaces.
  b. Correct the sign on the penetration depth so that colliding
     objects report positive penetration depth.
2. Refactors box-box intersection code:
  a. Removes redundant implementation to limit repeated bugs. This
     encompasses completely redundant function implementations as well
     as unnecessary duplicatio in branches.
  b. Add additional documentation/todos on the implementation.
  c. Replace the `fudge2` parameter with something more reasoned.
3. Adds unit tests confirming the behavior.
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

No branches or pull requests

2 participants