Merge pull request #2044 from hannobraun/boundary

Add `CurveBoundaries`

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

@@ -1,65 +1,34 @@
-use std::collections::VecDeque;
-
 use fj_math::Point;
 
-use crate::geometry::CurveBoundary;
+use crate::geometry::{CurveBoundaries, CurveBoundary};
 
 use super::{CurveApproxPoints, CurveApproxSegment};
 
 /// Partial approximation of a curve
 #[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct CurveApprox {
-    segments: Vec<(CurveBoundary<Point<1>>, CurveApproxPoints)>,
+    segments: CurveBoundaries<CurveApproxPoints>,
 }
 
 impl CurveApprox {
     /// Get the single segment that covers the provided boundary, if available
     pub fn into_single_segment(
-        mut self,
+        self,
         boundary: CurveBoundary<Point<1>>,
     ) -> Option<CurveApproxSegment> {
-        match self.segments.pop() {
+        self.segments
-            Some((b, points)) if self.segments.is_empty() && b == boundary => {
+            .into_single_payload(boundary)
-                // We just removed a single segment, there are no others, and
+            .map(|points| CurveApproxSegment { boundary, points })
-                // the removed segment's boundary matches the boundary provided
-                // to us.
-                //
-                // This is what the caller was asking for. Return it!
-                Some(CurveApproxSegment {
-                    boundary: b,
-                    points,
-                })
-            }
-            _ => {
-                // Either we don't have any segments in here, or we have more
-                // than one (which implies there are gaps between them), or we
-                // have a single one that doesn't cover the full boundary we
-                // were asked for.
-                //
-                // Either way, we don't have what the caller wants.
-                None
-            }
-        }
     }
 
     /// Reverse the approximation
     pub fn reverse(&mut self) {
         self.segments.reverse();
-
-        for (boundary, segment) in &mut self.segments {
-            *boundary = boundary.reverse();
-            segment.reverse();
-        }
     }
 
     /// Reduce the approximation to the subset defined by the provided boundary
     pub fn make_subset(&mut self, boundary: CurveBoundary<Point<1>>) {
-        for (b, segment) in &mut self.segments {
+        self.segments.make_subset(boundary);
-            *b = b.subset(boundary);
-            segment.make_subset(boundary.normalize());
-        }
-
-        self.segments.retain(|(boundary, _)| !boundary.is_empty());
     }
 
     /// Merge the provided segment into the approximation
@@ -67,39 +36,9 @@ impl CurveApprox {
         &mut self,
         new_segment: CurveApproxSegment,
     ) -> CurveApproxSegment {
-        let mut overlapping_segments = VecDeque::new();
+        let (merged_boundary, merged_segment) = self
-
+            .segments
-        let mut i = 0;
+            .merge(new_segment.boundary, new_segment.points);
-        loop {
-            let Some((boundary, _)) = self.segments.get(i) else {
-                break;
-            };
-
-            if boundary.overlaps(&new_segment.boundary) {
-                let segment = self.segments.swap_remove(i);
-                overlapping_segments.push_back(segment);
-                continue;
-            }
-
-            i += 1;
-        }
-
-        let mut merged_boundary = new_segment.boundary;
-        let mut merged_segment = new_segment.points;
-
-        for (boundary, segment) in overlapping_segments {
-            assert!(
-                merged_boundary.overlaps(&boundary),
-                "Shouldn't merge segments that don't overlap."
-            );
-
-            merged_boundary = merged_boundary.union(boundary);
-            merged_segment.merge(&segment, boundary);
-        }
-
-        self.segments
-            .push((merged_boundary, merged_segment.clone()));
-        self.segments.sort();
 
         CurveApproxSegment {
             boundary: merged_boundary,
@@ -111,10 +50,12 @@ impl CurveApprox {
 impl<const N: usize> From<[CurveApproxSegment; N]> for CurveApprox {
     fn from(segments: [CurveApproxSegment; N]) -> Self {
         Self {
-            segments: segments
+            segments: CurveBoundaries {
-                .into_iter()
+                inner: segments
-                .map(|segment| (segment.boundary, segment.points))
+                    .into_iter()
-                .collect(),
+                    .map(|segment| (segment.boundary, segment.points))
+                    .collect(),
+            },
         }
     }
 }

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

@@ -1,6 +1,9 @@
 use fj_math::Point;
 
-use crate::{algorithms::approx::ApproxPoint, geometry::CurveBoundary};
+use crate::{
+    algorithms::approx::ApproxPoint,
+    geometry::{CurveBoundariesPayload, CurveBoundary},
+};
 
 /// Points of a curve approximation
 #[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)]
@@ -16,9 +19,8 @@ impl CurveApproxPoints {
     }
 
     /// Reverse the orientation of the approximation
-    pub fn reverse(&mut self) -> &mut Self {
+    pub fn reverse(&mut self) {
         self.inner.reverse();
-        self
     }
 
     /// Reduce the approximation to the subset defined by the provided boundary
@@ -45,3 +47,17 @@ impl CurveApproxPoints {
         self.inner.sort();
     }
 }
+
+impl CurveBoundariesPayload for CurveApproxPoints {
+    fn reverse(&mut self) {
+        self.reverse();
+    }
+
+    fn make_subset(&mut self, boundary: CurveBoundary<Point<1>>) {
+        self.make_subset(boundary)
+    }
+
+    fn merge(&mut self, other: &Self, other_boundary: CurveBoundary<Point<1>>) {
+        self.merge(other, other_boundary)
+    }
+}

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

@@ -0,0 +1,2 @@
+pub mod multiple;
+pub mod single;

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

@@ -0,0 +1,134 @@
+use std::collections::VecDeque;
+
+use fj_math::Point;
+
+use crate::geometry::CurveBoundary;
+
+/// A collection of multiple [`CurveBoundary`] instances
+///
+/// Has a type parameter, `T`, which can be used to attach a payload to each
+/// boundary.
+#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
+pub struct CurveBoundaries<T: CurveBoundariesPayload = ()> {
+    /// The [`CurveBoundary`] instances
+    pub inner: Vec<(CurveBoundary<Point<1>>, T)>,
+}
+
+impl<T: CurveBoundariesPayload> CurveBoundaries<T> {
+    /// Transform `self` into the payload of the single boundary requested
+    ///
+    /// If there are no boundaries or multiple boundaries in `self`, or if the
+    /// one available boundary is not equal to the one requested, return `None`.
+    pub fn into_single_payload(
+        mut self,
+        boundary: CurveBoundary<Point<1>>,
+    ) -> Option<T> {
+        match self.inner.pop() {
+            Some((b, payload)) if self.inner.is_empty() && b == boundary => {
+                // We just removed a single element, there are no others, and
+                // the removed element's boundary matches the boundary provided
+                // to us.
+                //
+                // This is what the caller was asking for. Return it!
+                Some(payload)
+            }
+            _ => {
+                // Either we don't have any elements in here, or we have more
+                // than one (which implies there are gaps between them), or we
+                // have a single one that doesn't cover the full boundary we
+                // were asked for.
+                //
+                // Either way, we don't have what the caller wants.
+                None
+            }
+        }
+    }
+
+    /// Reverse each boundary, and their order
+    pub fn reverse(&mut self) {
+        self.inner.reverse();
+
+        for (boundary, payload) in &mut self.inner {
+            *boundary = boundary.reverse();
+            payload.reverse();
+        }
+    }
+
+    /// Reduce `self` to the subset defined by the provided boundary
+    pub fn make_subset(&mut self, boundary: CurveBoundary<Point<1>>) {
+        for (b, segment) in &mut self.inner {
+            *b = b.subset(boundary);
+            segment.make_subset(boundary);
+        }
+
+        self.inner.retain(|(boundary, _)| !boundary.is_empty());
+    }
+
+    /// Merge the provided boundary into `self`
+    ///
+    /// Return the merged boundary and payload.
+    pub fn merge(
+        &mut self,
+        new_boundary: CurveBoundary<Point<1>>,
+        new_payload: T,
+    ) -> (CurveBoundary<Point<1>>, T) {
+        let mut overlapping_payloads = VecDeque::new();
+
+        let mut i = 0;
+        loop {
+            let Some((boundary, _)) = self.inner.get(i) else {
+                break;
+            };
+
+            if boundary.overlaps(&new_boundary) {
+                let payload = self.inner.swap_remove(i);
+                overlapping_payloads.push_back(payload);
+                continue;
+            }
+
+            i += 1;
+        }
+
+        let mut merged_boundary = new_boundary;
+        let mut merged_payload = new_payload;
+
+        for (boundary, payload) in overlapping_payloads {
+            assert!(
+                merged_boundary.overlaps(&boundary),
+                "Shouldn't merge boundaries that don't overlap."
+            );
+
+            merged_boundary = merged_boundary.union(boundary);
+            merged_payload.merge(&payload, boundary);
+        }
+
+        self.inner.push((merged_boundary, merged_payload.clone()));
+        self.inner.sort();
+
+        (merged_boundary, merged_payload)
+    }
+}
+
+impl<T: CurveBoundariesPayload> Default for CurveBoundaries<T> {
+    fn default() -> Self {
+        Self { inner: Vec::new() }
+    }
+}
+
+/// A payload that can be used in [`CurveBoundaries`]
+pub trait CurveBoundariesPayload: Clone + Ord {
+    /// Reverse the orientation of the payload
+    fn reverse(&mut self);
+
+    /// Reduce the payload to the subset defined by the provided boundary
+    fn make_subset(&mut self, boundary: CurveBoundary<Point<1>>);
+
+    /// Merge the provided payload
+    fn merge(&mut self, other: &Self, other_boundary: CurveBoundary<Point<1>>);
+}
+
+impl CurveBoundariesPayload for () {
+    fn reverse(&mut self) {}
+    fn make_subset(&mut self, _: CurveBoundary<Point<1>>) {}
+    fn merge(&mut self, _: &Self, _: CurveBoundary<Point<1>>) {}
+}

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

@@ -67,7 +67,7 @@ impl CurveBoundary<Point<1>> {
 
     /// Indicate whether the boundary contains the given element
     pub fn contains(&self, point: Point<1>) -> bool {
-        let [min, max] = self.inner;
+        let [min, max] = self.normalize().inner;
         point > min && point < max
     }
 

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

@@ -5,7 +5,10 @@ mod path;
 mod surface;
 
 pub use self::{
-    boundary::{CurveBoundary, CurveBoundaryElement},
+    boundary::{
+        multiple::{CurveBoundaries, CurveBoundariesPayload},
+        single::{CurveBoundary, CurveBoundaryElement},
+    },
     path::{GlobalPath, SurfacePath},
     surface::SurfaceGeometry,
 };