Merge pull request #1023 from hannobraun/cycle

Validate `Cycle` on construction

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

@@ -13,6 +13,6 @@ impl Reverse for Cycle {
 
         edges.reverse();
 
-        Cycle::new(surface).with_edges(edges)
+        Cycle::new(surface, edges)
     }
 }

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

@@ -83,7 +83,7 @@ fn reverse_local_coordinates_in_cycle<'r>(
             Edge::from_curve_and_vertices(curve, vertices)
         });
 
-        Cycle::new(surface).with_edges(edges)
+        Cycle::new(surface, edges)
     })
 }
 

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

@@ -98,7 +98,7 @@ fn create_non_continuous_side_face(
             edges.push(edge);
         }
 
-        Cycle::new(surface).with_edges(edges)
+        Cycle::new(surface, edges)
     };
 
     Face::new(surface).with_exteriors([cycle]).with_color(color)
@@ -118,7 +118,7 @@ fn create_continuous_side_face(
     // won't start to matter at some point either.
     let placeholder = Surface::xy_plane();
 
-    let cycle = Cycle::new(placeholder).with_edges([edge]);
+    let cycle = Cycle::new(placeholder, [edge]);
     let approx = cycle.approx(tolerance, ());
 
     let mut quads = Vec::new();

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

@@ -46,8 +46,10 @@ impl TransformObject for Curve {
 
 impl TransformObject for Cycle {
     fn transform(self, transform: &Transform) -> Self {
-        Self::new(self.surface().transform(transform))
-            .with_edges(self.into_edges().map(|edge| edge.transform(transform)))
+        Self::new(
+            self.surface().transform(transform),
+            self.into_edges().map(|edge| edge.transform(transform)),
+        )
     }
 }
 

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

@@ -40,6 +40,6 @@ impl CycleBuilder {
             );
         }
 
-        Cycle::new(self.surface).with_edges(edges)
+        Cycle::new(self.surface, edges)
     }
 }

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

@@ -3,10 +3,6 @@ use crate::builder::CycleBuilder;
 use super::{Edge, Surface};
 
 /// A cycle of connected edges
-///
-/// The end of each edge in the cycle must connect to the beginning of the next
-/// edge. The end of the last edge must connect to the beginning of the first
-/// one.
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Cycle {
     surface: Surface,
@@ -20,19 +16,57 @@ impl Cycle {
     }
 
     /// Create a new cycle
-    pub fn new(surface: Surface) -> Self {
-        Self {
-            surface,
-            edges: Vec::new(),
+    ///
+    /// # Panics
+    ///
+    /// Panic, if the end of each edge does not connect to the beginning of the
+    /// next edge.
+    pub fn new(
+        surface: Surface,
+        edges: impl IntoIterator<Item = Edge>,
+    ) -> Self {
+        let edges = edges.into_iter().collect::<Vec<_>>();
+
+        if edges.len() != 1 {
+            // If the length is one, we might have a cycle made up of just one
+            // circle. If that isn't the case, we are dealing with line segments
+            // and can be sure that the following `get_or_panic` calls won't
+            // panic.
+
+            // Verify that all edges connect.
+            for edges in edges.windows(2) {
+                // Can't panic, as we passed `2` to `windows`.
+                //
+                // Can be cleaned up, once `array_windows` is stable"
+                // https://doc.rust-lang.org/std/primitive.slice.html#method.array_windows
+                let [a, b] = [&edges[0], &edges[1]];
+
+                let [_, prev] = a.vertices().get_or_panic();
+                let [next, _] = b.vertices().get_or_panic();
+
+                assert_eq!(
+                    prev.global(),
+                    next.global(),
+                    "Edges in cycle do not connect"
+                );
+            }
+
+            // Verify that the edges form a cycle
+            if let Some(first) = edges.first() {
+                if let Some(last) = edges.last() {
+                    let [first, _] = first.vertices().get_or_panic();
+                    let [_, last] = last.vertices().get_or_panic();
+
+                    assert_eq!(
+                        first.global(),
+                        last.global(),
+                        "Edges do not form a cycle"
+                    );
+                }
+            }
         }
-    }
 
-    /// Add edges to the cycle
-    ///
-    /// Consumes the cycle and returns the updated instance.
-    pub fn with_edges(mut self, edges: impl IntoIterator<Item = Edge>) -> Self {
-        self.edges.extend(edges);
-        self
+        Self { surface, edges }
     }
 
     /// Access the surface that this cycle is in

crates/fj-operations/src/sketch.rs

@@ -26,7 +26,7 @@ impl Shape for fj::Sketch {
 
                 let edge = Edge::build(surface)
                     .circle_from_radius(Scalar::from_f64(circle.radius()));
-                let cycle = Cycle::new(surface).with_edges([edge]);
+                let cycle = Cycle::new(surface, [edge]);
 
                 Face::new(surface)
                     .with_exteriors([cycle])