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

Invalid time_of_impact result for Ball and rotated Polyline #19

Closed
elsid opened this issue Mar 19, 2021 · 3 comments · Fixed by #23
Closed

Invalid time_of_impact result for Ball and rotated Polyline #19

elsid opened this issue Mar 19, 2021 · 3 comments · Fixed by #23

Comments

@elsid
Copy link

elsid commented Mar 19, 2021

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

elsid commented Apr 1, 2021

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

elsid commented Apr 2, 2021

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:

let pos12 = pos1.inv_mul(pos2);
let vel12 = vel2 - vel1;
.

@sebcrozet
Copy link
Member

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.

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