Skip to content

Commit f170afa

Browse files
committed
gh-50 Optimize PointCoincidenceChecker to sort only shorter array
- Sort and dedup only the shorter array (binary search target) - Iterate the longer array doing binary searches into the shorter - Reduces complexity from O(n log n + m log m) to O(n log n + m log n) - Fix test to expect false for shared segments (line, not point intersection)
1 parent 17f4b2f commit f170afa

1 file changed

Lines changed: 30 additions & 28 deletions

File tree

iOverlay/src/core/predicate.rs

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -38,28 +38,28 @@ impl PointCoincidenceChecker {
3838
/// - Similarly for clip-only interior segments
3939
#[inline]
4040
pub(crate) fn add_segment(&mut self, segment: &Segment<ShapeCountBoolean>, fill: SegmentFill) {
41-
let is_subj = segment.count.subj != 0;
42-
let is_clip = segment.count.clip != 0;
43-
4441
// Skip inner segments optimization:
4542
// If segment is entirely inside one shape's interior (filled on both sides)
4643
// and has no contribution from the other shape, it's not on a boundary
4744
// where coincidence could occur.
4845
let subj_interior = (fill & SUBJ_BOTH) == SUBJ_BOTH;
4946
let clip_interior = (fill & CLIP_BOTH) == CLIP_BOTH;
5047

51-
// Add to subj_points if:
52-
// - Segment belongs to subject AND
53-
// - Either it's not purely inside subject interior, OR clip is also present
54-
if is_subj && (!subj_interior || is_clip) {
55-
self.subj_points.push(segment.x_segment.a);
56-
self.subj_points.push(segment.x_segment.b);
48+
if subj_interior || clip_interior || fill == 0 {
49+
return;
5750
}
5851

59-
// Add to clip_points if:
60-
// - Segment belongs to clip AND
61-
// - Either it's not purely inside clip interior, OR subject is also present
62-
if is_clip && (!clip_interior || is_subj) {
52+
let is_subj = fill & SUBJ_BOTH != 0;
53+
let is_clip = fill & CLIP_BOTH != 0;
54+
if is_subj && is_clip {
55+
// Segment belongs to both shapes (boundary contact) - this is a shared edge, not a point coincidence.
56+
return;
57+
}
58+
if is_subj {
59+
self.subj_points.push(segment.x_segment.a);
60+
self.subj_points.push(segment.x_segment.b);
61+
} else {
62+
debug_assert!(is_clip);
6363
self.clip_points.push(segment.x_segment.a);
6464
self.clip_points.push(segment.x_segment.b);
6565
}
@@ -68,28 +68,29 @@ impl PointCoincidenceChecker {
6868
/// Check if any subject point coincides with any clip point.
6969
///
7070
/// Consumes self and returns true if coincidence found.
71+
///
72+
/// Optimization: Only sort/dedup the shorter array, then iterate the longer
73+
/// array doing binary searches into the shorter. This minimizes total work:
74+
/// O(n log n) sort + O(m log n) searches, where n ≤ m.
7175
#[inline]
7276
pub(crate) fn has_coincidence(mut self) -> bool {
7377
if self.subj_points.is_empty() || self.clip_points.is_empty() {
7478
return false;
7579
}
7680

77-
// Sort using sort_by_two_keys (radix sort for integer keys)
78-
self.subj_points.sort_by_two_keys(false, |p| p.x, |p| p.y);
79-
self.clip_points.sort_by_two_keys(false, |p| p.x, |p| p.y);
80-
81-
// Dedup (segment endpoints appear twice from adjacent segments)
82-
self.subj_points.dedup();
83-
self.clip_points.dedup();
84-
85-
// Binary search from shorter into longer array
81+
// Determine shorter/longer by pre-dedup size (good estimate of post-dedup)
8682
let (shorter, longer) = if self.subj_points.len() <= self.clip_points.len() {
87-
(&self.subj_points, &self.clip_points)
83+
(&mut self.subj_points, &self.clip_points)
8884
} else {
89-
(&self.clip_points, &self.subj_points)
85+
(&mut self.clip_points, &self.subj_points)
9086
};
9187

92-
shorter.iter().any(|p| longer.binary_search(p).is_ok())
88+
// Sort and dedup only the shorter array (binary search target)
89+
shorter.sort_by_two_keys(false, |p| p.x, |p| p.y);
90+
shorter.dedup();
91+
92+
// Iterate longer (unsorted) and binary search into shorter
93+
longer.iter().any(|p| shorter.binary_search(p).is_ok())
9394
}
9495
}
9596

@@ -365,11 +366,12 @@ mod tests {
365366
}
366367

367368
#[test]
368-
fn test_point_coincidence_shared_segment() {
369+
fn test_point_coincidence_shared_segment_is_line_not_point() {
369370
let mut checker = PointCoincidenceChecker::new(10);
370-
// Segment belonging to both shapes
371+
// Segment with both SUBJ and CLIP fill is a shared edge (line intersection),
372+
// not a point coincidence. Only one array gets populated, so no coincidence.
371373
checker.add_segment(&make_segment(0, 0, 10, 10, 1, 1), SUBJ_TOP | CLIP_BOTTOM);
372-
assert!(checker.has_coincidence());
374+
assert!(!checker.has_coincidence());
373375
}
374376

375377
#[test]

0 commit comments

Comments
 (0)