Merge pull request #1233 from hannobraun/vertex

Fix last known `SurfaceVertex` duplication issue

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

@@ -183,39 +183,54 @@ impl<'a> ShellBuilder<'a> {
                 .xy_plane()
                 .translate([Z, Z, h], self.objects);
 
-            let points = [[-h, -h], [-h, h], [h, h], [h, -h], [-h, -h]];
-
             let mut top_edges = top_edges;
             top_edges.reverse();
 
-            let mut vertex_prev = None;
+            let surface_vertices = {
+                let mut edges = top_edges.iter();
+
+                let a = edges.next().unwrap();
+                let b = edges.next().unwrap();
+                let c = edges.next().unwrap();
+                let d = edges.next().unwrap();
+
+                // Can be cleaned up, once `zip` is stable:
+                // https://doc.rust-lang.org/std/primitive.array.html#method.zip
+                let [a, b, c, d] =
+                    [([-h, -h], a), ([-h, h], b), ([h, h], c), ([h, -h], d)]
+                        .map(|(point, edge)| {
+                            let vertex = edge.back();
+
+                            Handle::<SurfaceVertex>::partial()
+                                .with_position(Some(point))
+                                .with_surface(Some(surface.clone()))
+                                .with_global_form(Some(
+                                    vertex.global_form().clone(),
+                                ))
+                                .build(self.objects)
+                        });
+
+                [a.clone(), b, c, d, a]
+            };
 
             let mut edges = Vec::new();
-            for (points, edge) in points.windows(2).zip(top_edges) {
+            for (surface_vertices, edge) in
+                surface_vertices.windows(2).zip(top_edges)
+            {
                 // This can't panic, as we passed `2` to `windows`. Can be
                 // cleaned up, once `array_windows` is stable.
-                let points = [points[0], points[1]];
+                let surface_vertices =
+                    [surface_vertices[0].clone(), surface_vertices[1].clone()];
 
                 // Can be cleaned up, once `zip` is stable:
                 // https://doc.rust-lang.org/std/primitive.array.html#method.zip
-                let [point_a, point_b] = points;
                 let [vertex_a, vertex_b] = edge.vertices().clone();
+                let [surface_vertex_a, surface_vertex_b] = surface_vertices;
                 let vertices = [
-                    (point_a, vertex_a, vertex_prev.clone()),
-                    (point_b, vertex_b, None),
+                    (vertex_a, surface_vertex_a),
+                    (vertex_b, surface_vertex_b),
                 ]
-                .map(|(point, vertex, surface_form)| {
-                    let surface_form = surface_form.unwrap_or_else(|| {
-                        Handle::<SurfaceVertex>::partial()
-                            .with_position(Some(point))
-                            .with_surface(Some(surface.clone()))
-                            .with_global_form(Some(
-                                vertex.global_form().clone(),
-                            ))
-                            .build(self.objects)
-                    });
-                    vertex_prev = Some(surface_form.clone());
-
+                .map(|(vertex, surface_form)| {
                     Vertex::partial()
                         .with_position(Some(vertex.position()))
                         .with_surface_form(Some(surface_form))

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

@@ -62,8 +62,8 @@ impl Cycle {
                 let [_, last] = last.vertices();
 
                 assert_eq!(
-                    first.surface_form(),
-                    last.surface_form(),
+                    first.surface_form().id(),
+                    last.surface_form().id(),
                     "Edges do not form a cycle"
                 );
             }

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

@@ -201,16 +201,16 @@ impl PartialCycle {
                 |(mut half_edges, previous_vertex), half_edge| {
                     let half_edge = half_edge
                         .update_partial(|half_edge| {
-                            let [from, _] = half_edge.vertices.clone();
-                            let from = from.map(|vertex| {
+                            let [back, _] = half_edge.vertices.clone();
+                            let back = back.map(|vertex| {
                                 vertex.update_partial(|partial| {
                                     partial.with_surface_form(previous_vertex)
                                 })
                             });
 
                             half_edge
                                 .with_surface(Some(surface_for_edges.clone()))
-                                .with_back_vertex(from)
+                                .with_back_vertex(back)
                         })
                         .into_full(objects);
 

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

@@ -90,8 +90,8 @@ impl PartialHalfEdge {
         vertices: Option<[impl Into<MaybePartial<Vertex>>; 2]>,
     ) -> Self {
         let vertices = vertices.map(|vertices| vertices.map(Into::into));
-        if let Some([a, b]) = vertices {
-            self.vertices = [Some(a), Some(b)];
+        if let Some([back, front]) = vertices {
+            self.vertices = [Some(back), Some(front)];
         }
         self
     }
@@ -108,13 +108,23 @@ impl PartialHalfEdge {
     }
 
     /// Update partial half-edge as a circle, from the given radius
-    pub fn as_circle_from_radius(mut self, radius: impl Into<Scalar>) -> Self {
+    ///
+    /// # 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,
+    ) -> Self {
         let curve = Handle::<Curve>::partial()
             .with_global_form(self.extract_global_curve())
             .with_surface(self.surface.clone())
             .as_circle_from_radius(radius);
 
-        let [a, b] = {
+        let [back, front] = {
             let [a_curve, b_curve] =
                 [Scalar::ZERO, Scalar::TAU].map(|coord| Point::from([coord]));
 
@@ -124,7 +134,9 @@ impl PartialHalfEdge {
             let path = curve.path.expect("Expected path that was just created");
             let surface_form = Handle::<SurfaceVertex>::partial()
                 .with_position(Some(path.point_from_path_coords(a_curve)))
-                .with_global_form(Some(global_form));
+                .with_surface(self.surface.clone())
+                .with_global_form(Some(global_form))
+                .build(objects);
 
             [a_curve, b_curve].map(|point_curve| {
                 Vertex::partial()
@@ -136,7 +148,7 @@ impl PartialHalfEdge {
         };
 
         self.curve = Some(curve.into());
-        self.vertices = [Some(a), Some(b)];
+        self.vertices = [Some(back), Some(front)];
 
         self
     }
@@ -186,7 +198,7 @@ impl PartialHalfEdge {
             .with_surface(Some(surface))
             .as_line_from_points(points);
 
-        let [a, b] = [(from, 0.), (to, 1.)].map(|(vertex, position)| {
+        let [back, front] = [(from, 0.), (to, 1.)].map(|(vertex, position)| {
             vertex.update_partial(|vertex| {
                 vertex
                     .with_position(Some([position]))
@@ -195,7 +207,7 @@ impl PartialHalfEdge {
         });
 
         self.curve = Some(curve.into());
-        self.vertices = [Some(a), Some(b)];
+        self.vertices = [Some(back), Some(front)];
 
         self
     }
@@ -229,12 +241,13 @@ impl PartialHalfEdge {
 
 impl From<&HalfEdge> for PartialHalfEdge {
     fn from(half_edge: &HalfEdge) -> Self {
-        let [a, b] = half_edge.vertices().clone().map(Into::into);
+        let [back_vertex, front_vertex] =
+            half_edge.vertices().clone().map(Into::into);
 
         Self {
             surface: Some(half_edge.curve().surface().clone()),
             curve: Some(half_edge.curve().clone().into()),
-            vertices: [Some(a), Some(b)],
+            vertices: [Some(back_vertex), Some(front_vertex)],
             global_form: Some(half_edge.global_form().clone().into()),
         }
     }

crates/fj-operations/src/sketch.rs

@@ -28,7 +28,7 @@ impl Shape for fj::Sketch {
 
                 let half_edge = HalfEdge::partial()
                     .with_surface(Some(surface.clone()))
-                    .as_circle_from_radius(circle.radius())
+                    .as_circle_from_radius(circle.radius(), objects)
                     .build(objects);
                 let cycle = Cycle::new(surface, [half_edge]);