-
-
Notifications
You must be signed in to change notification settings - Fork 108
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
Implement collision margin #393
Conversation
src/collision/narrow_phase.rs
Outdated
@@ -552,6 +608,7 @@ impl<'w, 's, C: AnyCollider> NarrowPhase<'w, 's, C> { | |||
body2: &RigidBodyQueryReadOnlyItem, | |||
collider1: &ColliderQueryItem<C>, | |||
collider2: &ColliderQueryItem<C>, | |||
collision_margin: Scalar, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
collision_margin: Scalar, | |
collision_margin: CollisionMargin, |
It's weird to refer to CollisionMargin
in the docs and then not actually use that type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like having to wrap everything in newtypes unnecessarily so I changed this (and the other similar cases) to impl Into<CollisionMargin>
and derived From<Scalar>
for now. Not sure if it's worth it though, since the newtypes don't really give an API benefit (other than explicitness and docs) and it just means that we need to call .into()
and use derefs in more places internally
src/dynamics/solver/contact/mod.rs
Outdated
@@ -91,6 +91,7 @@ impl ContactConstraint { | |||
collider_entity2: Entity, | |||
collider_transform1: Option<ColliderTransform>, | |||
collider_transform2: Option<ColliderTransform>, | |||
collision_margin: Scalar, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
collision_margin: Scalar, | |
collision_margin: CollisionMargin, |
if we already have the newtypes, we should also be consistently using them imo
src/collision/narrow_phase.rs
Outdated
let collision_margin1 = collider1.collision_margin.map_or( | ||
rb_collision_margin1.map_or(0.0, |margin| margin.0), | ||
|margin| margin.0, | ||
); | ||
let collision_margin2 = collider2.collision_margin.map_or( | ||
rb_collision_margin2.map_or(0.0, |margin| margin.0), | ||
|margin| margin.0, | ||
); | ||
let collision_margin_sum = collision_margin1 + collision_margin2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is duplicated. Consider extracting it into a method.
Math checks out, just some code quality issues, nothing big :) |
Co-authored-by: Jan Hohenheim <[email protected]>
Objective
Collisions for thin objects such as triangle meshes can sometimes have stability and performance issues, especially when dynamically colliding against other thin objects.
To allow users to mitigate these issues when necessary, engines such as Rapier and Bullet provide a collision margin or contact skin that forces the solver to maintain an artificial separation distance between objects. It can be thought of as a kind of shell that adds thickness to colliders, reducing the issues mentioned above.
Solution
Add a
CollisionMargin
component. It can be used to add artifical thickness to any collider for collisions.Without a collision margin:
2024-07-02.22-07-55.mp4
With a collision margin:
2024-07-02.22-09-28.mp4