Merge pull request #1309 from hannobraun/partial

Extract non-essential builder methods from partial object API

crates/fj-kernel/src/algorithms/intersect/curve_edge.rs

@@ -75,7 +75,7 @@ mod tests {
     use fj_math::Point;
 
     use crate::{
-        builder::CurveBuilder,
+        builder::{CurveBuilder, HalfEdgeBuilder},
         objects::{Curve, HalfEdge, Objects},
         partial::HasPartial,
     };
@@ -93,7 +93,7 @@ mod tests {
             .build(&objects)?;
         let half_edge = HalfEdge::partial()
             .with_surface(Some(surface))
-            .as_line_segment_from_points([[1., -1.], [1., 1.]])
+            .update_as_line_segment_from_points([[1., -1.], [1., 1.]])
             .build(&objects)?;
 
         let intersection = CurveEdgeIntersection::compute(&curve, &half_edge);
@@ -118,7 +118,7 @@ mod tests {
             .build(&objects)?;
         let half_edge = HalfEdge::partial()
             .with_surface(Some(surface))
-            .as_line_segment_from_points([[-1., -1.], [-1., 1.]])
+            .update_as_line_segment_from_points([[-1., -1.], [-1., 1.]])
             .build(&objects)?;
 
         let intersection = CurveEdgeIntersection::compute(&curve, &half_edge);
@@ -143,7 +143,7 @@ mod tests {
             .build(&objects)?;
         let half_edge = HalfEdge::partial()
             .with_surface(Some(surface))
-            .as_line_segment_from_points([[-1., -1.], [1., -1.]])
+            .update_as_line_segment_from_points([[-1., -1.], [1., -1.]])
             .build(&objects)?;
 
         let intersection = CurveEdgeIntersection::compute(&curve, &half_edge);
@@ -163,7 +163,7 @@ mod tests {
             .build(&objects)?;
         let half_edge = HalfEdge::partial()
             .with_surface(Some(surface))
-            .as_line_segment_from_points([[-1., 0.], [1., 0.]])
+            .update_as_line_segment_from_points([[-1., 0.], [1., 0.]])
             .build(&objects)?;
 
         let intersection = CurveEdgeIntersection::compute(&curve, &half_edge);

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

@@ -189,6 +189,7 @@ mod tests {
 
     use crate::{
         algorithms::{reverse::Reverse, sweep::Sweep},
+        builder::HalfEdgeBuilder,
         objects::{Cycle, Face, HalfEdge, Objects, SurfaceVertex, Vertex},
         partial::HasPartial,
     };
@@ -199,7 +200,7 @@ mod tests {
 
         let half_edge = HalfEdge::partial()
             .with_surface(Some(objects.surfaces.xy_plane()))
-            .as_line_segment_from_points([[0., 0.], [1., 0.]])
+            .update_as_line_segment_from_points([[0., 0.], [1., 0.]])
             .build(&objects)?;
 
         let face =
@@ -210,7 +211,7 @@ mod tests {
 
             let bottom = HalfEdge::partial()
                 .with_surface(Some(surface.clone()))
-                .as_line_segment_from_points([[0., 0.], [1., 0.]])
+                .update_as_line_segment_from_points([[0., 0.], [1., 0.]])
                 .build(&objects)?;
             let side_up = HalfEdge::partial()
                 .with_surface(Some(surface.clone()))
@@ -222,7 +223,7 @@ mod tests {
                         SurfaceVertex::partial().with_position(Some([1., 1.])),
                     ),
                 )))
-                .as_line_segment()
+                .update_as_line_segment()
                 .build(&objects)?;
             let top = HalfEdge::partial()
                 .with_surface(Some(surface.clone()))
@@ -234,7 +235,7 @@ mod tests {
                 .with_front_vertex(Some(Vertex::partial().with_surface_form(
                     Some(side_up.front().surface_form().clone()),
                 )))
-                .as_line_segment()
+                .update_as_line_segment()
                 .build(&objects)?
                 .reverse(&objects)?;
             let side_down = HalfEdge::partial()
@@ -245,7 +246,7 @@ mod tests {
                 .with_front_vertex(Some(Vertex::partial().with_surface_form(
                     Some(top.front().surface_form().clone()),
                 )))
-                .as_line_segment()
+                .update_as_line_segment()
                 .build(&objects)?
                 .reverse(&objects)?;
 

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

@@ -84,6 +84,7 @@ mod tests {
 
     use crate::{
         algorithms::{reverse::Reverse, transform::TransformObject},
+        builder::HalfEdgeBuilder,
         objects::{Face, HalfEdge, Objects, Sketch},
         partial::HasPartial,
     };
@@ -125,7 +126,7 @@ mod tests {
             .map(|&[a, b]| {
                 let half_edge = HalfEdge::partial()
                     .with_surface(Some(objects.surfaces.xy_plane()))
-                    .as_line_segment_from_points([a, b])
+                    .update_as_line_segment_from_points([a, b])
                     .build(&objects)?;
                 (half_edge, Color::default()).sweep(UP, &objects)
             })
@@ -167,7 +168,7 @@ mod tests {
             .map(|&[a, b]| {
                 let half_edge = HalfEdge::partial()
                     .with_surface(Some(objects.surfaces.xy_plane()))
-                    .as_line_segment_from_points([a, b])
+                    .update_as_line_segment_from_points([a, b])
                     .build(&objects)?
                     .reverse(&objects)?;
                 (half_edge, Color::default()).sweep(DOWN, &objects)

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

@@ -168,7 +168,7 @@ mod tests {
 
     use crate::{
         algorithms::sweep::Sweep,
-        builder::CurveBuilder,
+        builder::{CurveBuilder, HalfEdgeBuilder},
         objects::{Curve, HalfEdge, Objects, Vertex},
         partial::HasPartial,
     };
@@ -192,7 +192,7 @@ mod tests {
 
         let expected_half_edge = HalfEdge::partial()
             .with_surface(Some(surface))
-            .as_line_segment_from_points([[0., 0.], [0., 1.]])
+            .update_as_line_segment_from_points([[0., 0.], [0., 1.]])
             .build(&objects)?;
         assert_eq!(half_edge, expected_half_edge);
         Ok(())

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

@@ -13,25 +13,21 @@ impl TransformObject for PartialCycle {
         objects: &Objects,
     ) -> Result<Self, ValidationError> {
         let surface = self
-            .surface
+            .surface()
-            .clone()
             .map(|surface| surface.transform(transform, objects))
             .transpose()?;
         let half_edges = self
-            .half_edges
+            .half_edges()
-            .into_iter()
             .map(|edge| {
                 Ok(edge
                     .into_partial()
                     .transform(transform, objects)?
-                    .with_surface(surface.clone())
+                    .with_surface(surface.clone()))
-                    .into())
             })
-            .collect::<Result<_, ValidationError>>()?;
+            .collect::<Result<Vec<_>, ValidationError>>()?;
 
-        Ok(Self {
+        Ok(Self::default()
-            surface,
+            .with_surface(surface)
-            half_edges,
+            .with_half_edges(half_edges))
-        })
     }
 }

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

@@ -16,38 +16,35 @@ impl TransformObject for PartialHalfEdge {
         objects: &Objects,
     ) -> Result<Self, ValidationError> {
         let surface = self
-            .surface
+            .surface()
             .map(|surface| surface.transform(transform, objects))
             .transpose()?;
         let curve: MaybePartial<_> = self
-            .curve
+            .curve()
-            .clone()
             .into_partial()
             .transform(transform, objects)?
             .with_surface(surface.clone())
             .into();
-        let vertices = self.vertices.clone().try_map_ext(
+        let vertices = self.vertices().try_map_ext(
             |vertex| -> Result<_, ValidationError> {
                 Ok(vertex
                     .into_partial()
                     .transform(transform, objects)?
-                    .with_curve(Some(curve.clone()))
+                    .with_curve(Some(curve.clone())))
-                    .into())
             },
         )?;
         let global_form = self
-            .global_form
+            .global_form()
             .into_partial()
             .transform(transform, objects)?
             .with_curve(curve.global_form())
             .into();
 
-        Ok(Self {
+        Ok(Self::default()
-            surface,
+            .with_surface(surface)
-            curve,
+            .with_curve(Some(curve))
-            vertices,
+            .with_vertices(Some(vertices))
-            global_form,
+            .with_global_form(global_form))
-        })
     }
 }
 
@@ -57,9 +54,9 @@ impl TransformObject for PartialGlobalEdge {
         transform: &Transform,
         objects: &Objects,
     ) -> Result<Self, ValidationError> {
-        let curve = self.curve.transform(transform, objects)?;
+        let curve = self.curve().transform(transform, objects)?;
         let vertices = self
-            .vertices
+            .vertices()
             .map(|vertices| {
                 vertices.try_map_ext(|vertex| -> Result<_, ValidationError> {
                     vertex.transform(transform, objects)
@@ -67,6 +64,8 @@ impl TransformObject for PartialGlobalEdge {
             })
             .transpose()?;
 
-        Ok(Self { curve, vertices })
+        Ok(Self::default()
+            .with_curve(Some(curve))
+            .with_vertices(vertices))
     }
 }

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

@@ -0,0 +1,134 @@
+use fj_math::Point;
+
+use crate::{
+    objects::{Curve, HalfEdge, SurfaceVertex, Vertex},
+    partial::{HasPartial, MaybePartial, PartialCycle},
+};
+
+use super::{CurveBuilder, HalfEdgeBuilder};
+
+/// Builder API for [`PartialCycle`]
+pub trait CycleBuilder {
+    /// Update the partial cycle with a polygonal chain from the provided points
+    fn with_poly_chain(
+        self,
+        vertices: impl IntoIterator<Item = impl Into<MaybePartial<SurfaceVertex>>>,
+    ) -> Self;
+
+    /// Update the partial cycle with a polygonal chain from the provided points
+    fn with_poly_chain_from_points(
+        self,
+        points: impl IntoIterator<Item = impl Into<Point<2>>>,
+    ) -> Self;
+
+    /// Update the partial cycle by closing it with a line segment
+    ///
+    /// Builds a line segment from the last and first vertex, closing the cycle.
+    fn close_with_line_segment(self) -> Self;
+}
+
+impl CycleBuilder for PartialCycle {
+    fn with_poly_chain(
+        self,
+        vertices: impl IntoIterator<Item = impl Into<MaybePartial<SurfaceVertex>>>,
+    ) -> Self {
+        let vertices = vertices.into_iter().map(Into::into);
+
+        let iter = self
+            .half_edges()
+            .last()
+            .map(|half_edge| {
+                let [_, last] = half_edge.vertices();
+                last.surface_form()
+            })
+            .into_iter()
+            .chain(vertices);
+
+        let mut previous: Option<MaybePartial<SurfaceVertex>> = None;
+
+        let mut half_edges = Vec::new();
+        for vertex_next in iter {
+            if let Some(vertex_prev) = previous {
+                let surface = self
+                    .surface()
+                    .clone()
+                    .expect("Need surface to extend cycle with poly-chain");
+
+                let position_prev = vertex_prev
+                    .position()
+                    .expect("Need surface position to extend cycle");
+                let position_next = vertex_next
+                    .position()
+                    .expect("Need surface position to extend cycle");
+
+                let from = vertex_prev.update_partial(|partial| {
+                    partial.with_surface(Some(surface.clone()))
+                });
+                let to = vertex_next.update_partial(|partial| {
+                    partial.with_surface(Some(surface.clone()))
+                });
+
+                previous = Some(to.clone());
+
+                let curve = Curve::partial()
+                    .with_surface(Some(surface.clone()))
+                    .update_as_line_from_points([position_prev, position_next]);
+
+                let [from, to] =
+                    [(0., from), (1., to)].map(|(position, surface_form)| {
+                        Vertex::partial()
+                            .with_curve(Some(curve.clone()))
+                            .with_position(Some([position]))
+                            .with_surface_form(Some(surface_form))
+                    });
+
+                half_edges.push(
+                    HalfEdge::partial()
+                        .with_curve(Some(curve))
+                        .with_vertices(Some([from, to])),
+                );
+
+                continue;
+            }
+
+            previous = Some(vertex_next);
+        }
+
+        self.with_half_edges(half_edges)
+    }
+
+    fn with_poly_chain_from_points(
+        self,
+        points: impl IntoIterator<Item = impl Into<Point<2>>>,
+    ) -> Self {
+        self.with_poly_chain(points.into_iter().map(|position| {
+            SurfaceVertex::partial().with_position(Some(position))
+        }))
+    }
+
+    fn close_with_line_segment(self) -> Self {
+        let first = self.half_edges().next();
+        let last = self.half_edges().last();
+
+        let vertices = [first, last]
+            .map(|option| option.map(|half_edge| half_edge.vertices()));
+
+        let [Some([first, _]), Some([_, last])] = vertices else {
+            return self;
+        };
+
+        let vertices = [last, first].map(|vertex| {
+            vertex
+                .surface_form()
+                .position()
+                .expect("Need surface position to close cycle")
+        });
+        let surface = self.surface().expect("Need surface to close cycle");
+
+        self.with_half_edges(Some(
+            HalfEdge::partial()
+                .with_surface(Some(surface))
+                .update_as_line_segment_from_points(vertices),
+        ))
+    }
+}

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

@@ -0,0 +1,203 @@
+use fj_interop::ext::ArrayExt;
+use fj_math::{Point, Scalar};
+
+use crate::{
+    objects::{
+        Curve, GlobalVertex, Objects, SurfaceVertex, Vertex,
+        VerticesInNormalizedOrder,
+    },
+    partial::{HasPartial, PartialGlobalEdge, PartialHalfEdge},
+    storage::Handle,
+    validate::ValidationError,
+};
+
+use super::{CurveBuilder, GlobalVertexBuilder};
+
+/// Builder API for [`PartialHalfEdge`]
+pub trait HalfEdgeBuilder: Sized {
+    /// Update partial half-edge as a circle, from the given radius
+    ///
+    /// # Implementation Note
+    ///
+    /// In principle, only the `build` method should take a reference to
+    /// [`Objects`]. As of this writing, this method is the only one that
+    /// deviates from that. I couldn't think of a way to do it better.
+    fn update_as_circle_from_radius(
+        self,
+        radius: impl Into<Scalar>,
+        objects: &Objects,
+    ) -> Result<Self, ValidationError>;
+
+    /// Update partial half-edge as a line segment, from the given points
+    fn update_as_line_segment_from_points(
+        self,
+        points: [impl Into<Point<2>>; 2],
+    ) -> Self;
+
+    /// Update partial half-edge as a line segment, reusing existing vertices
+    fn update_as_line_segment(self) -> Self;
+}
+
+impl HalfEdgeBuilder for PartialHalfEdge {
+    fn update_as_circle_from_radius(
+        self,
+        radius: impl Into<Scalar>,
+        objects: &Objects,
+    ) -> Result<Self, ValidationError> {
+        let curve = Curve::partial()
+            .with_global_form(Some(self.extract_global_curve()))
+            .with_surface(self.surface())
+            .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
+            .extract_global_vertices()
+            .map(|[global_form, _]| global_form)
+            .unwrap_or_else(|| {
+                GlobalVertex::partial()
+                    .update_from_curve_and_position(curve.clone(), a_curve)
+                    .into()
+            });
+
+        let surface_vertex = SurfaceVertex::partial()
+            .with_position(Some(path.point_from_path_coords(a_curve)))
+            .with_surface(self.surface())
+            .with_global_form(Some(global_vertex))
+            .build(objects)?;
+
+        let [back, front] = [a_curve, b_curve].map(|point_curve| {
+            Vertex::partial()
+                .with_position(Some(point_curve))
+                .with_curve(Some(curve.clone()))
+                .with_surface_form(Some(surface_vertex.clone()))
+        });
+
+        Ok(self
+            .with_curve(Some(curve))
+            .with_vertices(Some([back, front])))
+    }
+
+    fn update_as_line_segment_from_points(
+        self,
+        points: [impl Into<Point<2>>; 2],
+    ) -> Self {
+        let surface = self.surface();
+        let vertices = points.map(|point| {
+            let surface_form = SurfaceVertex::partial()
+                .with_surface(surface.clone())
+                .with_position(Some(point));
+
+            Vertex::partial().with_surface_form(Some(surface_form))
+        });
+
+        self.with_vertices(Some(vertices)).update_as_line_segment()
+    }
+
+    fn update_as_line_segment(self) -> Self {
+        let [from, to] = self.vertices();
+        let [from_surface, to_surface] =
+            [&from, &to].map(|vertex| vertex.surface_form());
+
+        let surface = self
+            .surface()
+            .or_else(|| from_surface.surface())
+            .or_else(|| to_surface.surface())
+            .expect("Can't infer line segment without a surface");
+        let points = [&from_surface, &to_surface].map(|vertex| {
+            vertex
+                .position()
+                .expect("Can't infer line segment without surface position")
+        });
+
+        let curve = Curve::partial()
+            .with_global_form(Some(self.extract_global_curve()))
+            .with_surface(Some(surface))
+            .update_as_line_from_points(points);
+
+        let [back, front] = {
+            let vertices = [(from, 0.), (to, 1.)].map(|(vertex, position)| {
+                vertex.update_partial(|vertex| {
+                    vertex
+                        .with_position(Some([position]))
+                        .with_curve(Some(curve.clone()))
+                })
+            });
+
+            // The global vertices we extracted are in normalized order, which
+            // means we might need to switch their order here. This is a bit of
+            // a hack, but I can't think of something better.
+            let global_forms = {
+                let must_switch_order = {
+                    let objects = Objects::new();
+                    let vertices = vertices.clone().map(|vertex| {
+                        vertex
+                            .into_full(&objects)
+                            .unwrap()
+                            .global_form()
+                            .clone()
+                    });
+
+                    let (_, must_switch_order) =
+                        VerticesInNormalizedOrder::new(vertices);
+
+                    must_switch_order
+                };
+
+                self.extract_global_vertices()
+                    .map(
+                        |[a, b]| {
+                            if must_switch_order {
+                                [b, a]
+                            } else {
+                                [a, b]
+                            }
+                        },
+                    )
+                    .map(|[a, b]| [Some(a), Some(b)])
+                    .unwrap_or([None, None])
+            };
+
+            vertices.zip_ext(global_forms).map(|(vertex, global_form)| {
+                vertex.update_partial(|vertex| {
+                    vertex.clone().with_surface_form(Some(
+                        vertex.surface_form().update_partial(
+                            |surface_vertex| {
+                                surface_vertex.with_global_form(global_form)
+                            },
+                        ),
+                    ))
+                })
+            })
+        };
+
+        self.with_curve(Some(curve))
+            .with_vertices(Some([back, front]))
+    }
+}
+
+/// Builder API for [`PartialGlobalEdge`]
+pub trait GlobalEdgeBuilder {
+    /// Update partial global edge from the given curve and vertices
+    fn update_from_curve_and_vertices(
+        self,
+        curve: &Curve,
+        vertices: &[Handle<Vertex>; 2],
+    ) -> Self;
+}
+
+impl GlobalEdgeBuilder for PartialGlobalEdge {
+    fn update_from_curve_and_vertices(
+        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()),
+            ))
+    }
+}

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

@@ -7,6 +7,8 @@ use crate::{
     storage::Handle,
 };
 
+use super::CycleBuilder;
+
 /// API for building a [`Face`]
 ///
 /// Also see [`Face::builder`].

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

@@ -10,10 +10,14 @@ mod solid;
 
 // These are new-style builders that build on top of the partial object API.
 mod curve;
+mod cycle;
+mod edge;
 mod vertex;
 
 pub use self::{
     curve::CurveBuilder,
+    cycle::CycleBuilder,
+    edge::{GlobalEdgeBuilder, HalfEdgeBuilder},
     face::FaceBuilder,
     shell::ShellBuilder,
     sketch::SketchBuilder,

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

@@ -5,6 +5,7 @@ use fj_math::Scalar;
 
 use crate::{
     algorithms::transform::TransformObject,
+    builder::HalfEdgeBuilder,
     objects::{
         Curve, Cycle, Face, FaceSet, HalfEdge, Objects, Shell, Surface,
         SurfaceVertex, Vertex,
@@ -90,7 +91,10 @@ impl<'a> ShellBuilder<'a> {
                     HalfEdge::partial()
                         .with_surface(Some(surface.clone()))
                         .with_global_form(Some(half_edge.global_form().clone()))
-                        .as_line_segment_from_points([[Z, Z], [edge_length, Z]])
+                        .update_as_line_segment_from_points([
+                            [Z, Z],
+                            [edge_length, Z],
+                        ])
                         .build(self.objects)
                         .unwrap()
                 })
@@ -113,7 +117,7 @@ impl<'a> ShellBuilder<'a> {
                             Vertex::partial().with_surface_form(Some(from)),
                             Vertex::partial().with_surface_form(Some(to)),
                         ]))
-                        .as_line_segment()
+                        .update_as_line_segment()
                         .build(self.objects)
                         .unwrap()
                 })
@@ -150,7 +154,7 @@ impl<'a> ShellBuilder<'a> {
                                 Vertex::partial().with_surface_form(Some(from)),
                                 Vertex::partial().with_surface_form(Some(to)),
                             ]))
-                            .as_line_segment()
+                            .update_as_line_segment()
                             .build(self.objects)
                             .unwrap()
                     })
@@ -173,7 +177,7 @@ impl<'a> ShellBuilder<'a> {
 
                     HalfEdge::partial()
                         .with_vertices(Some([from, to]))
-                        .as_line_segment()
+                        .update_as_line_segment()
                         .build(self.objects)
                         .unwrap()
                 })
@@ -252,7 +256,7 @@ impl<'a> ShellBuilder<'a> {
                     HalfEdge::partial()
                         .with_vertices(Some(vertices))
                         .with_global_form(Some(edge.global_form().clone()))
-                        .as_line_segment()
+                        .update_as_line_segment()
                         .build(self.objects)
                         .unwrap(),
                 );

crates/fj-kernel/src/iter.rs

@@ -360,7 +360,7 @@ impl<T> Iterator for Iter<T> {
 #[cfg(test)]
 mod tests {
     use crate::{
-        builder::CurveBuilder,
+        builder::{CurveBuilder, CycleBuilder, HalfEdgeBuilder},
         objects::{
             Curve, Cycle, Face, GlobalCurve, GlobalVertex, HalfEdge, Objects,
             Shell, Sketch, Solid, SurfaceVertex, Vertex,
@@ -486,7 +486,7 @@ mod tests {
 
         let object = HalfEdge::partial()
             .with_surface(Some(objects.surfaces.xy_plane()))
-            .as_line_segment_from_points([[0., 0.], [1., 0.]])
+            .update_as_line_segment_from_points([[0., 0.], [1., 0.]])
             .build(&objects);
 
         assert_eq!(1, object.curve_iter().count());

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

@@ -149,7 +149,9 @@ impl VerticesInNormalizedOrder {
 mod tests {
     use pretty_assertions::assert_eq;
 
-    use crate::{objects::Objects, partial::HasPartial};
+    use crate::{
+        builder::HalfEdgeBuilder, objects::Objects, partial::HasPartial,
+    };
 
     use super::HalfEdge;
 
@@ -164,11 +166,11 @@ mod tests {
 
         let a_to_b = HalfEdge::partial()
             .with_surface(Some(surface.clone()))
-            .as_line_segment_from_points([a, b])
+            .update_as_line_segment_from_points([a, b])
             .build(&objects)?;
         let b_to_a = HalfEdge::partial()
             .with_surface(Some(surface))
-            .as_line_segment_from_points([b, a])
+            .update_as_line_segment_from_points([b, a])
             .build(&objects)?;
 
         assert_eq!(a_to_b.global_form(), b_to_a.global_form());

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

@@ -144,7 +144,7 @@ impl MaybePartial<GlobalEdge> {
     pub fn curve(&self) -> MaybePartial<GlobalCurve> {
         match self {
             Self::Full(full) => full.curve().clone().into(),
-            Self::Partial(partial) => partial.curve.clone(),
+            Self::Partial(partial) => partial.curve(),
         }
     }
 
@@ -154,7 +154,7 @@ impl MaybePartial<GlobalEdge> {
             Self::Full(full) => Some(
                 full.vertices().access_in_normalized_order().map(Into::into),
             ),
-            Self::Partial(partial) => partial.vertices.clone(),
+            Self::Partial(partial) => partial.vertices(),
         }
     }
 }
@@ -165,7 +165,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()
             }
         }
@@ -175,7 +175,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.clone(),
+            Self::Partial(partial) => partial.vertices(),
         }
     }
 }

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

@@ -1,11 +1,6 @@
-use fj_math::Point;
-
 use crate::{
-    builder::CurveBuilder,
+    objects::{Cycle, HalfEdge, Objects, Surface},
-    objects::{
+    partial::MaybePartial,
-        Curve, Cycle, HalfEdge, Objects, Surface, SurfaceVertex, Vertex,
-    },
-    partial::{HasPartial, MaybePartial},
     storage::Handle,
     validate::ValidationError,
 };
@@ -15,14 +10,21 @@ use crate::{
 /// See [`crate::partial`] for more information.
 #[derive(Clone, Debug, Default)]
 pub struct PartialCycle {
-    /// The surface that the [`Cycle`] is defined in
+    surface: Option<Handle<Surface>>,
-    pub surface: Option<Handle<Surface>>,
+    half_edges: Vec<MaybePartial<HalfEdge>>,
-
-    /// The half-edges that make up the [`Cycle`]
-    pub half_edges: Vec<MaybePartial<HalfEdge>>,
 }
 
 impl PartialCycle {
+    /// Access the surface that the [`Cycle`] is defined in
+    pub fn surface(&self) -> Option<Handle<Surface>> {
+        self.surface.clone()
+    }
+
+    /// Access the half-edges that make up the [`Cycle`]
+    pub fn half_edges(&self) -> impl Iterator<Item = MaybePartial<HalfEdge>> {
+        self.half_edges.clone().into_iter()
+    }
+
     /// Update the partial cycle with the given surface
     pub fn with_surface(mut self, surface: Option<Handle<Surface>>) -> Self {
         if let Some(surface) = surface {
@@ -34,121 +36,10 @@ impl PartialCycle {
     /// Update the partial cycle with the given half-edges
     pub fn with_half_edges(
         mut self,
-        half_edge: impl IntoIterator<Item = impl Into<MaybePartial<HalfEdge>>>,
+        half_edges: impl IntoIterator<Item = impl Into<MaybePartial<HalfEdge>>>,
     ) -> Self {
         self.half_edges
-            .extend(half_edge.into_iter().map(Into::into));
+            .extend(half_edges.into_iter().map(Into::into));
-        self
-    }
-
-    /// Update the partial cycle with a polygonal chain from the provided points
-    pub fn with_poly_chain(
-        mut self,
-        vertices: impl IntoIterator<Item = MaybePartial<SurfaceVertex>>,
-    ) -> Self {
-        let iter = self
-            .half_edges
-            .last()
-            .map(|half_edge| {
-                let [_, last] = half_edge.vertices();
-                last.surface_form()
-            })
-            .into_iter()
-            .chain(vertices);
-
-        let mut previous: Option<MaybePartial<SurfaceVertex>> = None;
-
-        for vertex_next in iter {
-            if let Some(vertex_prev) = previous {
-                let surface = self
-                    .surface
-                    .clone()
-                    .expect("Need surface to extend cycle with poly-chain");
-
-                let position_prev = vertex_prev
-                    .position()
-                    .expect("Need surface position to extend cycle");
-                let position_next = vertex_next
-                    .position()
-                    .expect("Need surface position to extend cycle");
-
-                let from = vertex_prev.update_partial(|partial| {
-                    partial.with_surface(Some(surface.clone()))
-                });
-                let to = vertex_next.update_partial(|partial| {
-                    partial.with_surface(Some(surface.clone()))
-                });
-
-                previous = Some(to.clone());
-
-                let curve = Curve::partial()
-                    .with_surface(Some(surface.clone()))
-                    .update_as_line_from_points([position_prev, position_next]);
-
-                let [from, to] =
-                    [(0., from), (1., to)].map(|(position, surface_form)| {
-                        Vertex::partial()
-                            .with_curve(Some(curve.clone()))
-                            .with_position(Some([position]))
-                            .with_surface_form(Some(surface_form))
-                    });
-
-                self.half_edges.push(
-                    HalfEdge::partial()
-                        .with_curve(Some(curve))
-                        .with_vertices(Some([from, to]))
-                        .into(),
-                );
-
-                continue;
-            }
-
-            previous = Some(vertex_next);
-        }
-
-        self
-    }
-
-    /// Update the partial cycle with a polygonal chain from the provided points
-    pub fn with_poly_chain_from_points(
-        self,
-        points: impl IntoIterator<Item = impl Into<Point<2>>>,
-    ) -> Self {
-        self.with_poly_chain(points.into_iter().map(|position| {
-            SurfaceVertex::partial()
-                .with_position(Some(position))
-                .into()
-        }))
-    }
-
-    /// Update the partial cycle by closing it with a line segment
-    ///
-    /// Builds a line segment from the last and first vertex, closing the cycle.
-    pub fn close_with_line_segment(mut self) -> Self {
-        let first = self.half_edges.first();
-        let last = self.half_edges.last();
-
-        let vertices = [first, last]
-            .map(|option| option.map(|half_edge| half_edge.vertices()));
-
-        if let [Some([first, _]), Some([_, last])] = vertices {
-            let vertices = [last, first].map(|vertex| {
-                vertex
-                    .surface_form()
-                    .position()
-                    .expect("Need surface position to close cycle")
-            });
-            let surface =
-                self.surface.clone().expect("Need surface to close cycle");
-
-            self.half_edges.push(
-                HalfEdge::partial()
-                    .with_surface(Some(surface))
-                    .as_line_segment_from_points(vertices)
-                    .into(),
-            );
-        }
-
         self
     }
 
@@ -202,7 +93,7 @@ impl PartialCycle {
 
                     let half_edge = half_edge
                         .update_partial(|half_edge| {
-                            let [back, _] = half_edge.vertices.clone();
+                            let [back, _] = half_edge.vertices();
                             let back = back.update_partial(|partial| {
                                 partial.with_surface_form(previous_vertex)
                             });

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

@@ -1,13 +1,12 @@
 use fj_interop::ext::ArrayExt;
-use fj_math::{Point, Scalar};
 
 use crate::{
-    builder::{CurveBuilder, GlobalVertexBuilder},
+    builder::GlobalEdgeBuilder,
     objects::{
         Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects,
-        Surface, SurfaceVertex, Vertex, VerticesInNormalizedOrder,
+        Surface, Vertex,
     },
-    partial::{HasPartial, MaybePartial},
+    partial::MaybePartial,
     storage::Handle,
     validate::ValidationError,
 };
@@ -17,22 +16,33 @@ use crate::{
 /// See [`crate::partial`] for more information.
 #[derive(Clone, Debug, Default)]
 pub struct PartialHalfEdge {
-    /// The surface that the [`HalfEdge`]'s [`Curve`] is defined in
+    surface: Option<Handle<Surface>>,
-    pub surface: Option<Handle<Surface>>,
+    curve: MaybePartial<Curve>,
-
+    vertices: [MaybePartial<Vertex>; 2],
-    /// The curve that the [`HalfEdge`] is defined in
+    global_form: MaybePartial<GlobalEdge>,
-    pub curve: MaybePartial<Curve>,
-
-    /// The vertices that bound this [`HalfEdge`] in the [`Curve`]
-    pub vertices: [MaybePartial<Vertex>; 2],
-
-    /// The global form of the [`HalfEdge`]
-    ///
-    /// Can be computed by [`PartialHalfEdge::build`], if not available.
-    pub global_form: MaybePartial<GlobalEdge>,
 }
 
 impl PartialHalfEdge {
+    /// Access the surface that the [`HalfEdge`]'s [`Curve`] is defined in
+    pub fn surface(&self) -> Option<Handle<Surface>> {
+        self.surface.clone()
+    }
+
+    /// Access the curve that the [`HalfEdge`] is defined in
+    pub fn curve(&self) -> MaybePartial<Curve> {
+        self.curve.clone()
+    }
+
+    /// Access the vertices that bound this [`HalfEdge`] in the [`Curve`]
+    pub fn vertices(&self) -> [MaybePartial<Vertex>; 2] {
+        self.vertices.clone()
+    }
+
+    /// Access the global form of the [`HalfEdge`]
+    pub fn global_form(&self) -> MaybePartial<GlobalEdge> {
+        self.global_form.clone()
+    }
+
     /// Extract the global curve from either the curve or global form
     ///
     /// If a global curve is available through both, the curve is preferred.
@@ -115,159 +125,6 @@ impl PartialHalfEdge {
         self
     }
 
-    /// Update partial half-edge as a circle, from the given radius
-    ///
-    /// # Implementation Note
-    ///
-    /// In principle, only the `build` method should take a reference to
-    /// [`Objects`]. As of this writing, this method is the only one that
-    /// deviates from that. I couldn't think of a way to do it better.
-    pub fn as_circle_from_radius(
-        mut self,
-        radius: impl Into<Scalar>,
-        objects: &Objects,
-    ) -> Result<Self, ValidationError> {
-        let curve = Curve::partial()
-            .with_global_form(Some(self.extract_global_curve()))
-            .with_surface(self.surface.clone())
-            .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
-            .extract_global_vertices()
-            .map(|[global_form, _]| global_form)
-            .unwrap_or_else(|| {
-                GlobalVertex::partial()
-                    .update_from_curve_and_position(curve.clone(), a_curve)
-                    .into()
-            });
-
-        let surface_vertex = SurfaceVertex::partial()
-            .with_position(Some(path.point_from_path_coords(a_curve)))
-            .with_surface(self.surface.clone())
-            .with_global_form(Some(global_vertex))
-            .build(objects)?;
-
-        let [back, front] = [a_curve, b_curve].map(|point_curve| {
-            Vertex::partial()
-                .with_position(Some(point_curve))
-                .with_curve(Some(curve.clone()))
-                .with_surface_form(Some(surface_vertex.clone()))
-                .into()
-        });
-
-        self.curve = curve.into();
-        self.vertices = [back, front];
-
-        Ok(self)
-    }
-
-    /// Update partial half-edge as a line segment, from the given points
-    pub fn as_line_segment_from_points(
-        self,
-        points: [impl Into<Point<2>>; 2],
-    ) -> Self {
-        let surface = self.surface.clone();
-        let vertices = points.map(|point| {
-            let surface_form = SurfaceVertex::partial()
-                .with_surface(surface.clone())
-                .with_position(Some(point));
-
-            Vertex::partial().with_surface_form(Some(surface_form))
-        });
-
-        self.with_vertices(Some(vertices)).as_line_segment()
-    }
-
-    /// Update partial half-edge as a line segment, reusing existing vertices
-    pub fn as_line_segment(mut self) -> Self {
-        let [from, to] = self.vertices.clone();
-        let [from_surface, to_surface] =
-            [&from, &to].map(|vertex| vertex.surface_form());
-
-        let surface = self
-            .surface
-            .clone()
-            .or_else(|| from_surface.surface())
-            .or_else(|| to_surface.surface())
-            .expect("Can't infer line segment without a surface");
-        let points = [&from_surface, &to_surface].map(|vertex| {
-            vertex
-                .position()
-                .expect("Can't infer line segment without surface position")
-        });
-
-        let curve = Curve::partial()
-            .with_global_form(Some(self.extract_global_curve()))
-            .with_surface(Some(surface))
-            .update_as_line_from_points(points);
-
-        let [back, front] = {
-            let vertices = [(from, 0.), (to, 1.)].map(|(vertex, position)| {
-                vertex.update_partial(|vertex| {
-                    vertex
-                        .with_position(Some([position]))
-                        .with_curve(Some(curve.clone()))
-                })
-            });
-
-            // The global vertices we extracted are in normalized order, which
-            // means we might need to switch their order here. This is a bit of
-            // a hack, but I can't think of something better.
-            let global_forms = {
-                let must_switch_order = {
-                    let objects = Objects::new();
-                    let vertices = vertices.clone().map(|vertex| {
-                        vertex
-                            .into_full(&objects)
-                            .unwrap()
-                            .global_form()
-                            .clone()
-                    });
-
-                    let (_, must_switch_order) =
-                        VerticesInNormalizedOrder::new(vertices);
-
-                    must_switch_order
-                };
-
-                self.extract_global_vertices()
-                    .map(
-                        |[a, b]| {
-                            if must_switch_order {
-                                [b, a]
-                            } else {
-                                [a, b]
-                            }
-                        },
-                    )
-                    .map(|[a, b]| [Some(a), Some(b)])
-                    .unwrap_or([None, None])
-            };
-
-            vertices.zip_ext(global_forms).map(|(vertex, global_form)| {
-                vertex.update_partial(|vertex| {
-                    vertex.clone().with_surface_form(Some(
-                        vertex.surface_form().update_partial(
-                            |surface_vertex| {
-                                surface_vertex.with_global_form(global_form)
-                            },
-                        ),
-                    ))
-                })
-            })
-        };
-
-        self.curve = curve.into();
-        self.vertices = [back, front];
-
-        self
-    }
-
     /// Build a full [`HalfEdge`] from the partial half-edge
     pub fn build(
         self,
@@ -287,7 +144,7 @@ impl PartialHalfEdge {
         let global_form = self
             .global_form
             .update_partial(|partial| {
-                partial.from_curve_and_vertices(&curve, &vertices)
+                partial.update_from_curve_and_vertices(&curve, &vertices)
             })
             .into_full(objects)?;
 
@@ -316,18 +173,21 @@ impl From<&HalfEdge> for PartialHalfEdge {
 /// See [`crate::partial`] for more information.
 #[derive(Clone, Debug, Default)]
 pub struct PartialGlobalEdge {
-    /// The curve that the [`GlobalEdge`] is defined in
+    curve: MaybePartial<GlobalCurve>,
-    ///
+    vertices: Option<[MaybePartial<GlobalVertex>; 2]>,
-    /// Must be provided before [`PartialGlobalEdge::build`] is called.
-    pub curve: MaybePartial<GlobalCurve>,
-
-    /// The vertices that bound the [`GlobalEdge`] in the curve
-    ///
-    /// Must be provided before [`PartialGlobalEdge::build`] is called.
-    pub vertices: Option<[MaybePartial<GlobalVertex>; 2]>,
 }
 
 impl PartialGlobalEdge {
+    /// Access the curve that the [`GlobalEdge`] is defined in
+    pub fn curve(&self) -> MaybePartial<GlobalCurve> {
+        self.curve.clone()
+    }
+
+    /// Access the vertices that bound the [`GlobalEdge`] in the curve
+    pub fn vertices(&self) -> Option<[MaybePartial<GlobalVertex>; 2]> {
+        self.vertices.clone()
+    }
+
     /// Update the partial global edge with the given curve
     pub fn with_curve(
         mut self,
@@ -350,18 +210,6 @@ impl PartialGlobalEdge {
         self
     }
 
-    /// Update partial global edge from the given curve and vertices
-    pub fn from_curve_and_vertices(
-        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()),
-            ))
-    }
-
     /// Build a full [`GlobalEdge`] from the partial global edge
     pub fn build(
         self,

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

@@ -196,7 +196,7 @@ mod tests {
     use fj_interop::ext::ArrayExt;
 
     use crate::{
-        builder::VertexBuilder,
+        builder::{HalfEdgeBuilder, VertexBuilder},
         objects::{GlobalCurve, HalfEdge, Objects},
         partial::HasPartial,
         validate::Validate2,
@@ -208,7 +208,7 @@ mod tests {
 
         let valid = HalfEdge::partial()
             .with_surface(Some(objects.surfaces.xy_plane()))
-            .as_line_segment_from_points([[0., 0.], [1., 0.]])
+            .update_as_line_segment_from_points([[0., 0.], [1., 0.]])
             .build(&objects)?;
         let invalid = {
             let mut vertices = valid.vertices().clone();
@@ -233,7 +233,7 @@ mod tests {
 
         let valid = HalfEdge::partial()
             .with_surface(Some(objects.surfaces.xy_plane()))
-            .as_line_segment_from_points([[0., 0.], [1., 0.]])
+            .update_as_line_segment_from_points([[0., 0.], [1., 0.]])
             .build(&objects)?;
         let invalid = HalfEdge::new(
             valid.vertices().clone(),
@@ -256,7 +256,7 @@ mod tests {
 
         let valid = HalfEdge::partial()
             .with_surface(Some(objects.surfaces.xy_plane()))
-            .as_line_segment_from_points([[0., 0.], [1., 0.]])
+            .update_as_line_segment_from_points([[0., 0.], [1., 0.]])
             .build(&objects)?;
         let invalid = HalfEdge::new(
             valid.vertices().clone(),
@@ -286,7 +286,7 @@ mod tests {
 
         let valid = HalfEdge::partial()
             .with_surface(Some(objects.surfaces.xy_plane()))
-            .as_line_segment_from_points([[0., 0.], [1., 0.]])
+            .update_as_line_segment_from_points([[0., 0.], [1., 0.]])
             .build(&objects)?;
         let invalid = HalfEdge::new(
             valid.vertices().clone().try_map_ext(|vertex| {

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

@@ -51,8 +51,8 @@ pub enum VertexValidationError {
     /// Mismatch between the surface's of the curve and surface form
     #[error(
         "Surface form of vertex must be defined on same surface as curve\n\
-        `- Surface` of curve: {curve_surface:#?}\n\
+        - `Surface` of curve: {curve_surface:#?}\n\
-        `- Surface` of surface form: {surface_form_surface:#?}"
+        - `Surface` of surface form: {surface_form_surface:#?}"
     )]
     SurfaceMismatch {
         /// The surface of the vertex' curve

crates/fj-operations/src/sketch.rs

@@ -2,6 +2,7 @@ use std::ops::Deref;
 
 use fj_interop::{debug::DebugInfo, mesh::Color};
 use fj_kernel::{
+    builder::HalfEdgeBuilder,
     objects::{Cycle, Face, HalfEdge, Objects, Sketch},
     partial::HasPartial,
     validate::{Validate, Validated, ValidationConfig, ValidationError},
@@ -28,7 +29,7 @@ impl Shape for fj::Sketch {
 
                 let half_edge = HalfEdge::partial()
                     .with_surface(Some(surface))
-                    .as_circle_from_radius(circle.radius(), objects)?
+                    .update_as_circle_from_radius(circle.radius(), objects)?
                     .build(objects)?;
                 let cycle = objects.cycles.insert(Cycle::new([half_edge]))?;