From 92177618558e388b84b8dafb3d492e270917847d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 15 Sep 2022 17:22:23 +0200 Subject: [PATCH 1/5] Improve type safety Make sure that a valid triangle is passed into the method. --- crates/fj-kernel/src/algorithms/triangulate/polygon.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/triangulate/polygon.rs b/crates/fj-kernel/src/algorithms/triangulate/polygon.rs index 21e057c69..f5d77519a 100644 --- a/crates/fj-kernel/src/algorithms/triangulate/polygon.rs +++ b/crates/fj-kernel/src/algorithms/triangulate/polygon.rs @@ -1,4 +1,4 @@ -use fj_math::{Point, PolyChain, Segment}; +use fj_math::{Point, PolyChain, Segment, Triangle}; use crate::algorithms::intersect::{ ray_segment::RaySegmentIntersection, HorizontalRayToTheRight, Intersect, @@ -42,11 +42,8 @@ impl Polygon { self } - pub fn contains_triangle( - &self, - triangle: [impl Into>; 3], - ) -> bool { - let [a, b, c] = triangle.map(Into::into); + pub fn contains_triangle(&self, triangle: impl Into>) -> bool { + let [a, b, c] = triangle.into().points(); let mut might_be_hole = true; From 14c0785a8995e1b617a556418688b0dd1f056427 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 12:24:35 +0200 Subject: [PATCH 2/5] Start curve approximation test suite It's just a single, simple test so far. I had planned to add more, but it turns out that they aren't easy to write, due to bugs and inconsistencies: https://github.com/hannobraun/Fornjot/issues/1079#issuecomment-1246622675 This is going to be much easier to fix, once #1079 is addressed, so I'm just going ahead with that instead. --- .../fj-kernel/src/algorithms/approx/curve.rs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 775d02d42..9a320cfae 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -130,3 +130,27 @@ pub struct GlobalCurveApprox { /// The points that approximate the curve pub points: Vec>, } + +#[cfg(test)] +mod tests { + + use crate::{ + algorithms::approx::{path::RangeOnPath, Approx}, + objects::{Curve, Surface}, + path::GlobalPath, + }; + + use super::CurveApprox; + + #[test] + fn approx_line_on_flat_surface() { + let surface = Surface::new(GlobalPath::x_axis(), [0., 0., 1.]); + let curve = + Curve::build(surface).line_from_points([[1., 1.], [2., 1.]]); + let range = RangeOnPath::from([[0.], [1.]]); + + let approx = (&curve, range).approx(1.); + + assert_eq!(approx, CurveApprox::empty()) + } +} From f77701425e180e3ce0f3a2284e20ff530772a667 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 15 Sep 2022 14:05:14 +0200 Subject: [PATCH 3/5] Implement `SurfacePath` approximation --- .../fj-kernel/src/algorithms/approx/path.rs | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/approx/path.rs b/crates/fj-kernel/src/algorithms/approx/path.rs index ef704642a..1ce214b79 100644 --- a/crates/fj-kernel/src/algorithms/approx/path.rs +++ b/crates/fj-kernel/src/algorithms/approx/path.rs @@ -32,10 +32,30 @@ use std::iter; use fj_math::{Circle, Point, Scalar, Sign}; -use crate::path::GlobalPath; +use crate::path::{GlobalPath, SurfacePath}; use super::{Approx, Tolerance}; +impl Approx for (SurfacePath, RangeOnPath) { + type Approximation = Vec<(Point<1>, Point<2>)>; + type Cache = (); + + fn approx_with_cache( + self, + tolerance: impl Into, + (): &mut Self::Cache, + ) -> Self::Approximation { + let (path, range) = self; + + match path { + SurfacePath::Circle(circle) => { + approx_circle(&circle, range, tolerance.into()) + } + SurfacePath::Line(_) => vec![], + } + } +} + impl Approx for (GlobalPath, RangeOnPath) { type Approximation = Vec<(Point<1>, Point<3>)>; type Cache = (); From a8f2586bfe6c828920319ed8cf8e025ae422ecde Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 14 Sep 2022 13:41:55 +0200 Subject: [PATCH 4/5] Bypass `GlobalCurve` approximation --- .../fj-kernel/src/algorithms/approx/curve.rs | 149 +++++++++++++++++- 1 file changed, 145 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 9a320cfae..3ddf1ef9d 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -11,7 +11,10 @@ use std::collections::BTreeMap; -use crate::objects::{Curve, GlobalCurve}; +use crate::{ + objects::{Curve, GlobalCurve}, + path::{GlobalPath, SurfacePath}, +}; use super::{path::RangeOnPath, Approx, ApproxPoint, Tolerance}; @@ -30,8 +33,7 @@ impl Approx for (&Curve, RangeOnPath) { let global_curve_approx = match cache.get(cache_key) { Some(approx) => approx, None => { - let approx = (curve.global_form(), range) - .approx_with_cache(tolerance, &mut ()); + let approx = approx_global_curve(curve, range, tolerance); cache.insert(cache_key, approx) } }; @@ -40,6 +42,7 @@ impl Approx for (&Curve, RangeOnPath) { global_curve_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)) }), @@ -47,6 +50,81 @@ impl Approx for (&Curve, RangeOnPath) { } } +fn approx_global_curve( + curve: &Curve, + range: RangeOnPath, + tolerance: impl Into, +) -> GlobalCurveApprox { + // There are different cases of varying complexity. Circles are the hard + // part here, as they need to be approximated, while lines don't need to be. + // + // This will probably all be unified eventually, as `SurfacePath` and + // `GlobalPath` grow APIs that are better suited to implementing this code + // in a more abstract way. + let points = match (curve.path(), curve.surface().u()) { + (SurfacePath::Circle(_), GlobalPath::Circle(_)) => { + todo!( + "Approximating a circle on a curved surface not supported yet." + ) + } + (SurfacePath::Circle(_), GlobalPath::Line(_)) => { + (curve.path(), range) + .approx_with_cache(tolerance, &mut ()) + .into_iter() + .map(|(point_curve, point_surface)| { + // We're throwing away `point_surface` here, which is a bit + // weird, as we're recomputing it later (outside of this + // function). + // + // It should be fine though: + // + // 1. We're throwing this version away, so there's no danger + // of inconsistency between this and the later version. + // 2. This version should have been computed using the same + // path and parameters and the later version will be, so + // they should be the same anyway. + // 3. Not all other cases handled in this function have a + // surface point available, so it needs to be computed + // later anyway, in the general case. + + let point_global = curve + .surface() + .point_from_surface_coords(point_surface); + (point_curve, point_global) + }) + .collect() + } + (SurfacePath::Line(line), _) => { + let range_u = + RangeOnPath::from(range.boundary.map(|point_curve| { + [curve.path().point_from_path_coords(point_curve).u] + })); + + let approx_u = (curve.surface().u(), range_u) + .approx_with_cache(tolerance, &mut ()); + + let mut points = Vec::new(); + for (u, _) in approx_u { + let t = (u.t - line.origin().u) / line.direction().u; + let point_surface = curve.path().point_from_path_coords([t]); + let point_global = + curve.surface().point_from_surface_coords(point_surface); + points.push((u, point_global)); + } + + points + } + }; + + let points = points + .into_iter() + .map(|(point_curve, point_global)| { + ApproxPoint::new(point_curve, point_global) + }) + .collect(); + GlobalCurveApprox { points } +} + impl Approx for (&GlobalCurve, RangeOnPath) { type Approximation = GlobalCurveApprox; type Cache = (); @@ -133,9 +211,12 @@ pub struct GlobalCurveApprox { #[cfg(test)] mod tests { + use std::f64::consts::TAU; + + use pretty_assertions::assert_eq; use crate::{ - algorithms::approx::{path::RangeOnPath, Approx}, + algorithms::approx::{path::RangeOnPath, Approx, ApproxPoint}, objects::{Curve, Surface}, path::GlobalPath, }; @@ -153,4 +234,64 @@ mod tests { assert_eq!(approx, CurveApprox::empty()) } + + #[test] + fn approx_line_on_curved_surface_but_not_along_curve() { + let surface = + Surface::new(GlobalPath::circle_from_radius(1.), [0., 0., 1.]); + let curve = + Curve::build(surface).line_from_points([[1., 1.], [1., 2.]]); + let range = RangeOnPath::from([[0.], [1.]]); + + let approx = (&curve, range).approx(1.); + + assert_eq!(approx, CurveApprox::empty()); + } + + #[test] + fn approx_line_on_curved_surface_along_curve() { + let path = GlobalPath::circle_from_radius(1.); + let surface = Surface::new(path, [0., 0., 1.]); + let curve = + Curve::build(surface).line_from_points([[0., 1.], [1., 1.]]); + + let range = RangeOnPath::from([[0.], [TAU]]); + let tolerance = 1.; + + let approx = (&curve, range).approx(tolerance); + + let expected_approx = (path, range) + .approx(tolerance) + .into_iter() + .map(|(point_local, _)| { + let point_surface = + curve.path().point_from_path_coords(point_local); + let point_global = + surface.point_from_surface_coords(point_surface); + ApproxPoint::new(point_surface, point_global) + }) + .collect::>(); + assert_eq!(approx.points, expected_approx); + } + + #[test] + fn approx_circle_on_flat_surface() { + let surface = Surface::new(GlobalPath::x_axis(), [0., 0., 1.]); + let curve = Curve::build(surface).circle_from_radius(1.); + + let range = RangeOnPath::from([[0.], [TAU]]); + let tolerance = 1.; + let approx = (&curve, range).approx(tolerance); + + let expected_approx = (curve.path(), range) + .approx(tolerance) + .into_iter() + .map(|(_, point_surface)| { + let point_global = + curve.surface().point_from_surface_coords(point_surface); + ApproxPoint::new(point_surface, point_global) + }) + .collect::>(); + assert_eq!(approx.points, expected_approx); + } } From a7e691c47696b9ac2760bb3f3e89b6b5121cee51 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 15 Sep 2022 17:15:12 +0200 Subject: [PATCH 5/5] Remove unused code --- .../fj-kernel/src/algorithms/approx/curve.rs | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 3ddf1ef9d..c62dfb4c4 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -125,29 +125,6 @@ fn approx_global_curve( GlobalCurveApprox { points } } -impl Approx for (&GlobalCurve, RangeOnPath) { - type Approximation = GlobalCurveApprox; - type Cache = (); - - fn approx_with_cache( - self, - tolerance: impl Into, - cache: &mut Self::Cache, - ) -> Self::Approximation { - let (curve, range) = self; - - let points = (curve.path(), range) - .approx_with_cache(tolerance, cache) - .into_iter() - .map(|(point_curve, point_global)| { - ApproxPoint::new(point_curve, point_global) - }) - .collect(); - - GlobalCurveApprox { points } - } -} - /// An approximation of a [`Curve`] #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CurveApprox {