From 733d13bc012781ee85baafb4ba94ab930bd84370 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 13 Jun 2023 14:06:56 +0200 Subject: [PATCH 1/5] Require only `&self` in `Reverse::reverse` --- crates/fj-core/src/algorithms/reverse/cycle.rs | 2 +- crates/fj-core/src/algorithms/reverse/face.rs | 2 +- crates/fj-core/src/algorithms/reverse/mod.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/algorithms/reverse/cycle.rs b/crates/fj-core/src/algorithms/reverse/cycle.rs index 5bef5fa7f..cd136eecc 100644 --- a/crates/fj-core/src/algorithms/reverse/cycle.rs +++ b/crates/fj-core/src/algorithms/reverse/cycle.rs @@ -10,7 +10,7 @@ use crate::{ use super::Reverse; impl Reverse for Handle { - fn reverse(self, services: &mut Services) -> Self { + fn reverse(&self, services: &mut Services) -> Self { let mut edges = self .half_edges() .cloned() diff --git a/crates/fj-core/src/algorithms/reverse/face.rs b/crates/fj-core/src/algorithms/reverse/face.rs index 089964680..e1ccc60ef 100644 --- a/crates/fj-core/src/algorithms/reverse/face.rs +++ b/crates/fj-core/src/algorithms/reverse/face.rs @@ -8,7 +8,7 @@ use crate::{ use super::Reverse; impl Reverse for Handle { - fn reverse(self, services: &mut Services) -> Self { + fn reverse(&self, services: &mut Services) -> Self { let exterior = self.region().exterior().clone().reverse(services); let interiors = self .region() diff --git a/crates/fj-core/src/algorithms/reverse/mod.rs b/crates/fj-core/src/algorithms/reverse/mod.rs index 76a1f74d9..933f62246 100644 --- a/crates/fj-core/src/algorithms/reverse/mod.rs +++ b/crates/fj-core/src/algorithms/reverse/mod.rs @@ -8,5 +8,5 @@ mod face; /// Reverse the direction/orientation of an object pub trait Reverse: Sized { /// Reverse the direction/orientation of the object - fn reverse(self, services: &mut Services) -> Self; + fn reverse(&self, services: &mut Services) -> Self; } From 0d0989bd3a3aeb6d274d4cb396d14b81f6f8e9fd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 13 Jun 2023 14:09:12 +0200 Subject: [PATCH 2/5] Implement `Reverse` for `Cycle` Make the implementation easier to use, by not requiring a full `Handle`. --- crates/fj-core/src/algorithms/reverse/cycle.rs | 5 ++--- crates/fj-core/src/algorithms/reverse/face.rs | 9 +++++++-- crates/fj-core/src/validate/face.rs | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/fj-core/src/algorithms/reverse/cycle.rs b/crates/fj-core/src/algorithms/reverse/cycle.rs index cd136eecc..0db018408 100644 --- a/crates/fj-core/src/algorithms/reverse/cycle.rs +++ b/crates/fj-core/src/algorithms/reverse/cycle.rs @@ -4,12 +4,11 @@ use crate::{ objects::{Cycle, HalfEdge}, operations::Insert, services::Services, - storage::Handle, }; use super::Reverse; -impl Reverse for Handle { +impl Reverse for Cycle { fn reverse(&self, services: &mut Services) -> Self { let mut edges = self .half_edges() @@ -33,6 +32,6 @@ impl Reverse for Handle { edges.reverse(); - Cycle::new(edges).insert(services) + Cycle::new(edges) } } diff --git a/crates/fj-core/src/algorithms/reverse/face.rs b/crates/fj-core/src/algorithms/reverse/face.rs index e1ccc60ef..81e5626b0 100644 --- a/crates/fj-core/src/algorithms/reverse/face.rs +++ b/crates/fj-core/src/algorithms/reverse/face.rs @@ -9,11 +9,16 @@ use super::Reverse; impl Reverse for Handle { fn reverse(&self, services: &mut Services) -> Self { - let exterior = self.region().exterior().clone().reverse(services); + let exterior = self + .region() + .exterior() + .clone() + .reverse(services) + .insert(services); let interiors = self .region() .interiors() - .map(|cycle| cycle.clone().reverse(services)) + .map(|cycle| cycle.clone().reverse(services).insert(services)) .collect::>(); let region = Region::new(exterior, interiors, self.region().color()) diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs index cf4f57303..5dcc383ee 100644 --- a/crates/fj-core/src/validate/face.rs +++ b/crates/fj-core/src/validate/face.rs @@ -106,7 +106,7 @@ mod tests { .region() .interiors() .cloned() - .map(|cycle| cycle.reverse(&mut services)) + .map(|cycle| cycle.reverse(&mut services).insert(&mut services)) .collect::>(); let region = Region::new( From 556804aaefd8002e2290e1c8695260f12bb0c3ca Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 13 Jun 2023 14:09:12 +0200 Subject: [PATCH 3/5] Implement `Reverse` for `Face` Make the implementation easier to use, by not requiring a full `Handle`. --- crates/fj-core/src/algorithms/reverse/face.rs | 5 ++--- crates/fj-core/src/algorithms/sweep/face.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/algorithms/reverse/face.rs b/crates/fj-core/src/algorithms/reverse/face.rs index 81e5626b0..deee5c378 100644 --- a/crates/fj-core/src/algorithms/reverse/face.rs +++ b/crates/fj-core/src/algorithms/reverse/face.rs @@ -2,12 +2,11 @@ use crate::{ objects::{Face, Region}, operations::Insert, services::Services, - storage::Handle, }; use super::Reverse; -impl Reverse for Handle { +impl Reverse for Face { fn reverse(&self, services: &mut Services) -> Self { let exterior = self .region() @@ -24,6 +23,6 @@ impl Reverse for Handle { let region = Region::new(exterior, interiors, self.region().color()) .insert(services); - Face::new(self.surface().clone(), region).insert(services) + Face::new(self.surface().clone(), region) } } diff --git a/crates/fj-core/src/algorithms/sweep/face.rs b/crates/fj-core/src/algorithms/sweep/face.rs index 627dfc98e..f2956891d 100644 --- a/crates/fj-core/src/algorithms/sweep/face.rs +++ b/crates/fj-core/src/algorithms/sweep/face.rs @@ -46,7 +46,7 @@ impl Sweep for Handle { if is_negative_sweep { self.clone() } else { - self.clone().reverse(services) + self.clone().reverse(services).insert(services) } }; faces.push(bottom_face.clone()); From 645ead6be3f48b3260577b5140d3662cc0814462 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 13 Jun 2023 14:13:26 +0200 Subject: [PATCH 4/5] Move `reverse` module to `operations` It fits well there. --- crates/fj-core/src/algorithms/mod.rs | 1 - crates/fj-core/src/algorithms/sweep/face.rs | 4 ++-- crates/fj-core/src/operations/mod.rs | 2 ++ .../fj-core/src/{algorithms => operations}/reverse/cycle.rs | 0 .../fj-core/src/{algorithms => operations}/reverse/face.rs | 0 crates/fj-core/src/{algorithms => operations}/reverse/mod.rs | 0 crates/fj-core/src/validate/face.rs | 5 +++-- 7 files changed, 7 insertions(+), 5 deletions(-) rename crates/fj-core/src/{algorithms => operations}/reverse/cycle.rs (100%) rename crates/fj-core/src/{algorithms => operations}/reverse/face.rs (100%) rename crates/fj-core/src/{algorithms => operations}/reverse/mod.rs (100%) diff --git a/crates/fj-core/src/algorithms/mod.rs b/crates/fj-core/src/algorithms/mod.rs index 2786c3f95..c7fa369d4 100644 --- a/crates/fj-core/src/algorithms/mod.rs +++ b/crates/fj-core/src/algorithms/mod.rs @@ -6,7 +6,6 @@ pub mod approx; pub mod bounding_volume; pub mod intersect; -pub mod reverse; pub mod sweep; pub mod transform; pub mod triangulate; diff --git a/crates/fj-core/src/algorithms/sweep/face.rs b/crates/fj-core/src/algorithms/sweep/face.rs index f2956891d..4e1f5981a 100644 --- a/crates/fj-core/src/algorithms/sweep/face.rs +++ b/crates/fj-core/src/algorithms/sweep/face.rs @@ -4,10 +4,10 @@ use fj_math::{Scalar, Vector}; use itertools::Itertools; use crate::{ - algorithms::{reverse::Reverse, transform::TransformObject}, + algorithms::transform::TransformObject, geometry::curve::GlobalPath, objects::{Cycle, Face, Region, Shell}, - operations::{BuildCycle, Insert, JoinCycle}, + operations::{BuildCycle, Insert, JoinCycle, Reverse}, services::Services, storage::Handle, }; diff --git a/crates/fj-core/src/operations/mod.rs b/crates/fj-core/src/operations/mod.rs index 4bd694904..5d43064e4 100644 --- a/crates/fj-core/src/operations/mod.rs +++ b/crates/fj-core/src/operations/mod.rs @@ -3,6 +3,7 @@ mod build; mod insert; mod join; +mod reverse; mod update; pub use self::{ @@ -18,6 +19,7 @@ pub use self::{ }, insert::{Insert, IsInserted, IsInsertedNo, IsInsertedYes}, join::cycle::JoinCycle, + reverse::Reverse, update::{ cycle::UpdateCycle, edge::UpdateHalfEdge, face::UpdateFace, region::UpdateRegion, shell::UpdateShell, sketch::UpdateSketch, diff --git a/crates/fj-core/src/algorithms/reverse/cycle.rs b/crates/fj-core/src/operations/reverse/cycle.rs similarity index 100% rename from crates/fj-core/src/algorithms/reverse/cycle.rs rename to crates/fj-core/src/operations/reverse/cycle.rs diff --git a/crates/fj-core/src/algorithms/reverse/face.rs b/crates/fj-core/src/operations/reverse/face.rs similarity index 100% rename from crates/fj-core/src/algorithms/reverse/face.rs rename to crates/fj-core/src/operations/reverse/face.rs diff --git a/crates/fj-core/src/algorithms/reverse/mod.rs b/crates/fj-core/src/operations/reverse/mod.rs similarity index 100% rename from crates/fj-core/src/algorithms/reverse/mod.rs rename to crates/fj-core/src/operations/reverse/mod.rs diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs index 5dcc383ee..ba059e9c9 100644 --- a/crates/fj-core/src/validate/face.rs +++ b/crates/fj-core/src/validate/face.rs @@ -71,10 +71,11 @@ impl FaceValidationError { #[cfg(test)] mod tests { use crate::{ - algorithms::reverse::Reverse, assert_contains_err, objects::{Cycle, Face, Region}, - operations::{BuildCycle, BuildFace, Insert, UpdateFace, UpdateRegion}, + operations::{ + BuildCycle, BuildFace, Insert, Reverse, UpdateFace, UpdateRegion, + }, services::Services, validate::{FaceValidationError, Validate, ValidationError}, }; From 8e6354d496bf401bd9adc2eab7d569c19063b5f2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 13 Jun 2023 14:15:19 +0200 Subject: [PATCH 5/5] Remove unused trait bound --- crates/fj-core/src/operations/reverse/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/operations/reverse/mod.rs b/crates/fj-core/src/operations/reverse/mod.rs index 933f62246..bb99a3bd7 100644 --- a/crates/fj-core/src/operations/reverse/mod.rs +++ b/crates/fj-core/src/operations/reverse/mod.rs @@ -6,7 +6,7 @@ mod cycle; mod face; /// Reverse the direction/orientation of an object -pub trait Reverse: Sized { +pub trait Reverse { /// Reverse the direction/orientation of the object fn reverse(&self, services: &mut Services) -> Self; }