Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
2 changes: 1 addition & 1 deletion src/dynamics/rigid_body_components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ impl RigidBodyColliders {

// Set the modification flag so we can benefit from the modification-tracking
// when updating the narrow-phase/broad-phase afterwards.
co.changes |= ColliderChanges::POSITION;
co.changes |= ColliderChanges::MODIFIED | ColliderChanges::POSITION;
Copy link
Copy Markdown
Contributor Author

@ThierryBerger ThierryBerger Mar 5, 2025

Choose a reason for hiding this comment

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

This makes me wonder if we should change ColliderChanges::POSITION (and the likes) from:
const POSITION = 1 << 3
to
const POSITION = 1 << 3 | 1 << 0; (adding ColliderChanges::MODIFIED to it)

This would clarify their relation and make mistakes harder to make.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change is probably not possible because MODIFIED is used to check if a collider is part of the modified_colliders list. This seems difficult to maintain, I imagine searching for modified_colliders.push should be able to lead to a MODIFIED bit to be added to that collider, but it's sometimes tricky to assess.

I imagine changing modified_colliders to a Hashmap may be less performant than storing that in the bitfield, but I'm throwing the idea.

co.pos = ColliderPosition(new_pos);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/geometry/collider_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ impl ColliderSet {
self.modified_colliders.clear();
let modified_colliders = &mut self.modified_colliders;
self.colliders.iter_mut().map(move |(h, b)| {
b.changes |= ColliderChanges::MODIFIED;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure this is correct ; the fix may be more around https://github.com/Vrixyz/rapier/blob/607f0988596d346b6afa70e5cfbc8f0f3519e02c/src/geometry/collider.rs#L271 ; to set ColliderChanges::MODIFIED flag too (see https://github.com/dimforge/rapier/pull/803/files#r1980964215 comment for a longer term fix maybe ?)

And then add then refill self.modified_colliders only when applicable (check changes flag)

modified_colliders.push(ColliderHandle(h));
(ColliderHandle(h), b)
})
Expand Down
62 changes: 62 additions & 0 deletions src/pipeline/physics_pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1011,4 +1011,66 @@ mod test {
assert!(rotation.w.is_finite());
}
}

#[test]
#[cfg(feature = "dim2")]
fn test_multi_sap_disable_body() {
use na::vector;
let mut rigid_body_set = RigidBodySet::new();
let mut collider_set = ColliderSet::new();

/* Create the ground. */
let collider = ColliderBuilder::cuboid(100.0, 0.1).build();
collider_set.insert(collider);

/* Create the bouncing ball. */
let rigid_body = RigidBodyBuilder::dynamic()
.translation(vector![0.0, 10.0])
.build();
let collider = ColliderBuilder::ball(0.5).restitution(0.7).build();
let ball_body_handle = rigid_body_set.insert(rigid_body);
collider_set.insert_with_parent(collider, ball_body_handle, &mut rigid_body_set);

/* Create other structures necessary for the simulation. */
let gravity = vector![0.0, -9.81];
let integration_parameters = IntegrationParameters::default();
let mut physics_pipeline = PhysicsPipeline::new();
let mut island_manager = IslandManager::new();
let mut broad_phase = BroadPhaseMultiSap::new();
let mut narrow_phase = NarrowPhase::new();
let mut impulse_joint_set = ImpulseJointSet::new();
let mut multibody_joint_set = MultibodyJointSet::new();
let mut ccd_solver = CCDSolver::new();
let physics_hooks = ();
let event_handler = ();

/* Run the game loop, stepping the simulation once per frame. */
for _ in 0..200 {
physics_pipeline.step(
&gravity,
&integration_parameters,
&mut island_manager,
&mut broad_phase,
&mut narrow_phase,
&mut rigid_body_set,
&mut collider_set,
&mut impulse_joint_set,
&mut multibody_joint_set,
&mut ccd_solver,
None,
&physics_hooks,
&event_handler,
);

let ball_body = &mut rigid_body_set[ball_body_handle];
println!("Ball altitude: {}", ball_body.translation().y);

// This causes the error:
// Modify the translation (or rotation) (RigidBodyChanges::POSITION)
// Modify enabled
ball_body.set_translation(vector![0.0, 0.0], true);
// ball_body.set_rotation(nalgebra::UnitComplex::new(1.0), true);
ball_body.set_enabled(false);
}
}
Copy link
Copy Markdown
Contributor

@Johannes0021 Johannes0021 Mar 5, 2025

Choose a reason for hiding this comment

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

This might be a better test because it covers both RigidBodyChanges::POSITION with enable and disable, and it doesn't run for 200 iterations.
What do you think?

use rapier2d::prelude::*;

fn main() {
    // Create other structures necessary for the simulation.
    let mut rigid_body_set = RigidBodySet::new();
    let mut collider_set = ColliderSet::new();
    let gravity = vector![0.0, -9.81];
    let integration_parameters = IntegrationParameters::default();
    let mut physics_pipeline = PhysicsPipeline::new();
    let mut island_manager = IslandManager::new();
    let mut broad_phase = DefaultBroadPhase::new();
    let mut narrow_phase = NarrowPhase::new();
    let mut impulse_joint_set = ImpulseJointSet::new();
    let mut multibody_joint_set = MultibodyJointSet::new();
    let mut ccd_solver = CCDSolver::new();
    let mut query_pipeline = QueryPipeline::new();
    let physics_hooks = ();
    let event_handler = ();


    // Create the ball with default values for translation, position and enabled state
    let rigid_body = RigidBodyBuilder::dynamic()
        .translation(vector![0.0, 0.0])
        .rotation(0.0)
        .build();
    let collider = ColliderBuilder::ball(0.5)
        .enabled(true)
        .build();
    let ball_body_handle = rigid_body_set.insert(rigid_body);
    collider_set.insert_with_parent(collider, ball_body_handle, &mut rigid_body_set);


    physics_pipeline.step(
        &gravity,
        &integration_parameters,
        &mut island_manager,
        &mut broad_phase,
        &mut narrow_phase,
        &mut rigid_body_set,
        &mut collider_set,
        &mut impulse_joint_set,
        &mut multibody_joint_set,
        &mut ccd_solver,
        Some(&mut query_pipeline),
        &physics_hooks,
        &event_handler,
    );


    // This causes the error:

    // Test RigidBodyChanges::POSITION and disable
    {
        let ball_body = &mut rigid_body_set[ball_body_handle];

        // Also, change the translation and rotation to different values
        ball_body.set_translation(vector![1.0, 1.0], true);
        ball_body.set_rotation(nalgebra::UnitComplex::new(1.0), true);

        ball_body.set_enabled(false);
    }

    physics_pipeline.step(
        &gravity,
        &integration_parameters,
        &mut island_manager,
        &mut broad_phase,
        &mut narrow_phase,
        &mut rigid_body_set,
        &mut collider_set,
        &mut impulse_joint_set,
        &mut multibody_joint_set,
        &mut ccd_solver,
        Some(&mut query_pipeline),
        &physics_hooks,
        &event_handler,
    );


    // Test RigidBodyChanges::POSITION and enable
    {
        let ball_body = &mut rigid_body_set[ball_body_handle];

        // Also, change the translation and rotation to different values
        ball_body.set_translation(vector![0.0, 0.0], true);
        ball_body.set_rotation(nalgebra::UnitComplex::new(0.0), true);

        ball_body.set_enabled(true);
    }

    physics_pipeline.step(
        &gravity,
        &integration_parameters,
        &mut island_manager,
        &mut broad_phase,
        &mut narrow_phase,
        &mut rigid_body_set,
        &mut collider_set,
        &mut impulse_joint_set,
        &mut multibody_joint_set,
        &mut ccd_solver,
        Some(&mut query_pipeline),
        &physics_hooks,
        &event_handler,
    );
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I rewrote it again, set default values at the beginning, and ensured that the values change.

}