Skip to content

Commit 973f5c2

Browse files
authored
Fix rare panic when violating total ordering invariant (#121)
Polyanya sometimes (very rarely) triggers the following panic in debug: ``` thread 'Compute Task Pool (5)' panicked at library/core/src/slice/sort/shared/smallsort.rs:860:5: user-provided comparison function does not correctly implement a total order stack backtrace: 0: __rustc::rust_begin_unwind 1: core::panicking::panic_fmt 2: core::slice::sort::shared::smallsort::panic_on_ord_violation 3: core::slice::sort::shared::smallsort::bidirectional_merge 4: core::slice::sort::shared::smallsort::small_sort_network 5: polyanya::input::trimesh::<impl core::convert::TryFrom<polyanya::input::trimesh::Trimesh> for polyanya::Mesh>::try_from ``` I fuzzed my way to a test case that reproduces it and am providing a fix for it here.
1 parent 1d73306 commit 973f5c2

File tree

1 file changed

+49
-2
lines changed

1 file changed

+49
-2
lines changed

src/input/trimesh.rs

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,25 @@ impl TryFrom<Trimesh> for Mesh {
7070
};
7171
let edge_a = get_counterclockwise_edge(*index_a);
7272
let edge_b = get_counterclockwise_edge(*index_b);
73-
if edge_a.perp_dot(edge_b) > 0. {
73+
let perp = edge_a.perp_dot(edge_b);
74+
75+
// As of Rust 1.81, sort functions panic in debug mode if they are not provided with
76+
// a total ordering. When edges are collinear, perp_dot(a, b) == 0, and in this case
77+
// the edges should be considered equal. However, due to floating point precision
78+
// errors, we can get situations where three collinear edges a, b, c have
79+
//
80+
// a > b, b > c, and c > a
81+
//
82+
// but this violates the ordering invariant and causes a panic. To mitigate, we allow
83+
// for some imprecision in perp_dot and treat these edges as collinear.
84+
const EPSILON: f32 = 1e-6;
85+
86+
if perp > EPSILON {
7487
Ordering::Less
75-
} else {
88+
} else if perp < -EPSILON {
7689
Ordering::Greater
90+
} else {
91+
Ordering::Equal
7792
}
7893
});
7994

@@ -249,6 +264,38 @@ mod tests {
249264
assert!(matches!(trimesh, Err(MeshError::InvalidMesh)));
250265
}
251266

267+
#[test]
268+
fn collinear_edges_total_ordering() {
269+
// This particular test case was found by fuzzing to look for the edge case
270+
// where floating point errors cause total ordering to be violated.
271+
let mut vertices = Vec::new();
272+
for ray_index in 0..12 {
273+
let angle = (ray_index as f32) * std::f32::consts::TAU / (12 as f32);
274+
let angle_offset = ((7 as f32) * 0.1).sin() * 0.01;
275+
let dir = Vec2::from_angle(angle + angle_offset);
276+
277+
for dist_index in 0..2 {
278+
let dist = 1.0 + (dist_index as f32) * 0.5;
279+
vertices.push(dir * dist);
280+
}
281+
}
282+
283+
let center_idx = vertices.len();
284+
vertices.push(Vec2::ZERO);
285+
286+
let triangles: Vec<[usize; 3]> = (0..vertices.len() - 1)
287+
.map(|i| [center_idx, i, (i + 1) % (vertices.len() - 1)])
288+
.collect();
289+
290+
let result: Result<Mesh, _> = Trimesh {
291+
vertices,
292+
triangles,
293+
}
294+
.try_into();
295+
296+
assert!(result.is_ok());
297+
}
298+
252299
fn wrap_to_first(polygons: &[u32], pred: impl Fn(&u32) -> bool) -> Option<Vec<u32>> {
253300
let offset = polygons.iter().position(pred)?;
254301
Some(

0 commit comments

Comments
 (0)