dimforge / parry

2D and 3D collision-detection library for Rust.
https://parry.rs
Apache License 2.0
557 stars 97 forks source link

Invalid time_of_impact result for Ball and rotated Polyline #19

Closed elsid closed 3 years ago

elsid commented 3 years ago

When using rotated Polyline time_of_impact returns a result as it is not rotated. See tests below for details.

This test fails because time_of_impact returns None:

#[test]
fn time_of_impact_should_return_toi_for_ball_and_rotated_polyline() {
    let ball_isometry = Isometry2::identity();
    let ball_velocity = Vector2::new(1.0, 0.0);
    let ball = Ball::new(0.5);
    let polyline_isometry = Isometry2::rotation(-std::f32::consts::FRAC_PI_2);
    let polyline_velocity = Vector2::zeros();
    let polyline = Polyline::new(vec![Point2::new(1.0, 1.0), Point2::new(-1.0, 1.0)], None);

    assert_eq!(polyline_isometry.transform_point(&Point2::new(1.0, 1.0)), Point2::new(0.99999994, -1.0));
    assert_eq!(polyline_isometry.transform_point(&Point2::new(-1.0, 1.0)), Point2::new(1.0, 0.99999994));

    let toi = query::time_of_impact(
        &ball_isometry, &ball_velocity, &ball,
        &polyline_isometry, &polyline_velocity, &polyline,
        1.0, 0.0,
    ).unwrap();

    assert_eq!(toi.unwrap().toi, 0.5);
}

But this one succeed when Ball with given velocity should not reach polyline at all:

#[test]
fn invalid_time_of_impact_should_return_toi_for_ball_and_rotated_polyline() {
    let ball_isometry = Isometry2::identity();
    let ball_velocity = Vector2::new(0.0, 1.0);
    let ball = Ball::new(0.5);
    let polyline_isometry = Isometry2::rotation(-std::f32::consts::FRAC_PI_2);
    let polyline_velocity = Vector2::zeros();
    let polyline = Polyline::new(vec![Point2::new(1.0, 1.0), Point2::new(-1.0, 1.0)], None);

    assert_eq!(polyline_isometry.transform_point(&Point2::new(1.0, 1.0)), Point2::new(0.99999994, -1.0));
    assert_eq!(polyline_isometry.transform_point(&Point2::new(-1.0, 1.0)), Point2::new(1.0, 0.99999994));

    let toi = query::time_of_impact(
        &ball_isometry, &ball_velocity, &ball,
        &polyline_isometry, &polyline_velocity, &polyline,
        1.0, 0.0,
    ).unwrap();

    assert_eq!(toi.unwrap().toi, 0.5);
}

However when using a Segment time_of_impact behaves correctly:

#[test]
fn time_of_impact_should_return_toi_for_ball_and_rotated_segment() {
    let ball_isometry = Isometry2::identity();
    let ball_velocity = Vector2::new(1.0, 0.0);
    let ball = Ball::new(0.5);
    let segment_isometry = Isometry2::rotation(-std::f32::consts::FRAC_PI_2);
    let segment_velocity = Vector2::zeros();
    let segment = Segment::new(Point2::new(1.0, 1.0), Point2::new(-1.0, 1.0));

    assert_eq!(segment_isometry.transform_point(&Point2::new(1.0, 1.0)), Point2::new(0.99999994, -1.0));
    assert_eq!(segment_isometry.transform_point(&Point2::new(-1.0, 1.0)), Point2::new(1.0, 0.99999994));

    let toi = query::time_of_impact(
        &ball_isometry, &ball_velocity, &ball,
        &segment_isometry, &segment_velocity, &segment,
        1.0, 0.0,
    ).unwrap();

    assert_eq!(toi.unwrap().toi, 0.49999994);
}
elsid commented 3 years ago

Not sure is it related but time_of_impact is not commutative for Ball and Segment. If ball and segment arguments are swapped in time_of_impact_should_return_toi_for_ball_and_rotated_segment then the test does not pass because time_of_impact returns None:

#[test]
fn time_of_impact_should_return_toi_for_rotated_segment_and_ball() {
    let ball_isometry = Isometry2::identity();
    let ball_velocity = Vector2::new(1.0, 0.0);
    let ball = Ball::new(0.5);
    let segment_isometry = Isometry2::rotation(-std::f32::consts::FRAC_PI_2);
    let segment_velocity = Vector2::zeros();
    let segment = Segment::new(Point2::new(1.0, 1.0), Point2::new(-1.0, 1.0));

    assert_eq!(segment_isometry.transform_point(&Point2::new(1.0, 1.0)), Point2::new(0.99999994, -1.0));
    assert_eq!(segment_isometry.transform_point(&Point2::new(-1.0, 1.0)), Point2::new(1.0, 0.99999994));

    let toi = query::time_of_impact(
        &segment_isometry, &segment_velocity, &segment,
        &ball_isometry, &ball_velocity, &ball,
        1.0, 0.0,
    ).unwrap();

    assert_eq!(toi.unwrap().toi, 0.49999994);
}
elsid commented 3 years ago

The same tests are passing with ncollide. The main difference I see between these libraries is an attempt to use only one transformation and velocity in parry: https://github.com/dimforge/parry/blob/f48629e173cd4e751d436799f8f2bef7bc5c0d5b/src/query/time_of_impact/time_of_impact.rs#L87-L88 .

sebcrozet commented 3 years ago

Thank you for the test cases! You are correct, there is a bug as it should be let vel12 = pos1.inverse_transform_vector(&(vel2 - vel1)) because the velocity must be expressed in the local-space of the first shape.

This will be fixed at part of today's release.