From 7f96aad3bbafd39511957dd6695eb4d8c431f674 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 14:29:32 +0200 Subject: [PATCH 1/9] Clarify variable names --- crates/fj-kernel/src/algorithms/sweep/face.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index af252266d..3d8feb8ec 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -17,16 +17,16 @@ impl Sweep for Face { let mut faces = Vec::new(); let is_negative_sweep = { - let a = match self.surface().u() { + let u = match self.surface().u() { GlobalPath::Circle(_) => todo!( "Sweeping from faces defined in round surfaces is not \ supported" ), GlobalPath::Line(line) => line.direction(), }; - let b = self.surface().v(); + let v = self.surface().v(); - let normal = a.cross(&b); + let normal = u.cross(&v); normal.dot(&path) < Scalar::ZERO }; From 7c29428556ce74a4e966594f1513eacc742b2803 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 14:18:29 +0200 Subject: [PATCH 2/9] Derive more traits for `RangeOnPath` --- crates/fj-kernel/src/path.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/path.rs b/crates/fj-kernel/src/path.rs index 0d1153d94..df3ff4e87 100644 --- a/crates/fj-kernel/src/path.rs +++ b/crates/fj-kernel/src/path.rs @@ -216,7 +216,7 @@ fn number_of_vertices_for_circle( } /// The range on which a path should be approximated -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)] pub struct RangeOnPath { boundary: [Point<1>; 2], is_reversed: bool, From 8769c5c7596d7b906885f04b3e65adf6b3302c04 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 15:19:06 +0200 Subject: [PATCH 3/9] Add `RangeOnPath::boundary` --- crates/fj-kernel/src/path.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/fj-kernel/src/path.rs b/crates/fj-kernel/src/path.rs index df3ff4e87..fd23214d4 100644 --- a/crates/fj-kernel/src/path.rs +++ b/crates/fj-kernel/src/path.rs @@ -257,6 +257,11 @@ impl RangeOnPath { self.is_reversed } + /// Access the boundary of the range + pub fn boundary(&self) -> [Point<1>; 2] { + self.boundary + } + /// Access the start of the range pub fn start(&self) -> Point<1> { self.boundary[0] From b3b8a2a20e741ce0dd7aad8dc78fff498cffb2e4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 15:54:58 +0200 Subject: [PATCH 4/9] Integrate path approximation into `approx` --- .../fj-kernel/src/algorithms/approx/curve.rs | 9 +- .../fj-kernel/src/algorithms/approx/edge.rs | 7 +- crates/fj-kernel/src/algorithms/approx/mod.rs | 1 + .../fj-kernel/src/algorithms/approx/path.rs | 207 ++++++++++++++++++ crates/fj-kernel/src/path.rs | 191 ---------------- 5 files changed, 216 insertions(+), 199 deletions(-) create mode 100644 crates/fj-kernel/src/algorithms/approx/path.rs diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 5e6bbdc7f..07cb5843c 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -9,12 +9,9 @@ //! done, to give the caller (who knows the boundary anyway) more options on how //! to further process the approximation. -use crate::{ - objects::{Curve, GlobalCurve}, - path::RangeOnPath, -}; +use crate::objects::{Curve, GlobalCurve}; -use super::{Approx, ApproxCache, ApproxPoint, Tolerance}; +use super::{path::RangeOnPath, Approx, ApproxCache, ApproxPoint, Tolerance}; impl Approx for (&Curve, RangeOnPath) { type Approximation = CurveApprox; @@ -55,7 +52,7 @@ impl Approx for (&GlobalCurve, RangeOnPath) { return approx; } - let points = curve.path().approx(range, tolerance); + let points = (curve.path(), range).approx_with_cache(tolerance, cache); cache.insert_global_curve(curve, GlobalCurveApprox { points }) } } diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 2e79c7149..8cabf2b30 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -5,9 +5,12 @@ //! approximations are usually used to build cycle approximations, and this way, //! the caller doesn't have to call with duplicate vertices. -use crate::{objects::HalfEdge, path::RangeOnPath}; +use crate::objects::HalfEdge; -use super::{curve::CurveApprox, Approx, ApproxCache, ApproxPoint, Tolerance}; +use super::{ + curve::CurveApprox, path::RangeOnPath, Approx, ApproxCache, ApproxPoint, + Tolerance, +}; impl Approx for &HalfEdge { type Approximation = HalfEdgeApprox; diff --git a/crates/fj-kernel/src/algorithms/approx/mod.rs b/crates/fj-kernel/src/algorithms/approx/mod.rs index e68421a79..908bdb8ab 100644 --- a/crates/fj-kernel/src/algorithms/approx/mod.rs +++ b/crates/fj-kernel/src/algorithms/approx/mod.rs @@ -4,6 +4,7 @@ pub mod curve; pub mod cycle; pub mod edge; pub mod face; +pub mod path; pub mod shell; pub mod sketch; pub mod solid; diff --git a/crates/fj-kernel/src/algorithms/approx/path.rs b/crates/fj-kernel/src/algorithms/approx/path.rs new file mode 100644 index 000000000..404978bd6 --- /dev/null +++ b/crates/fj-kernel/src/algorithms/approx/path.rs @@ -0,0 +1,207 @@ +//! # Path approximation +//! +//! Since paths are infinite (even circles have an infinite coordinate space, +//! even though they connect to themselves in global coordinates), a range must +//! be provided to approximate them. The approximation then returns points +//! within that range. +//! +//! The boundaries of the range are not included in the approximation. This is +//! done, to give the caller (who knows the boundary anyway) more options on how +//! to further process the approximation. + +use fj_math::{Circle, Point, Scalar}; + +use crate::path::GlobalPath; + +use super::{Approx, ApproxCache, ApproxPoint, Tolerance}; + +impl Approx for (GlobalPath, RangeOnPath) { + type Approximation = Vec>; + + fn approx_with_cache( + self, + tolerance: impl Into, + _: &mut ApproxCache, + ) -> Self::Approximation { + let (path, range) = self; + + match path { + GlobalPath::Circle(circle) => { + approx_circle(&circle, range, tolerance.into()) + } + GlobalPath::Line(_) => vec![], + } + } +} + +/// Approximate a circle +/// +/// `tolerance` specifies how much the approximation is allowed to deviate +/// from the circle. +fn approx_circle( + circle: &Circle<3>, + range: impl Into, + tolerance: Tolerance, +) -> Vec> { + let mut points = Vec::new(); + + let range = range.into(); + + // To approximate the circle, we use a regular polygon for which + // the circle is the circumscribed circle. The `tolerance` + // parameter is the maximum allowed distance between the polygon + // and the circle. This is the same as the difference between + // the circumscribed circle and the incircle. + + let n = number_of_vertices_for_circle( + tolerance, + circle.radius(), + range.length(), + ); + + for i in 1..n { + let angle = range.start().t + + (Scalar::TAU / n as f64 * i as f64) * range.direction(); + + let point_curve = Point::from([angle]); + let point_global = circle.point_from_circle_coords(point_curve); + + points.push(ApproxPoint::new(point_curve, point_global)); + } + + if range.is_reversed() { + points.reverse(); + } + + points +} + +fn number_of_vertices_for_circle( + tolerance: Tolerance, + radius: Scalar, + range: Scalar, +) -> u64 { + let n = (Scalar::PI / (Scalar::ONE - (tolerance.inner() / radius)).acos()) + .max(3.); + + (n / (Scalar::TAU / range)).ceil().into_u64() +} + +/// The range on which a path should be approximated +#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)] +pub struct RangeOnPath { + boundary: [Point<1>; 2], + is_reversed: bool, +} + +impl RangeOnPath { + /// Construct an instance of `RangeOnCurve` + /// + /// Ranges are normalized on construction, meaning that the order of + /// vertices passed to this constructor does not influence the range that is + /// constructed. + /// + /// This is done to prevent bugs during mesh construction: The curve + /// approximation code is regularly faced with ranges that are reversed + /// versions of each other. This can lead to slightly different + /// approximations, which in turn leads to the aforementioned invalid + /// meshes. + /// + /// The caller can use `is_reversed` to determine, if the range was reversed + /// during normalization, to adjust the approximation accordingly. + pub fn new(boundary: [impl Into>; 2]) -> Self { + let [a, b] = boundary.map(Into::into); + + let (boundary, is_reversed) = if a < b { + ([a, b], false) + } else { + ([b, a], true) + }; + + Self { + boundary, + is_reversed, + } + } + + /// Indicate whether the range was reversed during normalization + pub fn is_reversed(&self) -> bool { + self.is_reversed + } + + /// Access the boundary of the range + pub fn boundary(&self) -> [Point<1>; 2] { + self.boundary + } + + /// Access the start of the range + pub fn start(&self) -> Point<1> { + self.boundary[0] + } + + /// Access the end of the range + pub fn end(&self) -> Point<1> { + self.boundary[1] + } + + /// Compute the signed length of the range + pub fn signed_length(&self) -> Scalar { + (self.end() - self.start()).t + } + + /// Compute the absolute length of the range + pub fn length(&self) -> Scalar { + self.signed_length().abs() + } + + /// Compute the direction of the range + /// + /// Returns a [`Scalar`] that is zero or +/- one. + pub fn direction(&self) -> Scalar { + self.signed_length().sign() + } +} + +#[cfg(test)] +mod tests { + use fj_math::Scalar; + + use crate::algorithms::approx::Tolerance; + + #[test] + fn number_of_vertices_for_circle() { + verify_result(50., 100., Scalar::TAU, 3); + verify_result(50., 100., Scalar::PI, 2); + verify_result(10., 100., Scalar::TAU, 7); + verify_result(10., 100., Scalar::PI, 4); + verify_result(1., 100., Scalar::TAU, 23); + verify_result(1., 100., Scalar::PI, 12); + + fn verify_result( + tolerance: impl Into, + radius: impl Into, + range: impl Into, + n: u64, + ) { + let tolerance = tolerance.into(); + let radius = radius.into(); + let range = range.into(); + + assert_eq!( + n, + super::number_of_vertices_for_circle(tolerance, radius, range) + ); + + assert!(calculate_error(radius, range, n) <= tolerance.inner()); + if n > 3 { + assert!( + calculate_error(radius, range, n - 1) >= tolerance.inner() + ); + } + } + + fn calculate_error(radius: Scalar, range: Scalar, n: u64) -> Scalar { + radius - radius * (range / Scalar::from_u64(n) / 2.).cos() + } + } +} diff --git a/crates/fj-kernel/src/path.rs b/crates/fj-kernel/src/path.rs index fd23214d4..16e01a6b4 100644 --- a/crates/fj-kernel/src/path.rs +++ b/crates/fj-kernel/src/path.rs @@ -22,12 +22,8 @@ //! [`Surface`]: crate::objects::Surface //! [#1021]: https://github.com/hannobraun/Fornjot/issues/1021 -use std::cmp::max; - use fj_math::{Circle, Line, Point, Scalar, Vector}; -use crate::algorithms::approx::{ApproxPoint, Tolerance}; - /// A path through surface (2D) space #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub enum SurfacePath { @@ -145,191 +141,4 @@ impl GlobalPath { Self::Line(line) => line.vector_from_line_coords(vector), } } - - /// Approximate the path - pub fn approx( - &self, - range: RangeOnPath, - tolerance: impl Into, - ) -> Vec> { - match self { - GlobalPath::Circle(circle) => { - approx_circle(circle, range, tolerance.into()) - } - GlobalPath::Line(_) => vec![], - } - } -} - -/// Approximate a circle -/// -/// `tolerance` specifies how much the approximation is allowed to deviate -/// from the circle. -fn approx_circle( - circle: &Circle<3>, - range: impl Into, - tolerance: Tolerance, -) -> Vec> { - let mut points = Vec::new(); - - let range = range.into(); - - // To approximate the circle, we use a regular polygon for which - // the circle is the circumscribed circle. The `tolerance` - // parameter is the maximum allowed distance between the polygon - // and the circle. This is the same as the difference between - // the circumscribed circle and the incircle. - - let n = number_of_vertices_for_circle( - tolerance, - circle.radius(), - range.length(), - ); - - for i in 1..n { - let angle = range.start().t - + (Scalar::TAU / n as f64 * i as f64) * range.direction(); - - let point_curve = Point::from([angle]); - let point_global = circle.point_from_circle_coords(point_curve); - - points.push(ApproxPoint::new(point_curve, point_global)); - } - - if range.is_reversed() { - points.reverse(); - } - - points -} - -fn number_of_vertices_for_circle( - tolerance: Tolerance, - radius: Scalar, - range: Scalar, -) -> u64 { - let n = (range / (Scalar::ONE - (tolerance.inner() / radius)).acos() / 2.) - .ceil() - .into_u64(); - - max(n, 3) -} - -/// The range on which a path should be approximated -#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)] -pub struct RangeOnPath { - boundary: [Point<1>; 2], - is_reversed: bool, -} - -impl RangeOnPath { - /// Construct an instance of `RangeOnCurve` - /// - /// Ranges are normalized on construction, meaning that the order of - /// vertices passed to this constructor does not influence the range that is - /// constructed. - /// - /// This is done to prevent bugs during mesh construction: The curve - /// approximation code is regularly faced with ranges that are reversed - /// versions of each other. This can lead to slightly different - /// approximations, which in turn leads to the aforementioned invalid - /// meshes. - /// - /// The caller can use `is_reversed` to determine, if the range was reversed - /// during normalization, to adjust the approximation accordingly. - pub fn new(boundary: [impl Into>; 2]) -> Self { - let [a, b] = boundary.map(Into::into); - - let (boundary, is_reversed) = if a < b { - ([a, b], false) - } else { - ([b, a], true) - }; - - Self { - boundary, - is_reversed, - } - } - - /// Indicate whether the range was reversed during normalization - pub fn is_reversed(&self) -> bool { - self.is_reversed - } - - /// Access the boundary of the range - pub fn boundary(&self) -> [Point<1>; 2] { - self.boundary - } - - /// Access the start of the range - pub fn start(&self) -> Point<1> { - self.boundary[0] - } - - /// Access the end of the range - pub fn end(&self) -> Point<1> { - self.boundary[1] - } - - /// Compute the signed length of the range - pub fn signed_length(&self) -> Scalar { - (self.end() - self.start()).t - } - - /// Compute the absolute length of the range - pub fn length(&self) -> Scalar { - self.signed_length().abs() - } - - /// Compute the direction of the range - /// - /// Returns a [`Scalar`] that is zero or +/- one. - pub fn direction(&self) -> Scalar { - self.signed_length().sign() - } -} - -#[cfg(test)] -mod tests { - use fj_math::Scalar; - - use crate::algorithms::approx::Tolerance; - - #[test] - fn number_of_vertices_for_circle() { - verify_result(50., 100., Scalar::TAU, 3); - verify_result(50., 100., Scalar::PI, 3); - verify_result(10., 100., Scalar::TAU, 7); - verify_result(10., 100., Scalar::PI, 4); - verify_result(1., 100., Scalar::TAU, 23); - verify_result(1., 100., Scalar::PI, 12); - - fn verify_result( - tolerance: impl Into, - radius: impl Into, - range: impl Into, - n: u64, - ) { - let tolerance = tolerance.into(); - let radius = radius.into(); - let range = range.into(); - - assert_eq!( - n, - super::number_of_vertices_for_circle(tolerance, radius, range) - ); - - assert!(calculate_error(radius, range, n) <= tolerance.inner()); - if n > 3 { - assert!( - calculate_error(radius, range, n - 1) >= tolerance.inner() - ); - } - } - - fn calculate_error(radius: Scalar, range: Scalar, n: u64) -> Scalar { - radius - radius * (range / Scalar::from_u64(n) / 2.).cos() - } - } } From 36761ed6408ea232ec593ebe20644ec3f4febcfc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 16:11:21 +0200 Subject: [PATCH 5/9] Repurpose `GlobalCurveApprox` for paths --- .../fj-kernel/src/algorithms/approx/curve.rs | 18 +++++++----------- crates/fj-kernel/src/algorithms/approx/mod.rs | 10 +++++----- crates/fj-kernel/src/algorithms/approx/path.rs | 15 ++++++++++++--- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 07cb5843c..9b112eadd 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -11,7 +11,10 @@ use crate::objects::{Curve, GlobalCurve}; -use super::{path::RangeOnPath, Approx, ApproxCache, ApproxPoint, Tolerance}; +use super::{ + path::{GlobalPathApprox, RangeOnPath}, + Approx, ApproxCache, ApproxPoint, Tolerance, +}; impl Approx for (&Curve, RangeOnPath) { type Approximation = CurveApprox; @@ -39,7 +42,7 @@ impl Approx for (&Curve, RangeOnPath) { } impl Approx for (&GlobalCurve, RangeOnPath) { - type Approximation = GlobalCurveApprox; + type Approximation = GlobalPathApprox; fn approx_with_cache( self, @@ -52,8 +55,8 @@ impl Approx for (&GlobalCurve, RangeOnPath) { return approx; } - let points = (curve.path(), range).approx_with_cache(tolerance, cache); - cache.insert_global_curve(curve, GlobalCurveApprox { points }) + let approx = (curve.path(), range).approx_with_cache(tolerance, cache); + cache.insert_global_curve(curve, approx) } } @@ -79,10 +82,3 @@ impl CurveApprox { self } } - -/// An approximation of a [`GlobalCurve`] -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct GlobalCurveApprox { - /// The points that approximate the curve - pub points: Vec>, -} diff --git a/crates/fj-kernel/src/algorithms/approx/mod.rs b/crates/fj-kernel/src/algorithms/approx/mod.rs index 908bdb8ab..8ddadc56b 100644 --- a/crates/fj-kernel/src/algorithms/approx/mod.rs +++ b/crates/fj-kernel/src/algorithms/approx/mod.rs @@ -23,7 +23,7 @@ use fj_math::Point; use crate::objects::{Curve, GlobalCurve}; -use self::curve::GlobalCurveApprox; +use self::path::GlobalPathApprox; pub use self::tolerance::{InvalidTolerance, Tolerance}; /// Approximate an object @@ -51,7 +51,7 @@ pub trait Approx: Sized { /// A cache for results of an approximation #[derive(Default)] pub struct ApproxCache { - global_curves: BTreeMap, + global_curves: BTreeMap, } impl ApproxCache { @@ -64,8 +64,8 @@ impl ApproxCache { pub fn insert_global_curve( &mut self, object: &GlobalCurve, - approx: GlobalCurveApprox, - ) -> GlobalCurveApprox { + approx: GlobalPathApprox, + ) -> GlobalPathApprox { self.global_curves.insert(*object, approx.clone()); approx } @@ -74,7 +74,7 @@ impl ApproxCache { pub fn global_curve( &self, object: &GlobalCurve, - ) -> Option { + ) -> Option { self.global_curves.get(object).cloned() } } diff --git a/crates/fj-kernel/src/algorithms/approx/path.rs b/crates/fj-kernel/src/algorithms/approx/path.rs index 404978bd6..f02e9922c 100644 --- a/crates/fj-kernel/src/algorithms/approx/path.rs +++ b/crates/fj-kernel/src/algorithms/approx/path.rs @@ -16,7 +16,7 @@ use crate::path::GlobalPath; use super::{Approx, ApproxCache, ApproxPoint, Tolerance}; impl Approx for (GlobalPath, RangeOnPath) { - type Approximation = Vec>; + type Approximation = GlobalPathApprox; fn approx_with_cache( self, @@ -25,15 +25,24 @@ impl Approx for (GlobalPath, RangeOnPath) { ) -> Self::Approximation { let (path, range) = self; - match path { + let points = match path { GlobalPath::Circle(circle) => { approx_circle(&circle, range, tolerance.into()) } GlobalPath::Line(_) => vec![], - } + }; + + GlobalPathApprox { points } } } +/// An approximation of a [`GlobalPath`] +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct GlobalPathApprox { + /// The points that approximate the path + pub points: Vec>, +} + /// Approximate a circle /// /// `tolerance` specifies how much the approximation is allowed to deviate From ebe933f521336bf0fc2a55468fbbe61cee08f037 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 16:14:31 +0200 Subject: [PATCH 6/9] Refactor --- .../fj-kernel/src/algorithms/approx/curve.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 9b112eadd..1df9e6c6e 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -26,16 +26,14 @@ impl Approx for (&Curve, RangeOnPath) { ) -> Self::Approximation { let (curve, range) = self; - let points = (curve.global_form(), range) - .approx_with_cache(tolerance, cache) - .points - .into_iter() - .map(|point| { - let point_surface = - curve.path().point_from_path_coords(point.local_form); - ApproxPoint::new(point_surface, point.global_form) - .with_source((*curve, point.local_form)) - }); + let approx = + (curve.global_form(), range).approx_with_cache(tolerance, cache); + let points = approx.points.into_iter().map(|point| { + let point_surface = + curve.path().point_from_path_coords(point.local_form); + ApproxPoint::new(point_surface, point.global_form) + .with_source((*curve, point.local_form)) + }); CurveApprox::empty().with_points(points) } From 60f9dc9b47582c7ee1f1db7b4ea37791aa5d3fa6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 16:15:03 +0200 Subject: [PATCH 7/9] Add `GlobalPathApprox::points` --- crates/fj-kernel/src/algorithms/approx/curve.rs | 2 +- crates/fj-kernel/src/algorithms/approx/path.rs | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 1df9e6c6e..43f55c286 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -28,7 +28,7 @@ impl Approx for (&Curve, RangeOnPath) { let approx = (curve.global_form(), range).approx_with_cache(tolerance, cache); - let points = approx.points.into_iter().map(|point| { + let points = approx.points().map(|point| { let point_surface = curve.path().point_from_path_coords(point.local_form); ApproxPoint::new(point_surface, point.global_form) diff --git a/crates/fj-kernel/src/algorithms/approx/path.rs b/crates/fj-kernel/src/algorithms/approx/path.rs index f02e9922c..8cb8b8aea 100644 --- a/crates/fj-kernel/src/algorithms/approx/path.rs +++ b/crates/fj-kernel/src/algorithms/approx/path.rs @@ -43,6 +43,13 @@ pub struct GlobalPathApprox { pub points: Vec>, } +impl GlobalPathApprox { + /// Access the points that approximate the path + pub fn points(&self) -> impl Iterator> + '_ { + self.points.iter().cloned() + } +} + /// Approximate a circle /// /// `tolerance` specifies how much the approximation is allowed to deviate From 140a6b84b59e7636de097ae4b1a6780f76298f0e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 16:15:45 +0200 Subject: [PATCH 8/9] Remove unnecessary `pub` --- crates/fj-kernel/src/algorithms/approx/path.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/path.rs b/crates/fj-kernel/src/algorithms/approx/path.rs index 8cb8b8aea..ddb91d31c 100644 --- a/crates/fj-kernel/src/algorithms/approx/path.rs +++ b/crates/fj-kernel/src/algorithms/approx/path.rs @@ -39,8 +39,7 @@ impl Approx for (GlobalPath, RangeOnPath) { /// An approximation of a [`GlobalPath`] #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct GlobalPathApprox { - /// The points that approximate the path - pub points: Vec>, + points: Vec>, } impl GlobalPathApprox { From 15f0c357d877678184bf43eb52b878bdb25d0ed2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 15 Sep 2022 11:47:55 +0200 Subject: [PATCH 9/9] Update order of code --- .../fj-kernel/src/algorithms/approx/path.rs | 132 +++++++++--------- 1 file changed, 66 insertions(+), 66 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/path.rs b/crates/fj-kernel/src/algorithms/approx/path.rs index ddb91d31c..0575da869 100644 --- a/crates/fj-kernel/src/algorithms/approx/path.rs +++ b/crates/fj-kernel/src/algorithms/approx/path.rs @@ -36,72 +36,6 @@ impl Approx for (GlobalPath, RangeOnPath) { } } -/// An approximation of a [`GlobalPath`] -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct GlobalPathApprox { - points: Vec>, -} - -impl GlobalPathApprox { - /// Access the points that approximate the path - pub fn points(&self) -> impl Iterator> + '_ { - self.points.iter().cloned() - } -} - -/// Approximate a circle -/// -/// `tolerance` specifies how much the approximation is allowed to deviate -/// from the circle. -fn approx_circle( - circle: &Circle<3>, - range: impl Into, - tolerance: Tolerance, -) -> Vec> { - let mut points = Vec::new(); - - let range = range.into(); - - // To approximate the circle, we use a regular polygon for which - // the circle is the circumscribed circle. The `tolerance` - // parameter is the maximum allowed distance between the polygon - // and the circle. This is the same as the difference between - // the circumscribed circle and the incircle. - - let n = number_of_vertices_for_circle( - tolerance, - circle.radius(), - range.length(), - ); - - for i in 1..n { - let angle = range.start().t - + (Scalar::TAU / n as f64 * i as f64) * range.direction(); - - let point_curve = Point::from([angle]); - let point_global = circle.point_from_circle_coords(point_curve); - - points.push(ApproxPoint::new(point_curve, point_global)); - } - - if range.is_reversed() { - points.reverse(); - } - - points -} - -fn number_of_vertices_for_circle( - tolerance: Tolerance, - radius: Scalar, - range: Scalar, -) -> u64 { - let n = (Scalar::PI / (Scalar::ONE - (tolerance.inner() / radius)).acos()) - .max(3.); - - (n / (Scalar::TAU / range)).ceil().into_u64() -} - /// The range on which a path should be approximated #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)] pub struct RangeOnPath { @@ -177,6 +111,72 @@ impl RangeOnPath { } } +/// An approximation of a [`GlobalPath`] +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct GlobalPathApprox { + points: Vec>, +} + +impl GlobalPathApprox { + /// Access the points that approximate the path + pub fn points(&self) -> impl Iterator> + '_ { + self.points.iter().cloned() + } +} + +/// Approximate a circle +/// +/// `tolerance` specifies how much the approximation is allowed to deviate +/// from the circle. +fn approx_circle( + circle: &Circle<3>, + range: impl Into, + tolerance: Tolerance, +) -> Vec> { + let mut points = Vec::new(); + + let range = range.into(); + + // To approximate the circle, we use a regular polygon for which + // the circle is the circumscribed circle. The `tolerance` + // parameter is the maximum allowed distance between the polygon + // and the circle. This is the same as the difference between + // the circumscribed circle and the incircle. + + let n = number_of_vertices_for_circle( + tolerance, + circle.radius(), + range.length(), + ); + + for i in 1..n { + let angle = range.start().t + + (Scalar::TAU / n as f64 * i as f64) * range.direction(); + + let point_curve = Point::from([angle]); + let point_global = circle.point_from_circle_coords(point_curve); + + points.push(ApproxPoint::new(point_curve, point_global)); + } + + if range.is_reversed() { + points.reverse(); + } + + points +} + +fn number_of_vertices_for_circle( + tolerance: Tolerance, + radius: Scalar, + range: Scalar, +) -> u64 { + let n = (Scalar::PI / (Scalar::ONE - (tolerance.inner() / radius)).acos()) + .max(3.); + + (n / (Scalar::TAU / range)).ceil().into_u64() +} + #[cfg(test)] mod tests { use fj_math::Scalar;