Merge pull request #1343 from hannobraun/partial

Continue cleaning up partial object code

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

@@ -32,10 +32,7 @@ impl TransformObject for PartialCurve {
             .surface
             .map(|surface| surface.transform(transform, objects))
             .transpose()?;
-        let global_form = self
-            .global_form
-            .map(|global_form| global_form.transform(transform, objects))
-            .transpose()?;
+        let global_form = self.global_form.transform(transform, objects)?;
 
         // Don't need to transform `self.path`, as that's defined in surface
         // coordinates, and thus transforming `surface` takes care of it.

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

@@ -16,23 +16,23 @@ impl TransformObject for PartialHalfEdge {
         objects: &Objects,
     ) -> Result<Self, ValidationError> {
         let curve: MaybePartial<_> = self
-            .curve()
+            .curve
             .into_partial()
             .transform(transform, objects)?
             .into();
-        let vertices = self.vertices().try_map_ext(
+        let vertices = self.vertices.try_map_ext(
             |vertex| -> Result<_, ValidationError> {
                 let mut vertex =
                     vertex.into_partial().transform(transform, objects)?;
                 vertex.curve = curve.clone();
                 Ok(vertex)
             },
         )?;
-        let global_form = self
-            .global_form()
+        let mut global_form = self
+            .global_form
             .into_partial()
-            .transform(transform, objects)?
-            .with_curve(curve.global_form());
+            .transform(transform, objects)?;
+        global_form.curve = curve.global_form();
 
         Ok(Self::default()
             .with_curve(curve)
@@ -47,18 +47,13 @@ impl TransformObject for PartialGlobalEdge {
         transform: &Transform,
         objects: &Objects,
     ) -> Result<Self, ValidationError> {
-        let curve = self.curve().transform(transform, objects)?;
-        let vertices = self
-            .vertices()
-            .map(|vertices| {
-                vertices.try_map_ext(|vertex| -> Result<_, ValidationError> {
-                    vertex.transform(transform, objects)
-                })
-            })
-            .transpose()?;
+        let curve = self.curve.transform(transform, objects)?;
+        let vertices = self.vertices.try_map_ext(
+            |vertex| -> Result<_, ValidationError> {
+                vertex.transform(transform, objects)
+            },
+        )?;
 
-        Ok(Self::default()
-            .with_curve(Some(curve))
-            .with_vertices(vertices))
+        Ok(Self { curve, vertices })
     }
 }

crates/fj-kernel/src/builder/edge.rs

@@ -5,15 +5,14 @@ use crate::{
     insert::Insert,
     objects::{Curve, Objects, Surface, Vertex, VerticesInNormalizedOrder},
     partial::{
-        MaybePartial, MergeWith, PartialCurve, PartialGlobalEdge,
-        PartialGlobalVertex, PartialHalfEdge, PartialSurfaceVertex,
-        PartialVertex,
+        MaybePartial, MergeWith, PartialGlobalEdge, PartialHalfEdge,
+        PartialSurfaceVertex, PartialVertex,
     },
     storage::Handle,
     validate::ValidationError,
 };
 
-use super::{CurveBuilder, GlobalVertexBuilder};
+use super::CurveBuilder;
 
 /// Builder API for [`PartialHalfEdge`]
 pub trait HalfEdgeBuilder: Sized {
@@ -52,12 +51,12 @@ pub trait HalfEdgeBuilder: Sized {
 
 impl HalfEdgeBuilder for PartialHalfEdge {
     fn with_back_vertex(self, back: impl Into<MaybePartial<Vertex>>) -> Self {
-        let [_, front] = self.vertices();
+        let [_, front] = self.vertices.clone();
         self.with_vertices([back.into(), front])
     }
 
     fn with_front_vertex(self, front: impl Into<MaybePartial<Vertex>>) -> Self {
-        let [back, _] = self.vertices();
+        let [back, _] = self.vertices.clone();
         self.with_vertices([back, front.into()])
     }
 
@@ -66,26 +65,15 @@ impl HalfEdgeBuilder for PartialHalfEdge {
         radius: impl Into<Scalar>,
         objects: &Objects,
     ) -> Result<Self, ValidationError> {
-        let mut curve = self.curve().into_partial();
-        curve.global_form = Some(self.extract_global_curve());
+        let mut curve = self.curve.clone().into_partial();
         curve.update_as_circle_from_radius(radius);
 
         let path = curve.path.expect("Expected path that was just created");
 
         let [a_curve, b_curve] =
             [Scalar::ZERO, Scalar::TAU].map(|coord| Point::from([coord]));
 
-        let global_vertex = self
-            .global_form()
-            .vertices()
-            .map(|[global_form, _]| global_form)
-            .unwrap_or_else(|| {
-                PartialGlobalVertex::from_curve_and_position(
-                    curve.clone(),
-                    a_curve,
-                )
-                .into()
-            });
+        let [global_vertex, _] = self.global_form.vertices();
 
         let surface_vertex = PartialSurfaceVertex {
             position: Some(path.point_from_path_coords(a_curve)),
@@ -129,12 +117,12 @@ impl HalfEdgeBuilder for PartialHalfEdge {
     }
 
     fn update_as_line_segment(self) -> Self {
-        let [from, to] = self.vertices();
+        let [from, to] = self.vertices.clone();
         let [from_surface, to_surface] =
             [&from, &to].map(|vertex| vertex.surface_form());
 
         let surface = self
-            .curve()
+            .curve
             .surface()
             .merge_with(from_surface.surface())
             .merge_with(to_surface.surface())
@@ -145,11 +133,8 @@ impl HalfEdgeBuilder for PartialHalfEdge {
                 .expect("Can't infer line segment without surface position")
         });
 
-        let mut curve = PartialCurve {
-            surface: Some(surface),
-            global_form: Some(self.extract_global_curve()),
-            ..Default::default()
-        };
+        let mut curve = self.curve.clone().into_partial();
+        curve.surface = Some(surface);
         curve.update_as_line_from_points(points);
 
         let [back, front] = {
@@ -181,19 +166,12 @@ impl HalfEdgeBuilder for PartialHalfEdge {
                     must_switch_order
                 };
 
-                self.global_form()
-                    .vertices()
-                    .map(
-                        |[a, b]| {
-                            if must_switch_order {
-                                [b, a]
-                            } else {
-                                [a, b]
-                            }
-                        },
-                    )
-                    .map(|[a, b]| [Some(a), Some(b)])
-                    .unwrap_or([None, None])
+                let [a, b] = self.global_form.vertices();
+                if must_switch_order {
+                    [b, a]
+                } else {
+                    [a, b]
+                }
             };
 
             vertices
@@ -202,14 +180,12 @@ impl HalfEdgeBuilder for PartialHalfEdge {
                 .collect::<[_; 2]>()
                 .map(|(vertex, global_form)| {
                     vertex.update_partial(|mut vertex| {
-                        vertex.surface_form = vertex
-                            .surface_form
-                            .update_partial(|mut surface_vertex| {
-                                if let Some(global_form) = global_form {
-                                    surface_vertex.global_form = global_form;
-                                }
-                                surface_vertex
-                            });
+                        vertex.surface_form = vertex.surface_form.merge_with(
+                            PartialSurfaceVertex {
+                                global_form,
+                                ..Default::default()
+                            },
+                        );
                         vertex
                     })
                 })
@@ -235,13 +211,14 @@ pub trait GlobalEdgeBuilder {
 
 impl GlobalEdgeBuilder for PartialGlobalEdge {
     fn update_from_curve_and_vertices(
-        self,
+        mut self,
         curve: &Curve,
         vertices: &[Handle<Vertex>; 2],
     ) -> Self {
-        self.with_curve(Some(curve.global_form().clone()))
-            .with_vertices(Some(
-                vertices.clone().map(|vertex| vertex.global_form().clone()),
-            ))
+        self.curve = curve.global_form().clone().into();
+        self.vertices = vertices
+            .clone()
+            .map(|vertex| vertex.global_form().clone().into());
+        self
     }
 }

crates/fj-kernel/src/builder/shell.rs

@@ -161,13 +161,11 @@ impl<'a> ShellBuilder<'a> {
                         };
 
                         let curve = PartialCurve {
-                            global_form: Some(
-                                side_up_prev
-                                    .curve()
-                                    .global_form()
-                                    .clone()
-                                    .into(),
-                            ),
+                            global_form: side_up_prev
+                                .curve()
+                                .global_form()
+                                .clone()
+                                .into(),
                             ..Default::default()
                         };
 

crates/fj-kernel/src/partial/maybe_partial.rs

@@ -155,9 +155,9 @@ impl MaybePartial<Curve> {
     }
 
     /// Access the global form
-    pub fn global_form(&self) -> Option<MaybePartial<GlobalCurve>> {
+    pub fn global_form(&self) -> MaybePartial<GlobalCurve> {
         match self {
-            Self::Full(full) => Some(full.global_form().clone().into()),
+            Self::Full(full) => full.global_form().clone().into(),
             Self::Partial(partial) => partial.global_form.clone(),
         }
     }
@@ -168,17 +168,17 @@ impl MaybePartial<GlobalEdge> {
     pub fn curve(&self) -> MaybePartial<GlobalCurve> {
         match self {
             Self::Full(full) => full.curve().clone().into(),
-            Self::Partial(partial) => partial.curve(),
+            Self::Partial(partial) => partial.curve.clone(),
         }
     }
 
     /// Access the vertices
-    pub fn vertices(&self) -> Option<[MaybePartial<GlobalVertex>; 2]> {
+    pub fn vertices(&self) -> [MaybePartial<GlobalVertex>; 2] {
         match self {
-            Self::Full(full) => Some(
-                full.vertices().access_in_normalized_order().map(Into::into),
-            ),
-            Self::Partial(partial) => partial.vertices(),
+            Self::Full(full) => {
+                full.vertices().access_in_normalized_order().map(Into::into)
+            }
+            Self::Partial(partial) => partial.vertices.clone(),
         }
     }
 }
@@ -188,7 +188,7 @@ impl MaybePartial<HalfEdge> {
     pub fn curve(&self) -> MaybePartial<Curve> {
         match self {
             Self::Full(full) => full.curve().clone().into(),
-            Self::Partial(partial) => partial.curve(),
+            Self::Partial(partial) => partial.curve.clone(),
         }
     }
 
@@ -197,7 +197,7 @@ impl MaybePartial<HalfEdge> {
         match self {
             Self::Full(full) => full.front().clone().into(),
             Self::Partial(partial) => {
-                let [_, front] = &partial.vertices();
+                let [_, front] = &partial.vertices;
                 front.clone()
             }
         }
@@ -207,7 +207,7 @@ impl MaybePartial<HalfEdge> {
     pub fn vertices(&self) -> [MaybePartial<Vertex>; 2] {
         match self {
             Self::Full(full) => full.vertices().clone().map(Into::into),
-            Self::Partial(partial) => partial.vertices(),
+            Self::Partial(partial) => partial.vertices.clone(),
         }
     }
 }

crates/fj-kernel/src/partial/objects/curve.rs

@@ -1,7 +1,7 @@
 use crate::{
     geometry::path::SurfacePath,
     objects::{Curve, GlobalCurve, Objects, Surface},
-    partial::{MaybePartial, MergeWith, Mergeable},
+    partial::{MaybePartial, MergeWith},
     storage::Handle,
     validate::ValidationError,
 };
@@ -18,13 +18,7 @@ pub struct PartialCurve {
     pub surface: Option<Handle<Surface>>,
 
     /// The global form of the [`Curve`]
-    ///
-    /// # Implementation Note
-    ///
-    /// This can in principle be simplified to just `MaybePartial<GlobalForm`,
-    /// but as of this writing, there's still some code that relies on this
-    /// being an `Option`.
-    pub global_form: Option<MaybePartial<GlobalCurve>>,
+    pub global_form: MaybePartial<GlobalCurve>,
 }
 
 impl PartialCurve {
@@ -34,11 +28,7 @@ impl PartialCurve {
         let surface =
             self.surface.expect("Can't build `Curve` without surface");
 
-        let global_form = match self.global_form {
-            Some(global_form) => global_form,
-            None => objects.global_curves.insert(GlobalCurve)?.into(),
-        }
-        .into_full(objects)?;
+        let global_form = self.global_form.into_full(objects)?;
 
         Ok(Curve::new(surface, path, global_form))
     }
@@ -51,9 +41,7 @@ impl MergeWith for PartialCurve {
         Self {
             path: self.path.merge_with(other.path),
             surface: self.surface.merge_with(other.surface),
-            global_form: Mergeable(self.global_form)
-                .merge_with(Mergeable(other.global_form))
-                .0,
+            global_form: self.global_form.merge_with(other.global_form),
         }
     }
 }
@@ -63,7 +51,7 @@ impl From<&Curve> for PartialCurve {
         Self {
             path: Some(curve.path()),
             surface: Some(curve.surface().clone()),
-            global_form: Some(curve.global_form().clone().into()),
+            global_form: curve.global_form().clone().into(),
         }
     }
 }