Skip to content

Commit ea84f40

Browse files
izzymgsebcrozet
andauthored
Fix leaf traversal yielding non-passing leaves (#372)
* Fix leaf traversal bug causing non-passing leaves to be yielded The iterator did not check the parent node against the check fn. * feat: avoid checking every leaves twice in leaf iterator. * chore: cargo fmt * fix warning * fix test --------- Co-authored-by: Sébastien Crozet <sebcrozet@dimforge.com>
1 parent d955f75 commit ea84f40

File tree

4 files changed

+56
-25
lines changed

4 files changed

+56
-25
lines changed

src/partitioning/bvh/bvh_tests.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,31 @@ fn make_test_aabb(i: usize) -> Aabb {
66
Aabb::from_half_extents(Vector::repeat(i as Real).into(), Vector::repeat(1.0))
77
}
88

9+
#[test]
10+
fn test_leaves_iteration() {
11+
let leaves = [
12+
make_test_aabb(0), // mins at (0,0,0) - should pass
13+
make_test_aabb(5), // mins at (5,5,5) - should be filtered out
14+
];
15+
let bvh = Bvh::from_leaves(BvhBuildStrategy::Binned, &leaves);
16+
17+
// Only allow nodes with mins.x <= 3.0 (should only pass leaf 0)
18+
let check = |node: &crate::partitioning::BvhNode| -> bool { node.mins.x <= 3.0 };
19+
20+
let mut found_invalid_leaf = false;
21+
for leaf_index in bvh.leaves(check) {
22+
if leaf_index == 1 {
23+
// This is the leaf that should be filtered out
24+
found_invalid_leaf = true;
25+
break;
26+
}
27+
}
28+
29+
if found_invalid_leaf {
30+
panic!("Leaves iterator returned an invalid leaf");
31+
}
32+
}
33+
934
#[test]
1035
fn bvh_build_and_removal() {
1136
// Check various combination of building pattern and removal pattern.

src/partitioning/bvh/bvh_traverse.rs

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,30 @@ pub struct Leaves<'a, Check: Fn(&BvhNode) -> bool> {
1212
check: Check,
1313
}
1414

15+
impl<'a, Check: Fn(&BvhNode) -> bool> Leaves<'a, Check> {
16+
pub fn new(tree: &'a Bvh, check: Check) -> Leaves<'a, Check> {
17+
let mut stack = SmallVec::default();
18+
let mut next = None;
19+
20+
if let Some(root) = tree.nodes.first() {
21+
if check(&root.left) {
22+
next = Some(&root.left);
23+
}
24+
25+
if root.right.leaf_count() > 0 && check(&root.right) {
26+
stack.push(&root.right);
27+
}
28+
}
29+
30+
Leaves {
31+
tree,
32+
next,
33+
stack,
34+
check,
35+
}
36+
}
37+
}
38+
1539
impl<'a, Check: Fn(&BvhNode) -> bool> Iterator for Leaves<'a, Check> {
1640
type Item = u32;
1741
fn next(&mut self) -> Option<Self::Item> {
@@ -77,30 +101,11 @@ impl Bvh {
77101
/// See also the [`Bvh::traverse`] function which is slightly less convenient since it doesn’t
78102
/// rely on the iterator system, but takes a closure that implements [`FnMut`] instead of [`Fn`].
79103
pub fn leaves<F: Fn(&BvhNode) -> bool>(&self, check_node: F) -> Leaves<'_, F> {
80-
if let Some(root) = self.nodes.first() {
81-
let mut stack = SmallVec::default();
82-
if root.right.leaf_count() > 0 {
83-
stack.push(&root.right);
84-
}
85-
86-
Leaves {
87-
tree: self,
88-
next: Some(&root.left),
89-
stack,
90-
check: check_node,
91-
}
92-
} else {
93-
Leaves {
94-
tree: self,
95-
next: None,
96-
stack: Default::default(),
97-
check: check_node,
98-
}
99-
}
104+
Leaves::new(self, check_node)
100105
}
101106
}
102107

103-
/// Controls the execution flowo of [`Bvh::traverse`].
108+
/// Controls the execution flow of [`Bvh::traverse`].
104109
pub enum TraversalAction {
105110
/// The traversal will continue on the children of the tested node.
106111
Continue,

src/shape/shape.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1253,7 +1253,7 @@ impl Shape for ConvexPolygon {
12531253
ShapeType::ConvexPolygon
12541254
}
12551255

1256-
fn as_typed_shape(&self) -> TypedShape {
1256+
fn as_typed_shape(&self) -> TypedShape<'_> {
12571257
TypedShape::ConvexPolygon(self)
12581258
}
12591259

src/transformation/mesh_intersection/mesh_intersection.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,9 @@ fn triangulate_constraints_and_merge_duplicates(
439439
let p1_id = insert_into_set(point_pair[0], &mut point_set, epsilon);
440440
let p2_id = insert_into_set(point_pair[1], &mut point_set, epsilon);
441441

442-
edges.push([p1_id, p2_id]);
442+
if p1_id != p2_id {
443+
edges.push([p1_id, p2_id]);
444+
}
443445
}
444446

445447
let mut points: Vec<_> = point_set.iter().cloned().collect();
@@ -462,8 +464,7 @@ fn triangulate_constraints_and_merge_duplicates(
462464
utils::sanitize_spade_point(point_proj)
463465
})
464466
.collect();
465-
let cdt_triangulation =
466-
ConstrainedDelaunayTriangulation::bulk_load_cdt_stable(planar_points, edges)?;
467+
let cdt_triangulation = ConstrainedDelaunayTriangulation::bulk_load_cdt(planar_points, edges)?;
467468
debug_assert!(cdt_triangulation.vertices().len() == points.len());
468469

469470
let points = points.into_iter().map(|p| Point3::from(p.point)).collect();

0 commit comments

Comments
 (0)