From e543f4b6ea44448e01636a83028aa3523ba4b40b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 21 Sep 2022 12:49:34 +0200 Subject: [PATCH 1/3] Remove redundant argument from `Face::new` --- crates/fj-kernel/src/algorithms/reverse/face.rs | 4 +--- crates/fj-kernel/src/algorithms/sweep/edge.rs | 4 ++-- crates/fj-kernel/src/algorithms/transform.rs | 4 +--- crates/fj-kernel/src/builder/face.rs | 2 +- crates/fj-kernel/src/objects/face.rs | 4 ++-- crates/fj-operations/src/difference_2d.rs | 2 +- crates/fj-operations/src/sketch.rs | 2 +- 7 files changed, 9 insertions(+), 13 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse/face.rs b/crates/fj-kernel/src/algorithms/reverse/face.rs index 0879ad787..3ed1fbafa 100644 --- a/crates/fj-kernel/src/algorithms/reverse/face.rs +++ b/crates/fj-kernel/src/algorithms/reverse/face.rs @@ -4,12 +4,10 @@ use super::Reverse; impl Reverse for Face { fn reverse(self) -> Self { - let surface = *self.surface(); - let exterior = self.exterior().clone().reverse(); let interiors = self.interiors().map(|cycle| cycle.clone().reverse()); - Face::new(surface, exterior) + Face::new(exterior) .with_interiors(interiors) .with_color(self.color()) } diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 947c3820f..bc8497459 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -172,7 +172,7 @@ impl Sweep for (HalfEdge, Color) { Cycle::new(surface, edges) }; - Face::new(surface, cycle).with_color(color) + Face::new(cycle).with_color(color) } } @@ -212,7 +212,7 @@ mod tests { let cycle = Cycle::new(surface, [bottom, right, top, left]); - Face::new(surface, cycle) + Face::new(cycle) }; assert_eq!(face, expected_face); diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index 82b42684a..81e285f99 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/crates/fj-kernel/src/algorithms/transform.rs @@ -86,8 +86,6 @@ impl TransformObject for Face { type Transformed = Self; fn transform(self, transform: &Transform, stores: &Stores) -> Self { - let surface = self.surface().transform(transform, stores); - let exterior = self.exterior().clone().transform(transform, stores); let interiors = self .interiors() @@ -95,7 +93,7 @@ impl TransformObject for Face { let color = self.color(); - Face::new(surface, exterior) + Face::new(exterior) .with_interiors(interiors) .with_color(color) } diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 6dccad7b4..276b419e3 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -55,6 +55,6 @@ impl<'a> FaceBuilder<'a> { let exterior = self .exterior .expect("Can't build `Face` without exterior cycle"); - Face::new(self.surface, exterior).with_interiors(self.interiors) + Face::new(exterior).with_interiors(self.interiors) } } diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 948603d94..3ad03244e 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -79,9 +79,9 @@ impl Face { /// /// Creates the face with no interiors and the default color. This can be /// overridden using the `with_` methods. - pub fn new(surface: Surface, exterior: Cycle) -> Self { + pub fn new(exterior: Cycle) -> Self { Self { - surface, + surface: *exterior.surface(), exterior, interiors: Vec::new(), color: Color::default(), diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 01b68ccc7..3caa62004 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -83,7 +83,7 @@ impl Shape for fj::Difference2d { ); faces.push( - Face::new(*surface, exterior) + Face::new(exterior) .with_interiors(interiors) .with_color(Color(self.color())), ); diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index a22893d8a..78ff725ce 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -30,7 +30,7 @@ impl Shape for fj::Sketch { .build_circle_from_radius(circle.radius()); let cycle = Cycle::new(surface, [half_edge]); - Face::new(surface, cycle).with_color(Color(self.color())) + Face::new(cycle).with_color(Color(self.color())) } fj::Chain::PolyChain(poly_chain) => { let points = From 8ea30eabb885869534d313db3b1496f1d14ce9ae Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 21 Sep 2022 12:50:51 +0200 Subject: [PATCH 2/3] Rename `Face::new` to be more specific --- crates/fj-kernel/src/algorithms/reverse/face.rs | 2 +- crates/fj-kernel/src/algorithms/sweep/edge.rs | 4 ++-- crates/fj-kernel/src/algorithms/transform.rs | 2 +- crates/fj-kernel/src/builder/face.rs | 2 +- crates/fj-kernel/src/objects/face.rs | 2 +- crates/fj-operations/src/difference_2d.rs | 2 +- crates/fj-operations/src/sketch.rs | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse/face.rs b/crates/fj-kernel/src/algorithms/reverse/face.rs index 3ed1fbafa..20ec72ccc 100644 --- a/crates/fj-kernel/src/algorithms/reverse/face.rs +++ b/crates/fj-kernel/src/algorithms/reverse/face.rs @@ -7,7 +7,7 @@ impl Reverse for Face { let exterior = self.exterior().clone().reverse(); let interiors = self.interiors().map(|cycle| cycle.clone().reverse()); - Face::new(exterior) + Face::from_exterior(exterior) .with_interiors(interiors) .with_color(self.color()) } diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index bc8497459..bcb5c5eb3 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -172,7 +172,7 @@ impl Sweep for (HalfEdge, Color) { Cycle::new(surface, edges) }; - Face::new(cycle).with_color(color) + Face::from_exterior(cycle).with_color(color) } } @@ -212,7 +212,7 @@ mod tests { let cycle = Cycle::new(surface, [bottom, right, top, left]); - Face::new(cycle) + Face::from_exterior(cycle) }; assert_eq!(face, expected_face); diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index 81e285f99..0427c7f16 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/crates/fj-kernel/src/algorithms/transform.rs @@ -93,7 +93,7 @@ impl TransformObject for Face { let color = self.color(); - Face::new(exterior) + Face::from_exterior(exterior) .with_interiors(interiors) .with_color(color) } diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 276b419e3..e287c0e9f 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -55,6 +55,6 @@ impl<'a> FaceBuilder<'a> { let exterior = self .exterior .expect("Can't build `Face` without exterior cycle"); - Face::new(exterior).with_interiors(self.interiors) + Face::from_exterior(exterior).with_interiors(self.interiors) } } diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 3ad03244e..7e3837f77 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -79,7 +79,7 @@ impl Face { /// /// Creates the face with no interiors and the default color. This can be /// overridden using the `with_` methods. - pub fn new(exterior: Cycle) -> Self { + pub fn from_exterior(exterior: Cycle) -> Self { Self { surface: *exterior.surface(), exterior, diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 3caa62004..a1df9dc6f 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -83,7 +83,7 @@ impl Shape for fj::Difference2d { ); faces.push( - Face::new(exterior) + Face::from_exterior(exterior) .with_interiors(interiors) .with_color(Color(self.color())), ); diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 78ff725ce..1995f1f8e 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -30,7 +30,7 @@ impl Shape for fj::Sketch { .build_circle_from_radius(circle.radius()); let cycle = Cycle::new(surface, [half_edge]); - Face::new(cycle).with_color(Color(self.color())) + Face::from_exterior(cycle).with_color(Color(self.color())) } fj::Chain::PolyChain(poly_chain) => { let points = From 2dab910edaae1df31029464fa6a328cca28bd4b4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 21 Sep 2022 12:51:32 +0200 Subject: [PATCH 3/3] Update order of code `Face` is more widely used than `Faces`, so it makes sense to put it first. --- crates/fj-kernel/src/objects/face.rs | 96 ++++++++++++++-------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 7e3837f77..c0e283933 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -7,54 +7,6 @@ use crate::{builder::FaceBuilder, stores::Stores}; use super::{Cycle, Surface}; -/// A collection of faces -#[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Faces { - inner: BTreeSet, -} - -impl Faces { - /// Create an empty instance of `Faces` - pub fn new() -> Self { - Self::default() - } - - /// Find the given face - pub fn find(&self, face: &Face) -> Option { - for f in self { - if f == face { - return Some(f.clone()); - } - } - - None - } -} - -impl Extend for Faces { - fn extend>(&mut self, iter: T) { - self.inner.extend(iter) - } -} - -impl IntoIterator for Faces { - type Item = Face; - type IntoIter = btree_set::IntoIter; - - fn into_iter(self) -> Self::IntoIter { - self.inner.into_iter() - } -} - -impl<'a> IntoIterator for &'a Faces { - type Item = &'a Face; - type IntoIter = btree_set::Iter<'a, Face>; - - fn into_iter(self) -> Self::IntoIter { - self.inner.iter() - } -} - /// A face of a shape #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Face { @@ -165,6 +117,54 @@ impl Face { } } +/// A collection of faces +#[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct Faces { + inner: BTreeSet, +} + +impl Faces { + /// Create an empty instance of `Faces` + pub fn new() -> Self { + Self::default() + } + + /// Find the given face + pub fn find(&self, face: &Face) -> Option { + for f in self { + if f == face { + return Some(f.clone()); + } + } + + None + } +} + +impl Extend for Faces { + fn extend>(&mut self, iter: T) { + self.inner.extend(iter) + } +} + +impl IntoIterator for Faces { + type Item = Face; + type IntoIter = btree_set::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.inner.into_iter() + } +} + +impl<'a> IntoIterator for &'a Faces { + type Item = &'a Face; + type IntoIter = btree_set::Iter<'a, Face>; + + fn into_iter(self) -> Self::IntoIter { + self.inner.iter() + } +} + /// The handedness of a face's coordinate system /// /// See [`Face::coord_handedness`].