Fix user-change handling for colliders as well as disabled colliders (#900)

* feat: add debug-demo for disabling a collider

* feat: add a simple debug-demo with two cubes

* feat: rename RigidBodyChangnes::MODIFIED and ColliderChanges::MODIFIED to ::IN_MODIFIED_SET

* feat: render debug-colliders with a different color with the debug-renderer

* chore: wire up new examples to the testbed

* fix colliders user-modification being ignored after the first step

* fix broad-phase still taking into account disabled colliders with enabled dynamic rigid-bodies

* chore: update changelog

* fix cargo doc
This commit is contained in:
Sébastien Crozet
2025-11-14 09:35:02 +01:00
committed by GitHub
parent a68d0c600b
commit ef5dcaccaf
11 changed files with 146 additions and 26 deletions

View File

@@ -111,8 +111,8 @@ bitflags::bitflags! {
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
/// Flags describing how the rigid-body has been modified by the user.
pub struct RigidBodyChanges: u32 {
/// Flag indicating that any component of this rigid-body has been modified.
const MODIFIED = 1 << 0;
/// Flag indicating that this rigid-body is in the modified rigid-body set.
const IN_MODIFIED_SET = 1 << 0;
/// Flag indicating that the `RigidBodyPosition` component of this rigid-body has been modified.
const POSITION = 1 << 1;
/// Flag indicating that the `RigidBodyActivation` component of this rigid-body has been modified.
@@ -1061,10 +1061,7 @@ impl RigidBodyColliders {
co_handle: ColliderHandle,
) {
if let Some(i) = self.0.iter().position(|e| *e == co_handle) {
rb_changes.set(
RigidBodyChanges::MODIFIED | RigidBodyChanges::COLLIDERS,
true,
);
rb_changes.set(RigidBodyChanges::COLLIDERS, true);
self.0.swap_remove(i);
}
}
@@ -1083,10 +1080,7 @@ impl RigidBodyColliders {
co_shape: &ColliderShape,
co_mprops: &ColliderMassProps,
) {
rb_changes.set(
RigidBodyChanges::MODIFIED | RigidBodyChanges::COLLIDERS,
true,
);
rb_changes.set(RigidBodyChanges::COLLIDERS, true);
co_pos.0 = rb_pos.position * co_parent.pos_wrt_parent;
rb_ccd.ccd_thickness = rb_ccd.ccd_thickness.min(co_shape.ccd_thickness());
@@ -1113,6 +1107,8 @@ impl RigidBodyColliders {
) {
for handle in &self.0 {
// NOTE: the ColliderParent component must exist if we enter this method.
// NOTE: currently, we are propagating the position even if the collider is disabled.
// Is that the best behavior?
let co = colliders.index_mut_internal(*handle);
let new_pos = parent_pos * co.parent.as_ref().unwrap().pos_wrt_parent;

View File

@@ -31,12 +31,12 @@ pub(crate) type ModifiedRigidBodies = ModifiedObjects<RigidBodyHandle, RigidBody
impl HasModifiedFlag for RigidBody {
#[inline]
fn has_modified_flag(&self) -> bool {
self.changes.contains(RigidBodyChanges::MODIFIED)
self.changes.contains(RigidBodyChanges::IN_MODIFIED_SET)
}
#[inline]
fn set_modified_flag(&mut self) {
self.changes |= RigidBodyChanges::MODIFIED;
self.changes |= RigidBodyChanges::IN_MODIFIED_SET;
}
}

View File

@@ -69,10 +69,6 @@ impl BroadPhaseBvh {
/// sent previously and no `RemovePair` happened since then). Sending redundant events is allowed
/// but can result in a slight computational overhead.
///
/// The `colliders` set is mutable only to provide access to
/// [`collider.set_internal_broad_phase_proxy_index`]. Other properties of the collider should
/// **not** be modified during the broad-phase update.
///
/// # Parameters
/// - `params`: the integration parameters governing the simulation.
/// - `colliders`: the set of colliders. Change detection with `collider.needs_broad_phase_update()`

View File

@@ -35,8 +35,8 @@ bitflags::bitflags! {
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
/// Flags describing how the collider has been modified by the user.
pub struct ColliderChanges: u32 {
/// Flag indicating that any component of the collider has been modified.
const MODIFIED = 1 << 0;
/// Flag indicating that the collider handle is in the changed collider set.
const IN_MODIFIED_SET = 1 << 0;
/// Flag indicating that the density or mass-properties of this collider was changed.
const LOCAL_MASS_PROPERTIES = 1 << 1; // => RigidBody local mass-properties update.
/// Flag indicating that the `ColliderParent` component of the collider has been modified.

View File

@@ -11,12 +11,12 @@ pub type ModifiedColliders = ModifiedObjects<ColliderHandle, Collider>;
impl HasModifiedFlag for Collider {
#[inline]
fn has_modified_flag(&self) -> bool {
self.changes.contains(ColliderChanges::MODIFIED)
self.changes.contains(ColliderChanges::IN_MODIFIED_SET)
}
#[inline]
fn set_modified_flag(&mut self) {
self.changes |= ColliderChanges::MODIFIED;
self.changes |= ColliderChanges::IN_MODIFIED_SET;
}
}

View File

@@ -327,6 +327,8 @@ impl DebugRenderPipeline {
c[2] * coeff[2],
c[3] * coeff[3],
]
} else if !co.is_enabled() {
self.style.disabled_color_multiplier
} else {
self.style.collider_parentless_color
};

View File

@@ -82,6 +82,12 @@ impl PhysicsPipeline {
colliders: &mut ColliderSet,
modified_colliders: &mut ModifiedColliders,
) {
// TODO: we cant just iterate on `modified_colliders` here to clear the
// flags because the last substep will leave some colliders with
// changes flags set after solving, but without the collider being
// part of the `ModifiedColliders` set. This is a bit error-prone but
// is necessary for the modified information to carry on to the
// next frames narrow-phase for updating.
for co in colliders.colliders.iter_mut() {
co.1.changes = ColliderChanges::empty();
}
@@ -700,12 +706,32 @@ impl PhysicsPipeline {
// If we ran the last substep, just update the broad-phase bvh instead
// of a full collision-detection step.
for handle in modified_colliders.iter() {
let co = &colliders[*handle];
let aabb = co.compute_broad_phase_aabb(&integration_parameters, bodies);
broad_phase.set_aabb(&integration_parameters, *handle, aabb);
let co = colliders.index_mut_internal(*handle);
// NOTE: `advance_to_final_positions` might have added disabled colliders to
// `modified_colliders`. This raises the question: do we want
// rigid-body transform propagation to happen on disabled colliders if
// their parent rigid-body is enabled? For now, we are propagating as
// it feels less surprising to the user and makes handling collider
// re-enable less awkward.
if co.is_enabled() {
let aabb = co.compute_broad_phase_aabb(&integration_parameters, bodies);
broad_phase.set_aabb(&integration_parameters, *handle, aabb);
}
// Clear the modified collider set, but keep the other collider changes flags.
// This is needed so that the narrow-phase at the next timestep knows it must
// not skip these colliders for its update.
// TODO: this doesnt feel very clean, but leaving the collider in the modified
// set would be expensive as this will be traversed by all the user-changes
// functions. An alternative would be to maintain a second modified set,
// one for user changes, and one for changes applied by the solver but that
// feels a bit too much. Lets keep it simple for now and well see how it
// goes after the persistent island rework.
co.changes.remove(ColliderChanges::IN_MODIFIED_SET);
}
// Empty the modified colliders set. See comment for `co.change.remove(..)` above.
modified_colliders.clear();
// self.clear_modified_colliders(colliders, &mut modified_colliders);
}
}