Merge pull request #2010 from hannobraun/approx

Further prepare for more sophisticated curve approximation

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

@@ -13,8 +13,7 @@ use super::{CurveApprox, CurveApproxSegment};
 /// Cache for curve approximations
 #[derive(Default)]
 pub struct CurveApproxCache {
-    inner:
-        BTreeMap<(HandleWrapper<Curve>, CurveBoundary<Point<1>>), CurveApprox>,
+    inner: BTreeMap<HandleWrapper<Curve>, CurveApprox>,
 }
 
 impl CurveApproxCache {
@@ -33,15 +32,17 @@ impl CurveApproxCache {
         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();
-                }
+        let mut approx = self.inner.get(&curve).cloned()?;
 
-                approx
-            },
-        )
+        approx
+            .segments
+            .retain(|segment| segment.boundary == normalized_boundary);
+
+        if !was_already_normalized {
+            approx.reverse();
+        }
+
+        Some(approx)
     }
 
     /// Insert an approximated segment of the curve into the cache
@@ -50,31 +51,236 @@ impl CurveApproxCache {
         curve: Handle<Curve>,
         mut new_segment: CurveApproxSegment,
     ) -> CurveApproxSegment {
+        let curve = HandleWrapper::from(curve);
+
         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()],
+        let existing_approx = self.inner.remove(&curve);
+        let (approx, segment) = match existing_approx {
+            Some(mut existing_approx) => {
+                // An approximation for this curve already exists. We need to
+                // merge the new segment into it.
+
+                // We assume that approximated curve segments never overlap,
+                // unless they are completely congruent. As a consequence of
+                // this, we don't have to do any merging with existing segments
+                // here.
+                //
+                // For now, this is a valid assumption, as it matches the uses
+                // of this method, due to documented limitations elsewhere in
+                // the system.
+
+                let mut existing_segment = None;
+                for segment in existing_approx.segments.iter().cloned() {
+                    if segment.boundary == new_segment.boundary {
+                        existing_segment = Some(segment);
+                    }
+                }
+
+                let segment = match existing_segment {
+                    Some(segment) => segment,
+                    None => {
+                        existing_approx.segments.push(new_segment.clone());
+                        new_segment
+                    }
+                };
+
+                (existing_approx, segment)
+            }
+            None => {
+                // No approximation for this curve exists. We need to create a
+                // new one.
+                let new_approx = CurveApprox {
+                    segments: vec![new_segment.clone()],
+                };
+
+                (new_approx, new_segment)
+            }
         };
 
