Merge pull request #2066 from hannobraun/approx

Make some cleanups in approximation code

crates/fj-core/src/algorithms/approx/cycle.rs

@@ -9,7 +9,7 @@ use fj_math::Segment;
 use crate::objects::{Cycle, Surface};
 
 use super::{
-    edge::{EdgeApprox, EdgeApproxCache},
+    edge::{EdgeApproxCache, HalfEdgeApprox},
     Approx, ApproxPoint, Tolerance,
 };
 
@@ -25,32 +25,32 @@ impl Approx for (&Cycle, &Surface) {
         let (cycle, surface) = self;
         let tolerance = tolerance.into();
 
-        let edges = cycle
+        let half_edges = cycle
             .half_edges()
             .iter()
             .map(|edge| {
                 (edge.deref(), surface).approx_with_cache(tolerance, cache)
             })
             .collect();
 
-        CycleApprox { edges }
+        CycleApprox { half_edges }
     }
 }
 
 /// An approximation of a [`Cycle`]
 #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct CycleApprox {
-    /// The approximated edges that make up the approximated cycle
-    pub edges: Vec<EdgeApprox>,
+    /// The approximated half-edges that make up the approximated cycle
+    pub half_edges: Vec<HalfEdgeApprox>,
 }
 
 impl CycleApprox {
     /// Compute the points that approximate the cycle
     pub fn points(&self) -> Vec<ApproxPoint<2>> {
         let mut points = Vec::new();
 
-        for approx in &self.edges {
-            points.extend(approx.points());
+        for approx in &self.half_edges {
+            points.extend(approx.points.iter().copied());
         }
 
         if let Some(point) = points.first() {

crates/fj-core/src/algorithms/approx/edge.rs

@@ -23,7 +23,7 @@ use super::{
 };
 
 impl Approx for (&HalfEdge, &Surface) {
-    type Approximation = EdgeApprox;
+    type Approximation = HalfEdgeApprox;
     type Cache = EdgeApproxCache;
 
     fn approx_with_cache(
@@ -92,43 +92,26 @@ impl Approx for (&HalfEdge, &Surface) {
                 // immediately, and it will be there on the second iteration.
             };
 
-            segment
-                .points
-                .inner
-                .into_iter()
-                .map(|point| {
-                    let point_surface =
-                        edge.path().point_from_path_coords(point.local_form);
+            segment.points.inner.into_iter().map(|point| {
+                let point_surface =
+                    edge.path().point_from_path_coords(point.local_form);
 
-                    ApproxPoint::new(point_surface, point.global_form)
-                })
-                .collect()
+                ApproxPoint::new(point_surface, point.global_form)
+            })
         };
 
-        EdgeApprox { first, rest }
+        let mut points = vec![first];
+        points.extend(rest);
+
+        HalfEdgeApprox { points }
     }
 }
 
 /// An approximation of a [`HalfEdge`]
 #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub struct EdgeApprox {
-    /// The point that approximates the first vertex of the edge
-    first: ApproxPoint<2>,
-
-    /// The approximation of the rest of the edge
-    rest: Vec<ApproxPoint<2>>,
-}
-
-impl EdgeApprox {
-    /// Compute the points that approximate the edge
-    pub fn points(&self) -> Vec<ApproxPoint<2>> {
-        let mut points = Vec::new();
-
-        points.push(self.first);
-        points.extend(self.rest.iter().cloned());
-
-        points
-    }
+pub struct HalfEdgeApprox {
+    /// The points that approximate the half-edge
+    pub points: Vec<ApproxPoint<2>>,
 }
 
 fn approx_curve(
@@ -279,7 +262,11 @@ mod tests {
         let tolerance = 1.;
         let approx = (&edge, surface.deref()).approx(tolerance);
 
-        assert_eq!(approx.rest, Vec::new());
+        let expected_approx = vec![{
+            let point_surface = edge.start_position();
+            ApproxPoint::from_surface_point(point_surface, &surface)
+        }];
+        assert_eq!(approx.points, expected_approx);
     }
 
     #[test]
@@ -296,7 +283,11 @@ mod tests {
         let tolerance = 1.;
         let approx = (&edge, &surface).approx(tolerance);
 
-        assert_eq!(approx.rest, Vec::new());
+        let expected_approx = vec![{
+            let point_surface = edge.start_position();
+            ApproxPoint::from_surface_point(point_surface, &surface)
+        }];
+        assert_eq!(approx.points, expected_approx);
     }
 
     #[test]
@@ -319,18 +310,20 @@ mod tests {
         let tolerance = 1.;
         let approx = (&edge, &surface).approx(tolerance);
 
-        let expected_approx = (path, boundary)
-            .approx(tolerance)
-            .into_iter()
-            .map(|(point_local, _)| {
-                let point_surface =
-                    edge.path().point_from_path_coords(point_local);
-                let point_global =
-                    surface.geometry().point_from_surface_coords(point_surface);
-                ApproxPoint::new(point_surface, point_global)
-            })
-            .collect::<Vec<_>>();
-        assert_eq!(approx.rest, expected_approx);
+        let mut expected_approx = vec![{
+            let point_surface = edge.start_position();
+            ApproxPoint::from_surface_point(point_surface, &surface)
+        }];
+        expected_approx.extend(
+            (path, boundary).approx(tolerance).into_iter().map(
+                |(point_local, _)| {
+                    let point_surface =
+                        edge.path().point_from_path_coords(point_local);
+                    ApproxPoint::from_surface_point(point_surface, &surface)
+                },
+            ),
+        );
+        assert_eq!(approx.points, expected_approx);
     }
 
     #[test]
@@ -343,17 +336,18 @@ mod tests {
         let tolerance = 1.;
         let approx = (&edge, surface.deref()).approx(tolerance);
 
-        let expected_approx =
+        let mut expected_approx = vec![{
+            let point_surface = edge.start_position();
+            ApproxPoint::from_surface_point(point_surface, &surface)
+        }];
+        expected_approx.extend(
             (&edge.path(), CurveBoundary::from([[0.], [TAU]]))
                 .approx(tolerance)
                 .into_iter()
                 .map(|(_, point_surface)| {
-                    let point_global = surface
-                        .geometry()
-                        .point_from_surface_coords(point_surface);
-                    ApproxPoint::new(point_surface, point_global)
-                })
-                .collect::<Vec<_>>();
-        assert_eq!(approx.rest, expected_approx);
+                    ApproxPoint::from_surface_point(point_surface, &surface)
+                }),
+        );
+        assert_eq!(approx.points, expected_approx);
     }
 }

crates/fj-core/src/algorithms/approx/mod.rs

@@ -18,6 +18,8 @@ use std::{
 
 use fj_math::Point;
 
+use crate::objects::Surface;
+
 pub use self::tolerance::{InvalidTolerance, Tolerance};
 
 /// Approximate an object
@@ -59,7 +61,7 @@ pub struct ApproxPoint<const D: usize> {
 }
 
 impl<const D: usize> ApproxPoint<D> {
-    /// Create an instance of `ApproxPoint`, without a source
+    /// Create an instance of `ApproxPoint`
     pub fn new(
         local_form: impl Into<Point<D>>,
         global_form: impl Into<Point<3>>,
@@ -71,6 +73,19 @@ impl<const D: usize> ApproxPoint<D> {
     }
 }
 
+impl ApproxPoint<2> {
+    /// Create an instance of `ApproxPoint` from a surface point
+    pub fn from_surface_point(
+        point_surface: impl Into<Point<2>>,
+        surface: &Surface,
+    ) -> Self {
+        let point_surface = point_surface.into();
+        let point_global =
+            surface.geometry().point_from_surface_coords(point_surface);
+        ApproxPoint::new(point_surface, point_global)
+    }
+}
+
 impl<const D: usize> Eq for ApproxPoint<D> {}
 
 impl<const D: usize> PartialEq for ApproxPoint<D> {