From 70a5a4acd2451318a5264763cdabff6061e2c136 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 11:56:53 +0200 Subject: [PATCH 01/19] Add `CurveApproxPoints` --- .../src/algorithms/approx/curve/mod.rs | 4 +++- .../src/algorithms/approx/curve/points.rs | 8 +++++++ .../src/algorithms/approx/curve/segment.rs | 21 ++++++++++++------- crates/fj-core/src/algorithms/approx/edge.rs | 10 +++++++-- 4 files changed, 32 insertions(+), 11 deletions(-) create mode 100644 crates/fj-core/src/algorithms/approx/curve/points.rs diff --git a/crates/fj-core/src/algorithms/approx/curve/mod.rs b/crates/fj-core/src/algorithms/approx/curve/mod.rs index 8eb7f3bfe..0705cb15b 100644 --- a/crates/fj-core/src/algorithms/approx/curve/mod.rs +++ b/crates/fj-core/src/algorithms/approx/curve/mod.rs @@ -2,8 +2,10 @@ mod approx; mod cache; +mod points; mod segment; pub use self::{ - approx::CurveApprox, cache::CurveApproxCache, segment::CurveApproxSegment, + approx::CurveApprox, cache::CurveApproxCache, points::CurveApproxPoints, + segment::CurveApproxSegment, }; diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs new file mode 100644 index 000000000..0226a713a --- /dev/null +++ b/crates/fj-core/src/algorithms/approx/curve/points.rs @@ -0,0 +1,8 @@ +use crate::algorithms::approx::ApproxPoint; + +/// Points of a curve approximation +#[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct CurveApproxPoints { + /// Points of a curve approximation + pub inner: Vec>, +} diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index de37b008b..1c7bd6fe6 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -4,6 +4,8 @@ use fj_math::Point; use crate::{algorithms::approx::ApproxPoint, geometry::CurveBoundary}; +use super::points::CurveApproxPoints; + /// A segment of a curve approximation /// /// A curve is potentially infinite (at least its local coordinate space is @@ -16,7 +18,7 @@ pub struct CurveApproxSegment { pub boundary: CurveBoundary>, /// The points that approximate the curve segment - pub points: Vec>, + pub points: CurveApproxPoints, } impl CurveApproxSegment { @@ -26,7 +28,7 @@ impl CurveApproxSegment { if is_empty { assert!( - self.points.is_empty(), + self.points.inner.is_empty(), "Empty approximation still has points" ); } @@ -50,7 +52,7 @@ impl CurveApproxSegment { /// Reverse the orientation of the approximation pub fn reverse(&mut self) -> &mut Self { self.boundary = self.boundary.reverse(); - self.points.reverse(); + self.points.inner.reverse(); self } @@ -61,7 +63,7 @@ impl CurveApproxSegment { pub fn normalize(&mut self) -> &mut Self { if !self.is_normalized() { self.boundary = self.boundary.normalize(); - self.points.reverse(); + self.points.inner.reverse(); } self @@ -80,6 +82,7 @@ impl CurveApproxSegment { self.boundary = self.boundary.subset(boundary); self.points + .inner .retain(|point| self.boundary.contains(point.local_form)); } @@ -101,13 +104,13 @@ impl CurveApproxSegment { self.boundary = self.boundary.union(other.boundary); - self.points.retain(|point| { + self.points.inner.retain(|point| { // Only retain points that don't overlap with the other segment, or // we might end up with duplicates. !other.boundary.contains(point.local_form) }); - self.points.extend(&other.points); - self.points.sort(); + self.points.inner.extend(&other.points.inner); + self.points.inner.sort(); } } @@ -139,7 +142,9 @@ impl From<(CurveBoundary>, [ApproxPoint<1>; D])> ) -> Self { Self { boundary, - points: points.into_iter().collect(), + points: CurveApproxPoints { + inner: points.into_iter().collect(), + }, } } } diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 9abe833be..e3be305b8 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -16,7 +16,9 @@ use crate::{ }; use super::{ - curve::{CurveApprox, CurveApproxCache, CurveApproxSegment}, + curve::{ + CurveApprox, CurveApproxCache, CurveApproxPoints, CurveApproxSegment, + }, Approx, ApproxPoint, Tolerance, }; @@ -85,6 +87,7 @@ impl Approx for (&Edge, &Surface) { segment .points + .inner .into_iter() .map(|point| { let point_surface = @@ -194,7 +197,10 @@ fn approx_curve( ApproxPoint::new(point_curve, point_global) }) .collect(); - CurveApproxSegment { boundary, points } + CurveApproxSegment { + boundary, + points: CurveApproxPoints { inner: points }, + } } /// Cache for edge approximations From 049c313815574ba7ae1287dcc12a17628e43b27b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:21:22 +0200 Subject: [PATCH 02/19] Add `CurveApproxPoint::reverse` --- crates/fj-core/src/algorithms/approx/curve/points.rs | 8 ++++++++ crates/fj-core/src/algorithms/approx/curve/segment.rs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs index 0226a713a..ca7ff42b0 100644 --- a/crates/fj-core/src/algorithms/approx/curve/points.rs +++ b/crates/fj-core/src/algorithms/approx/curve/points.rs @@ -6,3 +6,11 @@ pub struct CurveApproxPoints { /// Points of a curve approximation pub inner: Vec>, } + +impl CurveApproxPoints { + /// Reverse the orientation of the approximation + pub fn reverse(&mut self) -> &mut Self { + self.inner.reverse(); + self + } +} diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index 1c7bd6fe6..59fc2fb59 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -52,7 +52,7 @@ impl CurveApproxSegment { /// Reverse the orientation of the approximation pub fn reverse(&mut self) -> &mut Self { self.boundary = self.boundary.reverse(); - self.points.inner.reverse(); + self.points.reverse(); self } From 3410410756590b4c18e4f9c2fdd5d57d60e9da26 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:24:46 +0200 Subject: [PATCH 03/19] Refactor to prepare for follow-on change --- crates/fj-core/src/algorithms/approx/curve/segment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index 59fc2fb59..7d096f005 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -83,7 +83,7 @@ impl CurveApproxSegment { self.boundary = self.boundary.subset(boundary); self.points .inner - .retain(|point| self.boundary.contains(point.local_form)); + .retain(|point| boundary.contains(point.local_form)); } /// Merge the provided segment into this one From 9785a9ed98aadd1a60bad561af3b84f577a9ecff Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:26:03 +0200 Subject: [PATCH 04/19] Add `CurveApproxPoints::make_subset` --- crates/fj-core/src/algorithms/approx/curve/points.rs | 10 +++++++++- crates/fj-core/src/algorithms/approx/curve/segment.rs | 4 +--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs index ca7ff42b0..4f4f4f6f0 100644 --- a/crates/fj-core/src/algorithms/approx/curve/points.rs +++ b/crates/fj-core/src/algorithms/approx/curve/points.rs @@ -1,4 +1,6 @@ -use crate::algorithms::approx::ApproxPoint; +use fj_math::Point; + +use crate::{algorithms::approx::ApproxPoint, geometry::CurveBoundary}; /// Points of a curve approximation #[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] @@ -13,4 +15,10 @@ impl CurveApproxPoints { self.inner.reverse(); self } + + /// Reduce the approximation to the subset defined by the provided boundary + pub fn make_subset(&mut self, boundary: CurveBoundary>) { + self.inner + .retain(|point| boundary.contains(point.local_form)); + } } diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index 7d096f005..c7831b223 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -81,9 +81,7 @@ impl CurveApproxSegment { ); self.boundary = self.boundary.subset(boundary); - self.points - .inner - .retain(|point| boundary.contains(point.local_form)); + self.points.make_subset(boundary); } /// Merge the provided segment into this one From 6689bc8295b21d7cd7cbff7cf4f63276ba31c74f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:27:55 +0200 Subject: [PATCH 05/19] Fix typo --- crates/fj-core/src/algorithms/approx/curve/segment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index c7831b223..8fa569088 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -86,7 +86,7 @@ impl CurveApproxSegment { /// Merge the provided segment into this one /// - /// It there is a true overlap between both segments (as opposed to them + /// If there is a true overlap between both segments (as opposed to them /// just touching), then the overlapping part is taken from the other /// segment, meaning parts of this one get overwritten. pub fn merge(&mut self, other: &Self) { From ec889d0d69d4aa4de0a1ea552989240c93c21ba8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:30:45 +0200 Subject: [PATCH 06/19] Add `CurveApproxPoints::merge` --- .../src/algorithms/approx/curve/points.rs | 18 ++++++++++++++++++ .../src/algorithms/approx/curve/segment.rs | 9 +-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs index 4f4f4f6f0..aac6de0be 100644 --- a/crates/fj-core/src/algorithms/approx/curve/points.rs +++ b/crates/fj-core/src/algorithms/approx/curve/points.rs @@ -21,4 +21,22 @@ impl CurveApproxPoints { self.inner .retain(|point| boundary.contains(point.local_form)); } + + /// Merge the provided points + /// + /// If there is a true overlap between these points and the other points + /// then the overlapping part is taken from the other points. + pub fn merge( + &mut self, + other: &Self, + other_boundary: CurveBoundary>, + ) { + self.inner.retain(|point| { + // Only retain points that don't overlap with the other points, or + // we might end up with duplicates. + !other_boundary.contains(point.local_form) + }); + self.inner.extend(&other.inner); + self.inner.sort(); + } } diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index 8fa569088..b065b619b 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -101,14 +101,7 @@ impl CurveApproxSegment { assert!(other.is_normalized(), "Can't merge non-normalized segment."); self.boundary = self.boundary.union(other.boundary); - - self.points.inner.retain(|point| { - // Only retain points that don't overlap with the other segment, or - // we might end up with duplicates. - !other.boundary.contains(point.local_form) - }); - self.points.inner.extend(&other.points.inner); - self.points.inner.sort(); + self.points.merge(&other.points, other.boundary); } } From 1f0131c781a994e144aac8faca7925a34e30eb1c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:34:02 +0200 Subject: [PATCH 07/19] Add `CurveApproxPoints::is_empty` --- crates/fj-core/src/algorithms/approx/curve/points.rs | 5 +++++ crates/fj-core/src/algorithms/approx/curve/segment.rs | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs index aac6de0be..0fc036245 100644 --- a/crates/fj-core/src/algorithms/approx/curve/points.rs +++ b/crates/fj-core/src/algorithms/approx/curve/points.rs @@ -10,6 +10,11 @@ pub struct CurveApproxPoints { } impl CurveApproxPoints { + /// Indicate whether there are any points + pub fn is_empty(&self) -> bool { + self.inner.is_empty() + } + /// Reverse the orientation of the approximation pub fn reverse(&mut self) -> &mut Self { self.inner.reverse(); diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index b065b619b..cb3809b72 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -28,7 +28,7 @@ impl CurveApproxSegment { if is_empty { assert!( - self.points.inner.is_empty(), + self.points.is_empty(), "Empty approximation still has points" ); } From 4a7aa14cc4f734e1a5a519e493dd9fc9729391a5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:48:40 +0200 Subject: [PATCH 08/19] Refactor to prepare for follow-on change --- .../src/algorithms/approx/curve/approx.rs | 22 ++++++++++++------- crates/fj-core/src/algorithms/approx/edge.rs | 2 +- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 07a7b45df..99fa05728 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -10,7 +10,7 @@ use super::CurveApproxSegment; #[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CurveApprox { /// The approximated segments that are part of this approximation - pub segments: Vec, + pub segments: Vec<(CurveBoundary>, CurveApproxSegment)>, } impl CurveApprox { @@ -18,7 +18,8 @@ impl CurveApprox { pub fn reverse(&mut self) -> &mut Self { self.segments.reverse(); - for segment in &mut self.segments { + for (boundary, segment) in &mut self.segments { + *boundary = boundary.reverse(); segment.reverse(); } @@ -27,11 +28,12 @@ impl CurveApprox { /// Reduce the approximation to the subset defined by the provided boundary pub fn make_subset(&mut self, boundary: CurveBoundary>) { - for segment in &mut self.segments { + for (b, segment) in &mut self.segments { + *b = b.subset(boundary); segment.make_subset(boundary.normalize()); } - self.segments.retain(|segment| !segment.is_empty()); + self.segments.retain(|(_, segment)| !segment.is_empty()); } /// Merge the provided segment into the approximation @@ -43,7 +45,7 @@ impl CurveApprox { let mut i = 0; loop { - let Some(segment) = self.segments.get(i) else { + let Some((_, segment)) = self.segments.get(i) else { break; }; @@ -57,11 +59,12 @@ impl CurveApprox { } let mut merged_segment = new_segment; - for segment in overlapping_segments { + for (_, segment) in overlapping_segments { merged_segment.merge(&segment); } - self.segments.push(merged_segment.clone()); + self.segments + .push((merged_segment.boundary, merged_segment.clone())); self.segments.sort(); merged_segment } @@ -70,7 +73,10 @@ impl CurveApprox { impl From<[CurveApproxSegment; N]> for CurveApprox { fn from(segments: [CurveApproxSegment; N]) -> Self { Self { - segments: segments.into_iter().collect(), + segments: segments + .into_iter() + .map(|segment| (segment.boundary, segment)) + .collect(), } } } diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index e3be305b8..1a34daa17 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -56,7 +56,7 @@ impl Approx for (&Edge, &Surface) { .get_curve_approx(edge.curve().clone(), edge.boundary()); match cached.segments.pop() { - Some(segment) if cached.segments.is_empty() => { + Some((_, segment)) 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. From 0c16be7f9848dd02c6290dc41694f60f1969860e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:51:37 +0200 Subject: [PATCH 09/19] Refactor to prepare for follow-on change --- crates/fj-core/src/algorithms/approx/curve/approx.rs | 4 ++++ crates/fj-core/src/algorithms/approx/curve/segment.rs | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 99fa05728..af87a0432 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -60,6 +60,10 @@ impl CurveApprox { let mut merged_segment = new_segment; for (_, segment) in overlapping_segments { + assert!( + merged_segment.boundary.overlaps(&segment.boundary), + "Shouldn't merge segments that don't overlap." + ); merged_segment.merge(&segment); } diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index cb3809b72..a1d99c0cf 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -90,10 +90,6 @@ impl CurveApproxSegment { /// just touching), then the overlapping part is taken from the other /// segment, meaning parts of this one get overwritten. pub fn merge(&mut self, other: &Self) { - assert!( - self.overlaps(other), - "Shouldn't merge segments that don't overlap." - ); assert!( self.is_normalized(), "Can't merge into non-normalized segment." From dc67f573e1c13e1fa29c1d91a7eaf2029d06264e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:52:14 +0200 Subject: [PATCH 10/19] Bypass redundant method --- crates/fj-core/src/algorithms/approx/curve/approx.rs | 4 ++-- crates/fj-core/src/algorithms/approx/curve/segment.rs | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index af87a0432..9199d8bda 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -45,11 +45,11 @@ impl CurveApprox { let mut i = 0; loop { - let Some((_, segment)) = self.segments.get(i) else { + let Some((boundary, _)) = self.segments.get(i) else { break; }; - if segment.overlaps(&new_segment) { + if boundary.overlaps(&new_segment.boundary) { let segment = self.segments.swap_remove(i); overlapping_segments.push_back(segment); continue; diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index a1d99c0cf..66e9f0687 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -41,14 +41,6 @@ impl CurveApproxSegment { self.boundary.is_normalized() } - /// Indicate whether this segment overlaps another - /// - /// Segments that touch (i.e. their closest boundary is equal) count as - /// overlapping. - pub fn overlaps(&self, other: &Self) -> bool { - self.boundary.overlaps(&other.boundary) - } - /// Reverse the orientation of the approximation pub fn reverse(&mut self) -> &mut Self { self.boundary = self.boundary.reverse(); From 0968ca160be0f8cbc0cdad3cdc9d0d132b4b94a8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:53:22 +0200 Subject: [PATCH 11/19] Refactor to prepare for follow-on change --- crates/fj-core/src/algorithms/approx/curve/approx.rs | 4 ++-- 1 file changed, 2 insertions(+), 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 9199d8bda..7c4e3cb66 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -59,9 +59,9 @@ impl CurveApprox { } let mut merged_segment = new_segment; - for (_, segment) in overlapping_segments { + for (boundary, segment) in overlapping_segments { assert!( - merged_segment.boundary.overlaps(&segment.boundary), + merged_segment.boundary.overlaps(&boundary), "Shouldn't merge segments that don't overlap." ); merged_segment.merge(&segment); From 19ece11225b8b3ad4851d0749456c66e52cd4f18 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:54:14 +0200 Subject: [PATCH 12/19] Update formatting --- crates/fj-core/src/algorithms/approx/curve/approx.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 7c4e3cb66..38f4884f5 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -59,6 +59,7 @@ impl CurveApprox { } let mut merged_segment = new_segment; + for (boundary, segment) in overlapping_segments { assert!( merged_segment.boundary.overlaps(&boundary), From a6fa52f70c94da38fc8dcfeed24d70d430234b78 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:57:22 +0200 Subject: [PATCH 13/19] Bypass redundant method --- .../src/algorithms/approx/curve/approx.rs | 13 ++++++++++--- .../src/algorithms/approx/curve/segment.rs | 16 ---------------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 38f4884f5..815c492cf 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -58,16 +58,23 @@ impl CurveApprox { i += 1; } - let mut merged_segment = new_segment; + let mut merged_boundary = new_segment.boundary; + let mut merged_segment = new_segment.points; for (boundary, segment) in overlapping_segments { assert!( - merged_segment.boundary.overlaps(&boundary), + merged_boundary.overlaps(&boundary), "Shouldn't merge segments that don't overlap." ); - merged_segment.merge(&segment); + + merged_boundary = merged_boundary.union(boundary); + merged_segment.merge(&segment.points, boundary); } + let merged_segment = CurveApproxSegment { + boundary: merged_boundary, + points: merged_segment, + }; self.segments .push((merged_segment.boundary, merged_segment.clone())); self.segments.sort(); diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index 66e9f0687..eb65cfda0 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -75,22 +75,6 @@ impl CurveApproxSegment { self.boundary = self.boundary.subset(boundary); self.points.make_subset(boundary); } - - /// Merge the provided segment into this one - /// - /// If there is a true overlap between both segments (as opposed to them - /// just touching), then the overlapping part is taken from the other - /// segment, meaning parts of this one get overwritten. - pub fn merge(&mut self, other: &Self) { - assert!( - self.is_normalized(), - "Can't merge into non-normalized segment." - ); - assert!(other.is_normalized(), "Can't merge non-normalized segment."); - - self.boundary = self.boundary.union(other.boundary); - self.points.merge(&other.points, other.boundary); - } } impl Ord for CurveApproxSegment { From e531895fcddaa59f954a5e0d262dbde8a74c165b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:58:13 +0200 Subject: [PATCH 14/19] Update formatting --- crates/fj-core/src/algorithms/approx/curve/approx.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 815c492cf..debd9919d 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -78,6 +78,7 @@ impl CurveApprox { self.segments .push((merged_segment.boundary, merged_segment.clone())); self.segments.sort(); + merged_segment } } From d40dc023c4ba4f7d55be96222c401f3d2a1c8c74 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 12:59:11 +0200 Subject: [PATCH 15/19] 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 debd9919d..0d3d9b72c 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -76,7 +76,7 @@ impl CurveApprox { points: merged_segment, }; self.segments - .push((merged_segment.boundary, merged_segment.clone())); + .push((merged_boundary, merged_segment.clone())); self.segments.sort(); merged_segment From 3a8763f28202ea85f8a338a1fd829b695275b37a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 13:05:37 +0200 Subject: [PATCH 16/19] Bypass redundant method --- .../fj-core/src/algorithms/approx/curve/approx.rs | 8 +++++++- .../src/algorithms/approx/curve/segment.rs | 15 --------------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 0d3d9b72c..e2e3d594e 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -30,7 +30,13 @@ impl CurveApprox { pub fn make_subset(&mut self, boundary: CurveBoundary>) { for (b, segment) in &mut self.segments { *b = b.subset(boundary); - segment.make_subset(boundary.normalize()); + + assert!( + segment.is_normalized(), + "Expected normalized segment for making subset." + ); + segment.boundary = *b; + segment.points.make_subset(boundary.normalize()); } self.segments.retain(|(_, segment)| !segment.is_empty()); diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index eb65cfda0..69dc643c2 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -60,21 +60,6 @@ impl CurveApproxSegment { self } - - /// Reduce the approximation to the subset defined by the provided boundary - pub fn make_subset(&mut self, boundary: CurveBoundary>) { - assert!( - self.is_normalized(), - "Expected normalized segment for making subset." - ); - assert!( - boundary.is_normalized(), - "Expected subset to be defined by normalized boundary." - ); - - self.boundary = self.boundary.subset(boundary); - self.points.make_subset(boundary); - } } impl Ord for CurveApproxSegment { From 4b42aedb01c55e1f7b83734e569314ff22d6fe7a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 13:01:19 +0200 Subject: [PATCH 17/19] Consolidate redundant boundaries in `CurveApprox` --- .../src/algorithms/approx/curve/approx.rs | 25 +++++++------------ crates/fj-core/src/algorithms/approx/edge.rs | 7 ++++-- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index e2e3d594e..c09227897 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -4,13 +4,13 @@ use fj_math::Point; use crate::geometry::CurveBoundary; -use super::CurveApproxSegment; +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>, CurveApproxSegment)>, + pub segments: Vec<(CurveBoundary>, CurveApproxPoints)>, } impl CurveApprox { @@ -30,13 +30,7 @@ impl CurveApprox { pub fn make_subset(&mut self, boundary: CurveBoundary>) { for (b, segment) in &mut self.segments { *b = b.subset(boundary); - - assert!( - segment.is_normalized(), - "Expected normalized segment for making subset." - ); - segment.boundary = *b; - segment.points.make_subset(boundary.normalize()); + segment.make_subset(boundary.normalize()); } self.segments.retain(|(_, segment)| !segment.is_empty()); @@ -74,18 +68,17 @@ impl CurveApprox { ); merged_boundary = merged_boundary.union(boundary); - merged_segment.merge(&segment.points, boundary); + merged_segment.merge(&segment, boundary); } - let merged_segment = CurveApproxSegment { - boundary: merged_boundary, - points: merged_segment, - }; self.segments .push((merged_boundary, merged_segment.clone())); self.segments.sort(); - merged_segment + CurveApproxSegment { + boundary: merged_boundary, + points: merged_segment, + } } } @@ -94,7 +87,7 @@ impl From<[CurveApproxSegment; N]> for CurveApprox { Self { segments: segments .into_iter() - .map(|segment| (segment.boundary, segment)) + .map(|segment| (segment.boundary, segment.points)) .collect(), } } diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 1a34daa17..51be77c8e 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -56,11 +56,14 @@ impl Approx for (&Edge, &Surface) { .get_curve_approx(edge.curve().clone(), edge.boundary()); match cached.segments.pop() { - Some((_, segment)) if cached.segments.is_empty() => { + Some((boundary, segment)) 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. - segment + CurveApproxSegment { + boundary, + points: segment, + } } _ => { // If we make it here, there are holes in the From 0dfe80b10b1a9ddab6610c5b46c49a47b53aa69d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 13:08:22 +0200 Subject: [PATCH 18/19] Update variable name --- crates/fj-core/src/algorithms/approx/edge.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 51be77c8e..c9e657b5b 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -56,14 +56,11 @@ impl Approx for (&Edge, &Surface) { .get_curve_approx(edge.curve().clone(), edge.boundary()); match cached.segments.pop() { - Some((boundary, segment)) if cached.segments.is_empty() => { + 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: segment, - } + CurveApproxSegment { boundary, points } } _ => { // If we make it here, there are holes in the From aa324cc698aead113437654faf21b2378152818f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 13:10:51 +0200 Subject: [PATCH 19/19] Remove unused method --- .../fj-core/src/algorithms/approx/curve/segment.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/segment.rs b/crates/fj-core/src/algorithms/approx/curve/segment.rs index 69dc643c2..47904bb70 100644 --- a/crates/fj-core/src/algorithms/approx/curve/segment.rs +++ b/crates/fj-core/src/algorithms/approx/curve/segment.rs @@ -22,20 +22,6 @@ pub struct CurveApproxSegment { } impl CurveApproxSegment { - /// Indicate whether the segment is empty - pub fn is_empty(&self) -> bool { - let is_empty = self.boundary.is_empty(); - - if is_empty { - assert!( - self.points.is_empty(), - "Empty approximation still has points" - ); - } - - is_empty - } - /// Indicate whether the segment is normalized pub fn is_normalized(&self) -> bool { self.boundary.is_normalized()