Skip to content

Commit

Permalink
Improved contact types (#616)
Browse files Browse the repository at this point in the history
# Objective

A lot of Avian's current contact types store some redundant data, aren't properly optimized, and aren't explicit enough about what they represent.

This PR is a clean-up pass to make the types a bit smaller and clearer.

## Solution

### `Contacts`

- Replace `total_normal_impulse` property with `total_normal_impulse`, `total_normal_impulse_magnitude`, and `max_normal_impulse` helpers
- Deprecate `total_normal_force` helper
  - You can just divide by the substep timestep
- Remove `total_tangent_impulse` property and `total_friction_force` helper
  - These could previously technically be wrong/misleading, since each contact point can have a different tangent direction, especially in 3D, and they can also change between substeps. The tangent impulse magnitudes of each individual point can still be accessed.

### `ContactManifold`

- Rename `ContactManifold::contacts` to `ContactManifold::points`
- Use an `ArrayVec` with a capacity of 2 instead of a `Vec` to store 2D `ContactManifold` points directly on the stack
- Replace the local `normal1` and `normal2` with a single world-space `normal`
  - Internals only need the world-space normal, and it makes e.g. contact modification more straightforward
- Add `total_normal_impulse` and `max_normal_impulse` helpers

### `ContactData`

- Rename `ContactData` to `ContactPoint` (represents a point in a contact manifold)
  - Another option would be `ManifoldPoint` like in Box2D; not sure which one is better
- Rename `point1` and `point2` to `local_point1` and `local_point2` for explicitness
- Remove `normal1` and `normal2` from `ContactPoint`, since the normal is already stored in the `ContactManifold`

### Other

- Many documentation improvements
- Contact force debug rendering uses per-point forces instead of the total force for the contact pair

---

## Migration Guide

There have been several changes to Avian's contact types to make them more optimized and clearer.

### `Contacts`

- The `total_normal_impulse` property has been replaced with a `total_normal_impulse` helper method.
- The `total_normal_force` helper has been deprecated. Instead, just divide the impulse by the substep timestep.
- The `total_tangent_impulse` property and `total_friction_force` helper have been removed for being inaccurate/misleading. The tangent impulse magnitudes of each individual point can still be accessed.

### `ContactManifold`

- `ContactManifold::contacts` has been renamed to `ContactManifold::points`.
- The local `normal1` and `normal2` have been replaced with a single world-space `normal`, pointing from the first shape to the second.

### `ContactData`

- `ContactData` has been renamed to `ContactPoint`, since it specifically represents a point in a contact manifold, not general contact data.
- `point1` and `point2` have been renamed to `local_point1` and `local_point2` for explicitness
- `normal1` and `normal2` have been removed, since the normal is already stored in the `ContactManifold`.
  • Loading branch information
Jondolf authored Feb 20, 2025
1 parent 95bbd06 commit 8bf77e0
Show file tree
Hide file tree
Showing 14 changed files with 270 additions and 262 deletions.
1 change: 1 addition & 0 deletions crates/avian2d/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ nalgebra = { version = "0.33", features = ["convert-glam029"], optional = true }
serde = { version = "1", features = ["derive"], optional = true }
derive_more = "1"
indexmap = "2.0.0"
arrayvec = "0.7"
fxhash = "0.2.1"
itertools = "0.13"
bitflags = "2.5.0"
Expand Down
27 changes: 12 additions & 15 deletions crates/avian2d/examples/custom_collider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,31 +79,28 @@ impl AnyCollider for CircleCollider {
let sum_radius = self.radius + other.radius;

if distance_squared < (sum_radius + prediction_distance).powi(2) {
let normal1 = if distance_squared != 0.0 {
let local_normal1 = if distance_squared != 0.0 {
delta_pos.normalize_or_zero()
} else {
Vector::X
};
let normal2 = delta_rot.inverse() * (-normal1);
let point1 = normal1 * self.radius;
let point2 = normal2 * other.radius;
let local_normal2 = delta_rot.inverse() * (-local_normal1);
let local_point1 = local_normal1 * self.radius;
let local_point2 = local_normal2 * other.radius;

vec![ContactManifold {
index: 0,
normal1,
normal2,
contacts: vec![ContactData {
feature_id1: PackedFeatureId::face(0),
feature_id2: PackedFeatureId::face(0),
point1,
point2,
normal1,
normal2,
normal: rotation1 * local_normal1,
points: avian2d::data_structures::ArrayVec::from_iter([ContactPoint {
local_point1,
local_point2,
penetration: sum_radius - distance_squared.sqrt(),
// Impulses are computed by the constraint solver
// Impulses are computed by the constraint solver.
normal_impulse: 0.0,
tangent_impulse: 0.0,
}],
feature_id1: PackedFeatureId::face(0),
feature_id2: PackedFeatureId::face(0),
}]),
}]
} else {
vec![]
Expand Down
15 changes: 5 additions & 10 deletions crates/avian2d/examples/kinematic_character_2d/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,7 @@ fn kinematic_controller_collisions(
bodies: Query<&RigidBody>,
collider_parents: Query<&ColliderParent, Without<Sensor>>,
mut character_controllers: Query<
(
&mut Position,
&Rotation,
&mut LinearVelocity,
Option<&MaxSlopeAngle>,
),
(&mut Position, &mut LinearVelocity, Option<&MaxSlopeAngle>),
(With<RigidBody>, With<CharacterController>),
>,
time: Res<Time>,
Expand All @@ -295,7 +290,7 @@ fn kinematic_controller_collisions(
let character_rb: RigidBody;
let is_other_dynamic: bool;

let (mut position, rotation, mut linear_velocity, max_slope_angle) =
let (mut position, mut linear_velocity, max_slope_angle) =
if let Ok(character) = character_controllers.get_mut(collider_parent1.get()) {
is_first = true;
character_rb = *bodies.get(collider_parent1.get()).unwrap();
Expand Down Expand Up @@ -323,15 +318,15 @@ fn kinematic_controller_collisions(
// Each contact in a single manifold shares the same contact normal.
for manifold in contacts.manifolds.iter() {
let normal = if is_first {
-manifold.global_normal1(rotation)
-manifold.normal
} else {
-manifold.global_normal2(rotation)
manifold.normal
};

let mut deepest_penetration: Scalar = Scalar::MIN;

// Solve each penetrating contact in the manifold.
for contact in manifold.contacts.iter() {
for contact in manifold.points.iter() {
if contact.penetration > 0.0 {
position.0 += normal * contact.penetration;
}
Expand Down
22 changes: 14 additions & 8 deletions crates/avian2d/examples/one_way_platform_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ fn pass_through_one_way_platform(
// It can have read-only access to queries, resources, and other system parameters.
#[derive(SystemParam)]
struct PlatformerCollisionHooks<'w, 's> {
one_way_platforms_query: Query<'w, 's, Read<OneWayPlatform>>,
one_way_platforms_query: Query<'w, 's, (Read<OneWayPlatform>, Read<GlobalTransform>)>,
// NOTE: This precludes a `OneWayPlatform` passing through a `OneWayPlatform`.
other_colliders_query: Query<
'w,
Expand Down Expand Up @@ -255,19 +255,24 @@ impl CollisionHooks for PlatformerCollisionHooks<'_, '_> {

// First, figure out which entity is the one-way platform, and which is the other.
// Choose the appropriate normal for pass-through depending on which is which.
let (platform_entity, one_way_platform, other_entity, relevant_normal) =
if let Ok(one_way_platform) = self.one_way_platforms_query.get(contacts.entity1) {
let (platform_entity, one_way_platform, platform_transform, other_entity, relevant_normal) =
if let Ok((one_way_platform, platform_transform)) =
self.one_way_platforms_query.get(contacts.entity1)
{
(
contacts.entity1,
one_way_platform,
platform_transform,
contacts.entity2,
RelevantNormal::Normal1,
)
} else if let Ok(one_way_platform) = self.one_way_platforms_query.get(contacts.entity2)
} else if let Ok((one_way_platform, platform_transform)) =
self.one_way_platforms_query.get(contacts.entity2)
{
(
contacts.entity2,
one_way_platform,
platform_transform,
contacts.entity1,
RelevantNormal::Normal2,
)
Expand All @@ -280,7 +285,7 @@ impl CollisionHooks for PlatformerCollisionHooks<'_, '_> {
if one_way_platform.0.contains(&other_entity) {
let any_penetrating = contacts.manifolds.iter().any(|manifold| {
manifold
.contacts
.points
.iter()
.any(|contact| contact.penetration > 0.0)
});
Expand Down Expand Up @@ -314,13 +319,14 @@ impl CollisionHooks for PlatformerCollisionHooks<'_, '_> {
Err(_) | Ok(None) | Ok(Some(PassThroughOneWayPlatform::ByNormal)) => {
// If all contact normals are in line with the local up vector of this platform,
// then this collision should occur: the entity is on top of the platform.
let platform_up = platform_transform.up().truncate().adjust_precision();
if contacts.manifolds.iter().all(|manifold| {
let normal = match relevant_normal {
RelevantNormal::Normal1 => manifold.normal1,
RelevantNormal::Normal2 => manifold.normal2,
RelevantNormal::Normal1 => manifold.normal,
RelevantNormal::Normal2 => -manifold.normal,
};

normal.length() > Scalar::EPSILON && normal.dot(Vector::Y) >= 0.5
normal.length() > Scalar::EPSILON && normal.dot(platform_up) >= 0.5
}) {
true
} else {
Expand Down
1 change: 1 addition & 0 deletions crates/avian3d/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ nalgebra = { version = "0.33", features = ["convert-glam029"], optional = true }
serde = { version = "1", features = ["derive"], optional = true }
derive_more = "1"
indexmap = "2.0.0"
arrayvec = "0.7"
fxhash = "0.2.1"
itertools = "0.13"
bitflags = "2.5.0"
Expand Down
15 changes: 5 additions & 10 deletions crates/avian3d/examples/kinematic_character_3d/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,7 @@ fn kinematic_controller_collisions(
bodies: Query<&RigidBody>,
collider_parents: Query<&ColliderParent, Without<Sensor>>,
mut character_controllers: Query<
(
&mut Position,
&Rotation,
&mut LinearVelocity,
Option<&MaxSlopeAngle>,
),
(&mut Position, &mut LinearVelocity, Option<&MaxSlopeAngle>),
(With<RigidBody>, With<CharacterController>),
>,
time: Res<Time>,
Expand All @@ -311,7 +306,7 @@ fn kinematic_controller_collisions(
let character_rb: RigidBody;
let is_other_dynamic: bool;

let (mut position, rotation, mut linear_velocity, max_slope_angle) =
let (mut position, mut linear_velocity, max_slope_angle) =
if let Ok(character) = character_controllers.get_mut(collider_parent1.get()) {
is_first = true;
character_rb = *bodies.get(collider_parent1.get()).unwrap();
Expand Down Expand Up @@ -339,15 +334,15 @@ fn kinematic_controller_collisions(
// Each contact in a single manifold shares the same contact normal.
for manifold in contacts.manifolds.iter() {
let normal = if is_first {
-manifold.global_normal1(rotation)
-manifold.normal
} else {
-manifold.global_normal2(rotation)
manifold.normal
};

let mut deepest_penetration: Scalar = Scalar::MIN;

// Solve each penetrating contact in the manifold.
for contact in manifold.contacts.iter() {
for contact in manifold.points.iter() {
if contact.penetration > 0.0 {
position.0 += normal * contact.penetration;
}
Expand Down
52 changes: 26 additions & 26 deletions src/collision/contact_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub fn contact(
let normal1: Vector = (rotation1.inverse() * Vector::from(contact.normal1)).normalize();
let normal2: Vector = (rotation2.inverse() * Vector::from(contact.normal2)).normalize();

// Make sure normals are valid
// Make sure the normal is valid
if !normal1.is_normalized() || !normal2.is_normalized() {
return None;
}
Expand Down Expand Up @@ -156,8 +156,12 @@ pub fn contact_manifolds(
rotation2: impl Into<Rotation>,
prediction_distance: Scalar,
) -> Vec<ContactManifold> {
let isometry1 = make_isometry(position1.into(), rotation1.into());
let isometry2 = make_isometry(position2.into(), rotation2.into());
let position1: Position = position1.into();
let position2: Position = position2.into();
let rotation1: Rotation = rotation1.into();
let rotation2: Rotation = rotation2.into();
let isometry1 = make_isometry(position1, rotation1);
let isometry2 = make_isometry(position2, rotation2);
let isometry12 = isometry1.inv_mul(&isometry2);

// TODO: Reuse manifolds from previous frame to improve performance
Expand All @@ -184,22 +188,25 @@ pub fn contact_manifolds(
shape2,
prediction_distance,
) {
let normal1 = Vector::from(contact.normal1);
let normal2 = Vector::from(contact.normal2);
let normal = rotation1 * Vector::from(contact.normal1);

// Make sure normals are valid
if !normal1.is_normalized() || !normal2.is_normalized() {
// Make sure the normal is valid
if !normal.is_normalized() {
return vec![];
}

return vec![ContactManifold {
normal1,
normal2,
contacts: vec![ContactData::new(
normal,
#[cfg(feature = "2d")]
points: arrayvec::ArrayVec::from_iter([ContactPoint::new(
contact.point1.into(),
contact.point2.into(),
-contact.dist,
)]),
#[cfg(feature = "3d")]
points: vec![ContactPoint::new(
contact.point1.into(),
contact.point2.into(),
normal1,
normal2,
-contact.dist,
)],
index: 0,
Expand All @@ -215,34 +222,27 @@ pub fn contact_manifolds(
.filter_map(|manifold| {
let subpos1 = manifold.subshape_pos1.unwrap_or_default();
let subpos2 = manifold.subshape_pos2.unwrap_or_default();
let normal1: Vector = subpos1
let local_normal: Vector = subpos1
.rotation
.transform_vector(&manifold.local_n1)
.normalize()
.into();
let normal2: Vector = subpos2
.rotation
.transform_vector(&manifold.local_n2)
.normalize()
.into();
let normal = rotation1 * local_normal;

// Make sure normals are valid
if !normal1.is_normalized() || !normal2.is_normalized() {
// Make sure the normal is valid
if !normal.is_normalized() {
return None;
}

let manifold = ContactManifold {
normal1,
normal2,
contacts: manifold
normal,
points: manifold
.contacts()
.iter()
.map(|contact| {
ContactData::new(
ContactPoint::new(
subpos1.transform_point(&contact.local_p1).into(),
subpos2.transform_point(&contact.local_p2).into(),
normal1,
normal2,
-contact.dist,
)
.with_feature_ids(contact.fid1.into(), contact.fid2.into())
Expand Down
Loading

0 comments on commit 8bf77e0

Please sign in to comment.