Merge pull request #1091 from hannobraun/cache

Improve curve approximation caching code

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

@@ -11,10 +11,7 @@
 
 use crate::objects::{Curve, GlobalCurve};
 
-use super::{
-    path::{GlobalPathApprox, RangeOnPath},
-    Approx, ApproxCache, ApproxPoint, Tolerance,
-};
+use super::{path::RangeOnPath, Approx, ApproxCache, ApproxPoint, Tolerance};
 
 impl Approx for (&Curve, RangeOnPath) {
     type Approximation = CurveApprox;
@@ -26,21 +23,29 @@ impl Approx for (&Curve, RangeOnPath) {
     ) -> Self::Approximation {
         let (curve, range) = self;
 
-        let approx =
-            (curve.global_form(), range).approx_with_cache(tolerance, cache);
-        let points = approx.points().map(|point| {
-            let point_surface =
-                curve.path().point_from_path_coords(point.local_form);
-            ApproxPoint::new(point_surface, point.global_form)
-                .with_source((*curve, point.local_form))
-        });
+        let cache_key = (*curve.global_form(), range);
+        let global_curve_approx = match cache.global_curve(cache_key) {
+            Some(approx) => approx,
+            None => {
+                let approx = (curve.global_form(), range)
+                    .approx_with_cache(tolerance, cache);
+                cache.insert_global_curve(cache_key, approx)
+            }
+        };
 
-        CurveApprox::empty().with_points(points)
+        CurveApprox::empty().with_points(
+            global_curve_approx.points.into_iter().map(|point| {
+                let point_surface =
+                    curve.path().point_from_path_coords(point.local_form);
+                ApproxPoint::new(point_surface, point.global_form)
+                    .with_source((*curve, point.local_form))
+            }),
+        )
     }
 }
 
 impl Approx for (&GlobalCurve, RangeOnPath) {
-    type Approximation = GlobalPathApprox;
+    type Approximation = GlobalCurveApprox;
 
     fn approx_with_cache(
         self,
@@ -49,12 +54,15 @@ impl Approx for (&GlobalCurve, RangeOnPath) {
     ) -> Self::Approximation {
         let (curve, range) = self;
 
-        if let Some(approx) = cache.global_curve(curve) {
-            return approx;
-        }
+        let points = (curve.path(), range)
+            .approx_with_cache(tolerance, cache)
+            .into_iter()
+            .map(|(point_curve, point_global)| {
+                ApproxPoint::new(point_curve, point_global)
+            })
+            .collect();
 
-        let approx = (curve.path(), range).approx_with_cache(tolerance, cache);
-        cache.insert_global_curve(curve, approx)
+        GlobalCurveApprox { points }
     }
 }
 
@@ -80,3 +88,10 @@ impl CurveApprox {
         self
     }
 }
+
+/// An approximation of a [`GlobalCurve`]
+#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
+pub struct GlobalCurveApprox {
+    /// The points that approximate the curve
+    pub points: Vec<ApproxPoint<1>>,
+}

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

@@ -23,8 +23,8 @@ use fj_math::Point;
 
 use crate::objects::{Curve, GlobalCurve};
 
-use self::path::GlobalPathApprox;
 pub use self::tolerance::{InvalidTolerance, Tolerance};
+use self::{curve::GlobalCurveApprox, path::RangeOnPath};
 
 /// Approximate an object
 pub trait Approx: Sized {
@@ -51,7 +51,7 @@ pub trait Approx: Sized {
 /// A cache for results of an approximation
 #[derive(Default)]
 pub struct ApproxCache {
-    global_curves: BTreeMap<GlobalCurve, GlobalPathApprox>,
+    global_curve: BTreeMap<(GlobalCurve, RangeOnPath), GlobalCurveApprox>,
 }
 
 impl ApproxCache {
@@ -63,19 +63,19 @@ impl ApproxCache {
     /// Insert the approximation of a [`GlobalCurve`]
     pub fn insert_global_curve(
         &mut self,
-        object: &GlobalCurve,
-        approx: GlobalPathApprox,
-    ) -> GlobalPathApprox {
-        self.global_curves.insert(*object, approx.clone());
+        key: (GlobalCurve, RangeOnPath),
+        approx: GlobalCurveApprox,
+    ) -> GlobalCurveApprox {
+        self.global_curve.insert(key, approx.clone());
         approx
     }
 
     /// Access the approximation for the given [`GlobalCurve`], if available
     pub fn global_curve(
         &self,
-        object: &GlobalCurve,
-    ) -> Option<GlobalPathApprox> {
-        self.global_curves.get(object).cloned()
+        key: (GlobalCurve, RangeOnPath),
+    ) -> Option<GlobalCurveApprox> {
+        self.global_curve.get(&key).cloned()
     }
 }
 

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

@@ -32,10 +32,10 @@ use fj_math::{Circle, Point, Scalar};
 
 use crate::path::GlobalPath;
 
-use super::{Approx, ApproxCache, ApproxPoint, Tolerance};
+use super::{Approx, ApproxCache, Tolerance};
 
 impl Approx for (GlobalPath, RangeOnPath) {
-    type Approximation = GlobalPathApprox;
+    type Approximation = Vec<(Point<1>, Point<3>)>;
 
     fn approx_with_cache(
         self,
@@ -44,14 +44,12 @@ impl Approx for (GlobalPath, RangeOnPath) {
     ) -> Self::Approximation {
         let (path, range) = self;
 
-        let points = match path {
+        match path {
             GlobalPath::Circle(circle) => {
                 approx_circle(&circle, range, tolerance.into())
             }
             GlobalPath::Line(_) => vec![],
-        };
-
-        GlobalPathApprox { points }
+        }
     }
 }
 
@@ -112,36 +110,23 @@ where
     }
 }
 
-/// An approximation of a [`GlobalPath`]
-#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub struct GlobalPathApprox {
-    points: Vec<ApproxPoint<1>>,
-}
-
-impl GlobalPathApprox {
-    /// Access the points that approximate the path
-    pub fn points(&self) -> impl Iterator<Item = ApproxPoint<1>> + '_ {
-        self.points.iter().cloned()
-    }
-}
-
 /// Approximate a circle
 ///
 /// `tolerance` specifies how much the approximation is allowed to deviate
 /// from the circle.
-fn approx_circle(
-    circle: &Circle<3>,
+fn approx_circle<const D: usize>(
+    circle: &Circle<D>,
     range: impl Into<RangeOnPath>,
     tolerance: Tolerance,
-) -> Vec<ApproxPoint<1>> {
+) -> Vec<(Point<1>, Point<D>)> {
     let range = range.into();
 
     let params = PathApproxParams::for_circle(circle, tolerance);
     let mut points = Vec::new();
 
     for point_curve in params.points(range) {
         let point_global = circle.point_from_circle_coords(point_curve);
-        points.push(ApproxPoint::new(point_curve, point_global));
+        points.push((point_curve, point_global));
     }
 
     if range.is_reversed() {