Remove redundant return values in curve approx code

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

@@ -32,18 +32,9 @@ impl CurveApprox {
     }
 
     /// Merge the provided segment into the approximation
-    pub fn merge(
-        &mut self,
-        new_segment: CurveApproxSegment,
-    ) -> CurveApproxSegment {
-        let (merged_boundary, merged_segment) = self
-            .segments
+    pub fn merge(&mut self, new_segment: CurveApproxSegment) {
+        self.segments
             .merge(new_segment.boundary, new_segment.points);
-
-        CurveApproxSegment {
-            boundary: merged_boundary,
-            points: merged_segment,
-        }
     }
 }
 

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

@@ -43,7 +43,7 @@ impl CurveApproxCache {
         &mut self,
         curve: Handle<Curve>,
         mut new_segment: CurveApproxSegment,
-    ) -> CurveApproxSegment {
+    ) {
         let curve = HandleWrapper::from(curve);
 
         // Overlapping approximations need to result in the same points,
@@ -52,7 +52,7 @@ impl CurveApproxCache {
         // approximated segment before doing *anything* with it.
         new_segment.normalize();
 
-        self.inner.entry(curve).or_default().merge(new_segment)
+        self.inner.entry(curve).or_default().merge(new_segment);
     }
 }
 
@@ -69,42 +69,6 @@ pub mod tests {
         services::Services,
     };
 
-    #[test]
-    fn insert_curve_already_exists_but_no_segment_merge_necessary() {
-        let mut services = Services::new();
-
-        let mut cache = CurveApproxCache::default();
-        let curve = Curve::new().insert(&mut services);
-
-        // An approximation of our curve already exists.
-        cache.insert(
-            curve.clone(),
-            CurveApproxSegment::from((
-                CurveBoundary::from([[2.], [3.]]),
-                [
-                    ApproxPoint::new([2.25], [2.25, 2.25, 2.25]),
-                    ApproxPoint::new([2.75], [2.75, 2.75, 2.75]),
-                ],
-            )),
-        );
-
-        // Here's another approximated segment for the same curve, but i doesn't
-        // overlap with the already existing one.
-        let boundary = CurveBoundary::from([[0.], [1.]]);
-        let segment = CurveApproxSegment::from((
-            boundary,
-            [
-                ApproxPoint::new([0.25], [0.25, 0.25, 0.25]),
-                ApproxPoint::new([0.75], [0.75, 0.75, 0.75]),
-            ],
-        ));
-
-        // When inserting the second segment, we expect to get it back
-        // unchanged.
-        let inserted = cache.insert(curve.clone(), segment.clone());
-        assert_eq!(inserted, segment);
-    }
-
     #[test]
     fn insert_congruent_segment_already_exists() {
         let mut services = Services::new();
@@ -132,11 +96,7 @@ pub mod tests {
                 ApproxPoint::new([0.76], [0.76, 0.76, 0.76]),
             ],
         ));
-
-        // When inserting the second segment, we expect to get the original one
-        // back.
-        let inserted = cache.insert(curve.clone(), new_segment);
-        assert_eq!(inserted, existing_segment);
+        cache.insert(curve.clone(), new_segment);
 
         // Also, the new segment should not have replaced the existing on in the
         // cache.

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

@@ -32,6 +32,7 @@ impl Approx for (&Edge, &Surface) {
         cache: &mut Self::Cache,
     ) -> Self::Approximation {
         let (edge, surface) = self;
+        let tolerance = tolerance.into();
 
         let start_position_surface = edge.start_position();
         let start_position =
@@ -51,42 +52,44 @@ impl Approx for (&Edge, &Surface) {
         let first = ApproxPoint::new(start_position_surface, start_position);
 
         let rest = {
-            let segment = {
+            let segment = loop {
                 let cached = cache
                     .get_curve_approx(edge.curve().clone(), edge.boundary());
 
                 // `cached` is the approximation of the curve that is available
                 // within the edge boundary. This approximation might or might
                 // not be complete.
 
-                match cached.into_single_segment(edge.boundary()) {
-                    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
-                        // in the future we might want to, for performance
-                        // reasons.
-                        //
-                        // For now, let's just approximate *all* we need and
-                        // insert that into the cache. The cache takes care of
-                        // merging that with whatever is already there.
-                        cache.insert_curve_approx(
-                            edge.curve().clone(),
-                            approx_curve(
-                                &edge.path(),
-                                surface,
-                                edge.boundary(),
-                                tolerance,
-                            ),
-                        )
-                    }
+                if let Some(segment) =
+                    cached.into_single_segment(edge.boundary())
+                {
+                    // 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.
+                    break segment;
                 }
+
+                // 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 in the future we might want to, for
+                // performance reasons.
+                //
+                // For now, let's just approximate *all* we need and insert that
+                // into the cache. The cache takes care of merging that with
+                // whatever is already there.
+                cache.insert_curve_approx(
+                    edge.curve().clone(),
+                    approx_curve(
+                        &edge.path(),
+                        surface,
+                        edge.boundary(),
+                        tolerance,
+                    ),
+                );
+
+                // We will never complete more than one full loop here. If we
+                // don't return the segment the first time, we'll insert it
+                // immediately, and it will be there on the second iteration.
             };
 
             segment
@@ -246,8 +249,8 @@ impl EdgeApproxCache {
         &mut self,
         handle: Handle<Curve>,
         approx: CurveApproxSegment,
-    ) -> CurveApproxSegment {
-        self.curve_approx.insert(handle, approx)
+    ) {
+        self.curve_approx.insert(handle, approx);
     }
 }
 

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

@@ -70,7 +70,7 @@ impl<T: CurveBoundariesPayload> CurveBoundaries<T> {
         &mut self,
         new_boundary: CurveBoundary<Point<1>>,
         new_payload: T,
-    ) -> (CurveBoundary<Point<1>>, T) {
+    ) {
         let mut overlapping_payloads = VecDeque::new();
 
         let mut i = 0;
@@ -101,10 +101,8 @@ impl<T: CurveBoundariesPayload> CurveBoundaries<T> {
             merged_payload.merge(&payload, boundary);
         }
 
-        self.inner.push((merged_boundary, merged_payload.clone()));
+        self.inner.push((merged_boundary, merged_payload));
         self.inner.sort();
-
-        (merged_boundary, merged_payload)
     }
 }