Merge pull request #1361 from hannobraun/transform

Clean up `MaybePartial` transform code
hannobraun · Nov 16, 2022 · b22f73f · b22f73f
2 parents ccaef87 + e34db7e
commit b22f73f
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 20 deletions.
diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs
@@ -3,7 +3,7 @@ use fj_math::Transform;
 
 use crate::{
     objects::Objects,
-    partial::{MaybePartial, PartialGlobalEdge, PartialHalfEdge},
+    partial::{PartialGlobalEdge, PartialHalfEdge},
     validate::ValidationError,
 };
 
@@ -15,11 +15,7 @@ impl TransformObject for PartialHalfEdge {
         transform: &Transform,
         objects: &Objects,
     ) -> Result<Self, ValidationError> {
-        let curve: MaybePartial<_> = self
-            .curve
-            .into_partial()
-            .transform(transform, objects)?
-            .into();
+        let curve = self.curve.transform(transform, objects)?;
         let vertices = self.vertices.try_map_ext(
             |vertex| -> Result<_, ValidationError> {
                 let mut vertex =

diff --git a/crates/fj-kernel/src/algorithms/transform/mod.rs b/crates/fj-kernel/src/algorithms/transform/mod.rs
@@ -90,13 +90,17 @@ where
         transform: &Transform,
         objects: &Objects,
     ) -> Result<Self, ValidationError> {
-        match self {
+        let transformed = match self {
             Self::Full(full) => {
-                Ok(Self::Full(full.transform(transform, objects)?))
+                full.to_partial().transform(transform, objects)?
             }
-            Self::Partial(partial) => {
-                Ok(Self::Partial(partial.transform(transform, objects)?))
-            }
-        }
+            Self::Partial(partial) => partial.transform(transform, objects)?,
+        };
+
+        // Transforming a `MaybePartial` *always* results in a partial object.
+        // This provides the most flexibility to the caller, who might want to
+        // use the transformed partial object for merging or whatever else,
+        // before building it themselves.
+        Ok(Self::Partial(transformed))
     }
 }
diff --git a/crates/fj-kernel/src/partial/maybe_partial.rs b/crates/fj-kernel/src/partial/maybe_partial.rs
@@ -207,6 +207,16 @@ impl MaybePartial<GlobalEdge> {
     }
 }
 
+impl MaybePartial<GlobalVertex> {
+    /// Access the position
+    pub fn position(&self) -> Option<Point<3>> {
+        match self {
+            Self::Full(full) => Some(full.position()),
+            Self::Partial(partial) => partial.position,
+        }
+    }
+}
+
 impl MaybePartial<HalfEdge> {
     /// Access the curve
     pub fn curve(&self) -> MaybePartial<Curve> {

diff --git a/crates/fj-kernel/src/partial/objects/vertex.rs b/crates/fj-kernel/src/partial/objects/vertex.rs
@@ -102,7 +102,7 @@ pub struct PartialSurfaceVertex {
 impl PartialSurfaceVertex {
     /// Build a full [`SurfaceVertex`] from the partial surface vertex
     pub fn build(
-        self,
+        mut self,
         objects: &Objects,
     ) -> Result<SurfaceVertex, ValidationError> {
         let position = self
@@ -112,13 +112,15 @@ impl PartialSurfaceVertex {
             .surface
             .expect("Can't build `SurfaceVertex` without `Surface`");
 
-        let global_form = self
-            .global_form
-            .merge_with(PartialGlobalVertex::from_surface_and_position(
-                &surface.geometry(),
-                position,
-            ))
-            .into_full(objects)?;
+        if self.global_form.position().is_none() {
+            self.global_form = self.global_form.merge_with(
+                PartialGlobalVertex::from_surface_and_position(
+                    &surface.geometry(),
+                    position,
+                ),
+            )
+        }
+        let global_form = self.global_form.into_full(objects)?;
 
         Ok(SurfaceVertex::new(position, surface, global_form))
     }