Merge pull request #1457 from hannobraun/builder

Continue cleaning up builder API

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

@@ -10,12 +10,17 @@ use super::HalfEdgeBuilder;
 /// Builder API for [`PartialCycle`]
 pub trait CycleBuilder {
     /// Create a cycle as a polygonal chain from the provided points
-    fn update_as_polygon(
+    fn update_as_polygon_from_points(
         &mut self,
         surface: impl Into<Partial<Surface>>,
         points: impl IntoIterator<Item = impl Into<Point<2>>>,
     ) -> Vec<Partial<HalfEdge>>;
 
+    /// Update cycle to be a polygon
+    ///
+    /// Will update each half-edge in the cycle to be a line segment.
+    fn update_as_polygon(&mut self);
+
     /// Add a new half-edge to the cycle
     ///
     /// Creates a half-edge and adds it to the cycle. The new half-edge is
@@ -26,41 +31,52 @@ pub trait CycleBuilder {
     /// If this is the first half-edge being added, it is connected to itself,
     /// meaning its front and back vertices are the same.
     fn add_half_edge(&mut self) -> Partial<HalfEdge>;
+
+    /// Add a new half-edge that starts at the provided point
+    ///
+    /// Opens the cycle between the last and first edge, updates the last edge
+    /// to go the provided point, and adds a new half-edge from the provided
+    /// point the the first edge.
+    ///
+    /// If the cycle doesn't have any edges yet, the new edge connects to
+    /// itself, starting and ending at the provided point.
+    fn add_half_edge_from_point_to_start(
+        &mut self,
+        point: impl Into<Point<2>>,
+    ) -> Partial<HalfEdge>;
 }
 
 impl CycleBuilder for PartialCycle {
-    fn update_as_polygon(
+    fn update_as_polygon_from_points(
         &mut self,
         surface: impl Into<Partial<Surface>>,
         points: impl IntoIterator<Item = impl Into<Point<2>>>,
     ) -> Vec<Partial<HalfEdge>> {
         let surface = surface.into();
+        let mut points = points.into_iter().map(Into::into);
 
         let mut half_edges = Vec::new();
 
-        for point in points.into_iter().map(Into::into) {
-            let mut half_edge = self.add_half_edge();
-
-            {
-                let mut half_edge = half_edge.write();
-
-                half_edge.curve().write().surface = surface.clone();
-
-                let mut back = half_edge.back_mut().write();
-                let mut back_surface = back.surface_form.write();
-
-                back_surface.position = Some(point);
-                back_surface.surface = surface.clone();
-            }
+        if let Some(point) = points.next() {
+            let mut half_edge = self.add_half_edge_from_point_to_start(point);
+            half_edge.write().replace_surface(surface);
+            half_edges.push(half_edge);
+        }
 
+        for point in points {
+            let half_edge = self.add_half_edge_from_point_to_start(point);
             half_edges.push(half_edge);
         }
 
+        self.update_as_polygon();
+
+        half_edges
+    }
+
+    fn update_as_polygon(&mut self) {
         for half_edge in &mut self.half_edges {
             half_edge.write().update_as_line_segment();
         }
-
-        half_edges
     }
 
     fn add_half_edge(&mut self) -> Partial<HalfEdge> {
@@ -81,34 +97,48 @@ impl CycleBuilder for PartialCycle {
                 None => (new_half_edge.clone(), new_half_edge.clone()),
             };
 
-        let shared_surface =
-            first_half_edge.read().curve().read().surface.clone();
-
         {
             let shared_surface_vertex =
                 new_half_edge.read().back().read().surface_form.clone();
 
             let mut last_half_edge = last_half_edge.write();
-            last_half_edge.curve().write().surface = shared_surface.clone();
+
             last_half_edge.front_mut().write().surface_form =
                 shared_surface_vertex;
-
             last_half_edge.infer_global_form();
         }
 
         {
             let shared_surface_vertex =
                 first_half_edge.read().back().read().surface_form.clone();
+            let shared_surface = shared_surface_vertex.read().surface.clone();
 
             let mut new_half_edge = new_half_edge.write();
 
-            new_half_edge.curve().write().surface = shared_surface;
             new_half_edge.front_mut().write().surface_form =
                 shared_surface_vertex;
+            new_half_edge.replace_surface(shared_surface);
             new_half_edge.infer_global_form();
         }
 
         self.half_edges.push(new_half_edge.clone());
         new_half_edge
     }
+
+    fn add_half_edge_from_point_to_start(
+        &mut self,
+        point: impl Into<Point<2>>,
+    ) -> Partial<HalfEdge> {
+        let mut half_edge = self.add_half_edge();
+
+        half_edge
+            .write()
+            .back_mut()
+            .write()
+            .surface_form
+            .write()
+            .position = Some(point.into());
+
+        half_edge
+    }
 }

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

@@ -6,10 +6,20 @@ use crate::{
     partial::{Partial, PartialGlobalEdge, PartialHalfEdge},
 };
 
-use super::CurveBuilder;
+use super::{CurveBuilder, VertexBuilder};
 
 /// Builder API for [`PartialHalfEdge`]
 pub trait HalfEdgeBuilder {
+    /// Completely replace the surface in this half-edge's object graph
+    ///
+    /// Please note that this operation will write to both vertices that the
+    /// half-edge references. If any of them were created from full objects,
+    /// this will break the connection to those, meaning that building the
+    /// partial objects won't result in those full objects again. This will be
+    /// the case, even if those full objects already referenced the provided
+    /// surface.
+    fn replace_surface(&mut self, surface: impl Into<Partial<Surface>>);
+
     /// Update partial half-edge to be a circle, from the given radius
     fn update_as_circle_from_radius(&mut self, radius: impl Into<Scalar>);
 
@@ -31,6 +41,14 @@ pub trait HalfEdgeBuilder {
 }
 
 impl HalfEdgeBuilder for PartialHalfEdge {
+    fn replace_surface(&mut self, surface: impl Into<Partial<Surface>>) {
+        let surface = surface.into();
+
+        for vertex in &mut self.vertices {
+            vertex.write().replace_surface(surface.clone());
+        }
+    }
+
     fn update_as_circle_from_radius(&mut self, radius: impl Into<Scalar>) {
         let mut curve = self.curve();
         curve.write().update_as_circle_from_radius(radius);

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

@@ -31,7 +31,7 @@ impl FaceBuilder for PartialFace {
         points: impl IntoIterator<Item = impl Into<Point<2>>>,
     ) -> Vec<Partial<HalfEdge>> {
         let mut cycle = PartialCycle::default();
-        let half_edges = cycle.update_as_polygon(surface, points);
+        let half_edges = cycle.update_as_polygon_from_points(surface, points);
 
         self.exterior = Partial::from_partial(cycle);
 
@@ -44,7 +44,7 @@ impl FaceBuilder for PartialFace {
         points: impl IntoIterator<Item = impl Into<Point<2>>>,
     ) {
         let mut cycle = PartialCycle::default();
-        cycle.update_as_polygon(surface, points);
+        cycle.update_as_polygon_from_points(surface, points);
 
         self.interiors.push(Partial::from_partial(cycle));
     }

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

@@ -1,16 +1,33 @@
 use fj_math::Point;
 
-use crate::partial::{
-    PartialGlobalVertex, PartialSurfaceVertex, PartialVertex,
+use crate::{
+    objects::Surface,
+    partial::{
+        Partial, PartialGlobalVertex, PartialSurfaceVertex, PartialVertex,
+    },
 };
 
 /// Builder API for [`PartialVertex`]
 pub trait VertexBuilder {
-    // No methods are currently defined. This trait serves as a placeholder, to
-    // make it clear where to add such methods, once necessary.
+    /// Completely replace the surface in this vertex' object graph
+    ///
+    /// Please note that this operation will write to every partial object that
+    /// the vertex references. If any of them were created from full objects,
+    /// this will break the connection to those, meaning that building the
+    /// partial objects won't result in those full objects again. This will be
+    /// the case, even if those full objects already referenced the provided
+    /// surface.
+    fn replace_surface(&mut self, surface: impl Into<Partial<Surface>>);
 }
 
-impl VertexBuilder for PartialVertex {}
+impl VertexBuilder for PartialVertex {
+    fn replace_surface(&mut self, surface: impl Into<Partial<Surface>>) {
+        let surface = surface.into();
+
+        self.curve.write().surface = surface.clone();
+        self.surface_form.write().surface = surface;
+    }
+}
 
 /// Builder API for [`PartialSurfaceVertex`]
 pub trait SurfaceVertexBuilder {

crates/fj-kernel/src/iter.rs

@@ -409,7 +409,10 @@ mod tests {
         let surface = services.objects.surfaces.xy_plane();
         let object = {
             let mut cycle = PartialCycle::default();
-            cycle.update_as_polygon(surface, [[0., 0.], [1., 0.], [0., 1.]]);
+            cycle.update_as_polygon_from_points(
+                surface,
+                [[0., 0.], [1., 0.], [0., 1.]],
+            );
             cycle
                 .build(&mut services.objects)
                 .insert(&mut services.objects)

crates/fj-kernel/src/validate/cycle.rs

@@ -79,7 +79,7 @@ mod tests {
 
         let valid = {
             let mut cycle = PartialCycle::default();
-            cycle.update_as_polygon(
+            cycle.update_as_polygon_from_points(
                 services.objects.surfaces.xy_plane(),
                 [[0., 0.], [1., 0.], [0., 1.]],
             );

crates/fj-kernel/src/validate/face.rs

@@ -131,7 +131,7 @@ mod tests {
         };
         let invalid = {
             let mut cycle = PartialCycle::default();
-            cycle.update_as_polygon(
+            cycle.update_as_polygon_from_points(
                 services.objects.surfaces.xz_plane(),
                 [[1., 1.], [1., 2.], [2., 1.]],
             );