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

Implement interaction groups test mode and cofficient combine rule #741

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,17 @@

### Added

- `InteractionTestMode`: Specifies which method should be used to test interactions. Supports `AND` and `OR`.
- `CoefficientCombineRule::Sum` - Adds the two coefficients and does a clamp to have at most 1.
- `RigidBodySet` and `ColliderSet` have a new constructor `with_capacity`.

### Modified

- `InteractionGroups` default value for `memberships` is now `GROUP_1` (#706)
- `ImpulseJointSet::get_mut` has a new parameter `wake_up: bool`, to wake up connected bodies.
- `InteractionGroups` struct now contains `InteractionTestMode`. Continues [rapier/pull/170](https://github.com/dimforge/rapier/pull/170) for [rapier/issues/622](https://github.com/dimforge/rapier/issues/622)
- `InteractionGroups` constructor now requires an `InteractionTestMode` parameter. If you want same behaviour as before, use `InteractionTestMode::AND` (eg. `InteractionGroups::new(Group::GROUP_1, Group::GROUP_1, InteractionTestMode::AND)`)
- `CoefficientCombineRule::Min` - now makes sure it uses a non zero value as result by using `coeff1.min(coeff2).abs()`

## v0.22.0 (20 July 2024)

Expand Down
6 changes: 4 additions & 2 deletions examples2d/collision_groups2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ pub fn init_world(testbed: &mut Testbed) {
/*
* Setup groups
*/
const GREEN_GROUP: InteractionGroups = InteractionGroups::new(Group::GROUP_1, Group::GROUP_1);
const BLUE_GROUP: InteractionGroups = InteractionGroups::new(Group::GROUP_2, Group::GROUP_2);
const GREEN_GROUP: InteractionGroups =
InteractionGroups::new(Group::GROUP_1, Group::GROUP_1, InteractionTestMode::AND);
const BLUE_GROUP: InteractionGroups =
InteractionGroups::new(Group::GROUP_2, Group::GROUP_2, InteractionTestMode::AND);

/*
* A green floor that will collide with the GREEN group only.
Expand Down
6 changes: 4 additions & 2 deletions examples3d/collision_groups3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ pub fn init_world(testbed: &mut Testbed) {
/*
* Setup groups
*/
const GREEN_GROUP: InteractionGroups = InteractionGroups::new(Group::GROUP_1, Group::GROUP_1);
const BLUE_GROUP: InteractionGroups = InteractionGroups::new(Group::GROUP_2, Group::GROUP_2);
const GREEN_GROUP: InteractionGroups =
InteractionGroups::new(Group::GROUP_1, Group::GROUP_1, InteractionTestMode::AND);
const BLUE_GROUP: InteractionGroups =
InteractionGroups::new(Group::GROUP_2, Group::GROUP_2, InteractionTestMode::AND);

/*
* A green floor that will collide with the GREEN group only.
Expand Down
12 changes: 10 additions & 2 deletions examples3d/vehicle_joints3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ pub fn init_world(testbed: &mut Testbed) {

let body_co = ColliderBuilder::cuboid(0.65, 0.3, 0.9)
.density(100.0)
.collision_groups(InteractionGroups::new(CAR_GROUP, !CAR_GROUP));
.collision_groups(InteractionGroups::new(
CAR_GROUP,
!CAR_GROUP,
InteractionTestMode::AND,
));
let body_rb = RigidBodyBuilder::dynamic()
.position(body_position.into())
.build();
Expand Down Expand Up @@ -85,7 +89,11 @@ pub fn init_world(testbed: &mut Testbed) {
// is mathematically simpler than a cylinder and cheaper to compute for collision-detection.
let wheel_co = ColliderBuilder::ball(wheel_radius)
.density(100.0)
.collision_groups(InteractionGroups::new(CAR_GROUP, !CAR_GROUP))
.collision_groups(InteractionGroups::new(
CAR_GROUP,
!CAR_GROUP,
InteractionTestMode::AND,
))
.friction(1.0);
let wheel_rb = RigidBodyBuilder::dynamic().position(wheel_center.into());
let wheel_handle = bodies.insert(wheel_rb);
Expand Down
6 changes: 5 additions & 1 deletion src/dynamics/coefficient_combine_rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ pub enum CoefficientCombineRule {
Multiply,
/// The greatest coefficient is chosen.
Max,
/// The sum of the two coefficients.
Sum,
}

impl CoefficientCombineRule {
Expand All @@ -27,8 +29,10 @@ impl CoefficientCombineRule {

match effective_rule {
0 => (coeff1 + coeff2) / 2.0,
1 => coeff1.min(coeff2),
1 => coeff1.min(coeff2).abs(),
Copy link
Contributor

@Vrixyz Vrixyz Nov 15, 2024

Choose a reason for hiding this comment

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

Should it be max(0.0) ?

Copy link
Contributor

@Vrixyz Vrixyz Nov 15, 2024

Choose a reason for hiding this comment

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

(outside of this PR's scope) @sebcrozet why does this function takes u8 rather than CoefficientCombineRule ?

Copy link
Member

Choose a reason for hiding this comment

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

Should it be max(0.0) ?

I think there is some use-case for this in godot but that would be worth clarifying by @Ughuuu.

(outside of this PR's scope) @sebcrozet why does this function takes u8 rather than CoefficientCombineRule ?

I don’t think there is a particular reason for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about the godot use case, this is just the exact code they have right now, so for parity I have that. Their use case is that they also have negative coefficients, and use this with absorbent field.
https://github.com/godotengine/godot/blob/0eadbdb5d0709e4e557e52377fa075d3e2f0ad1f/scene/resources/physics_material.h#L57
Then they do abs here, so the negative one becomes positive:
https://github.com/godotengine/godot/blob/0eadbdb5d0709e4e557e52377fa075d3e2f0ad1f/modules/godot_physics_3d/godot_body_pair_3d.cpp#L259

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should i add a comment in code around that match explaining that the coefficient can be negative in some cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should i add a comment in code around that match explaining that the coefficient can be negative in some cases?

Yes ; you can add a link to godot documentation so the reader knows where this comes from 👍

2 => coeff1 * coeff2,
4 => (coeff1 + coeff2).clamp(0.0, 1.0),
// 3 is missing as Max is the default one in case of mismatch.
_ => coeff1.max(coeff2),
}
}
Expand Down
59 changes: 49 additions & 10 deletions src/geometry/interaction_groups.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,18 @@
/// - The interaction groups filter.
///
/// An interaction is allowed between two filters `a` and `b` when two conditions
/// are met simultaneously:
/// are met simultaneously for [`InteractionTestMode::AND`] or individually for [`InteractionTestMode::OR`]::
/// - The groups membership of `a` has at least one bit set to `1` in common with the groups filter of `b`.
/// - The groups membership of `b` has at least one bit set to `1` in common with the groups filter of `a`.
///
/// In other words, interactions are allowed between two filter iff. the following condition is met:
/// In other words, interactions are allowed between two filter iff. the following condition is met
/// for [`InteractionTestMode::AND`]:
/// ```ignore
/// (self.memberships & rhs.filter) != 0 && (rhs.memberships & self.filter) != 0
/// (self.memberships.bits() & rhs.filter.bits()) != 0 && (rhs.memberships.bits() & self.filter.bits()) != 0
/// ```
/// or for [`InteractionTestMode::OR`]:
/// ```ignore
/// (self.memberships.bits() & rhs.filter.bits()) != 0 || (rhs.memberships.bits() & self.filter.bits()) != 0
/// ```
#[cfg_attr(feature = "serde-serialize", derive(Serialize, Deserialize))]
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
Expand All @@ -23,25 +28,39 @@ pub struct InteractionGroups {
pub memberships: Group,
/// Groups filter.
pub filter: Group,
/// Interaction test mode
pub test_mode: InteractionTestMode,
}

#[cfg_attr(feature = "serde-serialize", derive(Serialize, Deserialize))]
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, Default)]
/// Specifies which method should be used to test interactions
pub enum InteractionTestMode {
/// Use [`InteractionGroups::test_and`].
#[default]
AND,
/// Use [`InteractionGroups::test_or`].
OR,
Ughuuu marked this conversation as resolved.
Show resolved Hide resolved
}

impl InteractionGroups {
/// Initializes with the given interaction groups and interaction mask.
pub const fn new(memberships: Group, filter: Group) -> Self {
pub const fn new(memberships: Group, filter: Group, test_mode: InteractionTestMode) -> Self {
Self {
memberships,
filter,
test_mode,
}
}
Ughuuu marked this conversation as resolved.
Show resolved Hide resolved

/// Allow interaction with everything.
pub const fn all() -> Self {
Self::new(Group::ALL, Group::ALL)
Self::new(Group::ALL, Group::ALL, InteractionTestMode::AND)
}

/// Prevent all interactions.
pub const fn none() -> Self {
Self::new(Group::NONE, Group::NONE)
Self::new(Group::NONE, Group::NONE, InteractionTestMode::AND)
}

/// Sets the group this filter is part of.
Expand All @@ -59,21 +78,41 @@ impl InteractionGroups {
/// Check if interactions should be allowed based on the interaction memberships and filter.
///
/// An interaction is allowed iff. the memberships of `self` contain at least one bit set to 1 in common
/// with the filter of `rhs`, and vice-versa.
/// with the filter of `rhs`, **and** vice-versa.
#[inline]
pub const fn test(self, rhs: Self) -> bool {
// NOTE: since const ops is not stable, we have to convert `Group` into u32
// to use & operator in const context.
pub const fn test_and(self, rhs: Self) -> bool {
(self.memberships.bits() & rhs.filter.bits()) != 0
&& (rhs.memberships.bits() & self.filter.bits()) != 0
}

/// Check if interactions should be allowed based on the interaction memberships and filter.
///
/// An interaction is allowed iff. the groups of `self` contain at least one bit set to 1 in common
/// with the mask of `rhs`, **or** vice-versa.
#[inline]
pub const fn test_or(self, rhs: Self) -> bool {
(self.memberships.bits() & rhs.filter.bits()) != 0
|| (rhs.memberships.bits() & self.filter.bits()) != 0
}

/// Check if interactions should be allowed based on the interaction memberships and filter.
///
/// See [`InteractionTestMode`] for more info.
#[inline]
pub const fn test(self, rhs: Self) -> bool {
match self.test_mode {
InteractionTestMode::AND => self.test_and(rhs),
InteractionTestMode::OR => self.test_or(rhs),
}
}
Ughuuu marked this conversation as resolved.
Show resolved Hide resolved
}

impl Default for InteractionGroups {
fn default() -> Self {
Self {
memberships: Group::GROUP_1,
filter: Group::ALL,
test_mode: InteractionTestMode::AND,
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/geometry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub use self::contact_pair::{
pub use self::interaction_graph::{
ColliderGraphIndex, InteractionGraph, RigidBodyGraphIndex, TemporaryInteractionIndex,
};
pub use self::interaction_groups::{Group, InteractionGroups};
pub use self::interaction_groups::{Group, InteractionGroups, InteractionTestMode};
pub use self::mesh_converter::{MeshConverter, MeshConverterError};
pub use self::narrow_phase::NarrowPhase;

Expand Down