Merge pull request #854 from hannobraun/edge

Clean up `Edge`

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

@@ -21,12 +21,12 @@ impl CycleApprox {
 
         for edge in &cycle.edges {
             let mut edge_points = Vec::new();
-            approx_curve(&edge.curve(), tolerance, &mut edge_points);
-            approx_edge(edge.vertices, &mut edge_points);
+            approx_curve(&edge.curve().global(), tolerance, &mut edge_points);
+            approx_edge(*edge.vertices(), &mut edge_points);
 
             points.extend(edge_points.into_iter().map(|point| {
                 let local =
-                    edge.curve.local().point_from_curve_coords(point.local());
+                    edge.curve().local().point_from_curve_coords(point.local());
                 Local::new(local, point.global())
             }));
         }

crates/fj-kernel/src/algorithms/intersection/curve_face.rs

@@ -40,15 +40,15 @@ impl CurveFaceIntersectionList {
                 edges
             })
             .map(|edge| {
-                let line = match edge.curve.local() {
+                let line = match edge.curve().local() {
                     Curve::Line(line) => line,
                     _ => {
                         todo!("Curve-face intersection only supports polygons")
                     }
                 };
 
-                let vertices = match edge.vertices() {
-                    Some(vertices) => vertices,
+                let vertices = match edge.vertices().get() {
+                    Some(vertices) => vertices.map(|&vertex| vertex),
                     None => todo!(
                         "Curve-face intersection does not support faces with \
                     continuous edges"

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

@@ -31,7 +31,7 @@ fn reverse_local_coordinates_in_cycle(
             .iter()
             .map(|edge| {
                 let curve = {
-                    let local = match edge.curve.local() {
+                    let local = match edge.curve().local() {
                         Curve::Circle(Circle { center, a, b }) => {
                             let center = Point::from([center.u, -center.v]);
 
@@ -49,13 +49,10 @@ fn reverse_local_coordinates_in_cycle(
                         }
                     };
 
-                    Local::new(local, edge.curve.global())
+                    Local::new(local, edge.curve().global())
                 };
 
-                Edge {
-                    curve,
-                    vertices: edge.vertices,
-                }
+                Edge::new(curve, *edge.vertices())
             })
             .collect();
 

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

@@ -40,7 +40,7 @@ pub fn sweep(
 
         for cycle in face.all_cycles() {
             for edge in cycle.edges {
-                if let Some(vertices) = edge.vertices() {
+                if let Some(vertices) = edge.vertices().get() {
                     create_non_continuous_side_face(
                         path,
                         is_sweep_along_negative_direction,
@@ -152,7 +152,7 @@ fn create_non_continuous_side_face(
                 Vertex::new(Point::from([1.]), b.1),
             ]);
 
-            let edge = Edge { curve, vertices };
+            let edge = Edge::new(curve, vertices);
 
             edges.push(edge);
         }

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

@@ -59,13 +59,14 @@ impl TransformObject for Cycle {
 impl TransformObject for Edge {
     fn transform(self, transform: &Transform) -> Self {
         let curve = Local::new(
-            self.curve.local(),
-            self.curve.global().transform(transform),
+            self.curve().local(),
+            self.curve().global().transform(transform),
         );
 
-        let vertices = self.vertices.map(|vertex| vertex.transform(transform));
+        let vertices =
+            self.vertices().map(|vertex| vertex.transform(transform));
 
-        Self { curve, vertices }
+        Self::new(curve, vertices)
     }
 }
 

crates/fj-kernel/src/iter.rs

@@ -165,19 +165,19 @@ impl ObjectIters for Cycle {
 
 impl ObjectIters for Edge {
     fn curve_iter(&self) -> Iter<Curve<3>> {
-        let mut iter = Iter::empty().with(self.curve().curve_iter());
+        let mut iter = Iter::empty().with(self.curve().global().curve_iter());
 
-        for vertex in self.vertices().into_iter().flatten() {
+        for vertex in self.vertices().iter() {
             iter = iter.with(vertex.curve_iter());
         }
 
         iter
     }
 
     fn cycle_iter(&self) -> Iter<Cycle> {
-        let mut iter = Iter::empty().with(self.curve().cycle_iter());
+        let mut iter = Iter::empty().with(self.curve().global().cycle_iter());
 
-        for vertex in self.vertices().into_iter().flatten() {
+        for vertex in self.vertices().iter() {
             iter = iter.with(vertex.cycle_iter());
         }
 
@@ -189,59 +189,60 @@ impl ObjectIters for Edge {
     }
 
     fn face_iter(&self) -> Iter<Face> {
-        let mut iter = Iter::empty().with(self.curve().face_iter());
+        let mut iter = Iter::empty().with(self.curve().global().face_iter());
 
-        for vertex in self.vertices().into_iter().flatten() {
+        for vertex in self.vertices().iter() {
             iter = iter.with(vertex.face_iter());
         }
 
         iter
     }
 
     fn global_vertex_iter(&self) -> Iter<GlobalVertex> {
-        let mut iter = Iter::empty().with(self.curve().global_vertex_iter());
+        let mut iter =
+            Iter::empty().with(self.curve().global().global_vertex_iter());
 
-        for vertex in self.vertices().into_iter().flatten() {
+        for vertex in self.vertices().iter() {
             iter = iter.with(vertex.global_vertex_iter());
         }
 
         iter
     }
 
     fn sketch_iter(&self) -> Iter<Sketch> {
-        let mut iter = Iter::empty().with(self.curve().sketch_iter());
+        let mut iter = Iter::empty().with(self.curve().global().sketch_iter());
 
-        for vertex in self.vertices().into_iter().flatten() {
+        for vertex in self.vertices().iter() {
             iter = iter.with(vertex.sketch_iter());
         }
 
         iter
     }
 
     fn solid_iter(&self) -> Iter<Solid> {
-        let mut iter = Iter::empty().with(self.curve().solid_iter());
+        let mut iter = Iter::empty().with(self.curve().global().solid_iter());
 
-        for vertex in self.vertices().into_iter().flatten() {
+        for vertex in self.vertices().iter() {
             iter = iter.with(vertex.solid_iter());
         }
 
         iter
     }
 
     fn surface_iter(&self) -> Iter<Surface> {
-        let mut iter = Iter::empty().with(self.curve().surface_iter());
+        let mut iter = Iter::empty().with(self.curve().global().surface_iter());
 
-        for vertex in self.vertices().into_iter().flatten() {
+        for vertex in self.vertices().iter() {
             iter = iter.with(vertex.surface_iter());
         }
 
         iter
     }
 
     fn vertex_iter(&self) -> Iter<Vertex> {
-        let mut iter = Iter::empty().with(self.curve().vertex_iter());
+        let mut iter = Iter::empty().with(self.curve().global().vertex_iter());
 
-        for vertex in self.vertices().into_iter().flatten() {
+        for vertex in self.vertices().iter() {
             iter = iter.with(vertex.vertex_iter());
         }
 

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

@@ -9,21 +9,16 @@ use super::{Curve, GlobalVertex, Surface, Vertex};
 /// An edge of a shape
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Edge {
-    /// Access the curve that defines the edge's geometry
-    ///
-    /// The edge can be a segment of the curve that is bounded by two vertices,
-    /// or if the curve is continuous (i.e. connects to itself), the edge could
-    /// be defined by the whole curve, and have no bounding vertices.
-    pub curve: Local<Curve<2>>,
-
-    /// Access the vertices that bound the edge on the curve
-    ///
-    /// If there are no such vertices, that means that both the curve and the
-    /// edge are continuous (i.e. connected to themselves).
-    pub vertices: VerticesOfEdge,
+    curve: Local<Curve<2>>,
+    vertices: VerticesOfEdge,
 }
 
 impl Edge {
+    /// Create a new instance
+    pub fn new(curve: Local<Curve<2>>, vertices: VerticesOfEdge) -> Self {
+        Self { curve, vertices }
+    }
+
     /// Create a circle from the given radius
     pub fn circle_from_radius(radius: Scalar) -> Self {
         let curve_local = Curve::Circle(Circle {
@@ -76,28 +71,36 @@ impl Edge {
         }
     }
 
-    /// Access this edge's curve
-    pub fn curve(&self) -> Curve<3> {
-        self.curve.global()
+    /// Access the curve that defines the edge's geometry
+    ///
+    /// The edge can be a segment of the curve that is bounded by two vertices,
+    /// or if the curve is continuous (i.e. connects to itself), the edge could
+    /// be defined by the whole curve, and have no bounding vertices.
+    pub fn curve(&self) -> &Local<Curve<2>> {
+        &self.curve
     }
 
-    /// Access this edge's vertices
-    pub fn vertices(&self) -> Option<[Vertex; 2]> {
-        self.vertices.0
+    /// Access the vertices that bound the edge on the curve
+    ///
+    /// An edge has either two bounding vertices or none. The latter is possible
+    /// if the edge's curve is continuous (i.e. connects to itself), and defines
+    /// the whole edge.
+    pub fn vertices(&self) -> &VerticesOfEdge {
+        &self.vertices
     }
 }
 
 impl fmt::Display for Edge {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self.vertices() {
+        match self.vertices().0 {
             Some(vertices) => {
                 let [a, b] = vertices.map(|vertex| vertex.position());
                 write!(f, "edge from {:?} to {:?}", a, b)?
             }
             None => write!(f, "continuous edge")?,
         }
 
-        write!(f, " on {}", self.curve())?;
+        write!(f, " on {}", self.curve().global())?;
 
         Ok(())
     }
@@ -136,6 +139,16 @@ impl VerticesOfEdge {
         false
     }
 
+    /// Access the vertices
+    pub fn get(&self) -> Option<[&Vertex; 2]> {
+        self.0.as_ref().map(|vertices| {
+            // Can be cleaned up once `each_ref` is stable:
+            // https://doc.rust-lang.org/std/primitive.array.html#method.each_ref
+            let [a, b] = vertices;
+            [a, b]
+        })
+    }
+
     /// Access the two vertices
     ///
     /// # Panics
@@ -145,7 +158,7 @@ impl VerticesOfEdge {
         self.0.expect("Expected edge to have vertices")
     }
 
-    /// Iterate over the vertices, if any
+    /// Iterate over the vertices
     pub fn iter(&self) -> impl Iterator<Item = &Vertex> {
         self.0.iter().flatten()
     }

crates/fj-kernel/src/validation/coherence.rs

@@ -16,9 +16,10 @@ pub fn validate_edge(
 
     let mut edge_vertex_mismatches = Vec::new();
 
-    for vertex in edge.vertices.iter() {
+    for vertex in edge.vertices().iter() {
         let local = vertex.position();
-        let local_as_global = edge.curve().point_from_curve_coords(local);
+        let local_as_global =
+            edge.curve().global().point_from_curve_coords(local);
         let global = vertex.global().position();
         let distance = (local_as_global - global).magnitude();
 

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

@@ -155,7 +155,7 @@ mod tests {
         let b = Vertex::new(Point::from([Scalar::ONE]), b);
         let vertices = VerticesOfEdge::from_vertices([a, b]);
 
-        let edge = Edge { curve, vertices };
+        let edge = Edge::new(curve, vertices);
 
         let result = validate(
             edge,

crates/fj-operations/src/difference_2d.rs

@@ -92,30 +92,25 @@ impl Shape for fj::Difference2d {
 fn add_cycle(cycle: Cycle, reverse: bool) -> Cycle {
     let mut edges = Vec::new();
     for edge in cycle.edges {
-        let curve_local = edge.curve.local();
         let curve_local = if reverse {
-            curve_local.reverse()
+            edge.curve().local().reverse()
         } else {
-            curve_local
+            edge.curve().local()
         };
 
-        let curve_canonical = edge.curve();
-        let curve_canonical = if reverse {
-            curve_canonical.reverse()
+        let curve_global = if reverse {
+            edge.curve().global().reverse()
         } else {
-            curve_canonical
+            edge.curve().global()
         };
 
         let vertices = if reverse {
-            edge.vertices.reverse()
+            edge.vertices().reverse()
         } else {
-            edge.vertices
+            *edge.vertices()
         };
 
-        let edge = Edge {
-            curve: Local::new(curve_local, curve_canonical),
-            vertices,
-        };
+        let edge = Edge::new(Local::new(curve_local, curve_global), vertices);
 
         edges.push(edge);
     }