Skip to content

Commit

Permalink
Fix collider scale propagation issues (#472)
Browse files Browse the repository at this point in the history
# Objective

Fixes #287 and part of #471.

Scale is currently not propagated correctly for colliders if the rigid body is not the root entity. Additionally, negative scale is prohibited for child colliders: scale is clamped to be greater than zero.

## Solution

Fix scale propagation and allow negative scale.

Note that mirroring is *not* handled properly for child colliders and some shapes yet. That will be fixed later.
  • Loading branch information
Jondolf authored Jul 28, 2024
1 parent fb99129 commit 5c2188c
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 70 deletions.
15 changes: 6 additions & 9 deletions src/collision/collider/hierarchy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ pub(crate) fn propagate_collider_transforms(
let changed = transform.is_changed() || parent.is_changed();
let parent_transform = ColliderTransform::from(*transform);
let child_transform = ColliderTransform::from(*child_transform);
let scale = (parent_transform.scale * child_transform.scale).max(Vector::splat(Scalar::EPSILON));
let scale = parent_transform.scale * child_transform.scale;

// SAFETY:
// - `child` must have consistent parentage, or the above assertion would panic.
Expand Down Expand Up @@ -301,9 +301,9 @@ pub(crate) fn propagate_collider_transforms(
/// # Safety
///
/// - While this function is running, `collider_query` must not have any fetches for `entity`,
/// nor any of its descendants.
/// nor any of its descendants.
/// - The caller must ensure that the hierarchy leading to `entity`
/// is well-formed and must remain as a tree or a forest. Each entity must have at most one parent.
/// is well-formed and must remain as a tree or a forest. Each entity must have at most one parent.
#[allow(clippy::type_complexity)]
unsafe fn propagate_collider_transforms_recursive(
transform: ColliderTransform,
Expand Down Expand Up @@ -372,6 +372,7 @@ unsafe fn propagate_collider_transforms_recursive(
);

let child_transform = ColliderTransform::from(*child_transform);
let scale = transform.scale * child_transform.scale;

// SAFETY: The caller guarantees that `collider_query` will not be fetched
// for any descendants of `entity`, so it is safe to call `propagate_collider_transforms_recursive` for each child.
Expand All @@ -381,19 +382,15 @@ unsafe fn propagate_collider_transforms_recursive(
unsafe {
propagate_collider_transforms_recursive(
if is_rb {
ColliderTransform {
scale: child_transform.scale,
..default()
}
ColliderTransform { scale, ..default() }
} else {
ColliderTransform {
translation: transform.transform_point(child_transform.translation),
#[cfg(feature = "2d")]
rotation: transform.rotation * child_transform.rotation,
#[cfg(feature = "3d")]
rotation: Rotation(transform.rotation.0 * child_transform.rotation.0),
scale: (transform.scale * child_transform.scale)
.max(Vector::splat(Scalar::EPSILON)),
scale,
}
},
collider_query,
Expand Down
23 changes: 12 additions & 11 deletions src/collision/collider/parry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ impl Collider {
/// - `ray_direction`: What direction the ray is cast in.
/// - `max_time_of_impact`: The maximum distance that the ray can travel.
/// - `solid`: If true and the ray origin is inside of a collider, the hit point will be the ray origin itself.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
pub fn cast_ray(
&self,
translation: impl Into<Position>,
Expand Down Expand Up @@ -1261,13 +1261,14 @@ fn scale_shape(
scale: Vector,
num_subdivisions: u32,
) -> Result<SharedShape, UnsupportedShape> {
let scale = scale.abs();
match shape.as_typed_shape() {
TypedShape::Cuboid(s) => Ok(SharedShape::new(s.scaled(&scale.into()))),
TypedShape::Cuboid(s) => Ok(SharedShape::new(s.scaled(&scale.abs().into()))),
TypedShape::RoundCuboid(s) => Ok(SharedShape::new(RoundShape {
border_radius: s.border_radius,
inner_shape: s.inner_shape.scaled(&scale.into()),
inner_shape: s.inner_shape.scaled(&scale.abs().into()),
})),
TypedShape::Capsule(c) => match c.scaled(&scale.into(), num_subdivisions) {
TypedShape::Capsule(c) => match c.scaled(&scale.abs().into(), num_subdivisions) {
None => {
log::error!("Failed to apply scale {} to Capsule shape.", scale);
Ok(SharedShape::ball(0.0))
Expand All @@ -1279,16 +1280,16 @@ fn scale_shape(
#[cfg(feature = "2d")]
{
if scale.x == scale.y {
Ok(SharedShape::ball(b.radius * scale.x))
Ok(SharedShape::ball(b.radius * scale.x.abs()))
} else {
// A 2D circle becomes an ellipse when scaled non-uniformly.
Ok(SharedShape::new(EllipseWrapper(Ellipse {
half_size: Vec2::splat(b.radius as f32) * scale.f32(),
half_size: Vec2::splat(b.radius as f32) * scale.f32().abs(),
})))
}
}
#[cfg(feature = "3d")]
match b.scaled(&scale.into(), num_subdivisions) {
match b.scaled(&scale.abs().into(), num_subdivisions) {
None => {
log::error!("Failed to apply scale {} to Ball shape.", scale);
Ok(SharedShape::ball(0.0))
Expand Down Expand Up @@ -1360,7 +1361,7 @@ fn scale_shape(
}
}
#[cfg(feature = "3d")]
TypedShape::Cylinder(c) => match c.scaled(&scale.into(), num_subdivisions) {
TypedShape::Cylinder(c) => match c.scaled(&scale.abs().into(), num_subdivisions) {
None => {
log::error!("Failed to apply scale {} to Cylinder shape.", scale);
Ok(SharedShape::ball(0.0))
Expand All @@ -1370,7 +1371,7 @@ fn scale_shape(
},
#[cfg(feature = "3d")]
TypedShape::RoundCylinder(c) => {
match c.inner_shape.scaled(&scale.into(), num_subdivisions) {
match c.inner_shape.scaled(&scale.abs().into(), num_subdivisions) {
None => {
log::error!("Failed to apply scale {} to RoundCylinder shape.", scale);
Ok(SharedShape::ball(0.0))
Expand Down Expand Up @@ -1434,15 +1435,15 @@ fn scale_shape(
if _id == 1 {
if let Some(ellipse) = shape.as_shape::<EllipseWrapper>() {
return Ok(SharedShape::new(EllipseWrapper(Ellipse {
half_size: ellipse.half_size * scale.f32(),
half_size: ellipse.half_size * scale.f32().abs(),
})));
}
} else if _id == 2 {
if let Some(polygon) = shape.as_shape::<RegularPolygonWrapper>() {
if scale.x == scale.y {
return Ok(SharedShape::new(RegularPolygonWrapper(
RegularPolygon::new(
polygon.circumradius() * scale.x as f32,
polygon.circumradius() * scale.x.abs() as f32,
polygon.sides,
),
)));
Expand Down
2 changes: 1 addition & 1 deletion src/collision/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub type PackedFeatureId = parry::shape::PackedFeatureId;
/// - [`iter`](Self::iter) and [`iter_mut`](Self::iter_mut)
/// - [`contains`](Self::contains)
/// - [`collisions_with_entity`](Self::collisions_with_entity) and
/// [`collisions_with_entity_mut`](Self::collisions_with_entity_mut)
/// [`collisions_with_entity_mut`](Self::collisions_with_entity_mut)
///
/// The collisions can be accessed at any time, but modifications to contacts should be performed
/// in the [`PostProcessCollisions`] schedule. Otherwise, the physics solver will use the old contact data.
Expand Down
2 changes: 2 additions & 0 deletions src/dynamics/rigid_body/forces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ pub(crate) type Torque = Scalar;
#[cfg(feature = "3d")]
pub(crate) type Torque = Vector;

#[cfg(feature = "2d")]
pub(crate) trait FloatZero {
const ZERO: Self;
}

#[cfg(feature = "2d")]
impl FloatZero for Scalar {
const ZERO: Self = 0.0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/dynamics/solver/xpbd/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ pub trait XpbdConstraint<const ENTITY_COUNT: usize>: MapEntities {
/// There are two main steps to solving a constraint:
///
/// 1. Compute the generalized inverse masses, [gradients](self#constraint-gradients)
/// and the [Lagrange multiplier](self#lagrange-multipliers) update.
/// and the [Lagrange multiplier](self#lagrange-multipliers) update.
/// 2. Apply corrections along the gradients using the Lagrange multiplier update.
///
/// [`XpbdConstraint`] provides the [`compute_lagrange_update`](XpbdConstraint::compute_lagrange_update)
Expand Down
2 changes: 1 addition & 1 deletion src/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Default for PreparePlugin {
/// 4. `PropagateTransforms`: Responsible for propagating transforms.
/// 5. `InitMassProperties`: Responsible for initializing missing mass properties for [`RigidBody`] components.
/// 6. `InitTransforms`: Responsible for initializing [`Transform`] based on [`Position`] and [`Rotation`]
/// or vice versa.
/// or vice versa.
/// 7. `Finalize`: Responsible for performing final updates after everything is initialized.
#[derive(SystemSet, Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub enum PrepareSet {
Expand Down
10 changes: 5 additions & 5 deletions src/schedule/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ use bevy::{
/// This plugin initializes and configures the following schedules and system sets:
///
/// - [`PhysicsSet`]: High-level system sets for the main phases of the physics engine.
/// You can use these to schedule your own systems before or after physics is run without
/// having to worry about implementation details.
/// You can use these to schedule your own systems before or after physics is run without
/// having to worry about implementation details.
/// - [`PhysicsSchedule`]: Responsible for advancing the simulation in [`PhysicsSet::StepSimulation`].
/// - [`PhysicsStepSet`]: System sets for the steps of the actual physics simulation loop, like
/// the broad phase and the substepping loop.
/// the broad phase and the substepping loop.
/// - [`SubstepSchedule`]: Responsible for running the substepping loop in [`SolverSet::Substep`].
pub struct PhysicsSchedulePlugin {
schedule: Interned<dyn ScheduleLabel>,
Expand Down Expand Up @@ -191,10 +191,10 @@ pub struct PostProcessCollisions;
/// having to worry about implementation details.
///
/// 1. `Prepare`: Responsible for initializing [rigid bodies](RigidBody) and [colliders](Collider) and
/// updating several components.
/// updating several components.
/// 2. `StepSimulation`: Responsible for advancing the simulation by running the steps in [`PhysicsStepSet`].
/// 3. `Sync`: Responsible for synchronizing physics components with other data, like keeping [`Position`]
/// and [`Rotation`] in sync with `Transform`.
/// and [`Rotation`] in sync with `Transform`.
///
/// ## See also
///
Expand Down
22 changes: 11 additions & 11 deletions src/spatial_query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@
//! There are two ways to perform raycasts.
//!
//! 1. For simple raycasts, use the [`RayCaster`] component. It returns the results of the raycast
//! in the [`RayHits`] component every frame. It uses local coordinates, so it will automatically follow the entity
//! it's attached to or its parent.
//! in the [`RayHits`] component every frame. It uses local coordinates, so it will automatically follow the entity
//! it's attached to or its parent.
//! 2. When you need more control or don't want to cast every frame, use the raycasting methods provided by
//! [`SpatialQuery`], like [`cast_ray`](SpatialQuery::cast_ray), [`ray_hits`](SpatialQuery::ray_hits) or
//! [`ray_hits_callback`](SpatialQuery::ray_hits_callback).
//! [`SpatialQuery`], like [`cast_ray`](SpatialQuery::cast_ray), [`ray_hits`](SpatialQuery::ray_hits) or
//! [`ray_hits_callback`](SpatialQuery::ray_hits_callback).
//!
//! See the documentation of the components and methods for more information.
//!
Expand Down Expand Up @@ -83,11 +83,11 @@
//! There are two ways to perform shapecasts.
//!
//! 1. For simple shapecasts, use the [`ShapeCaster`] component. It returns the results of the shapecast
//! in the [`ShapeHits`] component every frame. It uses local coordinates, so it will automatically follow the entity
//! it's attached to or its parent.
//! in the [`ShapeHits`] component every frame. It uses local coordinates, so it will automatically follow the entity
//! it's attached to or its parent.
//! 2. When you need more control or don't want to cast every frame, use the shapecasting methods provided by
//! [`SpatialQuery`], like [`cast_shape`](SpatialQuery::cast_shape), [`shape_hits`](SpatialQuery::shape_hits) or
//! [`shape_hits_callback`](SpatialQuery::shape_hits_callback).
//! [`SpatialQuery`], like [`cast_shape`](SpatialQuery::cast_shape), [`shape_hits`](SpatialQuery::shape_hits) or
//! [`shape_hits_callback`](SpatialQuery::shape_hits_callback).
//!
//! See the documentation of the components and methods for more information.
//!
Expand Down Expand Up @@ -142,11 +142,11 @@
//! and they all have callback variants that call a given callback on each intersection.
//!
//! - [`point_intersections`](SpatialQuery::point_intersections): Finds all entities with a collider that contains
//! the given point.
//! the given point.
//! - [`aabb_intersections_with_aabb`](SpatialQuery::aabb_intersections_with_aabb):
//! Finds all entities with a [`ColliderAabb`] that is intersecting the given [`ColliderAabb`].
//! Finds all entities with a [`ColliderAabb`] that is intersecting the given [`ColliderAabb`].
//! - [`shape_intersections`](SpatialQuery::shape_intersections): Finds all entities with a [collider](Collider)
//! that is intersecting the given shape.
//! that is intersecting the given shape.
//!
//! See the documentation of the components and methods for more information.
//!
Expand Down
24 changes: 12 additions & 12 deletions src/spatial_query/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl SpatialQueryPipeline {
/// - `direction`: What direction the ray is cast in.
/// - `max_time_of_impact`: The maximum distance that the ray can travel.
/// - `solid`: If true and the ray origin is inside of a collider, the hit point will be the ray origin itself.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
/// - `query_filter`: A [`SpatialQueryFilter`] that determines which colliders are taken into account in the query.
///
/// See also: [`SpatialQuery::cast_ray`]
Expand Down Expand Up @@ -196,10 +196,10 @@ impl SpatialQueryPipeline {
/// - `direction`: What direction the ray is cast in.
/// - `max_time_of_impact`: The maximum distance that the ray can travel.
/// - `solid`: If true and the ray origin is inside of a collider, the hit point will be the ray origin itself.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
/// - `query_filter`: A [`SpatialQueryFilter`] that determines which colliders are taken into account in the query.
/// - `predicate`: A function with which the colliders are filtered. Given the Entity it should return false, if the
/// entity should be ignored.
/// entity should be ignored.
///
/// See also: [`SpatialQuery::cast_ray`]
pub fn cast_ray_predicate(
Expand Down Expand Up @@ -241,7 +241,7 @@ impl SpatialQueryPipeline {
/// - `max_time_of_impact`: The maximum distance that the ray can travel.
/// - `max_hits`: The maximum number of hits. Additional hits will be missed.
/// - `solid`: If true and the ray origin is inside of a collider, the hit point will be the ray origin itself.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
/// - `query_filter`: A [`SpatialQueryFilter`] that determines which colliders are taken into account in the query.
///
/// See also: [`SpatialQuery::ray_hits`]
Expand Down Expand Up @@ -280,7 +280,7 @@ impl SpatialQueryPipeline {
/// - `direction`: What direction the ray is cast in.
/// - `max_time_of_impact`: The maximum distance that the ray can travel.
/// - `solid`: If true and the ray origin is inside of a collider, the hit point will be the ray origin itself.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
/// - `query_filter`: A [`SpatialQueryFilter`] that determines which colliders are taken into account in the query.
/// - `callback`: A callback function called for each hit.
///
Expand Down Expand Up @@ -339,8 +339,8 @@ impl SpatialQueryPipeline {
/// - `direction`: What direction the shape is cast in.
/// - `max_time_of_impact`: The maximum distance that the shape can travel.
/// - `ignore_origin_penetration`: If true and the shape is already penetrating a collider at the
/// shape origin, the hit will be ignored and only the next hit will be computed. Otherwise, the initial
/// hit will be returned.
/// shape origin, the hit will be ignored and only the next hit will be computed. Otherwise, the initial
/// hit will be returned.
/// - `query_filter`: A [`SpatialQueryFilter`] that determines which colliders are taken into account in the query.
///
/// See also: [`SpatialQuery::cast_shape`]
Expand Down Expand Up @@ -405,8 +405,8 @@ impl SpatialQueryPipeline {
/// - `max_time_of_impact`: The maximum distance that the shape can travel.
/// - `max_hits`: The maximum number of hits. Additional hits will be missed.
/// - `ignore_origin_penetration`: If true and the shape is already penetrating a collider at the
/// shape origin, the hit will be ignored and only the next hit will be computed. Otherwise, the initial
/// hit will be returned.
/// shape origin, the hit will be ignored and only the next hit will be computed. Otherwise, the initial
/// hit will be returned.
/// - `query_filter`: A [`SpatialQueryFilter`] that determines which colliders are taken into account in the query.
/// - `callback`: A callback function called for each hit.
///
Expand Down Expand Up @@ -452,8 +452,8 @@ impl SpatialQueryPipeline {
/// - `direction`: What direction the shape is cast in.
/// - `max_time_of_impact`: The maximum distance that the shape can travel.
/// - `ignore_origin_penetration`: If true and the shape is already penetrating a collider at the
/// shape origin, the hit will be ignored and only the next hit will be computed. Otherwise, the initial
/// hit will be returned.
/// shape origin, the hit will be ignored and only the next hit will be computed. Otherwise, the initial
/// hit will be returned.
/// - `query_filter`: A [`SpatialQueryFilter`] that determines which colliders are taken into account in the query.
/// - `callback`: A callback function called for each hit.
///
Expand Down Expand Up @@ -528,7 +528,7 @@ impl SpatialQueryPipeline {
///
/// - `point`: The point that should be projected.
/// - `solid`: If true and the point is inside of a collider, the projection will be at the point.
/// Otherwise, the collider will be treated as hollow, and the projection will be at the collider's boundary.
/// Otherwise, the collider will be treated as hollow, and the projection will be at the collider's boundary.
/// - `query_filter`: A [`SpatialQueryFilter`] that determines which colliders are taken into account in the query.
///
/// See also: [`SpatialQuery::project_point`]
Expand Down
Loading

0 comments on commit 5c2188c

Please sign in to comment.