Merge pull request #2009 from hannobraun/approx

Further prepare for more sophisticated curve approximation

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

@@ -1,6 +1,9 @@
 //! Curve approximation
 
+mod approx;
 mod cache;
 mod segment;
 
-pub use self::{cache::CurveApproxCache, segment::CurveApproxSegment};
+pub use self::{
+    approx::CurveApprox, cache::CurveApproxCache, segment::CurveApproxSegment,
+};

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

@@ -0,0 +1,74 @@
+use super::CurveApproxSegment;
+
+/// Partial approximation of a curve
+#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
+pub struct CurveApprox {
+    /// The approximated segments that are part of this approximation
+    pub segments: Vec<CurveApproxSegment>,
+}
+
+impl CurveApprox {
+    /// Reverse the approximation
+    pub fn reverse(&mut self) -> &mut Self {
+        self.segments.reverse();
+
+        for segment in &mut self.segments {
+            segment.reverse();
+        }
+
+        self
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::algorithms::approx::{curve::CurveApproxSegment, ApproxPoint};
+
+    use super::CurveApprox;
+
+    #[test]
+    fn reverse() {
+        let mut approx = CurveApprox {
+            segments: vec![
+                CurveApproxSegment {
+                    boundary: [[0.1], [0.4]].into(),
+                    points: vec![
+                        ApproxPoint::new([0.1], [0.1, 0.1, 0.1]),
+                        ApproxPoint::new([0.4], [0.4, 0.4, 0.4]),
+                    ],
+                },
+                CurveApproxSegment {
+                    boundary: [[0.6], [0.9]].into(),
+                    points: vec![
+                        ApproxPoint::new([0.6], [0.6, 0.6, 0.6]),
+                        ApproxPoint::new([0.9], [0.9, 0.9, 0.9]),
+                    ],
+                },
+            ],
+        };
+
+        approx.reverse();
+
+        assert_eq!(
+            approx,
+            CurveApprox {
+                segments: vec![
+                    CurveApproxSegment {
+                        boundary: [[0.9], [0.6]].into(),
+                        points: vec![
+                            ApproxPoint::new([0.9], [0.9, 0.9, 0.9]),
+                            ApproxPoint::new([0.6], [0.6, 0.6, 0.6]),
+                        ],
+                    },
+                    CurveApproxSegment {
+                        boundary: [[0.4], [0.1]].into(),
+                        points: vec![
+                            ApproxPoint::new([0.4], [0.4, 0.4, 0.4]),
+                            ApproxPoint::new([0.1], [0.1, 0.1, 0.1]),
+                        ],
+                    },
+                ],
+            }
+        )
+    }
+}

crates/fj-core/src/algorithms/approx/curve/cache.rs

@@ -8,15 +8,13 @@ use crate::{
     storage::{Handle, HandleWrapper},
 };
 
-use super::CurveApproxSegment;
+use super::{CurveApprox, CurveApproxSegment};
 
 /// Cache for curve approximations
 #[derive(Default)]
 pub struct CurveApproxCache {
-    inner: BTreeMap<
-        (HandleWrapper<Curve>, CurveBoundary<Point<1>>),
-        CurveApproxSegment,
-    >,
+    inner:
+        BTreeMap<(HandleWrapper<Curve>, CurveBoundary<Point<1>>), CurveApprox>,
 }
 
 impl CurveApproxCache {
@@ -25,29 +23,58 @@ impl CurveApproxCache {
         &self,
         curve: &Handle<Curve>,
         boundary: &CurveBoundary<Point<1>>,
-    ) -> Option<CurveApproxSegment> {
-        if let Some(approx) = self.inner.get(&(curve.clone().into(), *boundary))
-        {
-            return Some(approx.clone());
-        }
-        if let Some(approx) =
-            self.inner.get(&(curve.clone().into(), boundary.reverse()))
-        {
-            // If we have a cache entry for the reverse boundary, we need to use
-            // that too!
-            return Some(approx.clone().reverse());
-        }
+    ) -> Option<CurveApprox> {
+        let curve = HandleWrapper::from(curve.clone());
 
-        None
+        // Approximations within the cache are all stored in normalized form. If
+        // the caller requests a non-normalized boundary, that means we need to
+        // adjust the result before we return it, so let's remember whether the
+        // normalization resulted in a reversal.
+        let was_already_normalized = boundary.is_normalized();
+        let normalized_boundary = boundary.normalize();
+
+        self.inner.get(&(curve, normalized_boundary)).cloned().map(
+            |mut approx| {
+                if !was_already_normalized {
+                    approx.reverse();
+                }
+
+                approx
+            },
+        )
     }
 
     /// Insert an approximated segment of the curve into the cache
     pub fn insert(
         &mut self,
         curve: Handle<Curve>,
-        new_segment: CurveApproxSegment,
-    ) -> Option<CurveApproxSegment> {
+        mut new_segment: CurveApproxSegment,
+    ) -> CurveApproxSegment {
+        new_segment.normalize();
+
+        // We assume that curve approximation segments never overlap, so so we
+        // don't have to do any merging of this segment with a possibly existing
+        // approximation for this curve.
+        //
+        // For now, this is a valid assumption, as it matches the uses of this
+        // method, due to documented limitations elsewhere in the system.
+        let approx = CurveApprox {
+            segments: vec![new_segment.clone()],
+        };
+
         self.inner
-            .insert((curve.into(), new_segment.boundary), new_segment)
+            .insert((curve.into(), new_segment.boundary), approx)
+            .map(|approx| {
+                let mut segments = approx.segments.into_iter();
+                let segment = segments
+                    .next()
+                    .expect("Empty approximations should not exist in cache");
+                assert!(
+                    segments.next().is_none(),
+                    "Cached approximations should have exactly 1 segment"
+                );
+                segment
+            })
+            .unwrap_or(new_segment)
     }
 }

crates/fj-core/src/algorithms/approx/curve/segment.rs

@@ -20,13 +20,30 @@ pub struct CurveApproxSegment {
 }
 
 impl CurveApproxSegment {
+    /// Indicate whether the segment is normalized
+    pub fn is_normalized(&self) -> bool {
+        self.boundary.is_normalized()
+    }
+
     /// Reverse the orientation of the approximation
-    #[must_use]
-    pub fn reverse(mut self) -> Self {
+    pub fn reverse(&mut self) -> &mut Self {
         self.boundary = self.boundary.reverse();
         self.points.reverse();
         self
     }
+
+    /// Normalize the segment
+    ///
+    /// Puts the points of the approximation in a defined order. This can be
+    /// used to compare segments while disregarding their direction.
+    pub fn normalize(&mut self) -> &mut Self {
+        if !self.is_normalized() {
+            self.boundary = self.boundary.normalize();
+            self.points.reverse();
+        }
+
+        self
+    }
 }
 
 impl Ord for CurveApproxSegment {

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

@@ -245,7 +245,17 @@ impl EdgeApproxCache {
         handle: Handle<Curve>,
         boundary: CurveBoundary<Point<1>>,
     ) -> Option<CurveApproxSegment> {
-        self.curve_approx.get(&handle, &boundary)
+        self.curve_approx.get(&handle, &boundary).map(|approx| {
+            let mut segments = approx.segments.into_iter();
+            let segment = segments
+                .next()
+                .expect("Empty approximations should not exist in cache");
+            assert!(
+                segments.next().is_none(),
+                "Cached approximations should have exactly 1 segment"
+            );
+            segment
+        })
     }
 
     fn insert_curve_approx(
@@ -253,9 +263,7 @@ impl EdgeApproxCache {
         handle: Handle<Curve>,
         approx: CurveApproxSegment,
     ) -> CurveApproxSegment {
-        self.curve_approx
-            .insert(handle, approx.clone())
-            .unwrap_or(approx)
+        self.curve_approx.insert(handle, approx)
     }
 }