From 9ea1394b8aff221b321c3a7e5ae253b0404788d3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 10:28:40 +0200 Subject: [PATCH 01/13] Add dedicated directory for `boundary` module --- crates/fj-core/src/geometry/{boundary.rs => boundary/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename crates/fj-core/src/geometry/{boundary.rs => boundary/mod.rs} (100%) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary/mod.rs similarity index 100% rename from crates/fj-core/src/geometry/boundary.rs rename to crates/fj-core/src/geometry/boundary/mod.rs From 9a2cf9363ecd4cbae75754889b006b43fa457a2c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 10:29:54 +0200 Subject: [PATCH 02/13] Refactor to prepare for follow-on change --- crates/fj-core/src/geometry/boundary/mod.rs | 211 +----------------- .../fj-core/src/geometry/boundary/single.rs | 210 +++++++++++++++++ crates/fj-core/src/geometry/mod.rs | 2 +- 3 files changed, 212 insertions(+), 211 deletions(-) create mode 100644 crates/fj-core/src/geometry/boundary/single.rs diff --git a/crates/fj-core/src/geometry/boundary/mod.rs b/crates/fj-core/src/geometry/boundary/mod.rs index 7580e0b71..50c072992 100644 --- a/crates/fj-core/src/geometry/boundary/mod.rs +++ b/crates/fj-core/src/geometry/boundary/mod.rs @@ -1,210 +1 @@ -use std::{ - cmp::{self, Ordering}, - hash::{Hash, Hasher}, -}; - -use fj_math::Point; - -use crate::{objects::Vertex, storage::HandleWrapper}; - -/// A boundary on a curve -/// -/// This struct is generic, because different situations require different -/// representations of a boundary. In some cases, curve coordinates are enough, -/// in other cases, vertices are required, and sometimes you need both. -#[derive(Clone, Copy, Debug)] -pub struct CurveBoundary { - /// The raw representation of the boundary - pub inner: [T::Repr; 2], -} - -impl CurveBoundary { - /// Indicate whether the boundary is normalized - /// - /// If the boundary is normalized, its bounding elements are in a defined - /// order, and calling `normalize` will return an identical instance. - pub fn is_normalized(&self) -> bool { - let [a, b] = &self.inner; - a <= b - } - - /// Reverse the direction of the boundary - /// - /// Returns a new instance of this struct, which has its direction reversed. - #[must_use] - pub fn reverse(self) -> Self { - let [a, b] = self.inner; - Self { inner: [b, a] } - } - - /// Normalize the boundary - /// - /// Returns a new instance of this struct, which has the bounding elements - /// in a defined order. This can be used to compare boundaries while - /// disregarding their direction. - #[must_use] - pub fn normalize(self) -> Self { - if self.is_normalized() { - self - } else { - self.reverse() - } - } -} - -// Technically, these methods could be implemented for all -// `CurveBoundaryElement`s, but that would be misleading. While -// `HandleWrapper` implements `Ord`, which is useful for putting it (and -// by extension, `CurveBoundary`) into `BTreeMap`s, this `Ord` -// implementation doesn't actually define the geometrically meaningful ordering -// that the following methods rely on. -impl CurveBoundary> { - /// Indicate whether the boundary is empty - pub fn is_empty(&self) -> bool { - let [min, max] = &self.inner; - min >= max - } - - /// Indicate whether the boundary contains the given element - pub fn contains(&self, point: Point<1>) -> bool { - let [min, max] = self.inner; - point > min && point < max - } - - /// Indicate whether the boundary overlaps another - /// - /// Boundaries that touch (i.e. their closest boundary elements are equal) - /// count as overlapping. - pub fn overlaps(&self, other: &Self) -> bool { - let [a_low, a_high] = self.normalize().inner; - let [b_low, b_high] = other.normalize().inner; - - a_low <= b_high && a_high >= b_low - } - - /// Create the subset of this boundary and another - /// - /// The result will be normalized. - #[must_use] - pub fn subset(self, other: Self) -> Self { - let self_ = self.normalize(); - let other = other.normalize(); - - let [self_min, self_max] = self_.inner; - let [other_min, other_max] = other.inner; - - let min = cmp::max(self_min, other_min); - let max = cmp::min(self_max, other_max); - - Self { inner: [min, max] } - } - - /// Create the union of this boundary and another - /// - /// The result will be normalized. - /// - /// # Panics - /// - /// Panics, if the two boundaries don't overlap (touching counts as - /// overlapping). - pub fn union(self, other: Self) -> Self { - let self_ = self.normalize(); - let other = other.normalize(); - - assert!( - self.overlaps(&other), - "Can't merge boundaries that don't at least touch" - ); - - let [self_min, self_max] = self_.inner; - let [other_min, other_max] = other.inner; - - let min = cmp::min(self_min, other_min); - let max = cmp::max(self_max, other_max); - - Self { inner: [min, max] } - } -} - -impl From<[S; 2]> for CurveBoundary -where - S: Into, -{ - fn from(boundary: [S; 2]) -> Self { - let inner = boundary.map(Into::into); - Self { inner } - } -} - -impl Eq for CurveBoundary {} - -impl PartialEq for CurveBoundary { - fn eq(&self, other: &Self) -> bool { - self.inner == other.inner - } -} - -impl Hash for CurveBoundary { - fn hash(&self, state: &mut H) { - self.inner.hash(state); - } -} - -impl Ord for CurveBoundary { - fn cmp(&self, other: &Self) -> Ordering { - self.inner.cmp(&other.inner) - } -} - -impl PartialOrd for CurveBoundary { - fn partial_cmp(&self, other: &Self) -> Option { - self.inner.partial_cmp(&other.inner) - } -} - -/// An element of a curve boundary -/// -/// Used for the type parameter of [`CurveBoundary`]. -pub trait CurveBoundaryElement { - /// The representation the curve boundary element - /// - /// This is the actual data stored in [`CurveBoundary`]. - type Repr: Eq + Hash + Ord; -} - -impl CurveBoundaryElement for Point<1> { - type Repr = Self; -} - -impl CurveBoundaryElement for Vertex { - type Repr = HandleWrapper; -} - -#[cfg(test)] -mod tests { - use fj_math::Point; - - use crate::geometry::CurveBoundary; - - #[test] - fn overlaps() { - assert!(overlap([0., 2.], [1., 3.])); // regular overlap - assert!(overlap([0., 1.], [1., 2.])); // just touching - assert!(overlap([2., 0.], [3., 1.])); // not normalized - assert!(overlap([1., 3.], [0., 2.])); // lower boundary comes second - - assert!(!overlap([0., 1.], [2., 3.])); // regular non-overlap - assert!(!overlap([2., 3.], [0., 1.])); // lower boundary comes second - - fn overlap(a: [f64; 2], b: [f64; 2]) -> bool { - let a = array_to_boundary(a); - let b = array_to_boundary(b); - - a.overlaps(&b) - } - - fn array_to_boundary(array: [f64; 2]) -> CurveBoundary> { - CurveBoundary::from(array.map(|element| [element])) - } - } -} +pub mod single; diff --git a/crates/fj-core/src/geometry/boundary/single.rs b/crates/fj-core/src/geometry/boundary/single.rs new file mode 100644 index 000000000..7580e0b71 --- /dev/null +++ b/crates/fj-core/src/geometry/boundary/single.rs @@ -0,0 +1,210 @@ +use std::{ + cmp::{self, Ordering}, + hash::{Hash, Hasher}, +}; + +use fj_math::Point; + +use crate::{objects::Vertex, storage::HandleWrapper}; + +/// A boundary on a curve +/// +/// This struct is generic, because different situations require different +/// representations of a boundary. In some cases, curve coordinates are enough, +/// in other cases, vertices are required, and sometimes you need both. +#[derive(Clone, Copy, Debug)] +pub struct CurveBoundary { + /// The raw representation of the boundary + pub inner: [T::Repr; 2], +} + +impl CurveBoundary { + /// Indicate whether the boundary is normalized + /// + /// If the boundary is normalized, its bounding elements are in a defined + /// order, and calling `normalize` will return an identical instance. + pub fn is_normalized(&self) -> bool { + let [a, b] = &self.inner; + a <= b + } + + /// Reverse the direction of the boundary + /// + /// Returns a new instance of this struct, which has its direction reversed. + #[must_use] + pub fn reverse(self) -> Self { + let [a, b] = self.inner; + Self { inner: [b, a] } + } + + /// Normalize the boundary + /// + /// Returns a new instance of this struct, which has the bounding elements + /// in a defined order. This can be used to compare boundaries while + /// disregarding their direction. + #[must_use] + pub fn normalize(self) -> Self { + if self.is_normalized() { + self + } else { + self.reverse() + } + } +} + +// Technically, these methods could be implemented for all +// `CurveBoundaryElement`s, but that would be misleading. While +// `HandleWrapper` implements `Ord`, which is useful for putting it (and +// by extension, `CurveBoundary`) into `BTreeMap`s, this `Ord` +// implementation doesn't actually define the geometrically meaningful ordering +// that the following methods rely on. +impl CurveBoundary> { + /// Indicate whether the boundary is empty + pub fn is_empty(&self) -> bool { + let [min, max] = &self.inner; + min >= max + } + + /// Indicate whether the boundary contains the given element + pub fn contains(&self, point: Point<1>) -> bool { + let [min, max] = self.inner; + point > min && point < max + } + + /// Indicate whether the boundary overlaps another + /// + /// Boundaries that touch (i.e. their closest boundary elements are equal) + /// count as overlapping. + pub fn overlaps(&self, other: &Self) -> bool { + let [a_low, a_high] = self.normalize().inner; + let [b_low, b_high] = other.normalize().inner; + + a_low <= b_high && a_high >= b_low + } + + /// Create the subset of this boundary and another + /// + /// The result will be normalized. + #[must_use] + pub fn subset(self, other: Self) -> Self { + let self_ = self.normalize(); + let other = other.normalize(); + + let [self_min, self_max] = self_.inner; + let [other_min, other_max] = other.inner; + + let min = cmp::max(self_min, other_min); + let max = cmp::min(self_max, other_max); + + Self { inner: [min, max] } + } + + /// Create the union of this boundary and another + /// + /// The result will be normalized. + /// + /// # Panics + /// + /// Panics, if the two boundaries don't overlap (touching counts as + /// overlapping). + pub fn union(self, other: Self) -> Self { + let self_ = self.normalize(); + let other = other.normalize(); + + assert!( + self.overlaps(&other), + "Can't merge boundaries that don't at least touch" + ); + + let [self_min, self_max] = self_.inner; + let [other_min, other_max] = other.inner; + + let min = cmp::min(self_min, other_min); + let max = cmp::max(self_max, other_max); + + Self { inner: [min, max] } + } +} + +impl From<[S; 2]> for CurveBoundary +where + S: Into, +{ + fn from(boundary: [S; 2]) -> Self { + let inner = boundary.map(Into::into); + Self { inner } + } +} + +impl Eq for CurveBoundary {} + +impl PartialEq for CurveBoundary { + fn eq(&self, other: &Self) -> bool { + self.inner == other.inner + } +} + +impl Hash for CurveBoundary { + fn hash(&self, state: &mut H) { + self.inner.hash(state); + } +} + +impl Ord for CurveBoundary { + fn cmp(&self, other: &Self) -> Ordering { + self.inner.cmp(&other.inner) + } +} + +impl PartialOrd for CurveBoundary { + fn partial_cmp(&self, other: &Self) -> Option { + self.inner.partial_cmp(&other.inner) + } +} + +/// An element of a curve boundary +/// +/// Used for the type parameter of [`CurveBoundary`]. +pub trait CurveBoundaryElement { + /// The representation the curve boundary element + /// + /// This is the actual data stored in [`CurveBoundary`]. + type Repr: Eq + Hash + Ord; +} + +impl CurveBoundaryElement for Point<1> { + type Repr = Self; +} + +impl CurveBoundaryElement for Vertex { + type Repr = HandleWrapper; +} + +#[cfg(test)] +mod tests { + use fj_math::Point; + + use crate::geometry::CurveBoundary; + + #[test] + fn overlaps() { + assert!(overlap([0., 2.], [1., 3.])); // regular overlap + assert!(overlap([0., 1.], [1., 2.])); // just touching + assert!(overlap([2., 0.], [3., 1.])); // not normalized + assert!(overlap([1., 3.], [0., 2.])); // lower boundary comes second + + assert!(!overlap([0., 1.], [2., 3.])); // regular non-overlap + assert!(!overlap([2., 3.], [0., 1.])); // lower boundary comes second + + fn overlap(a: [f64; 2], b: [f64; 2]) -> bool { + let a = array_to_boundary(a); + let b = array_to_boundary(b); + + a.overlaps(&b) + } + + fn array_to_boundary(array: [f64; 2]) -> CurveBoundary> { + CurveBoundary::from(array.map(|element| [element])) + } + } +} diff --git a/crates/fj-core/src/geometry/mod.rs b/crates/fj-core/src/geometry/mod.rs index 0ed948703..d67ca02b4 100644 --- a/crates/fj-core/src/geometry/mod.rs +++ b/crates/fj-core/src/geometry/mod.rs @@ -5,7 +5,7 @@ mod path; mod surface; pub use self::{ - boundary::{CurveBoundary, CurveBoundaryElement}, + boundary::single::{CurveBoundary, CurveBoundaryElement}, path::{GlobalPath, SurfacePath}, surface::SurfaceGeometry, }; From 68332dbf28b1ec2e509693eadb358c77c7664b7d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 11:02:36 +0200 Subject: [PATCH 03/13] Add `CurveBoundaries` --- crates/fj-core/src/geometry/boundary/mod.rs | 1 + .../fj-core/src/geometry/boundary/multiple.rs | 19 +++++++++++++++++++ crates/fj-core/src/geometry/mod.rs | 5 ++++- 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 crates/fj-core/src/geometry/boundary/multiple.rs diff --git a/crates/fj-core/src/geometry/boundary/mod.rs b/crates/fj-core/src/geometry/boundary/mod.rs index 50c072992..4cb2b429d 100644 --- a/crates/fj-core/src/geometry/boundary/mod.rs +++ b/crates/fj-core/src/geometry/boundary/mod.rs @@ -1 +1,2 @@ +pub mod multiple; pub mod single; diff --git a/crates/fj-core/src/geometry/boundary/multiple.rs b/crates/fj-core/src/geometry/boundary/multiple.rs new file mode 100644 index 000000000..fb11fb0ad --- /dev/null +++ b/crates/fj-core/src/geometry/boundary/multiple.rs @@ -0,0 +1,19 @@ +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 { + /// The [`CurveBoundary`] instances + pub inner: Vec<(CurveBoundary>, T)>, +} + +impl Default for CurveBoundaries { + fn default() -> Self { + Self { inner: Vec::new() } + } +} diff --git a/crates/fj-core/src/geometry/mod.rs b/crates/fj-core/src/geometry/mod.rs index d67ca02b4..6e435af82 100644 --- a/crates/fj-core/src/geometry/mod.rs +++ b/crates/fj-core/src/geometry/mod.rs @@ -5,7 +5,10 @@ mod path; mod surface; pub use self::{ - boundary::single::{CurveBoundary, CurveBoundaryElement}, + boundary::{ + multiple::CurveBoundaries, + single::{CurveBoundary, CurveBoundaryElement}, + }, path::{GlobalPath, SurfacePath}, surface::SurfaceGeometry, }; From fb264b2e0781855b6af9901ffd818d42154c0e1f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 5 Oct 2023 11:04:31 +0200 Subject: [PATCH 04/13] Use `CurveBoundaries` in `CurveApproxSegment` --- .../src/algorithms/approx/curve/approx.rs | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 546ccafb0..01ce6229e 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -2,14 +2,14 @@ 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>, CurveApproxPoints)>, + segments: CurveBoundaries, } impl CurveApprox { @@ -18,8 +18,10 @@ impl CurveApprox { mut self, boundary: CurveBoundary>, ) -> Option { - match self.segments.pop() { - Some((b, points)) if self.segments.is_empty() && b == boundary => { + match self.segments.inner.pop() { + Some((b, points)) + if self.segments.inner.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. @@ -44,9 +46,9 @@ impl CurveApprox { /// Reverse the approximation pub fn reverse(&mut self) { - self.segments.reverse(); + self.segments.inner.reverse(); - for (boundary, segment) in &mut self.segments { + for (boundary, segment) in &mut self.segments.inner { *boundary = boundary.reverse(); segment.reverse(); } @@ -54,12 +56,14 @@ impl CurveApprox { /// Reduce the approximation to the subset defined by the provided boundary pub fn make_subset(&mut self, boundary: CurveBoundary>) { - for (b, segment) in &mut self.segments { + for (b, segment) in &mut self.segments.inner { *b = b.subset(boundary); segment.make_subset(boundary.normalize()); } - self.segments.retain(|(boundary, _)| !boundary.is_empty()); + self.segments + .inner + .retain(|(boundary, _)| !boundary.is_empty()); } /// Merge the provided segment into the approximation @@ -71,12 +75,12 @@ impl CurveApprox { let mut i = 0; loop { - let Some((boundary, _)) = self.segments.get(i) else { + let Some((boundary, _)) = self.segments.inner.get(i) else { break; }; if boundary.overlaps(&new_segment.boundary) { - let segment = self.segments.swap_remove(i); + let segment = self.segments.inner.swap_remove(i); overlapping_segments.push_back(segment); continue; } @@ -98,8 +102,9 @@ impl CurveApprox { } self.segments + .inner .push((merged_boundary, merged_segment.clone())); - self.segments.sort(); + self.segments.inner.sort(); CurveApproxSegment { boundary: merged_boundary, @@ -111,10 +116,12 @@ impl CurveApprox { impl From<[CurveApproxSegment; N]> for CurveApprox { fn from(segments: [CurveApproxSegment; N]) -> Self { Self { - segments: segments - .into_iter() - .map(|segment| (segment.boundary, segment.points)) - .collect(), + segments: CurveBoundaries { + inner: segments + .into_iter() + .map(|segment| (segment.boundary, segment.points)) + .collect(), + }, } } } From 3cc7aaf1017932e8f7bda0b1ba03f89f66ce58f6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 11:53:13 +0200 Subject: [PATCH 05/13] Add `CurveBoundaries::into_single_payload` --- .../src/algorithms/approx/curve/approx.rs | 29 +++-------------- .../fj-core/src/geometry/boundary/multiple.rs | 31 +++++++++++++++++++ 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 01ce6229e..190ded714 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -15,33 +15,12 @@ pub struct CurveApprox { impl CurveApprox { /// Get the single segment that covers the provided boundary, if available pub fn into_single_segment( - mut self, + self, boundary: CurveBoundary>, ) -> Option { - match self.segments.inner.pop() { - Some((b, points)) - if self.segments.inner.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 - } - } + self.segments + .into_single_payload(boundary) + .map(|points| CurveApproxSegment { boundary, points }) } /// Reverse the approximation diff --git a/crates/fj-core/src/geometry/boundary/multiple.rs b/crates/fj-core/src/geometry/boundary/multiple.rs index fb11fb0ad..ca1644e23 100644 --- a/crates/fj-core/src/geometry/boundary/multiple.rs +++ b/crates/fj-core/src/geometry/boundary/multiple.rs @@ -12,6 +12,37 @@ pub struct CurveBoundaries { pub inner: Vec<(CurveBoundary>, T)>, } +impl CurveBoundaries { + /// 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>, + ) -> Option { + 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 + } + } + } +} + impl Default for CurveBoundaries { fn default() -> Self { Self { inner: Vec::new() } From 83fc0e473234d1d9d5a659799b1fbe5f0fc080bd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 12:34:12 +0200 Subject: [PATCH 06/13] Add `CurveBoundariesPayload` --- crates/fj-core/src/algorithms/approx/curve/points.rs | 7 ++++++- crates/fj-core/src/geometry/boundary/multiple.rs | 11 ++++++++--- crates/fj-core/src/geometry/mod.rs | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs index 0fc036245..4d70699b6 100644 --- a/crates/fj-core/src/algorithms/approx/curve/points.rs +++ b/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)] @@ -45,3 +48,5 @@ impl CurveApproxPoints { self.inner.sort(); } } + +impl CurveBoundariesPayload for CurveApproxPoints {} diff --git a/crates/fj-core/src/geometry/boundary/multiple.rs b/crates/fj-core/src/geometry/boundary/multiple.rs index ca1644e23..f5b133157 100644 --- a/crates/fj-core/src/geometry/boundary/multiple.rs +++ b/crates/fj-core/src/geometry/boundary/multiple.rs @@ -7,12 +7,12 @@ use crate::geometry::CurveBoundary; /// 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 { +pub struct CurveBoundaries { /// The [`CurveBoundary`] instances pub inner: Vec<(CurveBoundary>, T)>, } -impl CurveBoundaries { +impl CurveBoundaries { /// Transform `self` into the payload of the single boundary requested /// /// If there are no boundaries or multiple boundaries in `self`, or if the @@ -43,8 +43,13 @@ impl CurveBoundaries { } } -impl Default for CurveBoundaries { +impl Default for CurveBoundaries { fn default() -> Self { Self { inner: Vec::new() } } } + +/// A payload that can be used in [`CurveBoundaries`] +pub trait CurveBoundariesPayload {} + +impl CurveBoundariesPayload for () {} diff --git a/crates/fj-core/src/geometry/mod.rs b/crates/fj-core/src/geometry/mod.rs index 6e435af82..fa971aa74 100644 --- a/crates/fj-core/src/geometry/mod.rs +++ b/crates/fj-core/src/geometry/mod.rs @@ -6,7 +6,7 @@ mod surface; pub use self::{ boundary::{ - multiple::CurveBoundaries, + multiple::{CurveBoundaries, CurveBoundariesPayload}, single::{CurveBoundary, CurveBoundaryElement}, }, path::{GlobalPath, SurfacePath}, From 578f1ad1dd0010457ec624734d6f4744b2af97d7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 12:36:14 +0200 Subject: [PATCH 07/13] Remove unused return value --- crates/fj-core/src/algorithms/approx/curve/points.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs index 4d70699b6..9bfd65c89 100644 --- a/crates/fj-core/src/algorithms/approx/curve/points.rs +++ b/crates/fj-core/src/algorithms/approx/curve/points.rs @@ -19,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 From 372d0fc11a71427c7c978bfb2453d40b19d0653c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 12:38:30 +0200 Subject: [PATCH 08/13] Add `CurveBoundaries::reverse` --- .../src/algorithms/approx/curve/approx.rs | 7 +------ .../src/algorithms/approx/curve/points.rs | 6 +++++- .../fj-core/src/geometry/boundary/multiple.rs | 19 +++++++++++++++++-- 3 files changed, 23 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 190ded714..f740d65a7 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -25,12 +25,7 @@ impl CurveApprox { /// Reverse the approximation pub fn reverse(&mut self) { - self.segments.inner.reverse(); - - for (boundary, segment) in &mut self.segments.inner { - *boundary = boundary.reverse(); - segment.reverse(); - } + self.segments.reverse(); } /// Reduce the approximation to the subset defined by the provided boundary diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs index 9bfd65c89..4223548eb 100644 --- a/crates/fj-core/src/algorithms/approx/curve/points.rs +++ b/crates/fj-core/src/algorithms/approx/curve/points.rs @@ -48,4 +48,8 @@ impl CurveApproxPoints { } } -impl CurveBoundariesPayload for CurveApproxPoints {} +impl CurveBoundariesPayload for CurveApproxPoints { + fn reverse(&mut self) { + self.reverse(); + } +} diff --git a/crates/fj-core/src/geometry/boundary/multiple.rs b/crates/fj-core/src/geometry/boundary/multiple.rs index f5b133157..047e7bdf1 100644 --- a/crates/fj-core/src/geometry/boundary/multiple.rs +++ b/crates/fj-core/src/geometry/boundary/multiple.rs @@ -41,6 +41,16 @@ impl CurveBoundaries { } } } + + /// 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(); + } + } } impl Default for CurveBoundaries { @@ -50,6 +60,11 @@ impl Default for CurveBoundaries { } /// A payload that can be used in [`CurveBoundaries`] -pub trait CurveBoundariesPayload {} +pub trait CurveBoundariesPayload { + /// Reverse the orientation of the payload + fn reverse(&mut self); +} -impl CurveBoundariesPayload for () {} +impl CurveBoundariesPayload for () { + fn reverse(&mut self) {} +} From 018368f6603e015f761b1d2382a7a74f5d10e029 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 12:42:03 +0200 Subject: [PATCH 09/13] Improve robustness of `CurveBoundary::contains` --- crates/fj-core/src/geometry/boundary/single.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/geometry/boundary/single.rs b/crates/fj-core/src/geometry/boundary/single.rs index 7580e0b71..bd11aa384 100644 --- a/crates/fj-core/src/geometry/boundary/single.rs +++ b/crates/fj-core/src/geometry/boundary/single.rs @@ -67,7 +67,7 @@ impl CurveBoundary> { /// 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 } From aa0bef70b0ac5ee2c23756620459dc642fc2c47f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 12:42:20 +0200 Subject: [PATCH 10/13] Remove redundant call --- 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 f740d65a7..f706e85fd 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -32,7 +32,7 @@ impl CurveApprox { pub fn make_subset(&mut self, boundary: CurveBoundary>) { for (b, segment) in &mut self.segments.inner { *b = b.subset(boundary); - segment.make_subset(boundary.normalize()); + segment.make_subset(boundary); } self.segments From 2ef4d4a6bb546dd5dc35be1c9b09e6253d25a38a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 12:43:22 +0200 Subject: [PATCH 11/13] Add `CurveBoundaries::make_subset` --- .../fj-core/src/algorithms/approx/curve/approx.rs | 9 +-------- .../fj-core/src/algorithms/approx/curve/points.rs | 4 ++++ crates/fj-core/src/geometry/boundary/multiple.rs | 14 ++++++++++++++ 3 files changed, 19 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 f706e85fd..4f1bbc047 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -30,14 +30,7 @@ impl CurveApprox { /// Reduce the approximation to the subset defined by the provided boundary pub fn make_subset(&mut self, boundary: CurveBoundary>) { - for (b, segment) in &mut self.segments.inner { - *b = b.subset(boundary); - segment.make_subset(boundary); - } - - self.segments - .inner - .retain(|(boundary, _)| !boundary.is_empty()); + self.segments.make_subset(boundary); } /// Merge the provided segment into the approximation diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs index 4223548eb..631803fde 100644 --- a/crates/fj-core/src/algorithms/approx/curve/points.rs +++ b/crates/fj-core/src/algorithms/approx/curve/points.rs @@ -52,4 +52,8 @@ impl CurveBoundariesPayload for CurveApproxPoints { fn reverse(&mut self) { self.reverse(); } + + fn make_subset(&mut self, boundary: CurveBoundary>) { + self.make_subset(boundary) + } } diff --git a/crates/fj-core/src/geometry/boundary/multiple.rs b/crates/fj-core/src/geometry/boundary/multiple.rs index 047e7bdf1..db5a5b83c 100644 --- a/crates/fj-core/src/geometry/boundary/multiple.rs +++ b/crates/fj-core/src/geometry/boundary/multiple.rs @@ -51,6 +51,16 @@ impl CurveBoundaries { payload.reverse(); } } + + /// Reduce `self` to the subset defined by the provided boundary + pub fn make_subset(&mut self, boundary: CurveBoundary>) { + for (b, segment) in &mut self.inner { + *b = b.subset(boundary); + segment.make_subset(boundary); + } + + self.inner.retain(|(boundary, _)| !boundary.is_empty()); + } } impl Default for CurveBoundaries { @@ -63,8 +73,12 @@ impl Default for CurveBoundaries { pub trait CurveBoundariesPayload { /// 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>); } impl CurveBoundariesPayload for () { fn reverse(&mut self) {} + fn make_subset(&mut self, _: CurveBoundary>) {} } From 3e6bd8cd140699808c82e819c29ba74ed704e1b7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 12:48:44 +0200 Subject: [PATCH 12/13] Add trait bounds to `CurveBoundariesPayload` --- crates/fj-core/src/geometry/boundary/multiple.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/geometry/boundary/multiple.rs b/crates/fj-core/src/geometry/boundary/multiple.rs index db5a5b83c..5ef26469a 100644 --- a/crates/fj-core/src/geometry/boundary/multiple.rs +++ b/crates/fj-core/src/geometry/boundary/multiple.rs @@ -70,7 +70,7 @@ impl Default for CurveBoundaries { } /// A payload that can be used in [`CurveBoundaries`] -pub trait CurveBoundariesPayload { +pub trait CurveBoundariesPayload: Clone + Ord { /// Reverse the orientation of the payload fn reverse(&mut self); From 7cae10645e910faab63400da2a97171af79b7540 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Oct 2023 12:50:05 +0200 Subject: [PATCH 13/13] Add `CurveBoundaries::merge` --- .../src/algorithms/approx/curve/approx.rs | 39 ++------------- .../src/algorithms/approx/curve/points.rs | 4 ++ .../fj-core/src/geometry/boundary/multiple.rs | 50 +++++++++++++++++++ 3 files changed, 57 insertions(+), 36 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/approx.rs b/crates/fj-core/src/algorithms/approx/curve/approx.rs index 4f1bbc047..b0ee82d3f 100644 --- a/crates/fj-core/src/algorithms/approx/curve/approx.rs +++ b/crates/fj-core/src/algorithms/approx/curve/approx.rs @@ -1,5 +1,3 @@ -use std::collections::VecDeque; - use fj_math::Point; use crate::geometry::{CurveBoundaries, CurveBoundary}; @@ -38,40 +36,9 @@ impl CurveApprox { &mut self, new_segment: CurveApproxSegment, ) -> CurveApproxSegment { - let mut overlapping_segments = VecDeque::new(); - - let mut i = 0; - loop { - let Some((boundary, _)) = self.segments.inner.get(i) else { - break; - }; - - if boundary.overlaps(&new_segment.boundary) { - let segment = self.segments.inner.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 - .inner - .push((merged_boundary, merged_segment.clone())); - self.segments.inner.sort(); + let (merged_boundary, merged_segment) = self + .segments + .merge(new_segment.boundary, new_segment.points); CurveApproxSegment { boundary: merged_boundary, diff --git a/crates/fj-core/src/algorithms/approx/curve/points.rs b/crates/fj-core/src/algorithms/approx/curve/points.rs index 631803fde..ee0fbc56a 100644 --- a/crates/fj-core/src/algorithms/approx/curve/points.rs +++ b/crates/fj-core/src/algorithms/approx/curve/points.rs @@ -56,4 +56,8 @@ impl CurveBoundariesPayload for CurveApproxPoints { fn make_subset(&mut self, boundary: CurveBoundary>) { self.make_subset(boundary) } + + fn merge(&mut self, other: &Self, other_boundary: CurveBoundary>) { + self.merge(other, other_boundary) + } } diff --git a/crates/fj-core/src/geometry/boundary/multiple.rs b/crates/fj-core/src/geometry/boundary/multiple.rs index 5ef26469a..b4516e3b7 100644 --- a/crates/fj-core/src/geometry/boundary/multiple.rs +++ b/crates/fj-core/src/geometry/boundary/multiple.rs @@ -1,3 +1,5 @@ +use std::collections::VecDeque; + use fj_math::Point; use crate::geometry::CurveBoundary; @@ -61,6 +63,50 @@ impl CurveBoundaries { 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>, + new_payload: T, + ) -> (CurveBoundary>, 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 Default for CurveBoundaries { @@ -76,9 +122,13 @@ pub trait CurveBoundariesPayload: Clone + Ord { /// Reduce the payload to the subset defined by the provided boundary fn make_subset(&mut self, boundary: CurveBoundary>); + + /// Merge the provided payload + fn merge(&mut self, other: &Self, other_boundary: CurveBoundary>); } impl CurveBoundariesPayload for () { fn reverse(&mut self) {} fn make_subset(&mut self, _: CurveBoundary>) {} + fn merge(&mut self, _: &Self, _: CurveBoundary>) {} }