dimforge / rapier

2D and 3D physics engines focused on performance.
https://rapier.rs
Apache License 2.0
3.97k stars 248 forks source link

Far away object causes panic in broad phase #77

Closed e00E closed 2 years ago

e00E commented 3 years ago

thread 'main' panicked at 'assertion failed: proxy.aabb.maxs[dim] >= self.min_bound', .cargo/registry/src/github.com-1ecc6299db9ec823/rapier3d-0.4.2/src/geometry/broad_phase_multi_sap.rs:172:13 stack backtrace: 0: std::panicking::begin_panic 1: rapier3d::geometry::broad_phase_multi_sap::SAPAxis::batch_insert 2: rapier3d::geometry::broad_phase_multi_sap::BroadPhase::update_regions 3: rapier3d::geometry::broad_phase_multi_sap::BroadPhase::find_pairs 4: rapier3d::pipeline::physics_pipeline::PhysicsPipeline::step 5: bug_tests::main

This can happen when an object is free falling without a floor.

If the panic is intentional it should be documented under what conditions it can occur and what users can do to prevent it.

slasktotten commented 3 years ago

We started running into this as well and agree. Some more information regarding what might be causing it would be super helpful.

aristaeus commented 3 years ago

I've written an example to reproduce this issue.

use rapier3d_f64::dynamics::*;
use rapier3d_f64::geometry::*;
use rapier3d_f64::pipeline::*;
use rapier3d_f64::math::*;

pub struct DummyEventHandler;
impl EventHandler for DummyEventHandler {
    fn handle_intersection_event(&self, _: IntersectionEvent) {
    }

    fn handle_contact_event(&self, _: ContactEvent) {
    }
}

fn main() {
    let mut bodies = RigidBodySet::new();
    let mut colliders = ColliderSet::new();
    let mut joints = JointSet::new();
    let mut pipeline = PhysicsPipeline::new();
    let mut broad_phase = BroadPhase::new();
    let mut narrow_phase = NarrowPhase::new();
    let integration_params = IntegrationParameters::default();

    let rigid_body = RigidBodyBuilder::new_dynamic().position(Isometry::translation(0.0, 5.0e10, 0.0));
    let rigid_body = bodies.insert(rigid_body.build());
    colliders.insert(ColliderBuilder::ball(1.0).build(), rigid_body, &mut bodies);

    pipeline.step(
        &Vector::zeros(),
        &integration_params,
        &mut broad_phase,
        &mut narrow_phase,
        &mut bodies,
        &mut colliders,
        &mut joints,
        None,
        None,
        &DummyEventHandler
    );
}
aristaeus commented 3 years ago

After digging through the source code, I've identified the issue. When creating keys for the region hash map in the broad phase we hit ~undefined behaviour~ in the float -> int cast.

fn point_key(point: Point<Real>) -> Point<i32> {
    (point / CELL_WIDTH).coords.map(|e| e.floor() as i32).into()
}

When point / CELL_WIDTH is bigger than i32::MAX we saturate the int.

I think it's worth at least making the key a Point<i64> for the f64 library, and adding asserts to help users if they still encounter this issue in the future.

sebcrozet commented 3 years ago

@aristaeus Thank you for looking into this in details.

I think it's worth at least making the key a Point for the f64 library, and adding asserts to help users if they still encounter this issue in the future.

Yes, using i64 for the f64 version of the library makes sense. Adding an assert would be useful too.

Users will still encounter this issue in the future (at least with the f32 version), so even with these changes this issue should remain open. A proper fix would be to have and additional SAP regions that handles all these "out of bounds" AABBs.

DasEtwas commented 3 years ago

Those assertions also trigger when a NaN slips into the simulation. In my case, setting a Collider's density to 0.0 caused the proxy.aabb.min and max to have -4.49423e308 as value (the negative value of a constant with special meaning in the code).

darthdeus commented 2 years ago

I just ran into this yesterday setting a NaN velocity due to doing something like linvel = linvel.normalized() * 10.0, which would cause a NaN when linvel.magnitude() == 0.0. Posting this here in case someone else runs into the same issue like I did.

finnbear commented 2 years ago

Can confirm this crash, when a rigid body gets too far away, affects the f32 version of the library (0.12.0-alpha.1). Will investigate deleting out-of-bounds rigid bodies as a potential workaround.

ricochet1k commented 2 years ago

So rather than crashing, can we switch to to_int_unchecked instead? When objects are that far out of range I don't really care what the behavior is as long as it isn't crashing.