From 870ae51ceab97f819cabaa70ba0c0169e83ac319 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Jul 2023 09:30:00 +0200 Subject: [PATCH 1/4] Update comment --- crates/fj-core/src/algorithms/approx/edge.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index e8d4c5f0b..85c43369c 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -54,8 +54,8 @@ impl Approx for (&HalfEdge, &Surface) { // approximations. // // Caching works like this: We check whether there already is a - // cache entry for the `GlobalEdge`. If there isn't we create the 3D - // approximation from the 2D `HalfEdge`. Next time we check for a + // cache entry for the `GlobalEdge`. If there isn't, we create the + // 3D approximation from the 2D `HalfEdge`. Next time we check for a // coincident `HalfEdge`, we'll find the cache and use that, getting // the exact same 3D approximation, instead of generating a slightly // different one from the different 2D `HalfEdge`. @@ -74,16 +74,10 @@ impl Approx for (&HalfEdge, &Surface) { // forward, as it is, well, too limiting. This means things here // will need to change. // - // Basically, we're missing two things: - // - // 1. A "global curve" object that is referenced by `HalfEdge`s and - // can be used as the cache key, in combination with the range. - // 2. More intelligent caching, that can deliver partial results for - // the range given, while generating (and then caching) any - // unavailable parts of the range on the fly. - // - // Only item 2. is something we can do right here. Item 1. requires - // a change to the object graph. + // What we need here, is more intelligent caching based on `Curve` + // and the edge boundaries, instead of `GlobalEdge`. The cache needs + // to be able to deliver partial results for a given boundary, then + // generating (and caching) the rest of it on the fly. let cached_approx = cache.get_edge( half_edge.global_form().clone(), half_edge.boundary(), From dd4dfa776e2a22c25721676dcf03deceaad0fe94 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Jul 2023 09:40:41 +0200 Subject: [PATCH 2/4] Remove unnecessary `pub`s --- crates/fj-core/src/algorithms/approx/edge.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 85c43369c..1e67cfc78 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -228,7 +228,7 @@ impl EdgeCache { } /// Access the approximation for the given [`GlobalEdge`], if available - pub fn get_edge( + fn get_edge( &self, handle: Handle, boundary: BoundaryOnCurve, @@ -248,7 +248,7 @@ impl EdgeCache { } /// Insert the approximation of a [`GlobalEdge`] - pub fn insert_edge( + fn insert_edge( &mut self, handle: Handle, boundary: BoundaryOnCurve, @@ -276,14 +276,14 @@ impl EdgeCache { /// An approximation of a [`GlobalEdge`] #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct GlobalEdgeApprox { +struct GlobalEdgeApprox { /// The points that approximate the edge - pub points: Vec>, + points: Vec>, } impl GlobalEdgeApprox { /// Reverse the order of the approximation - pub fn reverse(mut self) -> Self { + fn reverse(mut self) -> Self { self.points.reverse(); self } From 627511fd4dde79dd9fa0abd40bf80cc5b2c63a06 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Jul 2023 09:53:52 +0200 Subject: [PATCH 3/4] Improve type safety in `EdgeCache` --- crates/fj-core/src/algorithms/approx/edge.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 1e67cfc78..96a745b90 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -12,7 +12,7 @@ use fj_math::Point; use crate::{ geometry::{BoundaryOnCurve, GlobalPath, SurfacePath}, objects::{GlobalEdge, HalfEdge, Surface, Vertex}, - storage::{Handle, ObjectId}, + storage::{Handle, HandleWrapper, ObjectId}, }; use super::{Approx, ApproxPoint, Tolerance}; @@ -217,7 +217,10 @@ fn approx_edge( /// A cache for results of an approximation #[derive(Default)] pub struct EdgeCache { - edge_approx: BTreeMap<(ObjectId, BoundaryOnCurve), GlobalEdgeApprox>, + edge_approx: BTreeMap< + (HandleWrapper, BoundaryOnCurve), + GlobalEdgeApprox, + >, vertex_approx: BTreeMap>, } @@ -233,11 +236,13 @@ impl EdgeCache { handle: Handle, boundary: BoundaryOnCurve, ) -> Option { - if let Some(approx) = self.edge_approx.get(&(handle.id(), boundary)) { + if let Some(approx) = + self.edge_approx.get(&(handle.clone().into(), boundary)) + { return Some(approx.clone()); } if let Some(approx) = - self.edge_approx.get(&(handle.id(), boundary.reverse())) + self.edge_approx.get(&(handle.into(), boundary.reverse())) { // If we have a cache entry for the reverse boundary, we need to use // that too! @@ -255,7 +260,7 @@ impl EdgeCache { approx: GlobalEdgeApprox, ) -> GlobalEdgeApprox { self.edge_approx - .insert((handle.id(), boundary), approx.clone()) + .insert((handle.into(), boundary), approx.clone()) .unwrap_or(approx) } From 7df17ecfe1674e56ed2d5b721f85a26c64e5c7e3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Jul 2023 09:53:52 +0200 Subject: [PATCH 4/4] Improve type safety in `EdgeCache` --- crates/fj-core/src/algorithms/approx/edge.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 96a745b90..de5f1e9d8 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -12,7 +12,7 @@ use fj_math::Point; use crate::{ geometry::{BoundaryOnCurve, GlobalPath, SurfacePath}, objects::{GlobalEdge, HalfEdge, Surface, Vertex}, - storage::{Handle, HandleWrapper, ObjectId}, + storage::{Handle, HandleWrapper}, }; use super::{Approx, ApproxPoint, Tolerance}; @@ -221,7 +221,7 @@ pub struct EdgeCache { (HandleWrapper, BoundaryOnCurve), GlobalEdgeApprox, >, - vertex_approx: BTreeMap>, + vertex_approx: BTreeMap, Point<3>>, } impl EdgeCache { @@ -265,7 +265,7 @@ impl EdgeCache { } fn get_position(&self, handle: &Handle) -> Option> { - self.vertex_approx.get(&handle.id()).cloned() + self.vertex_approx.get(&handle.clone().into()).cloned() } fn insert_position( @@ -274,7 +274,7 @@ impl EdgeCache { position: Point<3>, ) -> Point<3> { self.vertex_approx - .insert(handle.id(), position) + .insert(handle.clone().into(), position) .unwrap_or(position) } }