From ff2a452aa2488257937166cc723344c88dab8c4c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 16 May 2022 18:30:45 +0200 Subject: [PATCH 1/8] Update parameter names --- crates/fj-kernel/src/topology/faces.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/topology/faces.rs b/crates/fj-kernel/src/topology/faces.rs index 34461bedd..e6522a982 100644 --- a/crates/fj-kernel/src/topology/faces.rs +++ b/crates/fj-kernel/src/topology/faces.rs @@ -93,7 +93,7 @@ impl Face { pub fn exteriors(&self) -> impl Iterator> + '_ { match self { Self::Face { exteriors, .. } => { - exteriors.iter().map(|handle| handle.get()) + exteriors.iter().map(|edge| edge.get()) } _ => { // No code that still uses triangle representation is calling @@ -110,7 +110,7 @@ impl Face { pub fn interiors(&self) -> impl Iterator> + '_ { match self { Self::Face { interiors, .. } => { - interiors.iter().map(|handle| handle.get()) + interiors.iter().map(|edge| edge.get()) } _ => { // No code that still uses triangle representation is calling From 7085c1363769de25611afd2f9a191d904f2322cd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 16 May 2022 18:40:32 +0200 Subject: [PATCH 2/8] Add `CyclesInFace` --- crates/fj-kernel/src/topology/faces.rs | 7 +++++-- crates/fj-kernel/src/topology/mod.rs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/topology/faces.rs b/crates/fj-kernel/src/topology/faces.rs index e6522a982..7d42d4cc1 100644 --- a/crates/fj-kernel/src/topology/faces.rs +++ b/crates/fj-kernel/src/topology/faces.rs @@ -41,7 +41,7 @@ pub enum Face { /// /// It might be less error-prone to specify the cycles in surface /// coordinates. - exteriors: Vec>>, + exteriors: CyclesInFace, /// The cycles that bound the face on the inside /// @@ -50,7 +50,7 @@ pub enum Face { /// # Implementation note /// /// See note on `exterior` field. - interiors: Vec>>, + interiors: CyclesInFace, /// The color of the face color: [u8; 4], @@ -145,3 +145,6 @@ impl Hash for Face { } } } + +/// A list of cycles, as they are stored in `Face` +pub type CyclesInFace = Vec>>; diff --git a/crates/fj-kernel/src/topology/mod.rs b/crates/fj-kernel/src/topology/mod.rs index 108570065..dbfc15c39 100644 --- a/crates/fj-kernel/src/topology/mod.rs +++ b/crates/fj-kernel/src/topology/mod.rs @@ -26,6 +26,6 @@ pub use self::{ builder::{CycleBuilder, EdgeBuilder, FaceBuilder, VertexBuilder}, cycles::Cycle, edges::Edge, - faces::Face, + faces::{CyclesInFace, Face}, vertices::Vertex, }; From 9b51b44ad507adc72fdb219e2932735f6f9e7bb5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 16 May 2022 18:52:20 +0200 Subject: [PATCH 3/8] Add `Face::new` --- crates/fj-kernel/src/algorithms/sweep.rs | 32 +++++++---------------- crates/fj-kernel/src/shape/api.rs | 29 +++++++++----------- crates/fj-kernel/src/topology/builder.rs | 6 ++--- crates/fj-kernel/src/topology/faces.rs | 17 ++++++++++++ crates/fj-operations/src/circle.rs | 9 ++----- crates/fj-operations/src/difference_2d.rs | 7 +---- crates/fj-operations/src/group.rs | 16 ++++-------- 7 files changed, 49 insertions(+), 67 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index 7699a160a..ff203d78a 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -90,20 +90,15 @@ pub fn sweep_shape( let interiors_top = source_to_top.interiors_for_face(&face_source); target - .insert(Face::Face { - surface: surface_bottom, - exteriors: exteriors_bottom, - interiors: interiors_bottom, + .insert(Face::new( + surface_bottom, + exteriors_bottom, + interiors_bottom, color, - }) + )) .unwrap(); target - .insert(Face::Face { - surface: surface_top, - exteriors: exteriors_top, - interiors: interiors_top, - color, - }) + .insert(Face::new(surface_top, exteriors_top, interiors_top, color)) .unwrap(); } @@ -213,12 +208,7 @@ pub fn sweep_shape( .unwrap(); target - .insert(Face::Face { - surface, - exteriors: vec![cycle], - interiors: Vec::new(), - color, - }) + .insert(Face::new(surface, vec![cycle], Vec::new(), color)) .unwrap(); } } @@ -385,12 +375,8 @@ mod tests { let surface = if reverse { surface.reverse() } else { surface }; let surface = shape.insert(surface)?; - let abc = Face::Face { - surface, - exteriors: vec![cycles], - interiors: Vec::new(), - color: [255, 0, 0, 255], - }; + let abc = + Face::new(surface, vec![cycles], Vec::new(), [255, 0, 0, 255]); let face = shape.insert(abc)?; diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index 9ef11e54c..6b664603d 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -246,12 +246,7 @@ mod tests { let cycle = shape.insert(cycle)?; assert!(shape.get_handle(&cycle.get()).as_ref() == Some(&cycle)); - let face = Face::Face { - surface, - exteriors: Vec::new(), - interiors: Vec::new(), - color: [0, 0, 0, 0], - }; + let face = Face::new(surface, Vec::new(), Vec::new(), [0, 0, 0, 0]); assert!(shape.get_handle(&face).is_none()); let face = shape.insert(face)?; @@ -341,12 +336,12 @@ mod tests { // Nothing has been added to `shape`. Should fail. let err = shape - .insert(Face::Face { - surface: surface.clone(), - exteriors: vec![cycle.clone()], - interiors: Vec::new(), - color: [255, 0, 0, 255], - }) + .insert(Face::new( + surface.clone(), + vec![cycle.clone()], + Vec::new(), + [255, 0, 0, 255], + )) .unwrap_err(); assert!(err.missing_surface(&surface)); assert!(err.missing_cycle(&cycle)); @@ -355,12 +350,12 @@ mod tests { let cycle = shape.add_cycle()?; // Everything has been added to `shape` now. Should work! - shape.insert(Face::Face { + shape.insert(Face::new( surface, - exteriors: vec![cycle], - interiors: Vec::new(), - color: [255, 0, 0, 255], - })?; + vec![cycle], + Vec::new(), + [255, 0, 0, 255], + ))?; Ok(()) } diff --git a/crates/fj-kernel/src/topology/builder.rs b/crates/fj-kernel/src/topology/builder.rs index 2b1fc1b5a..1bf5adf75 100644 --- a/crates/fj-kernel/src/topology/builder.rs +++ b/crates/fj-kernel/src/topology/builder.rs @@ -197,11 +197,11 @@ impl<'r> FaceBuilder<'r> { interiors.push(cycle); } - self.shape.insert(Face::Face { + self.shape.insert(Face::new( surface, exteriors, interiors, - color: [255, 0, 0, 255], - }) + [255, 0, 0, 255], + )) } } diff --git a/crates/fj-kernel/src/topology/faces.rs b/crates/fj-kernel/src/topology/faces.rs index 7d42d4cc1..51344bf88 100644 --- a/crates/fj-kernel/src/topology/faces.rs +++ b/crates/fj-kernel/src/topology/faces.rs @@ -66,6 +66,23 @@ pub enum Face { } impl Face { + /// Construct a new instance of `Face` + pub fn new( + surface: Handle, + exteriors: impl IntoIterator>>, + interiors: impl IntoIterator>>, + color: [u8; 4], + ) -> Self { + let exteriors = exteriors.into_iter().collect(); + let interiors = interiors.into_iter().collect(); + + Self::Face { + surface, + exteriors, + interiors, + color, + } + } /// Build a face using the [`FaceBuilder`] API pub fn builder(surface: Surface, shape: &mut Shape) -> FaceBuilder { FaceBuilder::new(surface, shape) diff --git a/crates/fj-operations/src/circle.rs b/crates/fj-operations/src/circle.rs index 3c86e85fc..f3ab8b386 100644 --- a/crates/fj-operations/src/circle.rs +++ b/crates/fj-operations/src/circle.rs @@ -21,15 +21,10 @@ impl ToShape for fj::Circle { .unwrap(); shape.insert(Cycle::new(vec![edge])).unwrap(); - let cycles = shape.cycles().collect(); + let cycles = shape.cycles(); let surface = shape.insert(Surface::xy_plane()).unwrap(); shape - .insert(Face::Face { - exteriors: cycles, - interiors: Vec::new(), - surface, - color: self.color(), - }) + .insert(Face::new(surface, cycles, Vec::new(), self.color())) .unwrap(); shape diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 7011bac74..45a245831 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -72,12 +72,7 @@ impl ToShape for fj::Difference2d { let surface = shape.insert(face_a.surface()).unwrap(); shape - .insert(Face::Face { - surface, - exteriors, - interiors, - color: self.color(), - }) + .insert(Face::new(surface, exteriors, interiors, self.color())) .unwrap(); shape diff --git a/crates/fj-operations/src/group.rs b/crates/fj-operations/src/group.rs index 1e88d1789..4661f8380 100644 --- a/crates/fj-operations/src/group.rs +++ b/crates/fj-operations/src/group.rs @@ -94,18 +94,12 @@ fn copy_shape(orig: Shape, target: &mut Shape) { color, } => { target - .insert(Face::Face { - surface: surfaces[&surface].clone(), - exteriors: exteriors - .iter() - .map(|cycle| cycles[cycle].clone()) - .collect(), - interiors: interiors - .iter() - .map(|cycle| cycles[cycle].clone()) - .collect(), + .insert(Face::new( + surfaces[&surface].clone(), + exteriors.iter().map(|cycle| cycles[cycle].clone()), + interiors.iter().map(|cycle| cycles[cycle].clone()), color, - }) + )) .unwrap(); } face @ Face::Triangles(_) => { From 8cae216ba7f2499dbd0c94c2fcdfd1f1e1fee07b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 16 May 2022 18:56:04 +0200 Subject: [PATCH 4/8] Convert `CyclesInFace` into struct --- crates/fj-kernel/src/algorithms/sweep.rs | 2 ++ crates/fj-kernel/src/shape/validate.rs | 2 +- crates/fj-kernel/src/topology/faces.rs | 11 ++++++----- crates/fj-operations/src/group.rs | 4 ++-- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index ff203d78a..f1295ee24 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -266,6 +266,7 @@ impl Relation { }; exteriors + .0 .iter() .map(|cycle| self.cycles.get(cycle).unwrap().clone()) .collect() @@ -282,6 +283,7 @@ impl Relation { }; interiors + .0 .iter() .map(|cycle| self.cycles.get(cycle).unwrap().clone()) .collect() diff --git a/crates/fj-kernel/src/shape/validate.rs b/crates/fj-kernel/src/shape/validate.rs index 16a10b69b..deca757ad 100644 --- a/crates/fj-kernel/src/shape/validate.rs +++ b/crates/fj-kernel/src/shape/validate.rs @@ -149,7 +149,7 @@ impl Validate for Face { if !stores.surfaces.contains(surface) { missing_surface = Some(surface.clone()); } - for cycle in exteriors.iter().chain(interiors) { + for cycle in exteriors.0.iter().chain(&interiors.0) { if !stores.cycles.contains(cycle) { missing_cycles.insert(cycle.clone()); } diff --git a/crates/fj-kernel/src/topology/faces.rs b/crates/fj-kernel/src/topology/faces.rs index 51344bf88..4a7df6d36 100644 --- a/crates/fj-kernel/src/topology/faces.rs +++ b/crates/fj-kernel/src/topology/faces.rs @@ -73,8 +73,8 @@ impl Face { interiors: impl IntoIterator>>, color: [u8; 4], ) -> Self { - let exteriors = exteriors.into_iter().collect(); - let interiors = interiors.into_iter().collect(); + let exteriors = CyclesInFace(exteriors.into_iter().collect()); + let interiors = CyclesInFace(interiors.into_iter().collect()); Self::Face { surface, @@ -110,7 +110,7 @@ impl Face { pub fn exteriors(&self) -> impl Iterator> + '_ { match self { Self::Face { exteriors, .. } => { - exteriors.iter().map(|edge| edge.get()) + exteriors.0.iter().map(|edge| edge.get()) } _ => { // No code that still uses triangle representation is calling @@ -127,7 +127,7 @@ impl Face { pub fn interiors(&self) -> impl Iterator> + '_ { match self { Self::Face { interiors, .. } => { - interiors.iter().map(|edge| edge.get()) + interiors.0.iter().map(|edge| edge.get()) } _ => { // No code that still uses triangle representation is calling @@ -164,4 +164,5 @@ impl Hash for Face { } /// A list of cycles, as they are stored in `Face` -pub type CyclesInFace = Vec>>; +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct CyclesInFace(pub Vec>>); diff --git a/crates/fj-operations/src/group.rs b/crates/fj-operations/src/group.rs index 4661f8380..a18f0d922 100644 --- a/crates/fj-operations/src/group.rs +++ b/crates/fj-operations/src/group.rs @@ -96,8 +96,8 @@ fn copy_shape(orig: Shape, target: &mut Shape) { target .insert(Face::new( surfaces[&surface].clone(), - exteriors.iter().map(|cycle| cycles[cycle].clone()), - interiors.iter().map(|cycle| cycles[cycle].clone()), + exteriors.0.iter().map(|cycle| cycles[cycle].clone()), + interiors.0.iter().map(|cycle| cycles[cycle].clone()), color, )) .unwrap(); From 74b9f5fb2b880443e484a928828da76a24ec887e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 16 May 2022 18:58:35 +0200 Subject: [PATCH 5/8] Consolidate redundant code --- crates/fj-kernel/src/topology/faces.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/topology/faces.rs b/crates/fj-kernel/src/topology/faces.rs index 4a7df6d36..46bfadd11 100644 --- a/crates/fj-kernel/src/topology/faces.rs +++ b/crates/fj-kernel/src/topology/faces.rs @@ -73,8 +73,8 @@ impl Face { interiors: impl IntoIterator>>, color: [u8; 4], ) -> Self { - let exteriors = CyclesInFace(exteriors.into_iter().collect()); - let interiors = CyclesInFace(interiors.into_iter().collect()); + let exteriors = CyclesInFace::from_canonical(exteriors); + let interiors = CyclesInFace::from_canonical(interiors); Self::Face { surface, @@ -166,3 +166,11 @@ impl Hash for Face { /// A list of cycles, as they are stored in `Face` #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CyclesInFace(pub Vec>>); + +impl CyclesInFace { + fn from_canonical( + cycles: impl IntoIterator>>, + ) -> Self { + Self(cycles.into_iter().collect()) + } +} From 8d70529b831dc5cd9e9677799962214c46d21812 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 16 May 2022 19:00:39 +0200 Subject: [PATCH 6/8] Consolidate redundant code --- crates/fj-kernel/src/topology/faces.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/topology/faces.rs b/crates/fj-kernel/src/topology/faces.rs index 46bfadd11..cff40c993 100644 --- a/crates/fj-kernel/src/topology/faces.rs +++ b/crates/fj-kernel/src/topology/faces.rs @@ -109,9 +109,7 @@ impl Face { /// [`Handle`]s. pub fn exteriors(&self) -> impl Iterator> + '_ { match self { - Self::Face { exteriors, .. } => { - exteriors.0.iter().map(|edge| edge.get()) - } + Self::Face { exteriors, .. } => exteriors.as_canonical(), _ => { // No code that still uses triangle representation is calling // this method. @@ -126,9 +124,7 @@ impl Face { /// [`Handle`]s. pub fn interiors(&self) -> impl Iterator> + '_ { match self { - Self::Face { interiors, .. } => { - interiors.0.iter().map(|edge| edge.get()) - } + Self::Face { interiors, .. } => interiors.as_canonical(), _ => { // No code that still uses triangle representation is calling // this method. @@ -173,4 +169,8 @@ impl CyclesInFace { ) -> Self { Self(cycles.into_iter().collect()) } + + fn as_canonical(&self) -> impl Iterator> + '_ { + self.0.iter().map(|edge| edge.get()) + } } From 35fe5cc2e201148e70631a6128d8f5a6782e8d31 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 16 May 2022 19:06:47 +0200 Subject: [PATCH 7/8] Add `CyclesInFace::as_handle` --- crates/fj-kernel/src/algorithms/sweep.rs | 6 ++---- crates/fj-kernel/src/shape/validate.rs | 2 +- crates/fj-kernel/src/topology/faces.rs | 7 ++++++- crates/fj-operations/src/group.rs | 8 ++++++-- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index f1295ee24..ab8839bb5 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -266,8 +266,7 @@ impl Relation { }; exteriors - .0 - .iter() + .as_handle() .map(|cycle| self.cycles.get(cycle).unwrap().clone()) .collect() } @@ -283,8 +282,7 @@ impl Relation { }; interiors - .0 - .iter() + .as_handle() .map(|cycle| self.cycles.get(cycle).unwrap().clone()) .collect() } diff --git a/crates/fj-kernel/src/shape/validate.rs b/crates/fj-kernel/src/shape/validate.rs index deca757ad..823d335a9 100644 --- a/crates/fj-kernel/src/shape/validate.rs +++ b/crates/fj-kernel/src/shape/validate.rs @@ -149,7 +149,7 @@ impl Validate for Face { if !stores.surfaces.contains(surface) { missing_surface = Some(surface.clone()); } - for cycle in exteriors.0.iter().chain(&interiors.0) { + for cycle in exteriors.as_handle().chain(interiors.as_handle()) { if !stores.cycles.contains(cycle) { missing_cycles.insert(cycle.clone()); } diff --git a/crates/fj-kernel/src/topology/faces.rs b/crates/fj-kernel/src/topology/faces.rs index cff40c993..c66231928 100644 --- a/crates/fj-kernel/src/topology/faces.rs +++ b/crates/fj-kernel/src/topology/faces.rs @@ -171,6 +171,11 @@ impl CyclesInFace { } fn as_canonical(&self) -> impl Iterator> + '_ { - self.0.iter().map(|edge| edge.get()) + self.as_handle().map(|edge| edge.get()) + } + + /// Access an iterator over handles to the cycles + pub fn as_handle(&self) -> impl Iterator>> + '_ { + self.0.iter() } } diff --git a/crates/fj-operations/src/group.rs b/crates/fj-operations/src/group.rs index a18f0d922..5b4c21ba6 100644 --- a/crates/fj-operations/src/group.rs +++ b/crates/fj-operations/src/group.rs @@ -96,8 +96,12 @@ fn copy_shape(orig: Shape, target: &mut Shape) { target .insert(Face::new( surfaces[&surface].clone(), - exteriors.0.iter().map(|cycle| cycles[cycle].clone()), - interiors.0.iter().map(|cycle| cycles[cycle].clone()), + exteriors + .as_handle() + .map(|cycle| cycles[cycle].clone()), + interiors + .as_handle() + .map(|cycle| cycles[cycle].clone()), color, )) .unwrap(); From 8c8060ff06ae529c5d304a9eb03a29b3f8fa49bf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 16 May 2022 19:07:06 +0200 Subject: [PATCH 8/8] Remove unnecessary `pub` --- crates/fj-kernel/src/topology/faces.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/topology/faces.rs b/crates/fj-kernel/src/topology/faces.rs index c66231928..450106779 100644 --- a/crates/fj-kernel/src/topology/faces.rs +++ b/crates/fj-kernel/src/topology/faces.rs @@ -161,7 +161,7 @@ impl Hash for Face { /// A list of cycles, as they are stored in `Face` #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct CyclesInFace(pub Vec>>); +pub struct CyclesInFace(Vec>>); impl CyclesInFace { fn from_canonical(