From 22104576507039939e1e14ee26b6be81feeaae4f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 11:41:17 +0200 Subject: [PATCH 01/13] Rename `EdgeApprox` to `HalfEdgeApprox` --- crates/fj-core/src/algorithms/approx/cycle.rs | 4 ++-- crates/fj-core/src/algorithms/approx/edge.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/cycle.rs b/crates/fj-core/src/algorithms/approx/cycle.rs index ad6d4e489..c097b6603 100644 --- a/crates/fj-core/src/algorithms/approx/cycle.rs +++ b/crates/fj-core/src/algorithms/approx/cycle.rs @@ -9,7 +9,7 @@ use fj_math::Segment; use crate::objects::{Cycle, Surface}; use super::{ - edge::{EdgeApprox, EdgeApproxCache}, + edge::{EdgeApproxCache, HalfEdgeApprox}, Approx, ApproxPoint, Tolerance, }; @@ -41,7 +41,7 @@ impl Approx for (&Cycle, &Surface) { #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CycleApprox { /// The approximated edges that make up the approximated cycle - pub edges: Vec, + pub edges: Vec, } impl CycleApprox { diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index b4c05e8ec..f127bed0d 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -23,7 +23,7 @@ use super::{ }; impl Approx for (&HalfEdge, &Surface) { - type Approximation = EdgeApprox; + type Approximation = HalfEdgeApprox; type Cache = EdgeApproxCache; fn approx_with_cache( @@ -105,13 +105,13 @@ impl Approx for (&HalfEdge, &Surface) { .collect() }; - EdgeApprox { first, rest } + HalfEdgeApprox { first, rest } } } /// An approximation of a [`HalfEdge`] #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct EdgeApprox { +pub struct HalfEdgeApprox { /// The point that approximates the first vertex of the edge first: ApproxPoint<2>, @@ -119,7 +119,7 @@ pub struct EdgeApprox { rest: Vec>, } -impl EdgeApprox { +impl HalfEdgeApprox { /// Compute the points that approximate the edge pub fn points(&self) -> Vec> { let mut points = Vec::new(); From 3f30c61736e9b0b73b5e6e899c7efd1c5c24ce95 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 11:46:42 +0200 Subject: [PATCH 02/13] Refactor to prepare for follow-on change --- crates/fj-core/src/algorithms/approx/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index f127bed0d..ae3ac4ba7 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -102,7 +102,7 @@ impl Approx for (&HalfEdge, &Surface) { ApproxPoint::new(point_surface, point.global_form) }) - .collect() + .collect::>() }; HalfEdgeApprox { first, rest } From 3ab979581c71e0444c7d23321e05d4eeda824595 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 11:58:49 +0200 Subject: [PATCH 03/13] Remove obsolete remark from doc comment --- crates/fj-core/src/algorithms/approx/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/mod.rs b/crates/fj-core/src/algorithms/approx/mod.rs index 72497ffb1..0d069ff8e 100644 --- a/crates/fj-core/src/algorithms/approx/mod.rs +++ b/crates/fj-core/src/algorithms/approx/mod.rs @@ -59,7 +59,7 @@ pub struct ApproxPoint { } impl ApproxPoint { - /// Create an instance of `ApproxPoint`, without a source + /// Create an instance of `ApproxPoint` pub fn new( local_form: impl Into>, global_form: impl Into>, From 3b8ae0223144ecbbcc77bb0f437e6b2523074f3f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 12:00:35 +0200 Subject: [PATCH 04/13] Add `ApproxPoint::from_surface_point` --- crates/fj-core/src/algorithms/approx/edge.rs | 9 ++------- crates/fj-core/src/algorithms/approx/mod.rs | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index ae3ac4ba7..6b6a12140 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -325,9 +325,7 @@ mod tests { .map(|(point_local, _)| { let point_surface = edge.path().point_from_path_coords(point_local); - let point_global = - surface.geometry().point_from_surface_coords(point_surface); - ApproxPoint::new(point_surface, point_global) + ApproxPoint::from_surface_point(point_surface, &surface) }) .collect::>(); assert_eq!(approx.rest, expected_approx); @@ -348,10 +346,7 @@ mod tests { .approx(tolerance) .into_iter() .map(|(_, point_surface)| { - let point_global = surface - .geometry() - .point_from_surface_coords(point_surface); - ApproxPoint::new(point_surface, point_global) + ApproxPoint::from_surface_point(point_surface, &surface) }) .collect::>(); assert_eq!(approx.rest, expected_approx); diff --git a/crates/fj-core/src/algorithms/approx/mod.rs b/crates/fj-core/src/algorithms/approx/mod.rs index 0d069ff8e..94564c89c 100644 --- a/crates/fj-core/src/algorithms/approx/mod.rs +++ b/crates/fj-core/src/algorithms/approx/mod.rs @@ -18,6 +18,8 @@ use std::{ use fj_math::Point; +use crate::objects::Surface; + pub use self::tolerance::{InvalidTolerance, Tolerance}; /// Approximate an object @@ -71,6 +73,19 @@ impl ApproxPoint { } } +impl ApproxPoint<2> { + /// Create an instance of `ApproxPoint` from a surface point + pub fn from_surface_point( + point_surface: impl Into>, + surface: &Surface, + ) -> Self { + let point_surface = point_surface.into(); + let point_global = + surface.geometry().point_from_surface_coords(point_surface); + ApproxPoint::new(point_surface, point_global) + } +} + impl Eq for ApproxPoint {} impl PartialEq for ApproxPoint { From a4e722847f4699c012c4c72c6739364b86f4783f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 11:47:28 +0200 Subject: [PATCH 05/13] Don't store first point in `HalfEdgeApprox` It plays a special role for *creating* the `HalfEdgeApprox`, but there's not reason to store it separately once it has been created. --- crates/fj-core/src/algorithms/approx/edge.rs | 53 +++++++++++++------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 6b6a12140..cb3c8f38a 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -105,16 +105,16 @@ impl Approx for (&HalfEdge, &Surface) { .collect::>() }; - HalfEdgeApprox { first, rest } + let mut points = vec![first]; + points.extend(rest); + + HalfEdgeApprox { rest: points } } } /// An approximation of a [`HalfEdge`] #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdgeApprox { - /// The point that approximates the first vertex of the edge - first: ApproxPoint<2>, - /// The approximation of the rest of the edge rest: Vec>, } @@ -124,7 +124,6 @@ impl HalfEdgeApprox { pub fn points(&self) -> Vec> { let mut points = Vec::new(); - points.push(self.first); points.extend(self.rest.iter().cloned()); points @@ -279,7 +278,11 @@ mod tests { let tolerance = 1.; let approx = (&edge, surface.deref()).approx(tolerance); - assert_eq!(approx.rest, Vec::new()); + let expected_approx = vec![{ + let point_surface = edge.start_position(); + ApproxPoint::from_surface_point(point_surface, &surface) + }]; + assert_eq!(approx.rest, expected_approx); } #[test] @@ -296,7 +299,11 @@ mod tests { let tolerance = 1.; let approx = (&edge, &surface).approx(tolerance); - assert_eq!(approx.rest, Vec::new()); + let expected_approx = vec![{ + let point_surface = edge.start_position(); + ApproxPoint::from_surface_point(point_surface, &surface) + }]; + assert_eq!(approx.rest, expected_approx); } #[test] @@ -319,15 +326,19 @@ mod tests { let tolerance = 1.; let approx = (&edge, &surface).approx(tolerance); - let expected_approx = (path, boundary) - .approx(tolerance) - .into_iter() - .map(|(point_local, _)| { - let point_surface = - edge.path().point_from_path_coords(point_local); - ApproxPoint::from_surface_point(point_surface, &surface) - }) - .collect::>(); + let mut expected_approx = vec![{ + let point_surface = edge.start_position(); + ApproxPoint::from_surface_point(point_surface, &surface) + }]; + expected_approx.extend( + (path, boundary).approx(tolerance).into_iter().map( + |(point_local, _)| { + let point_surface = + edge.path().point_from_path_coords(point_local); + ApproxPoint::from_surface_point(point_surface, &surface) + }, + ), + ); assert_eq!(approx.rest, expected_approx); } @@ -341,14 +352,18 @@ mod tests { let tolerance = 1.; let approx = (&edge, surface.deref()).approx(tolerance); - let expected_approx = + let mut expected_approx = vec![{ + let point_surface = edge.start_position(); + ApproxPoint::from_surface_point(point_surface, &surface) + }]; + expected_approx.extend( (&edge.path(), CurveBoundary::from([[0.], [TAU]])) .approx(tolerance) .into_iter() .map(|(_, point_surface)| { ApproxPoint::from_surface_point(point_surface, &surface) - }) - .collect::>(); + }), + ); assert_eq!(approx.rest, expected_approx); } } From 85673463721706d6607bac831def0eea0e0eddc0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 12:04:52 +0200 Subject: [PATCH 06/13] Update struct field name --- crates/fj-core/src/algorithms/approx/edge.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index cb3c8f38a..1df9896c2 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -108,7 +108,7 @@ impl Approx for (&HalfEdge, &Surface) { let mut points = vec![first]; points.extend(rest); - HalfEdgeApprox { rest: points } + HalfEdgeApprox { points } } } @@ -116,7 +116,7 @@ impl Approx for (&HalfEdge, &Surface) { #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdgeApprox { /// The approximation of the rest of the edge - rest: Vec>, + points: Vec>, } impl HalfEdgeApprox { @@ -124,7 +124,7 @@ impl HalfEdgeApprox { pub fn points(&self) -> Vec> { let mut points = Vec::new(); - points.extend(self.rest.iter().cloned()); + points.extend(self.points.iter().cloned()); points } @@ -282,7 +282,7 @@ mod tests { let point_surface = edge.start_position(); ApproxPoint::from_surface_point(point_surface, &surface) }]; - assert_eq!(approx.rest, expected_approx); + assert_eq!(approx.points, expected_approx); } #[test] @@ -303,7 +303,7 @@ mod tests { let point_surface = edge.start_position(); ApproxPoint::from_surface_point(point_surface, &surface) }]; - assert_eq!(approx.rest, expected_approx); + assert_eq!(approx.points, expected_approx); } #[test] @@ -339,7 +339,7 @@ mod tests { }, ), ); - assert_eq!(approx.rest, expected_approx); + assert_eq!(approx.points, expected_approx); } #[test] @@ -364,6 +364,6 @@ mod tests { ApproxPoint::from_surface_point(point_surface, &surface) }), ); - assert_eq!(approx.rest, expected_approx); + assert_eq!(approx.points, expected_approx); } } From 1e7bbb80d6100d53bd30ae71db9a31f8951e9a9f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 12:05:40 +0200 Subject: [PATCH 07/13] Update doc comment --- crates/fj-core/src/algorithms/approx/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 1df9896c2..5757ae072 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -115,7 +115,7 @@ impl Approx for (&HalfEdge, &Surface) { /// An approximation of a [`HalfEdge`] #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdgeApprox { - /// The approximation of the rest of the edge + /// The points that approximate the half-edge points: Vec>, } From c75b0828dd9a9be9e7b4a8efdc73746df85b42e1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 12:05:51 +0200 Subject: [PATCH 08/13] Make struct field public --- crates/fj-core/src/algorithms/approx/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 5757ae072..7ba93799a 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -116,7 +116,7 @@ impl Approx for (&HalfEdge, &Surface) { #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdgeApprox { /// The points that approximate the half-edge - points: Vec>, + pub points: Vec>, } impl HalfEdgeApprox { From 62557d0b952fbf20a2d1467e64d77b603a0e3b3a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 12:08:09 +0200 Subject: [PATCH 09/13] Remove redundant method --- crates/fj-core/src/algorithms/approx/cycle.rs | 2 +- crates/fj-core/src/algorithms/approx/edge.rs | 11 ----------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/cycle.rs b/crates/fj-core/src/algorithms/approx/cycle.rs index c097b6603..8112324f4 100644 --- a/crates/fj-core/src/algorithms/approx/cycle.rs +++ b/crates/fj-core/src/algorithms/approx/cycle.rs @@ -50,7 +50,7 @@ impl CycleApprox { let mut points = Vec::new(); for approx in &self.edges { - points.extend(approx.points()); + points.extend(approx.points.iter().copied()); } if let Some(point) = points.first() { diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 7ba93799a..728ecca94 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -119,17 +119,6 @@ pub struct HalfEdgeApprox { pub points: Vec>, } -impl HalfEdgeApprox { - /// Compute the points that approximate the edge - pub fn points(&self) -> Vec> { - let mut points = Vec::new(); - - points.extend(self.points.iter().cloned()); - - points - } -} - fn approx_curve( path: &SurfacePath, surface: &Surface, From 154ea85f7073940dab884ed8c3b97c868672048b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 12:09:38 +0200 Subject: [PATCH 10/13] Remove unnecessary allocation --- crates/fj-core/src/algorithms/approx/edge.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 728ecca94..b199c442b 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -92,17 +92,12 @@ impl Approx for (&HalfEdge, &Surface) { // immediately, and it will be there on the second iteration. }; - segment - .points - .inner - .into_iter() - .map(|point| { - let point_surface = - edge.path().point_from_path_coords(point.local_form); + segment.points.inner.into_iter().map(|point| { + let point_surface = + edge.path().point_from_path_coords(point.local_form); - ApproxPoint::new(point_surface, point.global_form) - }) - .collect::>() + ApproxPoint::new(point_surface, point.global_form) + }) }; let mut points = vec![first]; From c7e2ca3b89e6b6dbf207fd40d3c358b867c3172f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 12:11:56 +0200 Subject: [PATCH 11/13] Update struct field name --- crates/fj-core/src/algorithms/approx/cycle.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/cycle.rs b/crates/fj-core/src/algorithms/approx/cycle.rs index 8112324f4..577664aca 100644 --- a/crates/fj-core/src/algorithms/approx/cycle.rs +++ b/crates/fj-core/src/algorithms/approx/cycle.rs @@ -33,7 +33,7 @@ impl Approx for (&Cycle, &Surface) { }) .collect(); - CycleApprox { edges } + CycleApprox { half_edges: edges } } } @@ -41,7 +41,7 @@ impl Approx for (&Cycle, &Surface) { #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CycleApprox { /// The approximated edges that make up the approximated cycle - pub edges: Vec, + pub half_edges: Vec, } impl CycleApprox { @@ -49,7 +49,7 @@ impl CycleApprox { pub fn points(&self) -> Vec> { let mut points = Vec::new(); - for approx in &self.edges { + for approx in &self.half_edges { points.extend(approx.points.iter().copied()); } From 7f4d16427c0edbe81553729a7bb18d152776bd9d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 12:12:15 +0200 Subject: [PATCH 12/13] Update variable name --- crates/fj-core/src/algorithms/approx/cycle.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/cycle.rs b/crates/fj-core/src/algorithms/approx/cycle.rs index 577664aca..6d111db9c 100644 --- a/crates/fj-core/src/algorithms/approx/cycle.rs +++ b/crates/fj-core/src/algorithms/approx/cycle.rs @@ -25,7 +25,7 @@ impl Approx for (&Cycle, &Surface) { let (cycle, surface) = self; let tolerance = tolerance.into(); - let edges = cycle + let half_edges = cycle .half_edges() .iter() .map(|edge| { @@ -33,7 +33,7 @@ impl Approx for (&Cycle, &Surface) { }) .collect(); - CycleApprox { half_edges: edges } + CycleApprox { half_edges } } } From 12d3bd1b516b249e6146b9c18fc7e52eb8f33b50 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 Oct 2023 12:12:40 +0200 Subject: [PATCH 13/13] Update doc comment --- crates/fj-core/src/algorithms/approx/cycle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/cycle.rs b/crates/fj-core/src/algorithms/approx/cycle.rs index 6d111db9c..163a523e1 100644 --- a/crates/fj-core/src/algorithms/approx/cycle.rs +++ b/crates/fj-core/src/algorithms/approx/cycle.rs @@ -40,7 +40,7 @@ impl Approx for (&Cycle, &Surface) { /// An approximation of a [`Cycle`] #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CycleApprox { - /// The approximated edges that make up the approximated cycle + /// The approximated half-edges that make up the approximated cycle pub half_edges: Vec, }