From 225fc17153b0225799445f1e213a30d2675b81a8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 4 Sep 2023 10:40:54 +0200 Subject: [PATCH 1/7] Rename function to improve clarity --- crates/fj-core/src/algorithms/approx/edge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index aa7d6bc6d..890bf36ca 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -86,7 +86,7 @@ impl Approx for (&Edge, &Surface) { let approx = match cached_approx { Some(approx) => approx, None => { - let approx = approx_edge( + let approx = approx_curve( &edge.path(), surface, edge.boundary(), @@ -138,7 +138,7 @@ impl EdgeApprox { } } -fn approx_edge( +fn approx_curve( path: &SurfacePath, surface: &Surface, boundary: CurveBoundary>, From 9912ee4e3324725148999e9515d1fa58f7f98b19 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 5 Sep 2023 10:07:30 +0200 Subject: [PATCH 2/7] Add `CurveApproxCache` --- crates/fj-core/src/algorithms/approx/curve.rs | 3 ++- .../src/algorithms/approx/curve/cache.rs | 17 ++++++++++++++ crates/fj-core/src/algorithms/approx/edge.rs | 23 +++++++++++-------- 3 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 crates/fj-core/src/algorithms/approx/curve/cache.rs diff --git a/crates/fj-core/src/algorithms/approx/curve.rs b/crates/fj-core/src/algorithms/approx/curve.rs index b80419791..481dca730 100644 --- a/crates/fj-core/src/algorithms/approx/curve.rs +++ b/crates/fj-core/src/algorithms/approx/curve.rs @@ -1,5 +1,6 @@ //! Curve approximation +mod cache; mod segment; -pub use self::segment::CurveApproxSegment; +pub use self::{cache::CurveApproxCache, segment::CurveApproxSegment}; diff --git a/crates/fj-core/src/algorithms/approx/curve/cache.rs b/crates/fj-core/src/algorithms/approx/curve/cache.rs new file mode 100644 index 000000000..0126d475f --- /dev/null +++ b/crates/fj-core/src/algorithms/approx/curve/cache.rs @@ -0,0 +1,17 @@ +use std::collections::BTreeMap; + +use fj_math::Point; + +use crate::{geometry::CurveBoundary, objects::Curve, storage::HandleWrapper}; + +use super::CurveApproxSegment; + +/// Cache for curve approximations +#[derive(Default)] +pub struct CurveApproxCache { + #[allow(missing_docs)] + pub inner: BTreeMap< + (HandleWrapper, CurveBoundary>), + CurveApproxSegment, + >, +} diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 890bf36ca..6602ed958 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -15,7 +15,10 @@ use crate::{ storage::{Handle, HandleWrapper}, }; -use super::{curve::CurveApproxSegment, Approx, ApproxPoint, Tolerance}; +use super::{ + curve::{CurveApproxCache, CurveApproxSegment}, + Approx, ApproxPoint, Tolerance, +}; impl Approx for (&Edge, &Surface) { type Approximation = EdgeApprox; @@ -218,10 +221,7 @@ fn approx_curve( #[derive(Default)] pub struct EdgeApproxCache { start_position_approx: BTreeMap, Point<3>>, - curve_approx: BTreeMap< - (HandleWrapper, CurveBoundary>), - CurveApproxSegment, - >, + curve_approx: CurveApproxCache, } impl EdgeApproxCache { @@ -254,13 +254,17 @@ impl EdgeApproxCache { handle: Handle, boundary: CurveBoundary>, ) -> Option { - if let Some(approx) = - self.curve_approx.get(&(handle.clone().into(), boundary)) + if let Some(approx) = self + .curve_approx + .inner + .get(&(handle.clone().into(), boundary)) { return Some(approx.clone()); } - if let Some(approx) = - self.curve_approx.get(&(handle.into(), boundary.reverse())) + if let Some(approx) = self + .curve_approx + .inner + .get(&(handle.into(), boundary.reverse())) { // If we have a cache entry for the reverse boundary, we need to use // that too! @@ -277,6 +281,7 @@ impl EdgeApproxCache { approx: CurveApproxSegment, ) -> CurveApproxSegment { self.curve_approx + .inner .insert((handle.into(), boundary), approx.clone()) .unwrap_or(approx) } From ddcbf47634869c39f12d67e75e54b3b9cc2196e6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 5 Sep 2023 10:12:14 +0200 Subject: [PATCH 3/7] Add `CurveApproxCache::get` --- .../src/algorithms/approx/curve/cache.rs | 29 ++++++++++++++++++- crates/fj-core/src/algorithms/approx/edge.rs | 19 +----------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/cache.rs b/crates/fj-core/src/algorithms/approx/curve/cache.rs index 0126d475f..4a308e65b 100644 --- a/crates/fj-core/src/algorithms/approx/curve/cache.rs +++ b/crates/fj-core/src/algorithms/approx/curve/cache.rs @@ -2,7 +2,11 @@ use std::collections::BTreeMap; use fj_math::Point; -use crate::{geometry::CurveBoundary, objects::Curve, storage::HandleWrapper}; +use crate::{ + geometry::CurveBoundary, + objects::Curve, + storage::{Handle, HandleWrapper}, +}; use super::CurveApproxSegment; @@ -15,3 +19,26 @@ pub struct CurveApproxCache { CurveApproxSegment, >, } + +impl CurveApproxCache { + /// Get an approximation from the cache + pub fn get( + &self, + curve: &Handle, + boundary: &CurveBoundary>, + ) -> Option { + 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()); + } + + None + } +} diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 6602ed958..48d515121 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -254,24 +254,7 @@ impl EdgeApproxCache { handle: Handle, boundary: CurveBoundary>, ) -> Option { - if let Some(approx) = self - .curve_approx - .inner - .get(&(handle.clone().into(), boundary)) - { - return Some(approx.clone()); - } - if let Some(approx) = self - .curve_approx - .inner - .get(&(handle.into(), boundary.reverse())) - { - // If we have a cache entry for the reverse boundary, we need to use - // that too! - return Some(approx.clone().reverse()); - } - - None + self.curve_approx.get(&handle, &boundary) } fn insert_curve_approx( From a13d77e2c91c40eb453020e0aaee49f3fd5c9f7c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 5 Sep 2023 10:26:05 +0200 Subject: [PATCH 4/7] Remove redundant method argument --- crates/fj-core/src/algorithms/approx/edge.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 48d515121..33b0c58fc 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -95,11 +95,7 @@ impl Approx for (&Edge, &Surface) { edge.boundary(), tolerance, ); - cache.insert_curve_approx( - edge.curve().clone(), - edge.boundary(), - approx, - ) + cache.insert_curve_approx(edge.curve().clone(), approx) } }; @@ -260,12 +256,11 @@ impl EdgeApproxCache { fn insert_curve_approx( &mut self, handle: Handle, - boundary: CurveBoundary>, approx: CurveApproxSegment, ) -> CurveApproxSegment { self.curve_approx .inner - .insert((handle.into(), boundary), approx.clone()) + .insert((handle.into(), approx.boundary), approx.clone()) .unwrap_or(approx) } } From c5d1d0e4be03ec321463323e80e76bfea4908c79 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 5 Sep 2023 10:16:43 +0200 Subject: [PATCH 5/7] Add `CurveApproxCache::insert` --- crates/fj-core/src/algorithms/approx/curve/cache.rs | 10 ++++++++++ crates/fj-core/src/algorithms/approx/edge.rs | 3 +-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/cache.rs b/crates/fj-core/src/algorithms/approx/curve/cache.rs index 4a308e65b..fdc712c07 100644 --- a/crates/fj-core/src/algorithms/approx/curve/cache.rs +++ b/crates/fj-core/src/algorithms/approx/curve/cache.rs @@ -41,4 +41,14 @@ impl CurveApproxCache { None } + + /// Insert an approximated segment of the curve into the cache + pub fn insert( + &mut self, + curve: Handle, + new_segment: CurveApproxSegment, + ) -> Option { + self.inner + .insert((curve.into(), new_segment.boundary), new_segment) + } } diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 33b0c58fc..476196497 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -259,8 +259,7 @@ impl EdgeApproxCache { approx: CurveApproxSegment, ) -> CurveApproxSegment { self.curve_approx - .inner - .insert((handle.into(), approx.boundary), approx.clone()) + .insert(handle, approx.clone()) .unwrap_or(approx) } } From 6fc3ccc206285f6561fd61b1f219ea8e9dc626cf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 5 Sep 2023 10:16:57 +0200 Subject: [PATCH 6/7] Remove unnecessary `pub` --- crates/fj-core/src/algorithms/approx/curve/cache.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve/cache.rs b/crates/fj-core/src/algorithms/approx/curve/cache.rs index fdc712c07..d10e3bada 100644 --- a/crates/fj-core/src/algorithms/approx/curve/cache.rs +++ b/crates/fj-core/src/algorithms/approx/curve/cache.rs @@ -13,8 +13,7 @@ use super::CurveApproxSegment; /// Cache for curve approximations #[derive(Default)] pub struct CurveApproxCache { - #[allow(missing_docs)] - pub inner: BTreeMap< + inner: BTreeMap< (HandleWrapper, CurveBoundary>), CurveApproxSegment, >, From a202d4d228e8d899ad6681b8dfd530b0930ee9e1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 5 Sep 2023 10:32:39 +0200 Subject: [PATCH 7/7] Remove unused constructor --- crates/fj-core/src/algorithms/approx/edge.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 476196497..599de8e93 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -221,11 +221,6 @@ pub struct EdgeApproxCache { } impl EdgeApproxCache { - /// Create an empty cache - pub fn new() -> Self { - Self::default() - } - fn get_start_position_approx( &self, handle: &Handle,