Merge pull request #1170 from hannobraun/transform

Fix `Face` transform code creating duplicate `Surface`s

crates/fj-kernel/src/algorithms/transform/curve.rs

@@ -1,25 +1,13 @@
 use fj_math::Transform;
 
 use crate::{
-    objects::{Curve, GlobalCurve},
+    objects::GlobalCurve,
     partial::PartialCurve,
     stores::{Handle, Stores},
 };
 
 use super::TransformObject;
 
-impl TransformObject for Curve {
-    fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        let surface = self.surface().clone().transform(transform, stores);
-        let global_form =
-            self.global_form().clone().transform(transform, stores);
-
-        // Don't need to transform `self.path`, as that's defined in surface
-        // coordinates, and thus transforming `surface` takes care of it.
-        Self::new(surface, self.path(), global_form)
-    }
-}
-
 impl TransformObject for Handle<GlobalCurve> {
     fn transform(self, _: &Transform, stores: &Stores) -> Self {
         // `GlobalCurve` doesn't contain any internal geometry. If it did, that

crates/fj-kernel/src/algorithms/transform/cycle.rs

@@ -1,15 +1,24 @@
 use fj_math::Transform;
 
-use crate::{objects::Cycle, stores::Stores};
+use crate::{partial::PartialCycle, stores::Stores};
 
 use super::TransformObject;
 
-impl TransformObject for Cycle {
+impl TransformObject for PartialCycle {
     fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        Self::new(
-            self.surface().clone().transform(transform, stores),
-            self.into_half_edges()
-                .map(|edge| edge.transform(transform, stores)),
-        )
+        let surface = self
+            .surface
+            .clone()
+            .map(|surface| surface.transform(transform, stores));
+        let half_edges = self
+            .half_edges
+            .into_iter()
+            .map(|edge| edge.transform(transform, stores))
+            .collect();
+
+        Self {
+            surface,
+            half_edges,
+        }
     }
 }

crates/fj-kernel/src/algorithms/transform/edge.rs

@@ -1,47 +1,46 @@
 use fj_math::Transform;
 
 use crate::{
-    objects::{GlobalEdge, HalfEdge},
-    partial::{HasPartial, PartialGlobalEdge},
+    partial::{PartialGlobalEdge, PartialHalfEdge},
     stores::Stores,
 };
 
 use super::TransformObject;
 
-impl TransformObject for HalfEdge {
+impl TransformObject for PartialHalfEdge {
     fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        let curve = self.curve().clone().transform(transform, stores);
-        let vertices = self
-            .vertices()
-            // The `clone` can be replaced with `each_ref`, once that is stable:
-            // https://doc.rust-lang.org/std/primitive.array.html#method.each_ref
+        let curve = self
+            .curve
             .clone()
-            .map(|vertex| {
-                vertex
-                    .to_partial()
-                    .transform(transform, stores)
-                    .with_curve(curve.clone())
-                    .build(stores)
-            });
-        let global_form = self
-            .global_form()
-            .to_partial()
-            .transform(transform, stores)
-            .with_curve(curve.global_form().clone())
-            .build(stores);
-
-        Self::new(curve, vertices, global_form)
-    }
-}
+            .map(|curve| curve.transform(transform, stores));
+        let vertices = self.vertices.clone().map(|vertices| {
+            vertices.map(|vertex| {
+                let vertex = vertex.into_partial().transform(transform, stores);
+                let vertex = match &curve {
+                    Some(curve) => vertex.with_curve(curve.clone()),
+                    None => vertex,
+                };
+                vertex.into()
+            })
+        });
+        let global_form = self.global_form.map(|global_form| {
+            let global_form =
+                global_form.into_partial().transform(transform, stores);
 
-impl TransformObject for GlobalEdge {
-    fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        let curve = self.curve().clone().transform(transform, stores);
-        let vertices = self
-            .vertices_in_normalized_order()
-            .map(|vertex| vertex.transform(transform, stores));
+            let curve = curve.as_ref().and_then(|curve| curve.global_form());
+            let global_form = match curve {
+                Some(curve) => global_form.with_curve(curve),
+                None => global_form,
+            };
 
-        Self::new(curve, vertices)
+            global_form.into()
+        });
+
+        Self {
+            curve,
+            vertices,
+            global_form,
+        }
     }
 }
 

crates/fj-kernel/src/algorithms/transform/face.rs

@@ -2,17 +2,28 @@ use fj_math::Transform;
 
 use crate::{
     objects::{Face, Faces},
+    partial::HasPartial,
     stores::Stores,
 };
 
 use super::TransformObject;
 
 impl TransformObject for Face {
     fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        let exterior = self.exterior().clone().transform(transform, stores);
-        let interiors = self
-            .interiors()
-            .map(|cycle| cycle.clone().transform(transform, stores));
+        let surface = self.surface().clone().transform(transform, stores);
+        let exterior = self
+            .exterior()
+            .to_partial()
+            .transform(transform, stores)
+            .with_surface(surface.clone())
+            .build(stores);
+        let interiors = self.interiors().map(|cycle| {
+            cycle
+                .to_partial()
+                .transform(transform, stores)
+                .with_surface(surface.clone())
+                .build(stores)
+        });
 
         let color = self.color();
 

crates/fj-kernel/src/algorithms/transform/mod.rs

@@ -14,7 +14,7 @@ mod vertex;
 use fj_math::{Transform, Vector};
 
 use crate::{
-    partial::{HasPartial, MaybePartial},
+    partial::{HasPartial, MaybePartial, Partial},
     stores::Stores,
 };
 
@@ -49,6 +49,16 @@ pub trait TransformObject: Sized {
     }
 }
 
+impl<T> TransformObject for T
+where
+    T: HasPartial,
+    T::Partial: TransformObject,
+{
+    fn transform(self, transform: &Transform, stores: &Stores) -> Self {
+        self.to_partial().transform(transform, stores).build(stores)
+    }
+}
+
 impl<T> TransformObject for MaybePartial<T>
 where
     T: HasPartial + TransformObject,

crates/fj-kernel/src/algorithms/transform/vertex.rs

@@ -1,44 +1,12 @@
 use fj_math::Transform;
 
 use crate::{
-    objects::{GlobalVertex, SurfaceVertex, Vertex},
     partial::{PartialGlobalVertex, PartialSurfaceVertex, PartialVertex},
     stores::Stores,
 };
 
 use super::TransformObject;
 
-impl TransformObject for Vertex {
-    fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        let curve = self.curve().clone().transform(transform, stores);
-        let surface_form =
-            self.surface_form().clone().transform(transform, stores);
-        let global_form = self.global_form().transform(transform, stores);
-
-        // Don't need to transform `self.position`, as that is in curve
-        // coordinates and thus transforming the curve takes care of it.
-        Self::new(self.position(), curve, surface_form, global_form)
-    }
-}
-
-impl TransformObject for SurfaceVertex {
-    fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        let surface = self.surface().clone().transform(transform, stores);
-        let global_form = self.global_form().transform(transform, stores);
-
-        // Don't need to transform `self.position`, as that is in surface
-        // coordinates and thus transforming the surface takes care of it.
-        Self::new(self.position(), surface, global_form)
-    }
-}
-
-impl TransformObject for GlobalVertex {
-    fn transform(self, transform: &Transform, _: &Stores) -> Self {
-        let position = transform.transform_point(&self.position());
-        Self::from_position(position)
-    }
-}
-
 impl TransformObject for PartialVertex {
     fn transform(self, transform: &Transform, stores: &Stores) -> Self {
         let curve = self.curve.map(|curve| curve.transform(transform, stores));