QueryOptions parameters (GjkOptions) #298
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Vrixyz
changed the title
Vrixyz
commented
Dec 27, 2024
Vrixyz
commented
Dec 27, 2024
Vrixyz
force-pushed
the
incorrect-shapecast
branch
from
b3b83c1 to
e9cf114
Compare
Vrixyz
commented
Jan 17, 2025
This was referenced Jun 13, 2025
…fix more path to eps_tol(), but serves as discussion starter
Vrixyz
commented
Jun 27, 2025
Vrixyz
commented
Jul 2, 2025
… shapes knowing their underlying algorithm.
It's not straightforward to do, that will likely need a solution similar to a dispatcher
Vrixyz
force-pushed
the
incorrect-shapecast
branch
from
02a37ec to
ca45b43
Compare
Vrixyz
commented
Jul 16, 2025
| #[cfg(feature = "alloc")] // TODO: can’t be used without alloc because of EPA | ||
| { | ||
| let Some(options) = options.as_any().downcast_ref() else { | ||
| warn!("Incorrect option passed to project_local_point: using default options."); |
There was a problem hiding this comment.
I'm not a huge fan of these warnings:
- I'd like to make it easy for users to use the "simple API" (pass default options)
- We can make a
project_local_point_with_query_options, but I'm not sure duplicating API is great - I chose to support
&()as parameter which will be interpreted as default options later- Going this path means we should not warn if user passed
&().
- Going this path means we should not warn if user passed
- We can make a
Vrixyz
changed the title
|
Vrixyz
commented
Jul 17, 2025
Vrixyz
commented
Jul 17, 2025
Vrixyz
commented
Jul 18, 2025
Comment on lines
+46
to
+53
| (TypeId::of::<Compound>(), self_ref), | ||
| #[cfg(feature = "dim2")] | ||
| ( | ||
| TypeId::of::<crate::shape::ConvexPolygon>(), | ||
| gjk_options.clone(), | ||
| ), | ||
| #[cfg(feature = "dim3")] | ||
| (TypeId::of::<crate::shape::ConvexPolyhedron>(), gjk_options), |
There was a problem hiding this comment.
Do roundshapes and other container shapes need to be specific ? like TypeId::of::<RolunShape<crate::shape::ConvexPolyhedron>>() ?
There was a problem hiding this comment.
No: roundshapes use support point algorithm which is gjk options.
Vrixyz
commented
Jul 18, 2025
Vrixyz
commented
Jul 25, 2025
Comment on lines
+62
to
+65
| // FIXME: This needs a query option dispatcher, because a user custom shape against a ball may need a different option. | ||
| // Currently, some paths lead to an unused option. | ||
| // (Gjk is used for intersections with convex shapes and cylinder) | ||
| &self.gjk_options, |
There was a problem hiding this comment.
This is an important point to tackle.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following discussion on discord, a shape cast of a ball sometimes fails when it should not.
time_of_impact_support_map_support_mapreturnsNonewhen it shouldn't with very specific inputs #180Current status
GjkOptions, this is very breaking as it impactsPointQuerytrait.DefaultQueryDispatcherstores relevant options to pass to their algorithms.DefaultQueryDispatcherto handle user-defined algorithms (and more elegantly our convex polygons).intersection_test_point_query_ball(https://github.com/dimforge/parry/pull/298/files#r2230938921)QueryOoptionsNotUsedin algorithms to make it easier to understand which paths need an option and which don´t. Ideally an associated trait would be used but it makes it difficult to mix different shapes...contains_local_point_with_option? to make the API more straightforward by default, but I'm not a huge fan of duplicated entriesBefore this PR:
After this PR, with customizable Gjk epsilon changing each frame:
Screencast.from.07-24-2025.04.49.01.PM.webm
I added an example for compound shape point query (using convex polygon, to test dispatching gjk options), and the result was showing false negatives (without custom gjk options ; multiplying the epsilon by 100 fixes it.):
Screencast.from.07-16-2025.04.09.26.PM.webm