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

Should we clear inside collide? #565

Open
lmontaut opened this issue Apr 12, 2024 · 4 comments
Open

Should we clear inside collide? #565

lmontaut opened this issue Apr 12, 2024 · 4 comments
Labels

Comments

@lmontaut
Copy link
Contributor

lmontaut commented Apr 12, 2024

When we call collide (or distance, it behaves the same) on a collision pair (collide(o1, T1, o2, T2, col_req, col_res)), the function collide does not clear the collision result col_res.
The problem is that if col_res is not cleared, collide will basically exit immediately, without doing any kind of computation.

The consequence of that is if we do:

collide(o1, T1, o2, T2, col_req, col_res);
collide(o1, T3, o2, T4, col_req, col_res);

then the results stored in col_res will actually not correspond to the poses (T3, T4) but (T1, T2).

This behavior of hpp-fcl has caused me a lot of bugs, which don't even throw an error so it's often very hard to debug this problem.
So I am wondering if this is something we should keep or not? Should we call clear inside collide? If not, what is the reason behind this choice?
Thanks!

@vicltz
Copy link

vicltz commented Apr 25, 2024

I had the same issue, debug was very hard because of this behavior.
Maybe it's a good idea to add the clear inside the distance or collide function ?

@jcarpent
Copy link
Contributor

Regarding this issue, we can add an extra argument to collide and distance, named bool clear_results = true, in such a way people can still do both.

@lmontaut
Copy link
Contributor Author

I guess we can but I just don't see the reason why we don't simply clear inside distance or collide. If the results are not cleared, it means that the results inside QueryResult have no relation to one another, because they may have been called on different poses, or worse, different collision pairs... Maybe there is a use case I am missing but I just don't see the point of not clearing. Multiple people have reported this issue to me already and it's a super annoying bug to track down when someone is not familiar with hpp-fcl.

@lmontaut
Copy link
Contributor Author

lmontaut commented Apr 30, 2024

EDIT: I think the rationale behind not clearing is that if you set the num_max_contacts in CollisionRequest, you can collect up to num_max_contacts on different collision pairs with the same CollisionResult.
On the contrary, I am very used to creating a single CollisionResult for each collision pair. If I have non-convex objects like BVHs then num_max_contacts allows me to collect multiple contact points; if I have convex objects, I know I will get only one contact, so I set num_max_contacts to 1.

Anyway, I think mixing collision pairs inside the same result is a bad idea, and the current design of hpp-fcl enforces that.

@jcarpent I will do a PR with your solution and set the default behavior to clear the result

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants