-
Notifications
You must be signed in to change notification settings - Fork 417
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
Add convexhull to convehull test #334
base: master
Are you sure you want to change the base?
Add convexhull to convehull test #334
Conversation
…e in ComputeVisiblePatch
This is to assist with issue #326 testing |
Here is th ply files if you want to view the convex hull for commit ab76114 |
Hi Levi, Thanks a lot for the PR! I think we can fix the problem in that assert with LIBCCD solver. I will file a PR with part of your test to expose the problem. |
…eNormalPointingOutward
I have added second of three tests which triggers another assert. |
Hi Levi, I believe #336 should fix the problem
I will look into the Best, |
@hongkai-dai I commented out the one test. I also added another test for different scenarios related to checking two convex hulls. Looking at the results of the test it has some large errors when two objects are in collision for distance and nearest points. The following case are currently tested for when the closest feature is:
Need to add:
|
/cc @DamrongGuoy |
Hi Levi, Sorry for my belated reply. I am looking into your test in https://github.com/Levi-Armstrong/fcl/blob/convexTests/test/test_fcl_convexhull_convexhull.cpp#L542L546. May I ask how you obtain these numbers In your comment https://github.com/Levi-Armstrong/fcl/blob/convexTests/test/test_fcl_convexhull_convexhull.cpp#L529, you mentioned that the closest feature between the two colliding objects are edge to edge. I think mathematically it is proved that the closest point on the boundary of the Minkowski difference A ⊖ B is an interior point within each facet (see theorem 4.8 of https://github.com/tianxiao/Recent-Advances-in-Real-Time-Collision-and-Proximity-Computations-for-Games-and-Simulations-/blob/master/books/Collision%20Detection%20in%20Interactive%203D%20Environments.pdf). Namely if your polytope is well posed (no two neighbouring faces are parallel), then the closest feature can never be edge to edge, at least one of the feature should be a face. |
Hi Levi, Sean @SeanCurtis-TRI and I looked into your test on the closest distance. We think in https://github.com/Levi-Armstrong/fcl/blob/convexTests/test/test_fcl_convexhull_convexhull.cpp#L542L546, what you find is not the penetration depth, but the distance between two edges. The definition of the penetration depth is Namely the penetration depth is the minimal translational distance, such that the two objects A and B can get out of contact. While the distance you find can get the two objects out of contact, it is not the minimal one. To show the minimal translation distance, Sean draws the two polytopes as We draw the two closest points And we translate one sphere with a vector The distance found from EPA is 0.2706222, smaller than the distance you computed (0.27552). So by definition of penetration depth, the EPA distance is more correct than 0.27552. Note that the closest points found from EPA are not on the edges, and the vector |
Wow! That is an awesome issue comment. |
@hongkai-dai and @SeanCurtis-TRI This is great and thank you for the detailed response. I came up with the number using blender and physically measuring two meshes. The one difference is I was measuring along the the same translation vector provided by using primitive shapes assume they should be same. As you pointed out this will not be the same so bad assumption on my part. I will revise the test with the correct information. |
@hongkai-dai and @SeanCurtis-TRI This is great and thank you for the detailed response. I came up with the number using blender and physically measuring two meshes. The one difference is I was measuring along the the same translation vector provided by using primitive shapes assume they should be same. As you pointed out this will not be the same so bad assumption on my part. I will keep looking. |
This test triggers the assert for isOutsidePolytopeFace in ComputeVisiblePatch
This change is