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

Add CollisionResult::nearest_points #303

Merged
merged 11 commits into from
Jun 13, 2022
Merged

Conversation

jcarpent
Copy link
Contributor

@jcarpent jcarpent commented Jun 4, 2022

This comes to replace #200 which was no more aligned with the current devel.
In addition, it fixes potential issues related to #200.

@jcarpent jcarpent mentioned this pull request Jun 4, 2022
4 tasks
Not sure yet if the change are all correct.
That's why, I decided to split with respect to the content of coal-library#200.
`distance_lower_bound` may be initialized with a lower value than the one contained in sqrDistLowerBound.
This is indeed expected as the lower bound has been updated in the collision call.
@@ -149,6 +149,9 @@ struct HPP_FCL_DLLAPI QueryRequest {
/// @brief enable timings when performing collision/distance request
bool enable_timings;

/// @brief threshold below which a collision is considered.
FCL_REAL collision_distance_threshold;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the same thing as what is called security_margin in other places of the code ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. These is just small tiny positive numbers to choose when there is a collision or not. In some contexts, the test (3e-18 <= 0) leads to invalid results. The idea is to make it a bit more robust.

Copy link
Contributor

@jmirabel jmirabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went quickly through it and it seems ok to me. I didn't have enough time for a thorough review.

Thanks for having updated my PR.

@jcarpent jcarpent merged commit 1263a06 into coal-library:devel Jun 13, 2022
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

Successfully merging this pull request may close these issues.

2 participants