Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Far away object causes panic in broad phase #77

Closed
e00E opened this issue Dec 13, 2020 · 8 comments · Fixed by #350
Closed

Far away object causes panic in broad phase #77

e00E opened this issue Dec 13, 2020 · 8 comments · Fixed by #350

Comments

@e00E
Copy link

e00E commented Dec 13, 2020

thread 'main' panicked at 'assertion failed: proxy.aabb.maxs[dim] >= self.min_bound', .cargo/registry/src/git.luolix.top-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.

@e00E e00E changed the title Far away object can cause panic in broad phase Far away object causes panic in broad phase Dec 13, 2020
@slasktotten
Copy link

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

@aristaeus
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

sebcrozet commented Mar 1, 2021

@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
Copy link
Contributor

DasEtwas commented Apr 14, 2021

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
Copy link
Contributor

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
Copy link

finnbear commented Jan 20, 2022

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants