Merge pull request #2037 from hannobraun/boundary

Expand `CurveBoundary` API

crates/fj-core/src/algorithms/approx/curve/segment.rs

@@ -22,10 +22,7 @@ pub struct CurveApproxSegment {
 impl CurveApproxSegment {
     /// Indicate whether the segment is empty
     pub fn is_empty(&self) -> bool {
-        let is_empty = {
+        let is_empty = self.boundary.is_empty();
-            let [min, max] = self.boundary.inner;
-            min >= max
-        };
 
         if is_empty {
             assert!(
@@ -47,10 +44,7 @@ impl CurveApproxSegment {
     /// Segments that touch (i.e. their closest boundary is equal) count as
     /// overlapping.
     pub fn overlaps(&self, other: &Self) -> bool {
-        let [a_low, a_high] = self.boundary.normalize().inner;
+        self.boundary.overlaps(&other.boundary)
-        let [b_low, b_high] = other.boundary.normalize().inner;
-
-        a_low <= b_high && a_high >= b_low
     }
 
     /// Reverse the orientation of the approximation
@@ -76,7 +70,7 @@ impl CurveApproxSegment {
     /// Reduce the approximation to the subset defined by the provided boundary
     pub fn make_subset(&mut self, boundary: CurveBoundary<Point<1>>) {
         assert!(
-            self.boundary.is_normalized(),
+            self.is_normalized(),
             "Expected normalized segment for making subset."
         );
         assert!(
@@ -84,19 +78,9 @@ impl CurveApproxSegment {
             "Expected subset to be defined by normalized boundary."
         );
 
-        let [min, max] = boundary.inner;
+        self.boundary = self.boundary.subset(boundary);
-
-        self.boundary.inner = {
-            let [self_min, self_max] = self.boundary.inner;
-
-            let min = cmp::max(self_min, min);
-            let max = cmp::min(self_max, max);
-
-            [min, max]
-        };
-
         self.points
-            .retain(|point| point.local_form > min && point.local_form < max);
+            .retain(|point| self.boundary.contains(point.local_form));
     }
 
     /// Merge the provided segment into this one
@@ -115,18 +99,12 @@ impl CurveApproxSegment {
         );
         assert!(other.is_normalized(), "Can't merge non-normalized segment.");
 
-        let [a_min, a_max] = self.boundary.inner;
+        self.boundary = self.boundary.union(other.boundary);
-        let [b_min, b_max] = other.boundary.inner;
-
-        let min = cmp::min(a_min, b_min);
-        let max = cmp::max(a_max, b_max);
-
-        self.boundary.inner = [min, max];
 
         self.points.retain(|point| {
             // Only retain points that don't overlap with the other segment, or
             // we might end up with duplicates.
-            point.local_form < b_min || point.local_form > b_max
+            !other.boundary.contains(point.local_form)
         });
         self.points.extend(&other.points);
         self.points.sort();
@@ -152,32 +130,3 @@ impl PartialOrd for CurveApproxSegment {
         Some(self.cmp(other))
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::algorithms::approx::curve::CurveApproxSegment;
-
-    #[test]
-    fn overlaps() {
-        assert!(overlap([0., 2.], [1., 3.])); // regular overlap
-        assert!(overlap([0., 1.], [1., 2.])); // just touching
-        assert!(overlap([2., 0.], [3., 1.])); // not normalized
-        assert!(overlap([1., 3.], [0., 2.])); // lower boundary comes second
-
-        assert!(!overlap([0., 1.], [2., 3.])); // regular non-overlap
-        assert!(!overlap([2., 3.], [0., 1.])); // lower boundary comes second
-
-        fn overlap(a: [f64; 2], b: [f64; 2]) -> bool {
-            let a = CurveApproxSegment {
-                boundary: a.map(|coord| [coord]).into(),
-                points: Vec::new(),
-            };
-            let b = CurveApproxSegment {
-                boundary: b.map(|coord| [coord]).into(),
-                points: Vec::new(),
-            };
-
-            a.overlaps(&b)
-        }
-    }
-}

crates/fj-core/src/geometry/boundary.rs

@@ -1,5 +1,5 @@
 use std::{
-    cmp::Ordering,
+    cmp::{self, Ordering},
     hash::{Hash, Hasher},
 };
 
@@ -19,15 +19,6 @@ pub struct CurveBoundary<T: CurveBoundaryElement> {
 }
 
 impl<T: CurveBoundaryElement> CurveBoundary<T> {
-    /// Reverse the direction of the boundary
-    ///
-    /// Returns a new instance of this struct, which has its direction reversed.
-    #[must_use]
-    pub fn reverse(self) -> Self {
-        let [a, b] = self.inner;
-        Self { inner: [b, a] }
-    }
-
     /// Indicate whether the boundary is normalized
     ///
     /// If the boundary is normalized, its bounding elements are in a defined
@@ -37,6 +28,15 @@ impl<T: CurveBoundaryElement> CurveBoundary<T> {
         a <= b
     }
 
+    /// Reverse the direction of the boundary
+    ///
+    /// Returns a new instance of this struct, which has its direction reversed.
+    #[must_use]
+    pub fn reverse(self) -> Self {
+        let [a, b] = self.inner;
+        Self { inner: [b, a] }
+    }
+
     /// Normalize the boundary
     ///
     /// Returns a new instance of this struct, which has the bounding elements
@@ -52,11 +52,77 @@ impl<T: CurveBoundaryElement> CurveBoundary<T> {
     }
 }
 
-impl<T: CurveBoundaryElement> Eq for CurveBoundary<T> {}
+// Technically, these methods could be implemented for all
+// `CurveBoundaryElement`s, but that would be misleading. While
+// `HandleWrapper<Vertex>` implements `Ord`, which is useful for putting it (and
+// by extension, `CurveBoundary<Vertex>`) into `BTreeMap`s, this `Ord`
+// implementation doesn't actually define the geometrically meaningful ordering
+// that the following methods rely on.
+impl CurveBoundary<Point<1>> {
+    /// Indicate whether the boundary is empty
+    pub fn is_empty(&self) -> bool {
+        let [min, max] = &self.inner;
+        min >= max
+    }
 
-impl<T: CurveBoundaryElement> PartialEq for CurveBoundary<T> {
+    /// Indicate whether the boundary contains the given element
-    fn eq(&self, other: &Self) -> bool {
+    pub fn contains(&self, point: Point<1>) -> bool {
-        self.inner == other.inner
+        let [min, max] = self.inner;
+        point > min && point < max
+    }
+
+    /// Indicate whether the boundary overlaps another
+    ///
+    /// Boundaries that touch (i.e. their closest boundary elements are equal)
+    /// count as overlapping.
+    pub fn overlaps(&self, other: &Self) -> bool {
+        let [a_low, a_high] = self.normalize().inner;
+        let [b_low, b_high] = other.normalize().inner;
+
+        a_low <= b_high && a_high >= b_low
+    }
+
+    /// Create the subset of this boundary and another
+    ///
+    /// The result will be normalized.
+    #[must_use]
+    pub fn subset(self, other: Self) -> Self {
+        let self_ = self.normalize();
+        let other = other.normalize();
+
+        let [self_min, self_max] = self_.inner;
+        let [other_min, other_max] = other.inner;
+
+        let min = cmp::max(self_min, other_min);
+        let max = cmp::min(self_max, other_max);
+
+        Self { inner: [min, max] }
+    }
+
+    /// Create the union of this boundary and another
+    ///
+    /// The result will be normalized.
+    ///
+    /// # Panics
+    ///
+    /// Panics, if the two boundaries don't overlap (touching counts as
+    /// overlapping).
+    pub fn union(self, other: Self) -> Self {
+        let self_ = self.normalize();
+        let other = other.normalize();
+
+        assert!(
+            self.overlaps(&other),
+            "Can't merge boundaries that don't at least touch"
+        );
+
+        let [self_min, self_max] = self_.inner;
+        let [other_min, other_max] = other.inner;
+
+        let min = cmp::min(self_min, other_min);
+        let max = cmp::max(self_max, other_max);
+
+        Self { inner: [min, max] }
     }
 }
 
@@ -70,6 +136,14 @@ where
     }
 }
 
+impl<T: CurveBoundaryElement> Eq for CurveBoundary<T> {}
+
+impl<T: CurveBoundaryElement> PartialEq for CurveBoundary<T> {
+    fn eq(&self, other: &Self) -> bool {
+        self.inner == other.inner
+    }
+}
+
 impl<T: CurveBoundaryElement> Hash for CurveBoundary<T> {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.inner.hash(state);
@@ -105,3 +179,32 @@ impl CurveBoundaryElement for Point<1> {
 impl CurveBoundaryElement for Vertex {
     type Repr = HandleWrapper<Vertex>;
 }
+
+#[cfg(test)]
+mod tests {
+    use fj_math::Point;
+
+    use crate::geometry::CurveBoundary;
+
+    #[test]
+    fn overlaps() {
+        assert!(overlap([0., 2.], [1., 3.])); // regular overlap
+        assert!(overlap([0., 1.], [1., 2.])); // just touching
+        assert!(overlap([2., 0.], [3., 1.])); // not normalized
+        assert!(overlap([1., 3.], [0., 2.])); // lower boundary comes second
+
+        assert!(!overlap([0., 1.], [2., 3.])); // regular non-overlap
+        assert!(!overlap([2., 3.], [0., 1.])); // lower boundary comes second
+
+        fn overlap(a: [f64; 2], b: [f64; 2]) -> bool {
+            let a = array_to_boundary(a);
+            let b = array_to_boundary(b);
+
+            a.overlaps(&b)
+        }
+
+        fn array_to_boundary(array: [f64; 2]) -> CurveBoundary<Point<1>> {
+            CurveBoundary::from(array.map(|element| [element]))
+        }
+    }
+}