From 84e60ce4e5997abdfa2702402b444998eb04de06 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 11:42:17 +0200 Subject: [PATCH 1/4] Add `CurveApprox::into_single_segment` --- .../src/algorithms/approx/curve/approx.rs | 29 +++++++++++++++++++ crates/fj-core/src/algorithms/approx/edge.rs | 20 ++++++++----- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index c09227897..f0091aebe 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -14,6 +14,35 @@ pub struct CurveApprox { } impl CurveApprox { + /// Get the single segment that covers the provided boundary, if available + pub fn into_single_segment( + mut self, + boundary: CurveBoundary>, + ) -> Option { + 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 { self.segments.reverse(); diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index c9e657b5b..d1bb79c0f 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/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() { - Some((boundary, points)) if cached.segments.is_empty() => { - // If the cached approximation has a single segment, - // that means everything we need is available, and we - // can use the cached approximation as-is. - CurveApproxSegment { boundary, points } + // `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 From 6f53ab1167ee2fc6bf79fdde0c649e6caeaaf44e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 12:15:15 +0200 Subject: [PATCH 2/4] Remove unnecessary `pub` --- crates/fj-core/src/algorithms/approx/curve/approx.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index f0091aebe..c444b474f 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -9,8 +9,7 @@ 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 - pub segments: Vec<(CurveBoundary>, CurveApproxPoints)>, + segments: Vec<(CurveBoundary>, CurveApproxPoints)>, } impl CurveApprox { From efe55e381b5b69463e3b251089f8d695044ebe6c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 11:54:38 +0200 Subject: [PATCH 3/4] Remove unused return value --- crates/fj-core/src/algorithms/approx/curve/approx.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index c444b474f..5b9f2d3f9 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -43,15 +43,13 @@ impl CurveApprox { } /// 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 From 02b7d9efe0bc3be5717ca51765db809fa99915f9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 11:57:47 +0200 Subject: [PATCH 4/4] Refactor to prepare for follow-on change --- crates/fj-core/src/algorithms/approx/curve/approx.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 5b9f2d3f9..546ccafb0 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -59,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