Merge pull request #1075 from hannobraun/validate

Improve validation of `HalfEdge` and `Vertex`

crates/fj-kernel/Cargo.toml

@@ -20,6 +20,7 @@ anymap = "1.0.0-beta.2"
 map-macro = "0.2.4"
 parking_lot = "0.12.0"
 parry2d-f64 = "0.9.0"
+pretty_assertions = "1.3.0"
 robust-predicates = "0.1.3"
 slotmap = "1.0.6"
 spade = "2.0.0"
@@ -36,4 +37,3 @@ path = "../fj-math"
 
 [dev-dependencies]
 anyhow = "1.0.64"
-pretty_assertions = "1.3.0"

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

@@ -1,4 +1,5 @@
 use fj_math::{Scalar, Winding};
+use pretty_assertions::assert_eq;
 
 use crate::builder::CycleBuilder;
 
@@ -40,11 +41,6 @@ impl Cycle {
         }
 
         if half_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 half_edges in half_edges.windows(2) {
                 // Can't panic, as we passed `2` to `windows`.

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

@@ -9,7 +9,7 @@ use super::{Curve, GlobalCurve, GlobalVertex, Surface, Vertex};
 pub struct HalfEdge {
     curve: Curve,
     vertices: [Vertex; 2],
-    global: GlobalEdge,
+    global_form: GlobalEdge,
 }
 
 impl HalfEdge {
@@ -26,26 +26,39 @@ impl HalfEdge {
     ///
     /// # Panics
     ///
+    /// Panics, if the provided `vertices` are not defined on the same curve as
+    /// `curve`.
+    ///
     /// Panics, if the provided [`GlobalEdge`] instance doesn't refer to the
     /// same [`GlobalCurve`] and [`GlobalVertex`] instances that the other
     /// objects that are passed refer to.
+    ///
+    /// Panics, if the provided vertices are coincident on the curve. If they
+    /// were, the edge would have no length, and thus not be valid. (It is
+    /// perfectly fine for global forms of the the vertices to be coincident.
+    /// That would just mean, that ends of the edge connect to each other.)
     pub fn new(
         curve: Curve,
         vertices: [Vertex; 2],
-        global: GlobalEdge,
+        global_form: GlobalEdge,
     ) -> Self {
-        assert_eq!(curve.global_form(), global.curve());
+        // Make sure `curve` and `vertices` match.
+        for vertex in vertices {
+            assert_eq!(
+                &curve,
+                vertex.curve(),
+                "An edge and its vertices must be defined on the same curve"
+            );
+        }
+
+        // Make sure `curve` and `vertices` match `global_form`.
+        assert_eq!(curve.global_form(), global_form.curve());
         assert_eq!(
             &vertices.map(|vertex| *vertex.global_form()),
-            global.vertices()
+            global_form.vertices()
         );
 
-        // Make sure that the edge vertices are not coincident on the curve. If
-        // they were, the edge would have no length, and not be valid.
-        //
-        // It is perfectly fine for global forms of the the vertices to be
-        // coincident (in 3D space). That would just mean, that ends of the edge
-        // connect to each other.
+        // Make sure that the edge vertices are not coincident on the curve.
         let [a, b] = vertices;
         assert_ne!(
             a.position(),
@@ -56,7 +69,7 @@ impl HalfEdge {
         Self {
             curve,
             vertices,
-            global,
+            global_form,
         }
     }
 
@@ -96,7 +109,7 @@ impl HalfEdge {
 
     /// Access the global form of this half-edge
     pub fn global_form(&self) -> &GlobalEdge {
-        &self.global
+        &self.global_form
     }
 }
 

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

@@ -1,4 +1,5 @@
 use fj_math::Point;
+use pretty_assertions::assert_eq;
 
 use super::{Curve, Surface};
 
@@ -17,13 +18,23 @@ pub struct Vertex {
 
 impl Vertex {
     /// Construct an instance of `Vertex`
+    ///
+    /// Panics, if `curve` and `surface_form` are not defined on the same
+    /// surface.
     pub fn new(
         position: impl Into<Point<1>>,
         curve: Curve,
         surface_form: SurfaceVertex,
         global_form: GlobalVertex,
     ) -> Self {
         let position = position.into();
+
+        assert_eq!(
+            curve.surface(),
+            surface_form.surface(),
+            "Surface form of vertex must be defined on same surface as curve",
+        );
+
         Self {
             position,
             curve,