-        self.inner
-            .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)
+        self.inner.insert(curve, approx);
+
+        segment
+    }
+}
+
+#[cfg(test)]
+pub mod tests {
+    use crate::{
+        algorithms::approx::{
+            curve::{CurveApprox, CurveApproxCache, CurveApproxSegment},
+            ApproxPoint,
+        },
+        geometry::CurveBoundary,
+        objects::Curve,
+        operations::Insert,
+        services::Services,
+    };
+
+    #[test]
+    fn insert_curve_already_exists_but_no_segment_merge_necessary() {
+        let mut services = Services::new();
+
+        let mut cache = CurveApproxCache::default();
+        let curve = Curve::new().insert(&mut services);
+
+        // An approximation of our curve already exists.
+        cache.insert(
+            curve.clone(),
+            CurveApproxSegment {
+                boundary: CurveBoundary::from([[2.], [3.]]),
+                points: vec![
+                    ApproxPoint::new([2.25], [2.25, 2.25, 2.25]),
+                    ApproxPoint::new([2.75], [2.75, 2.75, 2.75]),
+                ],
+            },
+        );
+
+        // Here's another approximated segment for the same curve, but i doesn't
+        // overlap with the already existing one.
+        let boundary = CurveBoundary::from([[0.], [1.]]);
+        let segment = CurveApproxSegment {
+            boundary,
+            points: vec![
+                ApproxPoint::new([0.25], [0.25, 0.25, 0.25]),
+                ApproxPoint::new([0.75], [0.75, 0.75, 0.75]),
+            ],
+        };
+
+        // When inserting the second segment, we expect to get it back
+        // unchanged.
+        let inserted = cache.insert(curve.clone(), segment.clone());
+        assert_eq!(inserted, segment);
+    }
+
+    #[test]
+    fn insert_congruent_segment_already_exists() {
+        let mut services = Services::new();
+
+        let mut cache = CurveApproxCache::default();
+        let curve = Curve::new().insert(&mut services);
+
+        // An approximation of our curve already exists.
+        let boundary = CurveBoundary::from([[0.], [1.]]);
+        let existing_segment = CurveApproxSegment {
+            boundary,
+            points: vec![
+                ApproxPoint::new([0.25], [0.25, 0.25, 0.25]),
+                ApproxPoint::new([0.75], [0.75, 0.75, 0.75]),
+            ],
+        };
+        cache.insert(curve.clone(), existing_segment.clone());
+
+        // Here's another approximated segment for the same curve that is
+        // congruent with the existing one.
+        let new_segment = CurveApproxSegment {
+            boundary,
+            points: vec![
+                ApproxPoint::new([0.24], [0.24, 0.24, 0.24]),
+                ApproxPoint::new([0.76], [0.76, 0.76, 0.76]),
+            ],
+        };
+
+        // When inserting the second segment, we expect to get the original one
+        // back.
+        let inserted = cache.insert(curve.clone(), new_segment);
+        assert_eq!(inserted, existing_segment);
+
+        // Also, the new segment should not have replaced the existing on in the
+        // cache.
+        let cached = cache.get(&curve, &boundary);
+        assert_eq!(
+            cached,
+            Some(CurveApprox {
+                segments: vec![existing_segment]
+            })
+        );
+    }
+
+    #[test]
+    fn get_exact_match() {
+        let mut services = Services::new();
+
+        let mut cache = CurveApproxCache::default();
+        let curve = Curve::new().insert(&mut services);
+
+        // An approximation of our curve already exists.
+        cache.insert(
+            curve.clone(),
+            CurveApproxSegment {
+                boundary: CurveBoundary::from([[2.], [3.]]),
+                points: vec![
+                    ApproxPoint::new([2.25], [2.25, 2.25, 2.25]),
+                    ApproxPoint::new([2.75], [2.75, 2.75, 2.75]),
+                ],
+            },
+        );
+
+        // Here's a second segment that doesn't overlap the existing one.
+        let boundary = CurveBoundary::from([[0.], [1.]]);
+        let segment = CurveApproxSegment {
+            boundary,
+            points: vec![
+                ApproxPoint::new([0.25], [0.25, 0.25, 0.25]),
+                ApproxPoint::new([0.75], [0.75, 0.75, 0.75]),
+            ],
+        };
+        cache.insert(curve.clone(), segment.clone());
+
+        // When asking for an approximation with the same boundary as the second
+        // segment we added, we expect to get it back exactly.
+        let cached = cache.get(&curve, &boundary);
+        assert_eq!(
+            cached,
+            Some(CurveApprox {
+                segments: vec![segment]
+            })
+        );
+    }
+
+    #[test]
+    fn get_exact_match_except_reversed() {
+        let mut services = Services::new();
+
+        let mut cache = CurveApproxCache::default();
+        let curve = Curve::new().insert(&mut services);
+
+        // An approximation of our curve already exists.
+        cache.insert(
+            curve.clone(),
+            CurveApproxSegment {
+                boundary: CurveBoundary::from([[2.], [3.]]),
+                points: vec![
+                    ApproxPoint::new([2.25], [2.25, 2.25, 2.25]),
+                    ApproxPoint::new([2.75], [2.75, 2.75, 2.75]),
+                ],
+            },
+        );
+
+        // Here's a second segment that doesn't overlap the existing one.
+        let boundary = CurveBoundary::from([[0.], [1.]]);
+        let segment = CurveApproxSegment {
+            points: vec![
+                ApproxPoint::new([0.25], [0.25, 0.25, 0.25]),
+                ApproxPoint::new([0.75], [0.75, 0.75, 0.75]),
+            ],
+            boundary,
+        };
+        cache.insert(curve.clone(), segment.clone());
+
+        // When asking for an approximation with the same boundary of the second
+        // segment we added but reversed, we expect to get back the segment, but
+        // reversed.
+        let cached = cache.get(&curve, &boundary.reverse());
+        assert_eq!(
+            cached,
+            Some(CurveApprox {
+                segments: vec![{
+                    let mut segment = segment;
+                    segment.reverse();
+                    segment
+                }]
+            })
+        );
     }
 }