From 121ab2c43d1a2a6c7f366aa74cc7365a7f33cb4b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:24:58 +0200 Subject: [PATCH 01/11] Convert `CurveFaceIntersection` into struct --- .../src/algorithms/intersection/curve_face.rs | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index cbbf730fe..3de335dc6 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -22,7 +22,10 @@ impl CurveFaceIntersectionList { ) -> Self { let intervals = intervals .into_iter() - .map(|interval| interval.map(Into::into)) + .map(|interval| { + let interval = interval.map(Into::into); + CurveFaceIntersection { interval } + }) .collect(); Self { intervals } } @@ -61,7 +64,8 @@ impl CurveFaceIntersectionList { .chunks(2) .map(|chunk| { // Can't panic, as we passed `2` to `chunks`. - [chunk[0], chunk[1]] + let interval = [chunk[0], chunk[1]]; + CurveFaceIntersection { interval } }) .collect(); @@ -82,8 +86,12 @@ impl CurveFaceIntersectionList { let mut intervals = Vec::new(); while let ( - Some([self_start, self_end]), - Some([other_start, other_end]), + Some(CurveFaceIntersection { + interval: [self_start, self_end], + }), + Some(CurveFaceIntersection { + interval: [other_start, other_end], + }), ) = (next_self, next_other) { // If we're starting another loop iteration, we have another @@ -98,7 +106,9 @@ impl CurveFaceIntersectionList { if overlap_start < overlap_end { // This is indeed a valid overlap. Add it to our list of // results. - intervals.push([overlap_start, overlap_end]); + intervals.push(CurveFaceIntersection { + interval: [overlap_start, overlap_end], + }); } // Only if the end of the overlap interval has overtaken one of the @@ -136,7 +146,11 @@ impl IntoIterator for CurveFaceIntersectionList { } /// An intersection between a curve and a face, in curve coordinates -pub type CurveFaceIntersection = [Point<1>; 2]; +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct CurveFaceIntersection { + /// The intersection interval, in curve coordinates + pub interval: [Point<1>; 2], +} #[cfg(test)] mod tests { From 8fea483d1a1510cda612f3c4367e2b01f44132db Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:25:55 +0200 Subject: [PATCH 02/11] Update doc comment --- crates/fj-kernel/src/algorithms/intersection/curve_face.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index 3de335dc6..612dee9cd 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -145,7 +145,7 @@ impl IntoIterator for CurveFaceIntersectionList { } } -/// An intersection between a curve and a face, in curve coordinates +/// An intersection between a curve and a face #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CurveFaceIntersection { /// The intersection interval, in curve coordinates From 65a1084b0d1282182b363a9a371436a5bb9ece30 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:26:53 +0200 Subject: [PATCH 03/11] Make struct name more specific --- .../src/algorithms/intersection/curve_face.rs | 16 ++++++++-------- .../fj-kernel/src/algorithms/intersection/mod.rs | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index 612dee9cd..5c4390771 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -10,7 +10,7 @@ use crate::{ /// The intersections between a [`Curve`] and a [`Face`], in curve coordinates #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CurveFaceIntersectionList { - intervals: Vec, + intervals: Vec, } impl CurveFaceIntersectionList { @@ -24,7 +24,7 @@ impl CurveFaceIntersectionList { .into_iter() .map(|interval| { let interval = interval.map(Into::into); - CurveFaceIntersection { interval } + CurveFaceIntersectionInterval { interval } }) .collect(); Self { intervals } @@ -65,7 +65,7 @@ impl CurveFaceIntersectionList { .map(|chunk| { // Can't panic, as we passed `2` to `chunks`. let interval = [chunk[0], chunk[1]]; - CurveFaceIntersection { interval } + CurveFaceIntersectionInterval { interval } }) .collect(); @@ -86,10 +86,10 @@ impl CurveFaceIntersectionList { let mut intervals = Vec::new(); while let ( - Some(CurveFaceIntersection { + Some(CurveFaceIntersectionInterval { interval: [self_start, self_end], }), - Some(CurveFaceIntersection { + Some(CurveFaceIntersectionInterval { interval: [other_start, other_end], }), ) = (next_self, next_other) @@ -106,7 +106,7 @@ impl CurveFaceIntersectionList { if overlap_start < overlap_end { // This is indeed a valid overlap. Add it to our list of // results. - intervals.push(CurveFaceIntersection { + intervals.push(CurveFaceIntersectionInterval { interval: [overlap_start, overlap_end], }); } @@ -137,7 +137,7 @@ impl CurveFaceIntersectionList { } impl IntoIterator for CurveFaceIntersectionList { - type Item = CurveFaceIntersection; + type Item = CurveFaceIntersectionInterval; type IntoIter = vec::IntoIter; fn into_iter(self) -> Self::IntoIter { @@ -147,7 +147,7 @@ impl IntoIterator for CurveFaceIntersectionList { /// An intersection between a curve and a face #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct CurveFaceIntersection { +pub struct CurveFaceIntersectionInterval { /// The intersection interval, in curve coordinates pub interval: [Point<1>; 2], } diff --git a/crates/fj-kernel/src/algorithms/intersection/mod.rs b/crates/fj-kernel/src/algorithms/intersection/mod.rs index df03b6abd..6e2a27de5 100644 --- a/crates/fj-kernel/src/algorithms/intersection/mod.rs +++ b/crates/fj-kernel/src/algorithms/intersection/mod.rs @@ -7,7 +7,7 @@ mod surface_surface; pub use self::{ curve_edge::CurveEdgeIntersection, - curve_face::{CurveFaceIntersection, CurveFaceIntersectionList}, + curve_face::{CurveFaceIntersectionInterval, CurveFaceIntersectionList}, line_segment::LineSegmentIntersection, surface_surface::SurfaceSurfaceIntersection, }; From e9219919f2c4556004d262d2ab437f607eb081bc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:29:14 +0200 Subject: [PATCH 04/11] Reorganize struct fields to make more explicit --- .../src/algorithms/intersection/curve_face.rs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index 5c4390771..e1920159e 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -23,8 +23,8 @@ impl CurveFaceIntersectionList { let intervals = intervals .into_iter() .map(|interval| { - let interval = interval.map(Into::into); - CurveFaceIntersectionInterval { interval } + let [start, end] = interval.map(Into::into); + CurveFaceIntersectionInterval { start, end } }) .collect(); Self { intervals } @@ -64,8 +64,8 @@ impl CurveFaceIntersectionList { .chunks(2) .map(|chunk| { // Can't panic, as we passed `2` to `chunks`. - let interval = [chunk[0], chunk[1]]; - CurveFaceIntersectionInterval { interval } + let [start, end] = [chunk[0], chunk[1]]; + CurveFaceIntersectionInterval { start, end } }) .collect(); @@ -87,10 +87,12 @@ impl CurveFaceIntersectionList { while let ( Some(CurveFaceIntersectionInterval { - interval: [self_start, self_end], + start: self_start, + end: self_end, }), Some(CurveFaceIntersectionInterval { - interval: [other_start, other_end], + start: other_start, + end: other_end, }), ) = (next_self, next_other) { @@ -107,7 +109,8 @@ impl CurveFaceIntersectionList { // This is indeed a valid overlap. Add it to our list of // results. intervals.push(CurveFaceIntersectionInterval { - interval: [overlap_start, overlap_end], + start: overlap_start, + end: overlap_end, }); } @@ -148,8 +151,11 @@ impl IntoIterator for CurveFaceIntersectionList { /// An intersection between a curve and a face #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CurveFaceIntersectionInterval { - /// The intersection interval, in curve coordinates - pub interval: [Point<1>; 2], + /// The start of the intersection interval, in curve coordinates + pub start: Point<1>, + + /// The end of the intersection interval, in curve coordinates + pub end: Point<1>, } #[cfg(test)] From 2a3fd26e2fa9b186c04805deb625e5287363d05e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:30:02 +0200 Subject: [PATCH 05/11] Refactor --- crates/fj-kernel/src/algorithms/intersection/curve_face.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index e1920159e..eb50830ce 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -64,8 +64,10 @@ impl CurveFaceIntersectionList { .chunks(2) .map(|chunk| { // Can't panic, as we passed `2` to `chunks`. - let [start, end] = [chunk[0], chunk[1]]; - CurveFaceIntersectionInterval { start, end } + CurveFaceIntersectionInterval { + start: chunk[0], + end: chunk[1], + } }) .collect(); From 52c0825921cf06fcba9d52ec5ace3865f5890538 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:31:38 +0200 Subject: [PATCH 06/11] Make variable name more explicit I'm about to make a change that would cause a conflict with the current name. --- crates/fj-kernel/src/algorithms/intersection/curve_face.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index eb50830ce..87ed22f0e 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -79,10 +79,10 @@ impl CurveFaceIntersectionList { /// The merged list will contain all overlaps of the intervals from the two /// other lists. pub fn merge(&self, other: &Self) -> Self { - let mut self_ = self.intervals.iter().copied(); + let mut self_intervals = self.intervals.iter().copied(); let mut other = other.intervals.iter().copied(); - let mut next_self = self_.next(); + let mut next_self = self_intervals.next(); let mut next_other = other.next(); let mut intervals = Vec::new(); @@ -123,7 +123,7 @@ impl CurveFaceIntersectionList { if self_end <= overlap_end { // Current interval from `self` has been overtaken. Let's grab // the next one. - next_self = self_.next(); + next_self = self_intervals.next(); } if other_end <= overlap_end { // Current interval from `other` has been overtaken. Let's grab From 59038ce12daf83adfe583c5be36d13e68e584de3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:32:04 +0200 Subject: [PATCH 07/11] Make variable name consistent with sibling var --- crates/fj-kernel/src/algorithms/intersection/curve_face.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index 87ed22f0e..32e8bf1e0 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -80,10 +80,10 @@ impl CurveFaceIntersectionList { /// other lists. pub fn merge(&self, other: &Self) -> Self { let mut self_intervals = self.intervals.iter().copied(); - let mut other = other.intervals.iter().copied(); + let mut other_interval = other.intervals.iter().copied(); let mut next_self = self_intervals.next(); - let mut next_other = other.next(); + let mut next_other = other_interval.next(); let mut intervals = Vec::new(); @@ -128,7 +128,7 @@ impl CurveFaceIntersectionList { if other_end <= overlap_end { // Current interval from `other` has been overtaken. Let's grab // the next one. - next_other = other.next(); + next_other = other_interval.next(); } } From 70a2174b687e95b4c6efbc902f41e1e5a3d824a6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:32:47 +0200 Subject: [PATCH 08/11] Refactor --- .../src/algorithms/intersection/curve_face.rs | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index 32e8bf1e0..cedfa0a4f 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -87,25 +87,15 @@ impl CurveFaceIntersectionList { let mut intervals = Vec::new(); - while let ( - Some(CurveFaceIntersectionInterval { - start: self_start, - end: self_end, - }), - Some(CurveFaceIntersectionInterval { - start: other_start, - end: other_end, - }), - ) = (next_self, next_other) - { + while let (Some(self_), Some(other)) = (next_self, next_other) { // If we're starting another loop iteration, we have another // interval available from both `self` and `other` each. Only if // that's the case, is there a chance for an overlap. // Build the overlap of the two next intervals, by comparing them. // At this point we don't know yet, if this is a valid interval. - let overlap_start = self_start.max(other_start); - let overlap_end = self_end.min(other_end); + let overlap_start = self_.start.max(other.start); + let overlap_end = self_.end.min(other.end); if overlap_start < overlap_end { // This is indeed a valid overlap. Add it to our list of @@ -120,12 +110,12 @@ impl CurveFaceIntersectionList { // input ones are we done with it. An input interval that hasn't // been overtaken by the overlap, could still overlap with another // interval. - if self_end <= overlap_end { + if self_.end <= overlap_end { // Current interval from `self` has been overtaken. Let's grab // the next one. next_self = self_intervals.next(); } - if other_end <= overlap_end { + if other.end <= overlap_end { // Current interval from `other` has been overtaken. Let's grab // the next one. next_other = other_interval.next(); From c4d59682b29407aed124269bdcef9aea54472a6e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:33:38 +0200 Subject: [PATCH 09/11] Simplify struct name The new name is also more accurate, I believe. --- .../src/algorithms/intersection/curve_face.rs | 24 +++++++++---------- .../src/algorithms/intersection/mod.rs | 2 +- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index cedfa0a4f..2d21a03da 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -9,11 +9,11 @@ use crate::{ /// The intersections between a [`Curve`] and a [`Face`], in curve coordinates #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct CurveFaceIntersectionList { +pub struct CurveFaceIntersection { intervals: Vec, } -impl CurveFaceIntersectionList { +impl CurveFaceIntersection { /// Create a new instance from the intersection intervals /// /// This method is useful for test code. @@ -71,7 +71,7 @@ impl CurveFaceIntersectionList { }) .collect(); - CurveFaceIntersectionList { intervals } + CurveFaceIntersection { intervals } } /// Merge this intersection list with another @@ -131,7 +131,7 @@ impl CurveFaceIntersectionList { } } -impl IntoIterator for CurveFaceIntersectionList { +impl IntoIterator for CurveFaceIntersection { type Item = CurveFaceIntersectionInterval; type IntoIter = vec::IntoIter; @@ -154,7 +154,7 @@ pub struct CurveFaceIntersectionInterval { mod tests { use crate::objects::{Curve, Face, Surface}; - use super::CurveFaceIntersectionList; + use super::CurveFaceIntersection; #[test] fn compute() { @@ -182,16 +182,14 @@ mod tests { .polygon_from_points(exterior) .with_hole(interior); - let expected = CurveFaceIntersectionList::from_intervals([ - [[1.], [2.]], - [[4.], [5.]], - ]); - assert_eq!(CurveFaceIntersectionList::compute(&curve, &face), expected); + let expected = + CurveFaceIntersection::from_intervals([[[1.], [2.]], [[4.], [5.]]]); + assert_eq!(CurveFaceIntersection::compute(&curve, &face), expected); } #[test] fn merge() { - let a = CurveFaceIntersectionList::from_intervals([ + let a = CurveFaceIntersection::from_intervals([ [[0.], [1.]], // 1: `a` and `b` are equal [[2.], [5.]], // 2: `a` contains `b` [[7.], [8.]], // 3: `b` contains `a` @@ -211,7 +209,7 @@ mod tests { [[62.], [63.]], // 13 [[65.], [66.]], // 14: one of `a` with no overlap in `b` ]); - let b = CurveFaceIntersectionList::from_intervals([ + let b = CurveFaceIntersection::from_intervals([ [[0.], [1.]], // 1 [[3.], [4.]], // 2 [[6.], [9.]], // 3 @@ -233,7 +231,7 @@ mod tests { let merged = a.merge(&b); - let expected = CurveFaceIntersectionList::from_intervals([ + let expected = CurveFaceIntersection::from_intervals([ [[0.], [1.]], // 1 [[3.], [4.]], // 2 [[7.], [8.]], // 3 diff --git a/crates/fj-kernel/src/algorithms/intersection/mod.rs b/crates/fj-kernel/src/algorithms/intersection/mod.rs index 6e2a27de5..06baac98b 100644 --- a/crates/fj-kernel/src/algorithms/intersection/mod.rs +++ b/crates/fj-kernel/src/algorithms/intersection/mod.rs @@ -7,7 +7,7 @@ mod surface_surface; pub use self::{ curve_edge::CurveEdgeIntersection, - curve_face::{CurveFaceIntersectionInterval, CurveFaceIntersectionList}, + curve_face::{CurveFaceIntersection, CurveFaceIntersectionInterval}, line_segment::LineSegmentIntersection, surface_surface::SurfaceSurfaceIntersection, }; From 09b3dcf2cd1f6c76b01657f10c7c7dcd20c028d7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:34:29 +0200 Subject: [PATCH 10/11] Make field public Previously, there was no way to actually access the result. --- crates/fj-kernel/src/algorithms/intersection/curve_face.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index 2d21a03da..751a46da9 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -10,7 +10,8 @@ use crate::{ /// The intersections between a [`Curve`] and a [`Face`], in curve coordinates #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CurveFaceIntersection { - intervals: Vec, + /// The intervals where the curve and face intersect, in curve coordinates + pub intervals: Vec, } impl CurveFaceIntersection { From 47655cc100811bf391bcad1c1d52f40992768b41 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 2 Aug 2022 15:37:21 +0200 Subject: [PATCH 11/11] Refactor --- .../src/algorithms/intersection/curve_face.rs | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs index 751a46da9..7a17a895a 100644 --- a/crates/fj-kernel/src/algorithms/intersection/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersection/curve_face.rs @@ -19,15 +19,11 @@ impl CurveFaceIntersection { /// /// This method is useful for test code. pub fn from_intervals( - intervals: impl IntoIterator>; 2]>, + intervals: impl IntoIterator< + Item = impl Into, + >, ) -> Self { - let intervals = intervals - .into_iter() - .map(|interval| { - let [start, end] = interval.map(Into::into); - CurveFaceIntersectionInterval { start, end } - }) - .collect(); + let intervals = intervals.into_iter().map(Into::into).collect(); Self { intervals } } @@ -151,6 +147,16 @@ pub struct CurveFaceIntersectionInterval { pub end: Point<1>, } +impl

From<[P; 2]> for CurveFaceIntersectionInterval +where + P: Into>, +{ + fn from(interval: [P; 2]) -> Self { + let [start, end] = interval.map(Into::into); + CurveFaceIntersectionInterval { start, end } + } +} + #[cfg(test)] mod tests { use crate::objects::{Curve, Face, Surface};