Merge pull request #667 from hannobraun/edge

Add custom data type to represent edge vertices

crates/fj-kernel/src/algorithms/approx/edges.rs

@@ -1,9 +1,7 @@
-use fj_math::Point;
-
-use crate::{geometry, shape::LocalForm, topology::Vertex};
+use crate::{geometry, topology::VerticesOfEdge};
 
 pub fn approximate_edge(
-    vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>,
+    vertices: VerticesOfEdge,
     points: &mut Vec<geometry::Point<1, 3>>,
 ) {
     // Insert the exact vertices of this edge into the approximation. This means
@@ -14,13 +12,8 @@ pub fn approximate_edge(
     // would lead to bugs in the approximation, as points that should refer to
     // the same vertex would be understood to refer to very close, but distinct
     // vertices.
-    let vertices = vertices.map(|vertices| {
-        vertices.map(|vertex| {
-            geometry::Point::new(
-                *vertex.local(),
-                vertex.canonical().get().point(),
-            )
-        })
+    let vertices = vertices.convert(|vertex| {
+        geometry::Point::new(*vertex.local(), vertex.canonical().get().point())
     });
     if let Some([a, b]) = vertices {
         points.insert(0, a);
@@ -44,7 +37,7 @@ mod test {
     use crate::{
         geometry,
         shape::{LocalForm, Shape},
-        topology::Vertex,
+        topology::{Vertex, VerticesOfEdge},
     };
 
     #[test]
@@ -59,10 +52,10 @@ mod test {
         let v1 = Vertex::builder(&mut shape).build_from_point(a)?;
         let v2 = Vertex::builder(&mut shape).build_from_point(d)?;
 
-        let vertices = [
+        let vertices = VerticesOfEdge::from_vertices([
             LocalForm::new(Point::from([0.]), v1),
             LocalForm::new(Point::from([1.]), v2),
-        ];
+        ]);
 
         let a = geometry::Point::new([0.0], a);
         let b = geometry::Point::new([0.25], b);
@@ -71,12 +64,12 @@ mod test {
 
         // Regular edge
         let mut points = vec![b, c];
-        super::approximate_edge(Some(vertices), &mut points);
+        super::approximate_edge(vertices, &mut points);
         assert_eq!(points, vec![a, b, c, d]);
 
         // Continuous edge
         let mut points = vec![b, c];
-        super::approximate_edge(None, &mut points);
+        super::approximate_edge(VerticesOfEdge::none(), &mut points);
         assert_eq!(points, vec![b, c, b]);
 
         Ok(())

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

@@ -210,11 +210,8 @@ fn create_side_edges(
 ) -> Result<[Handle<Edge<3>>; 2], ValidationError> {
     // Can't panic. We already ruled out the "continuous edge" case above, so
     // these edges must have vertices.
-    let [vertices_bottom, vertices_top] = [edge_bottom, edge_top].map(|edge| {
-        edge.get()
-            .vertices
-            .expect("Expected vertices on non-continuous edge")
-    });
+    let [vertices_bottom, vertices_top] = [edge_bottom, edge_top]
+        .map(|edge| edge.get().vertices.expect_vertices());
 
     // Can be simplified, once `zip` is stabilized:
     // https://doc.rust-lang.org/std/primitive.array.html#method.zip

crates/fj-kernel/src/shape/api.rs

@@ -313,7 +313,7 @@ mod tests {
     use crate::{
         geometry::{Curve, Surface},
         shape::{Handle, LocalForm, Shape, ValidationError, ValidationResult},
-        topology::{Cycle, Edge, Face, Vertex},
+        topology::{Cycle, Edge, Face, Vertex, VerticesOfEdge},
     };
 
     const MIN_DISTANCE: f64 = 5e-7;
@@ -340,7 +340,7 @@ mod tests {
         assert!(shape.get_handle(&surface.get()).as_ref() == Some(&surface));
 
         let vertex = Vertex { point };
-        let edge = Edge::new(curve, None);
+        let edge = Edge::new(curve, VerticesOfEdge::none());
 
         assert!(shape.get_handle(&vertex).is_none());
         assert!(shape.get_handle(&edge).is_none());
@@ -429,7 +429,10 @@ mod tests {
 
         // Shouldn't work. Nothing has been added to `shape`.
         let err = shape
-            .insert(Edge::new(curve.clone(), Some([a.clone(), b.clone()])))
+            .insert(Edge::new(
+                curve.clone(),
+                VerticesOfEdge::from_vertices([a.clone(), b.clone()]),
+            ))
             .unwrap_err();
         assert!(err.missing_curve(&curve));
         assert!(err.missing_vertex(&a.canonical()));
@@ -443,7 +446,8 @@ mod tests {
         let b = LocalForm::new(Point::from([2.]), b);
 
         // Everything has been added to `shape` now. Should work!
-        shape.insert(Edge::new(curve, Some([a, b])))?;
+        shape
+            .insert(Edge::new(curve, VerticesOfEdge::from_vertices([a, b])))?;
 
         Ok(())
     }

crates/fj-kernel/src/shape/object.rs

@@ -2,7 +2,7 @@ use fj_math::Point;
 
 use crate::{
     geometry::{Curve, Surface},
-    topology::{Cycle, Edge, Face, Vertex},
+    topology::{Cycle, Edge, Face, Vertex, VerticesOfEdge},
 };
 
 use super::{
@@ -117,26 +117,22 @@ impl Object for Edge<3> {
             mapping,
         )?;
 
-        // Can be cleaned up using `try_map`, once that is stable:
-        // https://doc.rust-lang.org/std/primitive.array.html#method.try_map
-        let vertices: Option<[Result<_, ValidationError>; 2]> =
-            self.vertices.map(|vertices| {
-                vertices.map(|vertex| {
+        let vertices =
+            self.vertices
+                .try_convert::<_, _, ValidationError>(|vertex| {
                     let canonical = vertex.canonical();
                     let canonical = canonical.get().merge_into(
                         Some(canonical),
                         shape,
                         mapping,
                     )?;
                     Ok(LocalForm::new(*vertex.local(), canonical))
-                })
-            });
-        let vertices = match vertices {
-            Some([a, b]) => Some([a?, b?]),
-            None => None,
-        };
+                })?;
 
-        let merged = shape.get_handle_or_insert(Edge::new(curve, vertices))?;
+        let merged = shape.get_handle_or_insert(Edge::new(
+            curve,
+            VerticesOfEdge::new(vertices),
+        ))?;
 
         if let Some(handle) = handle {
             mapping.edges.insert(handle, merged.clone());

crates/fj-kernel/src/shape/validate/geometric.rs

@@ -13,32 +13,31 @@ pub fn validate_edge(
     // Validate that the local and canonical forms of the vertices match. As a
     // side effect, this also happens to validate that the canonical forms of
     // the vertices lie on the curve.
-    if let Some(vertices) = &edge.vertices {
-        let mut edge_vertex_mismatches = Vec::new();
-
-        for vertex in vertices {
-            let local = *vertex.local();
-            let local_3d = edge.curve().point_from_curve_coords(local);
-            let canonical = vertex.canonical().get().point();
-            let distance = (local_3d - canonical).magnitude();
-
-            if distance > max_distance {
-                edge_vertex_mismatches.push(EdgeVertexMismatch {
-                    local,
-                    local_3d,
-                    canonical,
-                    distance,
-                });
-            }
-        }
 
-        if !edge_vertex_mismatches.is_empty() {
-            return Err(GeometricIssues {
-                edge_vertex_mismatches,
+    let mut edge_vertex_mismatches = Vec::new();
+
+    for vertex in edge.vertices.iter() {
+        let local = *vertex.local();
+        let local_3d = edge.curve().point_from_curve_coords(local);
+        let canonical = vertex.canonical().get().point();
+        let distance = (local_3d - canonical).magnitude();
+
+        if distance > max_distance {
+            edge_vertex_mismatches.push(EdgeVertexMismatch {
+                local,
+                local_3d,
+                canonical,
+                distance,
             });
         }
     }
 
+    if !edge_vertex_mismatches.is_empty() {
+        return Err(GeometricIssues {
+            edge_vertex_mismatches,
+        });
+    }
+
     Ok(())
 }
 
@@ -116,13 +115,11 @@ mod tests {
             .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]])?
             .get();
         let edge = Edge {
-            vertices: edge.vertices.clone().map(|vertices| {
-                vertices.map(|vertex| {
-                    LocalForm::new(
-                        *vertex.local() + [deviation],
-                        vertex.canonical(),
-                    )
-                })
+            vertices: edge.vertices.map(|vertex| {
+                LocalForm::new(
+                    *vertex.local() + [deviation],
+                    vertex.canonical(),
+                )
             }),
             ..edge
         };

crates/fj-kernel/src/shape/validate/structural.rs

@@ -32,11 +32,9 @@ pub fn validate_edge(
     if !stores.curves.contains(&edge.curve.canonical()) {
         missing_curve = Some(edge.curve.canonical());
     }
-    for vertices in &edge.vertices {
-        for vertex in vertices {
-            if !stores.vertices.contains(&vertex.canonical()) {
-                missing_vertices.insert(vertex.canonical().clone());
-            }
+    for vertex in edge.vertices.iter() {
+        if !stores.vertices.contains(&vertex.canonical()) {
+            missing_vertices.insert(vertex.canonical().clone());
         }
     }
 

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

@@ -5,7 +5,7 @@ use crate::{
     shape::{Handle, LocalForm, Shape, ValidationResult},
 };
 
-use super::{Cycle, Edge, Face, Vertex};
+use super::{Cycle, Edge, Face, Vertex, VerticesOfEdge};
 
 /// API for building a [`Vertex`]
 #[must_use]
@@ -53,7 +53,9 @@ impl<'r> EdgeBuilder<'r> {
             a: Vector::from([radius, Scalar::ZERO, Scalar::ZERO]),
             b: Vector::from([Scalar::ZERO, radius, Scalar::ZERO]),
         }))?;
-        let edge = self.shape.insert(Edge::new(curve, None))?;
+        let edge = self
+            .shape
+            .insert(Edge::new(curve, VerticesOfEdge::none()))?;
 
         Ok(edge)
     }
@@ -93,7 +95,10 @@ impl<'r> EdgeBuilder<'r> {
             LocalForm::new(Point::from([1.]), b),
         ];
 
-        let edge = self.shape.insert(Edge::new(curve, Some(vertices)))?;
+        let edge = self.shape.insert(Edge::new(
+            curve,
+            VerticesOfEdge::from_vertices(vertices),
+        ))?;
 
         Ok(edge)
     }

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

@@ -34,15 +34,12 @@ pub struct Edge<const D: usize> {
     ///
     /// If there are no such vertices, that means that both the curve and the
     /// edge are continuous (i.e. connected to themselves).
-    pub vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>,
+    pub vertices: VerticesOfEdge,
 }
 
 impl Edge<3> {
     /// Construct an instance of `Edge`
-    pub fn new(
-        curve: Handle<Curve<3>>,
-        vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>,
-    ) -> Self {
+    pub fn new(curve: Handle<Curve<3>>, vertices: VerticesOfEdge) -> Self {
         let curve = LocalForm::canonical_only(curve);
         Self { curve, vertices }
     }
@@ -68,6 +65,7 @@ impl<const D: usize> Edge<D> {
     /// [`Handle`]s.
     pub fn vertices(&self) -> Option<[Vertex; 2]> {
         self.vertices
+            .0
             .as_ref()
             .map(|[a, b]| [a.canonical().get(), b.canonical().get()])
     }
@@ -88,3 +86,82 @@ impl<const D: usize> fmt::Display for Edge<D> {
         Ok(())
     }
 }
+
+/// The vertices that bound an edge
+#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
+pub struct VerticesOfEdge(Option<[LocalForm<Point<1>, Vertex>; 2]>);
+
+impl VerticesOfEdge {
+    /// Construct an instance of `VerticesOfEdge` from zero or two vertices
+    pub fn new(vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>) -> Self {
+        Self(vertices)
+    }
+
+    /// Construct an instance of `VerticesOfEdge` from two vertices
+    pub fn from_vertices(vertices: [LocalForm<Point<1>, Vertex>; 2]) -> Self {
+        Self(Some(vertices))
+    }
+
+    /// Construct an instance of `VerticesOfEdge` without vertices
+    pub fn none() -> Self {
+        Self(None)
+    }
+
+    /// Access the two vertices
+    ///
+    /// # Panics
+    ///
+    /// Panics, if the edge has no vertices.
+    pub fn expect_vertices(self) -> [LocalForm<Point<1>, Vertex>; 2] {
+        self.0.expect("Expected edge to have vertices")
+    }
+
+    /// Iterate over the vertices, if any
+    pub fn iter(&self) -> impl Iterator<Item = &LocalForm<Point<1>, Vertex>> {
+        self.0.iter().flatten()
+    }
+
+    /// Reverse the order of vertices
+    ///
+    /// Makes sure that the local coordinates are still correct.
+    pub fn reverse(self) -> Self {
+        Self(self.0.map(|[a, b]| {
+            [
+                LocalForm::new(-(*b.local()), b.canonical()),
+                LocalForm::new(-(*a.local()), a.canonical()),
+            ]
+        }))
+    }
+
+    /// Map each vertex using the provided function
+    pub fn map<F>(self, f: F) -> Self
+    where
+        F: FnMut(LocalForm<Point<1>, Vertex>) -> LocalForm<Point<1>, Vertex>,
+    {
+        Self(self.convert(f))
+    }
+
+    /// Convert each vertex using the provided function
+    pub fn convert<F, T>(self, f: F) -> Option<[T; 2]>
+    where
+        F: FnMut(LocalForm<Point<1>, Vertex>) -> T,
+    {
+        self.0.map(|vertices| vertices.map(f))
+    }
+
+    /// Convert each vertex using the provided fallible function
+    pub fn try_convert<F, T, E>(self, f: F) -> Result<Option<[T; 2]>, E>
+    where
+        F: FnMut(LocalForm<Point<1>, Vertex>) -> Result<T, E>,
+    {
+        // Can be cleaned up using `try_map`, once that is stable:
+        // https://doc.rust-lang.org/std/primitive.array.html#method.try_map
+        let vertices: Option<[Result<_, E>; 2]> = self.convert(f);
+        let vertices = match vertices {
+            Some([a, b]) => Some([a?, b?]),
+            None => None,
+        };
+
+        Ok(vertices)
+    }
+}

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

@@ -25,7 +25,7 @@ mod vertex;
 pub use self::{
     builder::{CycleBuilder, EdgeBuilder, FaceBuilder, VertexBuilder},
     cycle::Cycle,
-    edge::Edge,
+    edge::{Edge, VerticesOfEdge},
     face::{CyclesInFace, Face},
     vertex::Vertex,
 };