From a0713c051f48ddb77f4cb404a7b41c3820697100 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 14 Nov 2023 08:40:40 +0100 Subject: [PATCH 1/2] Take `&self` in replace trait methods This makes those traits consistent with other operations, and avoids `clone`s at call sites. --- .../fj-core/src/operations/replace/curve.rs | 30 +++++++++---------- .../src/operations/replace/half_edge.rs | 26 ++++++++-------- .../fj-core/src/operations/replace/vertex.rs | 30 +++++++++---------- 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/crates/fj-core/src/operations/replace/curve.rs b/crates/fj-core/src/operations/replace/curve.rs index 13f8a491b..d55cff943 100644 --- a/crates/fj-core/src/operations/replace/curve.rs +++ b/crates/fj-core/src/operations/replace/curve.rs @@ -19,7 +19,7 @@ pub trait ReplaceCurve: Sized { /// Replace the curve #[must_use] fn replace_curve( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -30,7 +30,7 @@ impl ReplaceCurve for Handle { type BareObject = HalfEdge; fn replace_curve( - self, + &self, original: &Handle, replacement: Handle, _: &mut Services, @@ -38,7 +38,7 @@ impl ReplaceCurve for Handle { if original.id() == self.curve().id() { ReplaceOutput::Updated(self.update_curve(|_| replacement)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -47,7 +47,7 @@ impl ReplaceCurve for Handle { type BareObject = Cycle; fn replace_curve( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -68,7 +68,7 @@ impl ReplaceCurve for Handle { if replacement_happened { ReplaceOutput::Updated(Cycle::new(half_edges)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -77,7 +77,7 @@ impl ReplaceCurve for Handle { type BareObject = Region; fn replace_curve( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -109,7 +109,7 @@ impl ReplaceCurve for Handle { self.color(), )) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -118,7 +118,7 @@ impl ReplaceCurve for Handle { type BareObject = Sketch; fn replace_curve( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -139,7 +139,7 @@ impl ReplaceCurve for Handle { if replacement_happened { ReplaceOutput::Updated(Sketch::new(regions)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -148,7 +148,7 @@ impl ReplaceCurve for Handle { type BareObject = Face; fn replace_curve( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -165,7 +165,7 @@ impl ReplaceCurve for Handle { region.into_inner(services), )) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -174,7 +174,7 @@ impl ReplaceCurve for Handle { type BareObject = Shell; fn replace_curve( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -195,7 +195,7 @@ impl ReplaceCurve for Handle { if replacement_happened { ReplaceOutput::Updated(Shell::new(faces)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -204,7 +204,7 @@ impl ReplaceCurve for Handle { type BareObject = Solid; fn replace_curve( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -225,7 +225,7 @@ impl ReplaceCurve for Handle { if replacement_happened { ReplaceOutput::Updated(Solid::new(shells)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } diff --git a/crates/fj-core/src/operations/replace/half_edge.rs b/crates/fj-core/src/operations/replace/half_edge.rs index b5c63b6c8..6d356683d 100644 --- a/crates/fj-core/src/operations/replace/half_edge.rs +++ b/crates/fj-core/src/operations/replace/half_edge.rs @@ -18,7 +18,7 @@ pub trait ReplaceHalfEdge: Sized { /// Replace the half-edge #[must_use] fn replace_half_edge( - self, + &self, original: &Handle, replacements: [Handle; N], services: &mut Services, @@ -29,7 +29,7 @@ impl ReplaceHalfEdge for Handle { type BareObject = Cycle; fn replace_half_edge( - self, + &self, original: &Handle, replacements: [Handle; N], _: &mut Services, @@ -39,7 +39,7 @@ impl ReplaceHalfEdge for Handle { { ReplaceOutput::Updated(Cycle::new(half_edges)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -48,7 +48,7 @@ impl ReplaceHalfEdge for Handle { type BareObject = Region; fn replace_half_edge( - self, + &self, original: &Handle, replacements: [Handle; N], services: &mut Services, @@ -80,7 +80,7 @@ impl ReplaceHalfEdge for Handle { self.color(), )) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -89,7 +89,7 @@ impl ReplaceHalfEdge for Handle { type BareObject = Sketch; fn replace_half_edge( - self, + &self, original: &Handle, replacements: [Handle; N], services: &mut Services, @@ -110,7 +110,7 @@ impl ReplaceHalfEdge for Handle { if replacement_happened { ReplaceOutput::Updated(Sketch::new(regions)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -119,7 +119,7 @@ impl ReplaceHalfEdge for Handle { type BareObject = Face; fn replace_half_edge( - self, + &self, original: &Handle, replacements: [Handle; N], services: &mut Services, @@ -136,7 +136,7 @@ impl ReplaceHalfEdge for Handle { region.into_inner(services), )) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -145,7 +145,7 @@ impl ReplaceHalfEdge for Handle { type BareObject = Shell; fn replace_half_edge( - self, + &self, original: &Handle, replacements: [Handle; N], services: &mut Services, @@ -166,7 +166,7 @@ impl ReplaceHalfEdge for Handle { if replacement_happened { ReplaceOutput::Updated(Shell::new(faces)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -175,7 +175,7 @@ impl ReplaceHalfEdge for Handle { type BareObject = Solid; fn replace_half_edge( - self, + &self, original: &Handle, replacements: [Handle; N], services: &mut Services, @@ -196,7 +196,7 @@ impl ReplaceHalfEdge for Handle { if replacement_happened { ReplaceOutput::Updated(Solid::new(shells)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } diff --git a/crates/fj-core/src/operations/replace/vertex.rs b/crates/fj-core/src/operations/replace/vertex.rs index 8cf982e7d..dd9d71e78 100644 --- a/crates/fj-core/src/operations/replace/vertex.rs +++ b/crates/fj-core/src/operations/replace/vertex.rs @@ -19,7 +19,7 @@ pub trait ReplaceVertex: Sized { /// Replace the vertex #[must_use] fn replace_vertex( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -30,7 +30,7 @@ impl ReplaceVertex for Handle { type BareObject = HalfEdge; fn replace_vertex( - self, + &self, original: &Handle, replacement: Handle, _: &mut Services, @@ -38,7 +38,7 @@ impl ReplaceVertex for Handle { if original.id() == self.start_vertex().id() { ReplaceOutput::Updated(self.update_start_vertex(|_| replacement)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -47,7 +47,7 @@ impl ReplaceVertex for Handle { type BareObject = Cycle; fn replace_vertex( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -68,7 +68,7 @@ impl ReplaceVertex for Handle { if replacement_happened { ReplaceOutput::Updated(Cycle::new(half_edges)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -77,7 +77,7 @@ impl ReplaceVertex for Handle { type BareObject = Region; fn replace_vertex( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -109,7 +109,7 @@ impl ReplaceVertex for Handle { self.color(), )) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -118,7 +118,7 @@ impl ReplaceVertex for Handle { type BareObject = Sketch; fn replace_vertex( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -139,7 +139,7 @@ impl ReplaceVertex for Handle { if replacement_happened { ReplaceOutput::Updated(Sketch::new(regions)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -148,7 +148,7 @@ impl ReplaceVertex for Handle { type BareObject = Face; fn replace_vertex( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -165,7 +165,7 @@ impl ReplaceVertex for Handle { region.into_inner(services), )) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -174,7 +174,7 @@ impl ReplaceVertex for Handle { type BareObject = Shell; fn replace_vertex( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -195,7 +195,7 @@ impl ReplaceVertex for Handle { if replacement_happened { ReplaceOutput::Updated(Shell::new(faces)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } @@ -204,7 +204,7 @@ impl ReplaceVertex for Handle { type BareObject = Solid; fn replace_vertex( - self, + &self, original: &Handle, replacement: Handle, services: &mut Services, @@ -225,7 +225,7 @@ impl ReplaceVertex for Handle { if replacement_happened { ReplaceOutput::Updated(Solid::new(shells)) } else { - ReplaceOutput::Original(self) + ReplaceOutput::Original(self.clone()) } } } From 64b488d08f4218326630192493cf3124e4a1afe9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 14 Nov 2023 08:44:30 +0100 Subject: [PATCH 2/2] Remove unnecessary `clone`s --- .../fj-core/src/operations/replace/curve.rs | 39 ++++++------------ .../src/operations/replace/half_edge.rs | 18 ++++----- .../fj-core/src/operations/replace/vertex.rs | 40 ++++++------------- 3 files changed, 33 insertions(+), 64 deletions(-) diff --git a/crates/fj-core/src/operations/replace/curve.rs b/crates/fj-core/src/operations/replace/curve.rs index d55cff943..6e130c91d 100644 --- a/crates/fj-core/src/operations/replace/curve.rs +++ b/crates/fj-core/src/operations/replace/curve.rs @@ -56,7 +56,7 @@ impl ReplaceCurve for Handle { let mut half_edges = Vec::new(); for half_edge in self.half_edges() { - let half_edge = half_edge.clone().replace_curve( + let half_edge = half_edge.replace_curve( original, replacement.clone(), services, @@ -84,7 +84,7 @@ impl ReplaceCurve for Handle { ) -> ReplaceOutput { let mut replacement_happened = false; - let exterior = self.exterior().clone().replace_curve( + let exterior = self.exterior().replace_curve( original, replacement.clone(), services, @@ -93,11 +93,8 @@ impl ReplaceCurve for Handle { let mut interiors = Vec::new(); for cycle in self.interiors() { - let cycle = cycle.clone().replace_curve( - original, - replacement.clone(), - services, - ); + let cycle = + cycle.replace_curve(original, replacement.clone(), services); replacement_happened |= cycle.was_updated(); interiors.push(cycle.into_inner(services)); } @@ -127,11 +124,8 @@ impl ReplaceCurve for Handle { let mut regions = Vec::new(); for region in self.regions() { - let region = region.clone().replace_curve( - original, - replacement.clone(), - services, - ); + let region = + region.replace_curve(original, replacement.clone(), services); replacement_happened |= region.was_updated(); regions.push(region.into_inner(services)); } @@ -153,11 +147,8 @@ impl ReplaceCurve for Handle { replacement: Handle, services: &mut Services, ) -> ReplaceOutput { - let region = self.region().clone().replace_curve( - original, - replacement, - services, - ); + let region = + self.region().replace_curve(original, replacement, services); if region.was_updated() { ReplaceOutput::Updated(Face::new( @@ -183,11 +174,8 @@ impl ReplaceCurve for Handle { let mut faces = Vec::new(); for face in self.faces() { - let face = face.clone().replace_curve( - original, - replacement.clone(), - services, - ); + let face = + face.replace_curve(original, replacement.clone(), services); replacement_happened |= face.was_updated(); faces.push(face.into_inner(services)); } @@ -213,11 +201,8 @@ impl ReplaceCurve for Handle { let mut shells = Vec::new(); for shell in self.shells() { - let shell = shell.clone().replace_curve( - original, - replacement.clone(), - services, - ); + let shell = + shell.replace_curve(original, replacement.clone(), services); replacement_happened |= shell.was_updated(); shells.push(shell.into_inner(services)); } diff --git a/crates/fj-core/src/operations/replace/half_edge.rs b/crates/fj-core/src/operations/replace/half_edge.rs index 6d356683d..3da5bf547 100644 --- a/crates/fj-core/src/operations/replace/half_edge.rs +++ b/crates/fj-core/src/operations/replace/half_edge.rs @@ -55,7 +55,7 @@ impl ReplaceHalfEdge for Handle { ) -> ReplaceOutput { let mut replacement_happened = false; - let exterior = self.exterior().clone().replace_half_edge( + let exterior = self.exterior().replace_half_edge( original, replacements.clone(), services, @@ -64,7 +64,7 @@ impl ReplaceHalfEdge for Handle { let mut interiors = Vec::new(); for cycle in self.interiors() { - let cycle = cycle.clone().replace_half_edge( + let cycle = cycle.replace_half_edge( original, replacements.clone(), services, @@ -98,7 +98,7 @@ impl ReplaceHalfEdge for Handle { let mut regions = Vec::new(); for region in self.regions() { - let region = region.clone().replace_half_edge( + let region = region.replace_half_edge( original, replacements.clone(), services, @@ -124,11 +124,9 @@ impl ReplaceHalfEdge for Handle { replacements: [Handle; N], services: &mut Services, ) -> ReplaceOutput { - let region = self.region().clone().replace_half_edge( - original, - replacements, - services, - ); + let region = + self.region() + .replace_half_edge(original, replacements, services); if region.was_updated() { ReplaceOutput::Updated(Face::new( @@ -154,7 +152,7 @@ impl ReplaceHalfEdge for Handle { let mut faces = Vec::new(); for face in self.faces() { - let face = face.clone().replace_half_edge( + let face = face.replace_half_edge( original, replacements.clone(), services, @@ -184,7 +182,7 @@ impl ReplaceHalfEdge for Handle { let mut shells = Vec::new(); for shell in self.shells() { - let shell = shell.clone().replace_half_edge( + let shell = shell.replace_half_edge( original, replacements.clone(), services, diff --git a/crates/fj-core/src/operations/replace/vertex.rs b/crates/fj-core/src/operations/replace/vertex.rs index dd9d71e78..617d945e2 100644 --- a/crates/fj-core/src/operations/replace/vertex.rs +++ b/crates/fj-core/src/operations/replace/vertex.rs @@ -56,7 +56,7 @@ impl ReplaceVertex for Handle { let mut half_edges = Vec::new(); for half_edge in self.half_edges() { - let half_edge = half_edge.clone().replace_vertex( + let half_edge = half_edge.replace_vertex( original, replacement.clone(), services, @@ -84,7 +84,7 @@ impl ReplaceVertex for Handle { ) -> ReplaceOutput { let mut replacement_happened = false; - let exterior = self.exterior().clone().replace_vertex( + let exterior = self.exterior().replace_vertex( original, replacement.clone(), services, @@ -93,11 +93,8 @@ impl ReplaceVertex for Handle { let mut interiors = Vec::new(); for cycle in self.interiors() { - let cycle = cycle.clone().replace_vertex( - original, - replacement.clone(), - services, - ); + let cycle = + cycle.replace_vertex(original, replacement.clone(), services); replacement_happened |= cycle.was_updated(); interiors.push(cycle.into_inner(services)); } @@ -127,11 +124,8 @@ impl ReplaceVertex for Handle { let mut regions = Vec::new(); for region in self.regions() { - let region = region.clone().replace_vertex( - original, - replacement.clone(), - services, - ); + let region = + region.replace_vertex(original, replacement.clone(), services); replacement_happened |= region.was_updated(); regions.push(region.into_inner(services)); } @@ -153,11 +147,9 @@ impl ReplaceVertex for Handle { replacement: Handle, services: &mut Services, ) -> ReplaceOutput { - let region = self.region().clone().replace_vertex( - original, - replacement, - services, - ); + let region = + self.region() + .replace_vertex(original, replacement, services); if region.was_updated() { ReplaceOutput::Updated(Face::new( @@ -183,11 +175,8 @@ impl ReplaceVertex for Handle { let mut faces = Vec::new(); for face in self.faces() { - let face = face.clone().replace_vertex( - original, - replacement.clone(), - services, - ); + let face = + face.replace_vertex(original, replacement.clone(), services); replacement_happened |= face.was_updated(); faces.push(face.into_inner(services)); } @@ -213,11 +202,8 @@ impl ReplaceVertex for Handle { let mut shells = Vec::new(); for shell in self.shells() { - let shell = shell.clone().replace_vertex( - original, - replacement.clone(), - services, - ); + let shell = + shell.replace_vertex(original, replacement.clone(), services); replacement_happened |= shell.was_updated(); shells.push(shell.into_inner(services)); }