Merge pull request #1049 from hannobraun/approx

Clean up and document approximation code

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

@@ -1,45 +1,56 @@
+//! Curve approximation
+//!
+//! Since curves are infinite (even circles have an infinite coordinate space,
+//! even though they connect to themselves in global coordinates), a range must
+//! be provided to approximate them. The approximation then returns points
+//! within that range.
+//!
+//! The boundaries of the range are not included in the approximation. This is
+//! done, to give the caller (who knows the boundary anyway) more options on how
+//! to further process the approximation.
+
 use std::cmp::max;
 
 use fj_math::{Circle, Point, Scalar};
 
-use crate::objects::{Curve, CurveKind, GlobalCurve};
+use crate::objects::{Curve, CurveKind, GlobalCurve, Vertex};
 
 use super::{Approx, Tolerance};
 
-impl Approx for Curve {
+impl Approx for (&Curve, RangeOnCurve) {
     type Approximation = Vec<(Point<2>, Point<3>)>;
-    type Params = RangeOnCurve;
-
-    fn approx(
-        &self,
-        tolerance: Tolerance,
-        range: Self::Params,
-    ) -> Self::Approximation {
-        self.global_form()
-            .approx(tolerance, range)
+
+    fn approx(self, tolerance: Tolerance) -> Self::Approximation {
+        let (curve, range) = self;
+
+        (curve.global_form(), range)
+            .approx(tolerance)
             .into_iter()
             .map(|(point_curve, point_global)| {
                 let point_surface =
-                    self.kind().point_from_curve_coords(point_curve);
+                    curve.kind().point_from_curve_coords(point_curve);
                 (point_surface, point_global)
             })
             .collect()
     }
 }
 
-impl Approx for GlobalCurve {
+impl Approx for (&GlobalCurve, RangeOnCurve) {
     type Approximation = Vec<(Point<1>, Point<3>)>;
-    type Params = RangeOnCurve;
-
-    fn approx(
-        &self,
-        tolerance: Tolerance,
-        range: Self::Params,
-    ) -> Self::Approximation {
-        match self.kind() {
-            CurveKind::Circle(curve) => approx_circle(curve, range, tolerance),
-            CurveKind::Line(_) => vec![range.start()],
+
+    fn approx(self, tolerance: Tolerance) -> Self::Approximation {
+        let (curve, range) = self;
+
+        let mut points = Vec::new();
+
+        match curve.kind() {
+            CurveKind::Circle(curve) => {
+                approx_circle(curve, range, tolerance, &mut points);
+            }
+            CurveKind::Line(_) => {}
         }
+
+        points
     }
 }
 
@@ -51,7 +62,8 @@ fn approx_circle(
     circle: &Circle<3>,
     range: impl Into<RangeOnCurve>,
     tolerance: Tolerance,
-) -> Vec<(Point<1>, Point<3>)> {
+    points: &mut Vec<(Point<1>, Point<3>)>,
+) {
     let radius = circle.a().magnitude();
     let range = range.into();
 
@@ -63,20 +75,15 @@ fn approx_circle(
 
     let n = number_of_vertices_for_circle(tolerance, radius, range.length());
 
-    let mut points = Vec::new();
-    points.push(range.start());
-
     for i in 1..n {
-        let angle = range.start().0.t
+        let angle = range.start().position().t
             + (Scalar::TAU / n as f64 * i as f64) * range.direction();
 
         let point_curve = Point::from([angle]);
         let point_global = circle.point_from_circle_coords(point_curve);
 
         points.push((point_curve, point_global));
     }
-
-    points
 }
 
 fn number_of_vertices_for_circle(
@@ -91,28 +98,41 @@ fn number_of_vertices_for_circle(
     max(n, 3)
 }
 
+/// The range on which a curve should be approximated
+#[derive(Clone, Copy)]
 pub struct RangeOnCurve {
-    pub boundary: [(Point<1>, Point<3>); 2],
+    /// The boundary of the range
+    ///
+    /// The vertices that make up the boundary are themselves not included in
+    /// the approximation.
+    pub boundary: [Vertex; 2],
 }
 
 impl RangeOnCurve {
-    fn start(&self) -> (Point<1>, Point<3>) {
+    /// Access the start of the range
+    pub fn start(&self) -> Vertex {
         self.boundary[0]
     }
 
-    fn end(&self) -> (Point<1>, Point<3>) {
+    /// Access the end of the range
+    pub fn end(&self) -> Vertex {
         self.boundary[1]
     }
 
-    fn signed_length(&self) -> Scalar {
-        (self.end().0 - self.start().0).t
+    /// Compute the signed length of the range
+    pub fn signed_length(&self) -> Scalar {
+        (self.end().position() - self.start().position()).t
     }
 
-    fn length(&self) -> Scalar {
+    /// Compute the absolute length of the range
+    pub fn length(&self) -> Scalar {
         self.signed_length().abs()
     }
 
-    fn direction(&self) -> Scalar {
+    /// Compute the direction of the range
+    ///
+    /// Returns a [`Scalar`] that is zero or +/- one.
+    pub fn direction(&self) -> Scalar {
         self.signed_length().sign()
     }
 }

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

@@ -1,22 +1,21 @@
+//! Cycle approximation
+//!
+//! See [`CycleApprox`].
+
 use fj_math::{Point, Segment};
 
 use crate::objects::Cycle;
 
 use super::{Approx, Tolerance};
 
-impl Approx for Cycle {
+impl Approx for &Cycle {
     type Approximation = CycleApprox;
-    type Params = ();
 
-    fn approx(
-        &self,
-        tolerance: Tolerance,
-        (): Self::Params,
-    ) -> Self::Approximation {
+    fn approx(self, tolerance: Tolerance) -> Self::Approximation {
         let mut points = Vec::new();
 
         for edge in self.edges() {
-            let edge_points = edge.approx(tolerance, ());
+            let edge_points = edge.approx(tolerance);
             points.extend(edge_points);
         }
 

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

@@ -1,41 +1,85 @@
+//! Edge approximation
+//!
+//! The approximation of a curve is its first vertex, combined with the
+//! approximation of its curve. The second vertex is left off, as edge
+//! approximations are usually used to build cycle approximations, and this way,
+//! the caller doesn't have to call with duplicate vertices.
+
 use fj_math::{Point, Scalar};
 
-use crate::objects::Edge;
+use crate::objects::{Edge, GlobalVertex, SurfaceVertex, Vertex};
 
 use super::{curve::RangeOnCurve, Approx};
 
-impl Approx for Edge {
+impl Approx for &Edge {
     type Approximation = Vec<(Point<2>, Point<3>)>;
-    type Params = ();
 
-    fn approx(
-        &self,
-        tolerance: super::Tolerance,
-        (): Self::Params,
-    ) -> Self::Approximation {
+    fn approx(self, tolerance: super::Tolerance) -> Self::Approximation {
         // The range is only used for circles right now.
         let boundary = match self.vertices().get() {
-            Some(vertices) => vertices.map(|vertex| {
-                (vertex.position(), vertex.global_form().position())
-            }),
+            Some(vertices) => vertices.map(|&vertex| vertex),
             None => {
+                // Creating vertices from nothing, just for the sake of
+                // approximation is a bit weird. But this code is a temporary
+                // fallback anyway. It'll do for now, and it will likely be
+                // removed soon.
+
                 let start_curve = Point::from([Scalar::ZERO]);
                 let end_curve = Point::from([Scalar::TAU]);
 
                 // We're dealing with a circle here. Start and end are identical
                 // points, in global coordinates.
-                let point_global = self
-                    .global_form()
-                    .curve()
-                    .kind()
-                    .point_from_curve_coords(start_curve);
+                let vertex_global = {
+                    let point_global = self
+                        .global_form()
+                        .curve()
+                        .kind()
+                        .point_from_curve_coords(start_curve);
+
+                    GlobalVertex::from_position(point_global)
+                };
 
-                [(start_curve, point_global), (end_curve, point_global)]
+                let [start_surface, end_surface] = [start_curve, end_curve]
+                    .map(|point_curve| {
+                        let point_surface = self
+                            .curve()
+                            .kind()
+                            .point_from_curve_coords(point_curve);
+                        SurfaceVertex::new(
+                            point_surface,
+                            *self.curve().surface(),
+                            vertex_global,
+                        )
+                    });
+
+                let a = Vertex::new(
+                    start_curve,
+                    *self.curve(),
+                    start_surface,
+                    vertex_global,
+                );
+                let b = Vertex::new(
+                    end_curve,
+                    *self.curve(),
+                    end_surface,
+                    vertex_global,
+                );
+
+                [a, b]
             }
         };
 
         let range = RangeOnCurve { boundary };
 
-        self.curve().approx(tolerance, range)
+        let mut points = (self.curve(), range).approx(tolerance);
+        points.insert(
+            0,
+            (
+                range.start().surface_form().position(),
+                range.start().global_form().position(),
+            ),
+        );
+
+        points
     }
 }

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

@@ -1,20 +1,19 @@
+//! Face approximation
+//!
+//! See [`FaceApprox`].
+
 use std::collections::HashSet;
 
 use fj_math::Point;
 
 use crate::objects::Face;
 
-use super::{Approx, CycleApprox, Tolerance};
+use super::{cycle::CycleApprox, Approx, Tolerance};
 
-impl Approx for Face {
+impl Approx for &Face {
     type Approximation = FaceApprox;
-    type Params = ();
 
-    fn approx(
-        &self,
-        tolerance: Tolerance,
-        (): Self::Params,
-    ) -> Self::Approximation {
+    fn approx(self, tolerance: Tolerance) -> Self::Approximation {
         // Curved faces whose curvature is not fully defined by their edges
         // are not supported yet. For that reason, we can fully ignore `face`'s
         // `surface` field and just pass the edges to `Self::for_edges`.
@@ -33,13 +32,13 @@ impl Approx for Face {
         let mut interiors = HashSet::new();
 
         for cycle in self.exteriors() {
-            let cycle = cycle.approx(tolerance, ());
+            let cycle = cycle.approx(tolerance);
 
             points.extend(cycle.points.iter().copied());
             exteriors.push(cycle);
         }
         for cycle in self.interiors() {
-            let cycle = cycle.approx(tolerance, ());
+            let cycle = cycle.approx(tolerance);
 
             points.extend(cycle.points.iter().copied());
             interiors.insert(cycle);
@@ -123,7 +122,7 @@ mod tests {
         let g = (g, g.to_xyz());
         let h = (h, h.to_xyz());
 
-        let approx = face.approx(tolerance, ());
+        let approx = face.approx(tolerance);
         let expected = FaceApprox {
             points: set![a, b, c, d, e, f, g, h],
             exterior: CycleApprox {

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

@@ -1,32 +1,21 @@
 //! Approximation of objects
 
-mod curve;
-mod cycle;
-mod edge;
-mod face;
-mod tolerance;
+pub mod curve;
+pub mod cycle;
+pub mod edge;
+pub mod face;
+pub mod tolerance;
 
-pub use self::{
-    cycle::CycleApprox,
-    face::FaceApprox,
-    tolerance::{InvalidTolerance, Tolerance},
-};
+pub use self::tolerance::{InvalidTolerance, Tolerance};
 
 /// Approximate an object
 pub trait Approx {
     /// The approximation of the object
     type Approximation;
 
-    /// Additional parameters required for the approximation
-    type Params;
-
     /// Approximate the object
     ///
     /// `tolerance` defines how far the approximation is allowed to deviate from
     /// the actual object.
-    fn approx(
-        &self,
-        tolerance: Tolerance,
-        params: Self::Params,
-    ) -> Self::Approximation;
+    fn approx(self, tolerance: Tolerance) -> Self::Approximation;
 }

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

@@ -1,3 +1,7 @@
+//! Tolerance value for approximation
+//!
+//! See [`Tolerance`].
+
 use fj_math::Scalar;
 
 /// A tolerance value