Skip to content

Commit

Permalink
Prevent panic in epa::closest_points
Browse files Browse the repository at this point in the history
We now check whether at least one the proj_inside computations is true, and return None if not. Also rename some variables in epa2 to match the corresponding variables in epa3.
  • Loading branch information
wlinna committed Sep 5, 2024
1 parent e24a29c commit 4ab5a7d
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 8 deletions.
37 changes: 35 additions & 2 deletions crates/parry3d/tests/geometry/epa3.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use na::{self, Isometry3, Vector3};
use na::{self, Isometry3, Point3, Vector3};
use parry3d::query;
use parry3d::shape::Cuboid;
use parry3d::query::gjk::VoronoiSimplex;
use parry3d::shape::{Cuboid, Triangle};

#[test]
#[allow(non_snake_case)]
Expand All @@ -20,3 +21,35 @@ fn cuboid_cuboid_EPA() {
assert_eq!(res.dist, -1.8);
assert_eq!(res.normal1, -Vector3::y_axis());
}

#[test]
fn triangle_vertex_touches_triangle_edge_epa() {
// Related issues:
// https://github.com/dimforge/parry/issues/253
// https://github.com/dimforge/parry/issues/246

let mesh1 = Triangle::new(
Point3::new(-13.174434, 1.0, 8.736801),
Point3::new(3.5251038, 1.0, 12.1),
Point3::new(3.2048466, 1.0, 12.218325),
);
let mesh2 = Triangle::new(
Point3::new(-1.63, 0.0, 11.19),
Point3::new(-2.349647, 0.0, 11.037681),
Point3::new(-2.349647, 1.0, 11.037681),
);

// TODO: Check the return-value of the function once
// it the function's correctness issue have been taken care of
// Right now we just want it not to crash
let gjk_result = query::details::contact_support_map_support_map_with_params(
&Isometry3::identity(),
&mesh1,
&mesh2,
0.00999999977,
&mut VoronoiSimplex::new(),
None,
);

assert!(!matches!(gjk_result, query::gjk::GJKResult::NoIntersection(_)), "PARTIAL SUCCESS: contact_support_map_support_map_with_params did not crash but did not produce the desired result");
}
17 changes: 11 additions & 6 deletions src/query/epa/epa2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,28 +226,33 @@ impl EPA {
let pts2 = [1, 2];
let pts3 = [2, 0];

let (face1, proj_is_inside1) = Face::new(&self.vertices, pts1);
let (face2, proj_is_inside2) = Face::new(&self.vertices, pts2);
let (face3, proj_is_inside3) = Face::new(&self.vertices, pts3);
let (face1, proj_inside1) = Face::new(&self.vertices, pts1);
let (face2, proj_inside2) = Face::new(&self.vertices, pts2);
let (face3, proj_inside3) = Face::new(&self.vertices, pts3);

self.faces.push(face1);
self.faces.push(face2);
self.faces.push(face3);

if proj_is_inside1 {
if proj_inside1 {
let dist1 = self.faces[0].normal.dot(&self.vertices[0].point.coords);
self.heap.push(FaceId::new(0, -dist1)?);
}

if proj_is_inside2 {
if proj_inside2 {
let dist2 = self.faces[1].normal.dot(&self.vertices[1].point.coords);
self.heap.push(FaceId::new(1, -dist2)?);
}

if proj_is_inside3 {
if proj_inside3 {
let dist3 = self.faces[2].normal.dot(&self.vertices[2].point.coords);
self.heap.push(FaceId::new(2, -dist3)?);
}

if !(proj_inside1 || proj_inside2 || proj_inside3) {
log::debug!("Hit unexpected state in EPA: proj_inside1, proj_inside2 and proj_inside3 are all false. At least one of them should be true.");
return None;
}
} else {
let pts1 = [0, 1];
let pts2 = [1, 0];
Expand Down
5 changes: 5 additions & 0 deletions src/query/epa/epa3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,11 @@ impl EPA {
let dist4 = self.faces[3].normal.dot(&self.vertices[3].point.coords);
self.heap.push(FaceId::new(3, -dist4)?);
}

if !(proj_inside1 || proj_inside2 || proj_inside3 || proj_inside4) {
log::debug!("Hit unexpected state in EPA: proj_is_inside1, proj_is_inside2, proj_is_inside3 and proj_inside4 are all false. At least one of them should be true.");
return None;
}
} else {
if simplex.dimension() == 1 {
let dpt = self.vertices[1] - self.vertices[0];
Expand Down

0 comments on commit 4ab5a7d

Please sign in to comment.