Merge pull request #2043 from hannobraun/approx

Better encapsulate `CurveApprox`

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

@@ -9,21 +9,47 @@ use super::{CurveApproxPoints, CurveApproxSegment};
 /// Partial approximation of a curve
 #[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct CurveApprox {
-    /// The approximated segments that are part of this approximation
+    segments: Vec<(CurveBoundary<Point<1>>, CurveApproxPoints)>,
-    pub segments: Vec<(CurveBoundary<Point<1>>, CurveApproxPoints)>,
 }
 
 impl CurveApprox {
+    /// Get the single segment that covers the provided boundary, if available
+    pub fn into_single_segment(
+        mut self,
+        boundary: CurveBoundary<Point<1>>,
+    ) -> Option<CurveApproxSegment> {
+        match self.segments.pop() {
+            Some((b, points)) if self.segments.is_empty() && b == boundary => {
+                // We just removed a single segment, there are no others, and
+                // 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) -> &mut Self {
+    pub fn reverse(&mut self) {
         self.segments.reverse();
 
         for (boundary, segment) in &mut self.segments {
             *boundary = boundary.reverse();
             segment.reverse();
         }
-
-        self
     }
 
     /// Reduce the approximation to the subset defined by the provided boundary
@@ -33,7 +59,7 @@ impl CurveApprox {
             segment.make_subset(boundary.normalize());
         }
 
-        self.segments.retain(|(_, segment)| !segment.is_empty());
+        self.segments.retain(|(boundary, _)| !boundary.is_empty());
     }
 
     /// Merge the provided segment into the approximation

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

@@ -52,17 +52,21 @@ impl Approx for (&Edge, &Surface) {
 
         let rest = {
             let segment = {
-                let mut cached = cache
+                let cached = cache
                     .get_curve_approx(edge.curve().clone(), edge.boundary());
 
-                match cached.segments.pop() {
+                // `cached` is the approximation of the curve that is available
-                    Some((boundary, points)) if cached.segments.is_empty() => {
+                // within the edge boundary. This approximation might or might
-                        // If the cached approximation has a single segment,
+                // not be complete.
-                        // that means everything we need is available, and we
+
-                        // can use the cached approximation as-is.
+                match cached.into_single_segment(edge.boundary()) {
-                        CurveApproxSegment { boundary, points }
+                    Some(segment) => {
+                        // We've asked the approximation to give us a single
+                        // segment that covers the boundary, and we got it. We
+                        // can use it as-is.
+                        segment
                     }
-                    _ => {
+                    None => {
                         // If we make it here, there are holes in the
                         // approximation, in some way or another. We could be
                         // really surgical and fill in exactly those holes, and