Merge pull request #759 from hannobraun/edge

Unify `Edge<2>` and `Edge<3>`

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

@@ -168,7 +168,7 @@ fn create_non_continuous_side_face(
 }
 
 fn create_continuous_side_face(
-    edge: Edge<2>,
+    edge: Edge,
     path: Vector<3>,
     tolerance: Tolerance,
     color: [u8; 4],

crates/fj-kernel/src/iter.rs

@@ -16,7 +16,7 @@ pub trait ObjectIters {
     fn cycle_iter(&self) -> Iter<Cycle>;
 
     /// Iterate over all edges
-    fn edge_iter(&self) -> Iter<Edge<3>>;
+    fn edge_iter(&self) -> Iter<Edge>;
 
     /// Iterate over all faces
     fn face_iter(&self) -> Iter<Face>;
@@ -40,7 +40,7 @@ impl ObjectIters for Curve<3> {
         Iter::empty()
     }
 
-    fn edge_iter(&self) -> Iter<Edge<3>> {
+    fn edge_iter(&self) -> Iter<Edge> {
         Iter::empty()
     }
 
@@ -66,7 +66,7 @@ impl ObjectIters for Cycle {
         let mut iter = Iter::empty();
 
         for edge in self.edges() {
-            iter = iter.with(edge.to_canonical().curve_iter());
+            iter = iter.with(edge.curve_iter());
         }
 
         iter
@@ -76,11 +76,11 @@ impl ObjectIters for Cycle {
         Iter::from_object(self.clone())
     }
 
-    fn edge_iter(&self) -> Iter<Edge<3>> {
+    fn edge_iter(&self) -> Iter<Edge> {
         let mut iter = Iter::empty();
 
         for edge in self.edges() {
-            iter = iter.with(edge.to_canonical().edge_iter());
+            iter = iter.with(edge.edge_iter());
         }
 
         iter
@@ -90,7 +90,7 @@ impl ObjectIters for Cycle {
         let mut iter = Iter::empty();
 
         for edge in self.edges() {
-            iter = iter.with(edge.to_canonical().face_iter());
+            iter = iter.with(edge.face_iter());
         }
 
         iter
@@ -100,7 +100,7 @@ impl ObjectIters for Cycle {
         let mut iter = Iter::empty();
 
         for edge in self.edges() {
-            iter = iter.with(edge.to_canonical().global_vertex_iter());
+            iter = iter.with(edge.global_vertex_iter());
         }
 
         iter
@@ -110,7 +110,7 @@ impl ObjectIters for Cycle {
         let mut iter = Iter::empty();
 
         for edge in self.edges() {
-            iter = iter.with(edge.to_canonical().surface_iter());
+            iter = iter.with(edge.surface_iter());
         }
 
         iter
@@ -120,14 +120,14 @@ impl ObjectIters for Cycle {
         let mut iter = Iter::empty();
 
         for edge in self.edges() {
-            iter = iter.with(edge.to_canonical().vertex_iter());
+            iter = iter.with(edge.vertex_iter());
         }
 
         iter
     }
 }
 
-impl ObjectIters for Edge<3> {
+impl ObjectIters for Edge {
     fn curve_iter(&self) -> Iter<Curve<3>> {
         let mut iter = Iter::empty().with(self.curve().curve_iter());
 
@@ -148,7 +148,7 @@ impl ObjectIters for Edge<3> {
         iter
     }
 
-    fn edge_iter(&self) -> Iter<Edge<3>> {
+    fn edge_iter(&self) -> Iter<Edge> {
         Iter::from_object(self.clone())
     }
 
@@ -222,7 +222,7 @@ impl ObjectIters for Face {
         Iter::empty()
     }
 
-    fn edge_iter(&self) -> Iter<Edge<3>> {
+    fn edge_iter(&self) -> Iter<Edge> {
         if let Face::Face(face) = self {
             let mut iter = Iter::empty().with(face.surface().edge_iter());
 
@@ -293,7 +293,7 @@ impl ObjectIters for GlobalVertex {
         Iter::empty()
     }
 
-    fn edge_iter(&self) -> Iter<Edge<3>> {
+    fn edge_iter(&self) -> Iter<Edge> {
         Iter::empty()
     }
 
@@ -323,7 +323,7 @@ impl ObjectIters for Surface {
         Iter::empty()
     }
 
-    fn edge_iter(&self) -> Iter<Edge<3>> {
+    fn edge_iter(&self) -> Iter<Edge> {
         Iter::empty()
     }
 
@@ -353,7 +353,7 @@ impl ObjectIters for Vertex {
         Iter::empty()
     }
 
-    fn edge_iter(&self) -> Iter<Edge<3>> {
+    fn edge_iter(&self) -> Iter<Edge> {
         Iter::empty()
     }
 
@@ -404,7 +404,7 @@ where
         iter
     }
 
-    fn edge_iter(&self) -> Iter<Edge<3>> {
+    fn edge_iter(&self) -> Iter<Edge> {
         let mut iter = Iter::empty();
 
         for object in self.into_iter() {
@@ -532,7 +532,10 @@ mod tests {
 
     #[test]
     fn edge() {
-        let edge = Edge::line_segment_from_points([[0., 0., 0.], [1., 0., 0.]]);
+        let edge = Edge::line_segment_from_points(
+            &Surface::xy_plane(),
+            [[0., 0.], [1., 0.]],
+        );
 
         assert_eq!(1, edge.curve_iter().count());
         assert_eq!(0, edge.cycle_iter().count());

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

@@ -12,7 +12,7 @@ use super::{Curve, Edge, Surface};
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Cycle {
     /// The edges that make up the cycle
-    pub edges: Vec<Edge<2>>,
+    pub edges: Vec<Edge>,
 }
 
 impl Cycle {
@@ -36,10 +36,8 @@ impl Cycle {
             // Can be cleaned up, once `array_windows` is stable.
             let points = [points[0], points[1]];
 
-            let points_canonical =
-                points.map(|point| surface.point_from_surface_coords(point));
             let edge_canonical =
-                Edge::line_segment_from_points(points_canonical);
+                Edge::line_segment_from_points(surface, points);
 
             let edge_local = Edge {
                 curve: LocalForm::new(
@@ -59,7 +57,7 @@ impl Cycle {
     ///
     /// This is a convenience method that saves the caller from dealing with the
     /// [`Handle`]s.
-    pub fn edges(&self) -> impl Iterator<Item = Edge<2>> + '_ {
+    pub fn edges(&self) -> impl Iterator<Item = Edge> + '_ {
         self.edges.iter().cloned()
     }
 }

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

@@ -4,17 +4,17 @@ use fj_math::{Circle, Line, Point, Scalar, Vector};
 
 use crate::shape::LocalForm;
 
-use super::{Curve, GlobalVertex, Vertex};
+use super::{Curve, GlobalVertex, Surface, Vertex};
 
 /// An edge of a shape
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub struct Edge<const D: usize> {
+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: LocalForm<Curve<D>, Curve<3>>,
+    pub curve: LocalForm<Curve<2>, Curve<3>>,
 
     /// Access the vertices that bound the edge on the curve
     ///
@@ -23,27 +23,9 @@ pub struct Edge<const D: usize> {
     pub vertices: VerticesOfEdge,
 }
 
-impl<const D: usize> Edge<D> {
-    /// Access the curve that the edge refers to
-    ///
-    /// This is a convenience method that saves the caller from dealing with the
-    /// [`Handle`].
-    pub fn curve(&self) -> Curve<3> {
-        *self.curve.canonical()
-    }
-
-    /// Access the vertices that the edge refers to
-    ///
-    /// This is a convenience method that saves the caller from dealing with the
-    /// [`Handle`]s.
-    pub fn vertices(&self) -> Option<[Vertex; 2]> {
-        self.vertices.0
-    }
-}
-
-impl Edge<2> {
+impl Edge {
     /// Create a circle from the given radius
-    pub fn circle_from_radius(radius: Scalar) -> Edge<2> {
+    pub fn circle_from_radius(radius: Scalar) -> Self {
         let curve_local = Curve::Circle(Circle {
             center: Point::origin(),
             a: Vector::from([radius, Scalar::ZERO]),
@@ -61,50 +43,57 @@ impl Edge<2> {
         }
     }
 
-    /// Temporary utility method to aid refactoring
-    pub fn to_canonical(&self) -> Edge<3> {
-        let curve = *self.curve.canonical();
-        let curve = LocalForm::canonical_only(curve);
-
-        let vertices = self.vertices.clone();
-
-        Edge { curve, vertices }
-    }
-}
-
-impl Edge<3> {
     /// Create a line segment from two points
     pub fn line_segment_from_points(
-        vertices: [impl Into<Point<3>>; 2],
+        surface: &Surface,
+        points: [impl Into<Point<2>>; 2],
     ) -> Self {
-        let vertices = vertices.map(|position| {
-            let position = position.into();
+        let points = points.map(Into::into);
+
+        let global_vertices = points.map(|position| {
+            let position = surface.point_from_surface_coords(position);
             GlobalVertex::from_position(position)
         });
 
-        Self::line_segment_from_vertices(vertices)
-    }
-
-    /// Create a line segment from two vertices
-    pub fn line_segment_from_vertices([a, b]: [GlobalVertex; 2]) -> Self {
-        let curve = {
-            let points = [a, b].map(|vertex| vertex.position());
+        let curve_local = Curve::Line(Line::from_points(points));
+        let curve_canonical = {
+            let points =
+                global_vertices.map(|global_vertex| global_vertex.position());
             Curve::Line(Line::from_points(points))
         };
 
-        let vertices = [
-            Vertex::new(Point::from([0.]), a),
-            Vertex::new(Point::from([1.]), b),
-        ];
+        let vertices = {
+            let [a, b] = global_vertices;
+            [
+                Vertex::new(Point::from([0.]), a),
+                Vertex::new(Point::from([1.]), b),
+            ]
+        };
 
         Self {
-            curve: LocalForm::canonical_only(curve),
+            curve: LocalForm::new(curve_local, curve_canonical),
             vertices: VerticesOfEdge::from_vertices(vertices),
         }
     }
+
+    /// Access the curve that the edge refers to
+    ///
+    /// This is a convenience method that saves the caller from dealing with the
+    /// [`Handle`].
+    pub fn curve(&self) -> Curve<3> {
+        *self.curve.canonical()
+    }
+
+    /// Access the vertices that the edge refers to
+    ///
+    /// This is a convenience method that saves the caller from dealing with the
+    /// [`Handle`]s.
+    pub fn vertices(&self) -> Option<[Vertex; 2]> {
+        self.vertices.0
+    }
 }
 
-impl<const D: usize> fmt::Display for Edge<D> {
+impl fmt::Display for Edge {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self.vertices() {
             Some(vertices) => {

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

@@ -5,7 +5,7 @@ use fj_math::{Point, Scalar};
 use crate::objects::Edge;
 
 pub fn validate_edge(
-    edge: &Edge<3>,
+    edge: &Edge,
     max_distance: impl Into<Scalar>,
 ) -> Result<(), CoherenceIssues> {
     let max_distance = max_distance.into();

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

@@ -152,8 +152,9 @@ mod tests {
         let b = Point::from([1., 0., 0.]);
 
         let curve = {
-            let curve = Curve::line_from_points([a, b]);
-            LocalForm::canonical_only(curve)
+            let curve_local = Curve::line_from_points([[0., 0.], [1., 0.]]);
+            let curve_canonical = Curve::line_from_points([a, b]);
+            LocalForm::new(curve_local, curve_canonical)
         };
 
         let a = GlobalVertex::from_position(a);