From fddeaa0af740ac4176341ba0019ccc696a3c96dd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 09:03:26 +0200 Subject: [PATCH 01/14] Remove unnecessary lifetime bounds --- crates/fj-core/src/objects/kinds/region.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/objects/kinds/region.rs b/crates/fj-core/src/objects/kinds/region.rs index f39b8072f..b861db472 100644 --- a/crates/fj-core/src/objects/kinds/region.rs +++ b/crates/fj-core/src/objects/kinds/region.rs @@ -40,12 +40,12 @@ impl Region { /// Access the cycles that bound the region on the inside /// /// Each of these cycles defines a hole in the region . - pub fn interiors(&self) -> impl Iterator> + '_ { + pub fn interiors(&self) -> impl Iterator> { self.interiors.iter() } /// Access all cycles of the region (both exterior and interior) - pub fn all_cycles(&self) -> impl Iterator> + '_ { + pub fn all_cycles(&self) -> impl Iterator> { [self.exterior()].into_iter().chain(self.interiors()) } From 6c54c2f4fe7ae5ad243c086c9eda5249ec2cb8cd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 09:30:56 +0200 Subject: [PATCH 02/14] Remove redundant type hint --- crates/fj-core/src/objects/kinds/cycle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/objects/kinds/cycle.rs b/crates/fj-core/src/objects/kinds/cycle.rs index 0af5f5b87..0c6af8a90 100644 --- a/crates/fj-core/src/objects/kinds/cycle.rs +++ b/crates/fj-core/src/objects/kinds/cycle.rs @@ -12,7 +12,7 @@ pub struct Cycle { impl Cycle { /// Create an instance of `Cycle` pub fn new(edges: impl IntoIterator>) -> Self { - let edges = edges.into_iter().collect::>(); + let edges = edges.into_iter().collect(); Self { edges } } From 2575af05f18919c17e898d8640f3c0388b102493 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 10:22:15 +0200 Subject: [PATCH 03/14] Add `HandleSet` --- crates/fj-core/src/objects/handles.rs | 69 +++++++++++++++++++++++++++ crates/fj-core/src/objects/mod.rs | 1 + 2 files changed, 70 insertions(+) create mode 100644 crates/fj-core/src/objects/handles.rs diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs new file mode 100644 index 000000000..777d5eeeb --- /dev/null +++ b/crates/fj-core/src/objects/handles.rs @@ -0,0 +1,69 @@ +use std::{collections::BTreeSet, fmt::Debug}; + +use crate::storage::Handle; + +/// An ordered set of object handles +/// +/// This is an internal data structure that is used within objects that +/// reference multiple other objects of the same type. It does not contain any +/// duplicate elements, and it maintains the insertion order of those elements. +/// +/// `HandleSet` implement `FromIterator`, but it must never be constructed from +/// an iterator that contains duplicate handles. This will result in a panic. +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct HandleSet { + // This is supposed to be a set data structure, so what is that `Vec` doing + // here? Well, it's here because we need it to preserve insertion order, but + // that doesn't explain why it is here *alone*. + // + // If you look closely, you'll notice that this is an immutable data + // structure (since it is used in objects, and objects themselves are + // immutable). We make sure there are no duplicates when this is + // constructed (see the `FromIterator` implementation below), but after + // that, we're fine. + inner: Vec>, +} + +impl HandleSet { + pub fn len(&self) -> usize { + self.inner.len() + } + + pub fn is_empty(&self) -> bool { + self.inner.is_empty() + } + + pub fn nth(&self, index: usize) -> Option<&Handle> { + self.inner.get(index) + } + + pub fn iter( + &self, + ) -> impl Iterator> + Clone + ExactSizeIterator { + self.inner.iter() + } +} + +impl FromIterator> for HandleSet +where + O: Debug + Ord, +{ + fn from_iter>>(handles: T) -> Self { + let mut added = BTreeSet::new(); + let mut inner = Vec::new(); + + for handle in handles { + if added.contains(&handle) { + panic!( + "Constructing `HandleSet` with duplicate handle: {:?}", + handle + ); + } + + added.insert(handle.clone()); + inner.push(handle); + } + + Self { inner } + } +} diff --git a/crates/fj-core/src/objects/mod.rs b/crates/fj-core/src/objects/mod.rs index 8beedc65b..29c54e388 100644 --- a/crates/fj-core/src/objects/mod.rs +++ b/crates/fj-core/src/objects/mod.rs @@ -39,6 +39,7 @@ //! //! [`Handle`]: crate::storage::Handle +mod handles; mod kinds; mod object; mod set; From 05b00effd70b146489a6e339283563806f1ce9a9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 10:24:03 +0200 Subject: [PATCH 04/14] Use `HandleSet` in `Cycle` --- crates/fj-core/src/objects/kinds/cycle.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/objects/kinds/cycle.rs b/crates/fj-core/src/objects/kinds/cycle.rs index 0c6af8a90..8a5389e34 100644 --- a/crates/fj-core/src/objects/kinds/cycle.rs +++ b/crates/fj-core/src/objects/kinds/cycle.rs @@ -1,12 +1,16 @@ use fj_math::{Scalar, Winding}; use itertools::Itertools; -use crate::{geometry::SurfacePath, objects::Edge, storage::Handle}; +use crate::{ + geometry::SurfacePath, + objects::{handles::HandleSet, Edge}, + storage::Handle, +}; /// A cycle of connected edges #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Cycle { - edges: Vec>, + edges: HandleSet, } impl Cycle { @@ -30,7 +34,7 @@ impl Cycle { /// Access the edge with the provided index pub fn nth_edge(&self, index: usize) -> Option<&Handle> { - self.edges.get(index) + self.edges.nth(index) } /// Access the edge after the provided one @@ -39,7 +43,7 @@ impl Cycle { pub fn edge_after(&self, edge: &Handle) -> Option<&Handle> { self.index_of(edge).map(|index| { let next_index = (index + 1) % self.edges.len(); - &self.edges[next_index] + self.edges.nth(next_index).unwrap() }) } From fec81cf2c71b7ae3a7ef604e90967422cc3ea40c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 10:25:09 +0200 Subject: [PATCH 05/14] Use `HandleSet` in `Region`, `Sketch`, `Solid` --- crates/fj-core/src/objects/kinds/region.rs | 7 +++++-- crates/fj-core/src/objects/kinds/sketch.rs | 6 ++---- crates/fj-core/src/objects/kinds/solid.rs | 9 +++++---- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/crates/fj-core/src/objects/kinds/region.rs b/crates/fj-core/src/objects/kinds/region.rs index b861db472..40c0d3336 100644 --- a/crates/fj-core/src/objects/kinds/region.rs +++ b/crates/fj-core/src/objects/kinds/region.rs @@ -1,7 +1,10 @@ //! A single, continues 2d region use fj_interop::mesh::Color; -use crate::{objects::Cycle, storage::Handle}; +use crate::{ + objects::{handles::HandleSet, Cycle}, + storage::Handle, +}; /// A single, continuous 2d region, may contain holes /// @@ -14,7 +17,7 @@ use crate::{objects::Cycle, storage::Handle}; #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Region { exterior: Handle, - interiors: Vec>, + interiors: HandleSet, color: Option, } diff --git a/crates/fj-core/src/objects/kinds/sketch.rs b/crates/fj-core/src/objects/kinds/sketch.rs index 7cce6806e..163f11e7e 100644 --- a/crates/fj-core/src/objects/kinds/sketch.rs +++ b/crates/fj-core/src/objects/kinds/sketch.rs @@ -1,7 +1,5 @@ -use std::collections::BTreeSet; - use crate::{ - objects::{Face, FaceSet, Region, Surface}, + objects::{handles::HandleSet, Face, FaceSet, Region, Surface}, operations::Insert, services::Services, storage::Handle, @@ -10,7 +8,7 @@ use crate::{ /// A 2-dimensional shape #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Sketch { - regions: BTreeSet>, + regions: HandleSet, } impl Sketch { diff --git a/crates/fj-core/src/objects/kinds/solid.rs b/crates/fj-core/src/objects/kinds/solid.rs index 24aba5cdb..fefd1b32b 100644 --- a/crates/fj-core/src/objects/kinds/solid.rs +++ b/crates/fj-core/src/objects/kinds/solid.rs @@ -1,6 +1,7 @@ -use std::collections::BTreeSet; - -use crate::{objects::Shell, storage::Handle}; +use crate::{ + objects::{handles::HandleSet, Shell}, + storage::Handle, +}; /// A 3-dimensional shape, built from [`Shell`]s. Many Solids will contains only /// one shell, but if the Solid contains cavities they will be represented by a @@ -12,7 +13,7 @@ use crate::{objects::Shell, storage::Handle}; /// not currently validated. #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Solid { - shells: BTreeSet>, + shells: HandleSet, } impl Solid { From 9cf1bcbbf72ab1718380a2332d369cc51a191d9f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 10:56:40 +0200 Subject: [PATCH 06/14] Add `HandleIter` --- crates/fj-core/src/objects/handles.rs | 54 +++++++++++++++++++++++++-- crates/fj-core/src/objects/mod.rs | 1 + 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs index 777d5eeeb..518729e6b 100644 --- a/crates/fj-core/src/objects/handles.rs +++ b/crates/fj-core/src/objects/handles.rs @@ -37,10 +37,11 @@ impl HandleSet { self.inner.get(index) } - pub fn iter( - &self, - ) -> impl Iterator> + Clone + ExactSizeIterator { - self.inner.iter() + pub fn iter(&self) -> HandleIter { + HandleIter { + handles: &self.inner, + next_index: 0, + } } } @@ -67,3 +68,48 @@ where Self { inner } } } + +/// An iterator over handles to objects +/// +/// This struct is returned by the respective methods of all objects that +/// reference multiple objects of the same type. +pub struct HandleIter<'r, T> { + handles: &'r Vec>, + next_index: usize, +} + +impl<'r, T> Iterator for HandleIter<'r, T> { + // You might wonder why we're returning references to handles here, when + // `Handle` already is kind of reference, and easily cloned. + // + // Most of the time that doesn't make a difference, but there are use cases + // where dealing with owned `Handle`s is inconvenient, for example when + // using iterator adapters. You can't return a reference to the argument of + // an adapter's closure, if you own that argument. You can, if you just + // reference the argument. + type Item = &'r Handle; + + fn next(&mut self) -> Option { + let handle = self.handles.get(self.next_index); + self.next_index += 1; + handle + } + + fn size_hint(&self) -> (usize, Option) { + let size = self.handles.len(); + (size, Some(size)) + } +} + +impl ExactSizeIterator for HandleIter<'_, T> {} + +// Deriving won't work, as that only derives `Clone` where `T: Clone`. But +// `HandleIter` can be `Clone`d unconditionally. +impl Clone for HandleIter<'_, T> { + fn clone(&self) -> Self { + Self { + handles: self.handles, + next_index: self.next_index, + } + } +} diff --git a/crates/fj-core/src/objects/mod.rs b/crates/fj-core/src/objects/mod.rs index 29c54e388..c69fb8707 100644 --- a/crates/fj-core/src/objects/mod.rs +++ b/crates/fj-core/src/objects/mod.rs @@ -46,6 +46,7 @@ mod set; mod stores; pub use self::{ + handles::HandleIter, kinds::{ curve::Curve, cycle::Cycle, From d53f3d4abedaaf09ba175c5b2a74abae307a7275 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 11:00:21 +0200 Subject: [PATCH 07/14] Return `HandleIter`, where possible --- crates/fj-core/src/objects/kinds/cycle.rs | 4 ++-- crates/fj-core/src/objects/kinds/region.rs | 4 ++-- crates/fj-core/src/objects/kinds/sketch.rs | 4 ++-- crates/fj-core/src/objects/kinds/solid.rs | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/objects/kinds/cycle.rs b/crates/fj-core/src/objects/kinds/cycle.rs index 8a5389e34..12334f9b5 100644 --- a/crates/fj-core/src/objects/kinds/cycle.rs +++ b/crates/fj-core/src/objects/kinds/cycle.rs @@ -3,7 +3,7 @@ use itertools::Itertools; use crate::{ geometry::SurfacePath, - objects::{handles::HandleSet, Edge}, + objects::{handles::HandleSet, Edge, HandleIter}, storage::Handle, }; @@ -21,7 +21,7 @@ impl Cycle { } /// Access the edges that make up the cycle - pub fn edges(&self) -> impl Iterator> { + pub fn edges(&self) -> HandleIter { self.edges.iter() } diff --git a/crates/fj-core/src/objects/kinds/region.rs b/crates/fj-core/src/objects/kinds/region.rs index 40c0d3336..28ea8e5eb 100644 --- a/crates/fj-core/src/objects/kinds/region.rs +++ b/crates/fj-core/src/objects/kinds/region.rs @@ -2,7 +2,7 @@ use fj_interop::mesh::Color; use crate::{ - objects::{handles::HandleSet, Cycle}, + objects::{handles::HandleSet, Cycle, HandleIter}, storage::Handle, }; @@ -43,7 +43,7 @@ impl Region { /// Access the cycles that bound the region on the inside /// /// Each of these cycles defines a hole in the region . - pub fn interiors(&self) -> impl Iterator> { + pub fn interiors(&self) -> HandleIter { self.interiors.iter() } diff --git a/crates/fj-core/src/objects/kinds/sketch.rs b/crates/fj-core/src/objects/kinds/sketch.rs index 163f11e7e..657eb1270 100644 --- a/crates/fj-core/src/objects/kinds/sketch.rs +++ b/crates/fj-core/src/objects/kinds/sketch.rs @@ -1,5 +1,5 @@ use crate::{ - objects::{handles::HandleSet, Face, FaceSet, Region, Surface}, + objects::{handles::HandleSet, Face, FaceSet, HandleIter, Region, Surface}, operations::Insert, services::Services, storage::Handle, @@ -20,7 +20,7 @@ impl Sketch { } /// Access the regions of the sketch - pub fn regions(&self) -> impl Iterator> { + pub fn regions(&self) -> HandleIter { self.regions.iter() } diff --git a/crates/fj-core/src/objects/kinds/solid.rs b/crates/fj-core/src/objects/kinds/solid.rs index fefd1b32b..fe973df9d 100644 --- a/crates/fj-core/src/objects/kinds/solid.rs +++ b/crates/fj-core/src/objects/kinds/solid.rs @@ -1,5 +1,5 @@ use crate::{ - objects::{handles::HandleSet, Shell}, + objects::{handles::HandleSet, HandleIter, Shell}, storage::Handle, }; @@ -25,7 +25,7 @@ impl Solid { } /// Access the solid's shells - pub fn shells(&self) -> impl Iterator> { + pub fn shells(&self) -> HandleIter { self.shells.iter() } } From 8634c581c8a85fd5b4b32c7db4730bd491062cc4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 11:03:30 +0200 Subject: [PATCH 08/14] Add comment --- crates/fj-core/src/objects/kinds/region.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/fj-core/src/objects/kinds/region.rs b/crates/fj-core/src/objects/kinds/region.rs index 28ea8e5eb..e9028b5e2 100644 --- a/crates/fj-core/src/objects/kinds/region.rs +++ b/crates/fj-core/src/objects/kinds/region.rs @@ -49,6 +49,11 @@ impl Region { /// Access all cycles of the region (both exterior and interior) pub fn all_cycles(&self) -> impl Iterator> { + // It would be nice to return `HandleIter` here, but there's no + // straight-forward way to chain it to another iterator, given it's + // current implementation. + // + // Maybe this can be addressed, once the need arises. [self.exterior()].into_iter().chain(self.interiors()) } From 77501c33e2f82053b88f888191acdae68bbdcb93 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 11:08:02 +0200 Subject: [PATCH 09/14] Replace `FaceSet` in `Shell` --- crates/fj-core/src/algorithms/approx/face.rs | 4 ++-- crates/fj-core/src/algorithms/transform/shell.rs | 8 ++++---- crates/fj-core/src/objects/kinds/shell.rs | 8 ++++---- crates/fj-core/src/operations/update/shell.rs | 5 ++--- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/face.rs b/crates/fj-core/src/algorithms/approx/face.rs index 6cccce275..3babf79d3 100644 --- a/crates/fj-core/src/algorithms/approx/face.rs +++ b/crates/fj-core/src/algorithms/approx/face.rs @@ -7,7 +7,7 @@ use std::{collections::BTreeSet, ops::Deref}; use fj_interop::mesh::Color; use crate::{ - objects::{Face, FaceSet, Handedness}, + objects::{Face, Handedness, HandleIter}, validate::ValidationConfig, }; @@ -15,7 +15,7 @@ use super::{ cycle::CycleApprox, edge::EdgeApproxCache, Approx, ApproxPoint, Tolerance, }; -impl Approx for &FaceSet { +impl Approx for HandleIter<'_, Face> { type Approximation = BTreeSet; type Cache = EdgeApproxCache; diff --git a/crates/fj-core/src/algorithms/transform/shell.rs b/crates/fj-core/src/algorithms/transform/shell.rs index a8d91e0f9..0d1d474e0 100644 --- a/crates/fj-core/src/algorithms/transform/shell.rs +++ b/crates/fj-core/src/algorithms/transform/shell.rs @@ -11,10 +11,10 @@ impl TransformObject for Shell { services: &mut Services, cache: &mut TransformCache, ) -> Self { - let faces = - self.faces().clone().into_iter().map(|face| { - face.transform_with_cache(transform, services, cache) - }); + let faces = self + .faces() + .cloned() + .map(|face| face.transform_with_cache(transform, services, cache)); Self::new(faces) } diff --git a/crates/fj-core/src/objects/kinds/shell.rs b/crates/fj-core/src/objects/kinds/shell.rs index 8a1717255..c8654945f 100644 --- a/crates/fj-core/src/objects/kinds/shell.rs +++ b/crates/fj-core/src/objects/kinds/shell.rs @@ -1,12 +1,12 @@ use crate::{ - objects::{Face, FaceSet}, + objects::{handles::HandleSet, Face, HandleIter}, storage::Handle, }; /// A 3-dimensional closed shell #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Shell { - faces: FaceSet, + faces: HandleSet, } impl Shell { @@ -18,7 +18,7 @@ impl Shell { } /// Access the faces of the shell - pub fn faces(&self) -> &FaceSet { - &self.faces + pub fn faces(&self) -> HandleIter { + self.faces.iter() } } diff --git a/crates/fj-core/src/operations/update/shell.rs b/crates/fj-core/src/operations/update/shell.rs index cd68d70e4..52c9c6f36 100644 --- a/crates/fj-core/src/operations/update/shell.rs +++ b/crates/fj-core/src/operations/update/shell.rs @@ -24,7 +24,7 @@ pub trait UpdateShell { impl UpdateShell for Shell { fn add_faces(&self, faces: impl IntoIterator>) -> Self { - let faces = self.faces().into_iter().cloned().chain(faces); + let faces = self.faces().cloned().chain(faces); Shell::new(faces) } @@ -33,7 +33,7 @@ impl UpdateShell for Shell { original: &Handle, replacement: Handle, ) -> Self { - let faces = self.faces().into_iter().map(|face| { + let faces = self.faces().map(|face| { if face.id() == original.id() { replacement.clone() } else { @@ -47,7 +47,6 @@ impl UpdateShell for Shell { fn remove_face(&self, handle: &Handle) -> Self { let faces = self .faces() - .into_iter() .filter(|face| face.id() == handle.id()) .cloned(); From ca34d169a63062e86c470041f554a1da59447d46 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 11:09:00 +0200 Subject: [PATCH 10/14] Remove unused trait implementation --- .../fj-core/src/algorithms/transform/face.rs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/crates/fj-core/src/algorithms/transform/face.rs b/crates/fj-core/src/algorithms/transform/face.rs index 7c99c97dc..1aa6e2278 100644 --- a/crates/fj-core/src/algorithms/transform/face.rs +++ b/crates/fj-core/src/algorithms/transform/face.rs @@ -1,7 +1,7 @@ use fj_math::Transform; use crate::{ - objects::{Face, FaceSet, Region}, + objects::{Face, Region}, operations::Insert, services::Services, }; @@ -36,20 +36,3 @@ impl TransformObject for Face { Self::new(surface, region) } } - -impl TransformObject for FaceSet { - fn transform_with_cache( - self, - transform: &Transform, - services: &mut Services, - cache: &mut TransformCache, - ) -> Self { - let mut faces = Self::new(); - faces.extend( - self.into_iter().map(|face| { - face.transform_with_cache(transform, services, cache) - }), - ); - faces - } -} From 1865f3db9bb4a5c9d61d5a12f0a87519aa3c99ef Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 11:11:43 +0200 Subject: [PATCH 11/14] Refactor --- crates/fj-core/src/objects/kinds/sketch.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-core/src/objects/kinds/sketch.rs b/crates/fj-core/src/objects/kinds/sketch.rs index 657eb1270..8deb53d9a 100644 --- a/crates/fj-core/src/objects/kinds/sketch.rs +++ b/crates/fj-core/src/objects/kinds/sketch.rs @@ -30,8 +30,7 @@ impl Sketch { surface: Handle, services: &mut Services, ) -> FaceSet { - self.regions - .iter() + self.regions() .map(|region| { Face::new(surface.clone(), region.clone()).insert(services) }) From 7e6e5bae9e161366ab3ed4fc516e9aa62131414f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 11:12:45 +0200 Subject: [PATCH 12/14] Refactor to improve clarity --- crates/fj-core/src/algorithms/sweep/sketch.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/sweep/sketch.rs b/crates/fj-core/src/algorithms/sweep/sketch.rs index c50b06777..6b95c04b1 100644 --- a/crates/fj-core/src/algorithms/sweep/sketch.rs +++ b/crates/fj-core/src/algorithms/sweep/sketch.rs @@ -18,10 +18,11 @@ impl Sweep for (Handle, Handle) { cache: &mut SweepCache, services: &mut Services, ) -> Self::Swept { + let (sketch, surface) = self; let path = path.into(); let mut shells = Vec::new(); - for face in self.0.faces(self.1, services) { + for face in sketch.faces(surface, services) { let shell = face.sweep_with_cache(path, cache, services); shells.push(shell); } From 24e38b806eda5ce6cb5224a9659c08f315fe2a03 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 11:14:52 +0200 Subject: [PATCH 13/14] Inline `Sketch::faces` into its only use site --- crates/fj-core/src/algorithms/sweep/sketch.rs | 6 ++++-- crates/fj-core/src/objects/kinds/sketch.rs | 17 +---------------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/crates/fj-core/src/algorithms/sweep/sketch.rs b/crates/fj-core/src/algorithms/sweep/sketch.rs index 6b95c04b1..fe026e8e4 100644 --- a/crates/fj-core/src/algorithms/sweep/sketch.rs +++ b/crates/fj-core/src/algorithms/sweep/sketch.rs @@ -1,7 +1,7 @@ use fj_math::Vector; use crate::{ - objects::{Sketch, Solid, Surface}, + objects::{Face, Sketch, Solid, Surface}, operations::Insert, services::Services, storage::Handle, @@ -22,7 +22,9 @@ impl Sweep for (Handle, Handle) { let path = path.into(); let mut shells = Vec::new(); - for face in sketch.faces(surface, services) { + for region in sketch.regions() { + let face = + Face::new(surface.clone(), region.clone()).insert(services); let shell = face.sweep_with_cache(path, cache, services); shells.push(shell); } diff --git a/crates/fj-core/src/objects/kinds/sketch.rs b/crates/fj-core/src/objects/kinds/sketch.rs index 8deb53d9a..3709783fd 100644 --- a/crates/fj-core/src/objects/kinds/sketch.rs +++ b/crates/fj-core/src/objects/kinds/sketch.rs @@ -1,7 +1,5 @@ use crate::{ - objects::{handles::HandleSet, Face, FaceSet, HandleIter, Region, Surface}, - operations::Insert, - services::Services, + objects::{handles::HandleSet, HandleIter, Region}, storage::Handle, }; @@ -23,17 +21,4 @@ impl Sketch { pub fn regions(&self) -> HandleIter { self.regions.iter() } - - /// Apply the regions of the sketch to some [`Surface`] - pub fn faces( - &self, - surface: Handle, - services: &mut Services, - ) -> FaceSet { - self.regions() - .map(|region| { - Face::new(surface.clone(), region.clone()).insert(services) - }) - .collect() - } } From c48449d500268d985d0eb2b76b6ca86c3b3ea77c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 21 Sep 2023 11:16:29 +0200 Subject: [PATCH 14/14] Remove unused code --- crates/fj-core/src/objects/kinds/face.rs | 47 ------------------------ crates/fj-core/src/objects/mod.rs | 2 +- 2 files changed, 1 insertion(+), 48 deletions(-) diff --git a/crates/fj-core/src/objects/kinds/face.rs b/crates/fj-core/src/objects/kinds/face.rs index 1a8df0b72..f97b4145d 100644 --- a/crates/fj-core/src/objects/kinds/face.rs +++ b/crates/fj-core/src/objects/kinds/face.rs @@ -1,5 +1,3 @@ -use std::collections::{btree_set, BTreeSet}; - use fj_math::Winding; use crate::{ @@ -71,51 +69,6 @@ impl Face { } } -/// A collection of faces -#[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct FaceSet { - inner: BTreeSet>, -} - -impl FaceSet { - /// Create an empty instance of `Faces` - pub fn new() -> Self { - Self::default() - } -} - -impl Extend> for FaceSet { - fn extend>>(&mut self, iter: T) { - self.inner.extend(iter); - } -} - -impl FromIterator> for FaceSet { - fn from_iter>>(iter: T) -> Self { - let mut faces = Self::new(); - faces.extend(iter); - faces - } -} - -impl IntoIterator for FaceSet { - type Item = Handle; - type IntoIter = btree_set::IntoIter>; - - fn into_iter(self) -> Self::IntoIter { - self.inner.into_iter() - } -} - -impl<'a> IntoIterator for &'a FaceSet { - type Item = &'a Handle; - type IntoIter = btree_set::Iter<'a, Handle>; - - fn into_iter(self) -> Self::IntoIter { - self.inner.iter() - } -} - /// The handedness of a face's coordinate system /// /// See [`Face::coord_handedness`]. diff --git a/crates/fj-core/src/objects/mod.rs b/crates/fj-core/src/objects/mod.rs index c69fb8707..0f170f05d 100644 --- a/crates/fj-core/src/objects/mod.rs +++ b/crates/fj-core/src/objects/mod.rs @@ -51,7 +51,7 @@ pub use self::{ curve::Curve, cycle::Cycle, edge::Edge, - face::{Face, FaceSet, Handedness}, + face::{Face, Handedness}, region::Region, shell::Shell, sketch::Sketch,