From 1e129862a156223cc5def5deb4cd7ce5e30bf776 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Feb 2023 14:36:35 +0100 Subject: [PATCH 01/25] Make function accessible from sibling modules I'm about to use it from `edge`, as part of an ongoing refactoring. --- crates/fj-kernel/src/algorithms/approx/curve.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 21d4d05df..c745d67c4 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -51,7 +51,7 @@ impl Approx for (&Handle, &Surface, Handle, RangeOnPath) { } } -fn approx_global_curve( +pub(super) fn approx_global_curve( curve: &Curve, surface: &Surface, range: RangeOnPath, From f1fbfa15cd77fa4ebf1dd6e593b29894255a02eb Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Feb 2023 14:39:26 +0100 Subject: [PATCH 02/25] Move tests I'm about to adapt them to be based on `HalfEdge`, therefore the new location. --- .../fj-kernel/src/algorithms/approx/curve.rs | 124 ------------------ .../fj-kernel/src/algorithms/approx/edge.rs | 124 ++++++++++++++++++ 2 files changed, 124 insertions(+), 124 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index c745d67c4..f367a1f14 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -206,127 +206,3 @@ impl GlobalCurveApprox { self } } - -#[cfg(test)] -mod tests { - use std::{f64::consts::TAU, ops::Deref}; - - use pretty_assertions::assert_eq; - - use crate::{ - algorithms::approx::{path::RangeOnPath, Approx, ApproxPoint}, - builder::{CurveBuilder, SurfaceBuilder}, - geometry::path::GlobalPath, - insert::Insert, - objects::GlobalCurve, - partial::{PartialCurve, PartialObject, PartialSurface}, - services::Services, - }; - - use super::CurveApprox; - - #[test] - fn approx_line_on_flat_surface() { - let mut services = Services::new(); - - let surface = services.objects.surfaces.xz_plane(); - let mut curve = PartialCurve::default(); - curve.update_as_line_from_points([[1., 1.], [2., 1.]]); - let curve = curve - .build(&mut services.objects) - .insert(&mut services.objects); - let global_curve = GlobalCurve.insert(&mut services.objects); - let range = RangeOnPath::from([[0.], [1.]]); - - let approx = (&curve, surface.deref(), global_curve, range).approx(1.); - - assert_eq!(approx, CurveApprox::empty()); - } - - #[test] - fn approx_line_on_curved_surface_but_not_along_curve() { - let mut services = Services::new(); - - let surface = PartialSurface::from_axes( - GlobalPath::circle_from_radius(1.), - [0., 0., 1.], - ) - .build(&mut services.objects) - .insert(&mut services.objects); - let mut curve = PartialCurve::default(); - curve.update_as_line_from_points([[1., 1.], [1., 2.]]); - let curve = curve - .build(&mut services.objects) - .insert(&mut services.objects); - let global_curve = GlobalCurve.insert(&mut services.objects); - let range = RangeOnPath::from([[0.], [1.]]); - - let approx = (&curve, surface.deref(), global_curve, range).approx(1.); - - assert_eq!(approx, CurveApprox::empty()); - } - - #[test] - fn approx_line_on_curved_surface_along_curve() { - let mut services = Services::new(); - - let path = GlobalPath::circle_from_radius(1.); - let surface = PartialSurface::from_axes(path, [0., 0., 1.]) - .build(&mut services.objects) - .insert(&mut services.objects); - let mut curve = PartialCurve::default(); - curve.update_as_line_from_points([[0., 1.], [1., 1.]]); - let curve = curve - .build(&mut services.objects) - .insert(&mut services.objects); - let global_curve = GlobalCurve.insert(&mut services.objects); - - let range = RangeOnPath::from([[0.], [TAU]]); - let tolerance = 1.; - - let approx = - (&curve, surface.deref(), global_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.geometry().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 mut services = Services::new(); - - let surface = services.objects.surfaces.xz_plane(); - let mut curve = PartialCurve::default(); - curve.update_as_circle_from_radius(1.); - let curve = curve - .build(&mut services.objects) - .insert(&mut services.objects); - let global_curve = GlobalCurve.insert(&mut services.objects); - - let range = RangeOnPath::from([[0.], [TAU]]); - let tolerance = 1.; - let approx = - (&curve, surface.deref(), global_curve, range).approx(tolerance); - - let expected_approx = (curve.path(), range) - .approx(tolerance) - .into_iter() - .map(|(_, point_surface)| { - let point_global = - surface.geometry().point_from_surface_coords(point_surface); - ApproxPoint::new(point_surface, point_global) - }) - .collect::>(); - assert_eq!(approx.points, expected_approx); - } -} diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 9d3226b24..ef4e8851a 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -71,3 +71,127 @@ impl HalfEdgeApprox { points } } + +#[cfg(test)] +mod tests { + use std::{f64::consts::TAU, ops::Deref}; + + use pretty_assertions::assert_eq; + + use crate::{ + algorithms::approx::{path::RangeOnPath, Approx, ApproxPoint}, + builder::{CurveBuilder, SurfaceBuilder}, + geometry::path::GlobalPath, + insert::Insert, + objects::GlobalCurve, + partial::{PartialCurve, PartialObject, PartialSurface}, + services::Services, + }; + + use super::CurveApprox; + + #[test] + fn approx_line_on_flat_surface() { + let mut services = Services::new(); + + let surface = services.objects.surfaces.xz_plane(); + let mut curve = PartialCurve::default(); + curve.update_as_line_from_points([[1., 1.], [2., 1.]]); + let curve = curve + .build(&mut services.objects) + .insert(&mut services.objects); + let global_curve = GlobalCurve.insert(&mut services.objects); + let range = RangeOnPath::from([[0.], [1.]]); + + let approx = (&curve, surface.deref(), global_curve, range).approx(1.); + + assert_eq!(approx, CurveApprox::empty()); + } + + #[test] + fn approx_line_on_curved_surface_but_not_along_curve() { + let mut services = Services::new(); + + let surface = PartialSurface::from_axes( + GlobalPath::circle_from_radius(1.), + [0., 0., 1.], + ) + .build(&mut services.objects) + .insert(&mut services.objects); + let mut curve = PartialCurve::default(); + curve.update_as_line_from_points([[1., 1.], [1., 2.]]); + let curve = curve + .build(&mut services.objects) + .insert(&mut services.objects); + let global_curve = GlobalCurve.insert(&mut services.objects); + let range = RangeOnPath::from([[0.], [1.]]); + + let approx = (&curve, surface.deref(), global_curve, range).approx(1.); + + assert_eq!(approx, CurveApprox::empty()); + } + + #[test] + fn approx_line_on_curved_surface_along_curve() { + let mut services = Services::new(); + + let path = GlobalPath::circle_from_radius(1.); + let surface = PartialSurface::from_axes(path, [0., 0., 1.]) + .build(&mut services.objects) + .insert(&mut services.objects); + let mut curve = PartialCurve::default(); + curve.update_as_line_from_points([[0., 1.], [1., 1.]]); + let curve = curve + .build(&mut services.objects) + .insert(&mut services.objects); + let global_curve = GlobalCurve.insert(&mut services.objects); + + let range = RangeOnPath::from([[0.], [TAU]]); + let tolerance = 1.; + + let approx = + (&curve, surface.deref(), global_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.geometry().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 mut services = Services::new(); + + let surface = services.objects.surfaces.xz_plane(); + let mut curve = PartialCurve::default(); + curve.update_as_circle_from_radius(1.); + let curve = curve + .build(&mut services.objects) + .insert(&mut services.objects); + let global_curve = GlobalCurve.insert(&mut services.objects); + + let range = RangeOnPath::from([[0.], [TAU]]); + let tolerance = 1.; + let approx = + (&curve, surface.deref(), global_curve, range).approx(tolerance); + + let expected_approx = (curve.path(), range) + .approx(tolerance) + .into_iter() + .map(|(_, point_surface)| { + let point_global = + surface.geometry().point_from_surface_coords(point_surface); + ApproxPoint::new(point_surface, point_global) + }) + .collect::>(); + assert_eq!(approx.points, expected_approx); + } +} From 3e9fe65f0ca49943637d248138baaf0e12585d3c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Feb 2023 14:47:39 +0100 Subject: [PATCH 03/25] Rewrite tests to approximate `HalfEdge` --- .../fj-kernel/src/algorithms/approx/edge.rs | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index ef4e8851a..320671e5c 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -80,11 +80,13 @@ mod tests { use crate::{ algorithms::approx::{path::RangeOnPath, Approx, ApproxPoint}, - builder::{CurveBuilder, SurfaceBuilder}, + builder::{CurveBuilder, HalfEdgeBuilder, SurfaceBuilder}, geometry::path::GlobalPath, insert::Insert, objects::GlobalCurve, - partial::{PartialCurve, PartialObject, PartialSurface}, + partial::{ + PartialCurve, PartialHalfEdge, PartialObject, PartialSurface, + }, services::Services, }; @@ -95,17 +97,20 @@ mod tests { let mut services = Services::new(); let surface = services.objects.surfaces.xz_plane(); - let mut curve = PartialCurve::default(); - curve.update_as_line_from_points([[1., 1.], [2., 1.]]); - let curve = curve - .build(&mut services.objects) - .insert(&mut services.objects); - let global_curve = GlobalCurve.insert(&mut services.objects); - let range = RangeOnPath::from([[0.], [1.]]); + let half_edge = { + let mut half_edge = PartialHalfEdge::default(); - let approx = (&curve, surface.deref(), global_curve, range).approx(1.); + half_edge.update_as_line_segment_from_points([[1., 1.], [2., 1.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); - assert_eq!(approx, CurveApprox::empty()); + half_edge + .build(&mut services.objects) + .insert(&mut services.objects) + }; + + let approx = (&half_edge, surface.deref()).approx(1.); + + assert_eq!(approx.curve_approx, CurveApprox::empty()); } #[test] @@ -118,17 +123,20 @@ mod tests { ) .build(&mut services.objects) .insert(&mut services.objects); - let mut curve = PartialCurve::default(); - curve.update_as_line_from_points([[1., 1.], [1., 2.]]); - let curve = curve - .build(&mut services.objects) - .insert(&mut services.objects); - let global_curve = GlobalCurve.insert(&mut services.objects); - let range = RangeOnPath::from([[0.], [1.]]); + let half_edge = { + let mut half_edge = PartialHalfEdge::default(); - let approx = (&curve, surface.deref(), global_curve, range).approx(1.); + half_edge.update_as_line_segment_from_points([[1., 1.], [2., 1.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); - assert_eq!(approx, CurveApprox::empty()); + half_edge + .build(&mut services.objects) + .insert(&mut services.objects) + }; + + let approx = (&half_edge, surface.deref()).approx(1.); + + assert_eq!(approx.curve_approx, CurveApprox::empty()); } #[test] From 703ecf7465ac9dc56dbb2429a4f2182edee0e81e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Feb 2023 14:52:07 +0100 Subject: [PATCH 04/25] Refactor --- crates/fj-kernel/src/algorithms/approx/edge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 320671e5c..26f2a1b46 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -171,7 +171,7 @@ mod tests { ApproxPoint::new(point_surface, point_global) }) .collect::>(); - assert_eq!(approx.points, expected_approx); + assert_eq!(approx, CurveApprox::empty().with_points(expected_approx)); } #[test] @@ -200,6 +200,6 @@ mod tests { ApproxPoint::new(point_surface, point_global) }) .collect::>(); - assert_eq!(approx.points, expected_approx); + assert_eq!(approx, CurveApprox::empty().with_points(expected_approx)); } } From e2428aadbfdf46027480abbe4e92edb467bf7a48 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Feb 2023 14:53:50 +0100 Subject: [PATCH 05/25] Refactor --- crates/fj-kernel/src/algorithms/approx/edge.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 26f2a1b46..e841e11ff 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -108,7 +108,8 @@ mod tests { .insert(&mut services.objects) }; - let approx = (&half_edge, surface.deref()).approx(1.); + let tolerance = 1.; + let approx = (&half_edge, surface.deref()).approx(tolerance); assert_eq!(approx.curve_approx, CurveApprox::empty()); } @@ -134,7 +135,8 @@ mod tests { .insert(&mut services.objects) }; - let approx = (&half_edge, surface.deref()).approx(1.); + let tolerance = 1.; + let approx = (&half_edge, surface.deref()).approx(tolerance); assert_eq!(approx.curve_approx, CurveApprox::empty()); } From 4d2c5e8e5834f6eff3939955cf5a144c489c2254 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Feb 2023 15:05:22 +0100 Subject: [PATCH 06/25] Refactor --- crates/fj-kernel/src/algorithms/approx/edge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index e841e11ff..3a21d66a0 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -146,6 +146,8 @@ mod tests { let mut services = Services::new(); let path = GlobalPath::circle_from_radius(1.); + let range = RangeOnPath::from([[0.], [TAU]]); + let surface = PartialSurface::from_axes(path, [0., 0., 1.]) .build(&mut services.objects) .insert(&mut services.objects); @@ -156,9 +158,7 @@ mod tests { .insert(&mut services.objects); let global_curve = GlobalCurve.insert(&mut services.objects); - let range = RangeOnPath::from([[0.], [TAU]]); let tolerance = 1.; - let approx = (&curve, surface.deref(), global_curve, range).approx(tolerance); From 4534580c9d4906bb5a818a0d1494abd1c2ccd3d3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Feb 2023 15:09:52 +0100 Subject: [PATCH 07/25] Fix word in doc comment --- crates/fj-kernel/src/algorithms/approx/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 3a21d66a0..1a69b455b 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -53,7 +53,7 @@ impl Approx for (&Handle, &Surface) { /// An approximation of an [`HalfEdge`] #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdgeApprox { - /// The point that approximates the first vertex of the curve + /// The point that approximates the first vertex of the edge pub first: ApproxPoint<2>, /// The approximation of the edge's curve From 7c38fc202367888072a6254c25edce8931508636 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 11:46:22 +0100 Subject: [PATCH 08/25] Rewrite tests to approximate `HalfEdge` --- .../fj-kernel/src/algorithms/approx/edge.rs | 86 +++++++++++-------- 1 file changed, 51 insertions(+), 35 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 1a69b455b..10a047db2 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -80,13 +80,10 @@ mod tests { use crate::{ algorithms::approx::{path::RangeOnPath, Approx, ApproxPoint}, - builder::{CurveBuilder, HalfEdgeBuilder, SurfaceBuilder}, + builder::{HalfEdgeBuilder, SurfaceBuilder}, geometry::path::GlobalPath, insert::Insert, - objects::GlobalCurve, - partial::{ - PartialCurve, PartialHalfEdge, PartialObject, PartialSurface, - }, + partial::{PartialHalfEdge, PartialObject, PartialSurface}, services::Services, }; @@ -151,29 +148,41 @@ mod tests { let surface = PartialSurface::from_axes(path, [0., 0., 1.]) .build(&mut services.objects) .insert(&mut services.objects); - let mut curve = PartialCurve::default(); - curve.update_as_line_from_points([[0., 1.], [1., 1.]]); - let curve = curve - .build(&mut services.objects) - .insert(&mut services.objects); - let global_curve = GlobalCurve.insert(&mut services.objects); + let half_edge = { + let mut half_edge = PartialHalfEdge::default(); + + half_edge.update_as_line_segment_from_points([[0., 1.], [1., 1.]]); + + half_edge.vertices[0].0 = Some(range.boundary[0]); + half_edge.vertices[1].0 = Some(range.boundary[1]); + + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); + + half_edge + .build(&mut services.objects) + .insert(&mut services.objects) + }; let tolerance = 1.; - let approx = - (&curve, surface.deref(), global_curve, range).approx(tolerance); + let approx = (&half_edge, surface.deref()).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_surface = half_edge + .curve() + .path() + .point_from_path_coords(point_local); let point_global = surface.geometry().point_from_surface_coords(point_surface); ApproxPoint::new(point_surface, point_global) }) .collect::>(); - assert_eq!(approx, CurveApprox::empty().with_points(expected_approx)); + assert_eq!( + approx.curve_approx, + CurveApprox::empty().with_points(expected_approx) + ); } #[test] @@ -181,27 +190,34 @@ mod tests { let mut services = Services::new(); let surface = services.objects.surfaces.xz_plane(); - let mut curve = PartialCurve::default(); - curve.update_as_circle_from_radius(1.); - let curve = curve - .build(&mut services.objects) - .insert(&mut services.objects); - let global_curve = GlobalCurve.insert(&mut services.objects); + let half_edge = { + let mut half_edge = PartialHalfEdge::default(); + + half_edge.update_as_circle_from_radius(1.); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); + + half_edge + .build(&mut services.objects) + .insert(&mut services.objects) + }; - let range = RangeOnPath::from([[0.], [TAU]]); let tolerance = 1.; - let approx = - (&curve, surface.deref(), global_curve, range).approx(tolerance); + let approx = (&half_edge, surface.deref()).approx(tolerance); - let expected_approx = (curve.path(), range) - .approx(tolerance) - .into_iter() - .map(|(_, point_surface)| { - let point_global = - surface.geometry().point_from_surface_coords(point_surface); - ApproxPoint::new(point_surface, point_global) - }) - .collect::>(); - assert_eq!(approx, CurveApprox::empty().with_points(expected_approx)); + let expected_approx = + (half_edge.curve().path(), RangeOnPath::from([[0.], [TAU]])) + .approx(tolerance) + .into_iter() + .map(|(_, point_surface)| { + let point_global = surface + .geometry() + .point_from_surface_coords(point_surface); + ApproxPoint::new(point_surface, point_global) + }) + .collect::>(); + assert_eq!( + approx.curve_approx, + CurveApprox::empty().with_points(expected_approx) + ); } } From f29ad93399ac18cc63b4b7886f20c7589289d948 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 11:52:04 +0100 Subject: [PATCH 09/25] Inline `Curve` approximation --- .../fj-kernel/src/algorithms/approx/curve.rs | 32 -------------- .../fj-kernel/src/algorithms/approx/edge.rs | 43 ++++++++++++++++--- 2 files changed, 36 insertions(+), 39 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index f367a1f14..8e057247d 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -19,38 +19,6 @@ use crate::{ use super::{path::RangeOnPath, Approx, ApproxPoint, Tolerance}; -impl Approx for (&Handle, &Surface, Handle, RangeOnPath) { - type Approximation = CurveApprox; - type Cache = CurveCache; - - fn approx_with_cache( - self, - tolerance: impl Into, - cache: &mut Self::Cache, - ) -> Self::Approximation { - let (curve, surface, global_curve, range) = self; - - let global_curve_approx = match cache.get(global_curve.clone(), range) { - Some(approx) => approx, - None => { - let approx = - approx_global_curve(curve, surface, range, tolerance); - cache.insert(global_curve, range, approx) - } - }; - - CurveApprox::empty().with_points( - 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.clone(), point.local_form)) - }), - ) - } -} - pub(super) fn approx_global_curve( curve: &Curve, surface: &Surface, diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 10a047db2..b51b91ae6 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -35,13 +35,42 @@ impl Approx for (&Handle, &Surface) { half_edge.start_vertex().global_form().position(), ) .with_source((half_edge.clone(), half_edge.boundary()[0])); - let curve_approx = ( - half_edge.curve(), - surface, - half_edge.global_form().curve().clone(), - range, - ) - .approx_with_cache(tolerance, cache); + + let curve_approx = { + let global_curve_approx = match cache + .get(half_edge.global_form().curve().clone().clone(), range) + { + Some(approx) => approx, + None => { + let approx = super::curve::approx_global_curve( + half_edge.curve(), + surface, + range, + tolerance, + ); + cache.insert( + half_edge.global_form().curve().clone(), + range, + approx, + ) + } + }; + + CurveApprox::empty().with_points( + global_curve_approx.points.into_iter().map(|point| { + let point_surface = half_edge + .curve() + .path() + .point_from_path_coords(point.local_form); + + ApproxPoint::new(point_surface, point.global_form) + .with_source(( + half_edge.curve().clone(), + point.local_form, + )) + }), + ) + }; HalfEdgeApprox { first, From 1809244fb74f5452cdec3f747392bba74a8cfdca Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 11:56:19 +0100 Subject: [PATCH 10/25] Move function to where it's used --- .../fj-kernel/src/algorithms/approx/curve.rs | 81 +----------------- .../fj-kernel/src/algorithms/approx/edge.rs | 85 ++++++++++++++++++- 2 files changed, 83 insertions(+), 83 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 8e057247d..5fcdb2933 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -12,88 +12,11 @@ use std::collections::BTreeMap; use crate::{ - geometry::path::{GlobalPath, SurfacePath}, - objects::{Curve, GlobalCurve, Surface}, + objects::GlobalCurve, storage::{Handle, ObjectId}, }; -use super::{path::RangeOnPath, Approx, ApproxPoint, Tolerance}; - -pub(super) fn approx_global_curve( - curve: &Curve, - surface: &Surface, - 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(), surface.geometry().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 = surface - .geometry() - .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 = (surface.geometry().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 = - surface.geometry().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 } -} +use super::{path::RangeOnPath, ApproxPoint}; /// An approximation of a [`Curve`] #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index b51b91ae6..2bcbca2e3 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -6,12 +6,13 @@ //! the caller doesn't have to call with duplicate vertices. use crate::{ - objects::{HalfEdge, Surface}, + geometry::path::{GlobalPath, SurfacePath}, + objects::{Curve, HalfEdge, Surface}, storage::Handle, }; use super::{ - curve::{CurveApprox, CurveCache}, + curve::{CurveApprox, CurveCache, GlobalCurveApprox}, path::RangeOnPath, Approx, ApproxPoint, Tolerance, }; @@ -38,11 +39,11 @@ impl Approx for (&Handle, &Surface) { let curve_approx = { let global_curve_approx = match cache - .get(half_edge.global_form().curve().clone().clone(), range) + .get(half_edge.global_form().curve().clone(), range) { Some(approx) => approx, None => { - let approx = super::curve::approx_global_curve( + let approx = approx_global_curve( half_edge.curve(), surface, range, @@ -101,6 +102,82 @@ impl HalfEdgeApprox { } } +fn approx_global_curve( + curve: &Curve, + surface: &Surface, + 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(), surface.geometry().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 = surface + .geometry() + .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 = (surface.geometry().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 = + surface.geometry().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 } +} + #[cfg(test)] mod tests { use std::{f64::consts::TAU, ops::Deref}; From e25c3ee6968b8130ed5cb78babe376b20303be9d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 11:57:44 +0100 Subject: [PATCH 11/25] Update function name --- crates/fj-kernel/src/algorithms/approx/edge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 2bcbca2e3..68ce4f163 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -43,7 +43,7 @@ impl Approx for (&Handle, &Surface) { { Some(approx) => approx, None => { - let approx = approx_global_curve( + let approx = approx_edge( half_edge.curve(), surface, range, @@ -102,7 +102,7 @@ impl HalfEdgeApprox { } } -fn approx_global_curve( +fn approx_edge( curve: &Curve, surface: &Surface, range: RangeOnPath, From 9ac29b061391624517a6d5b1759aa770cea17863 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 12:14:49 +0100 Subject: [PATCH 12/25] Merge `CurveApprox` into `HalfEdgeApprox` --- .../fj-kernel/src/algorithms/approx/curve.rs | 23 ------------- .../fj-kernel/src/algorithms/approx/edge.rs | 32 ++++++++----------- 2 files changed, 13 insertions(+), 42 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 5fcdb2933..1bf0bb0db 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -18,29 +18,6 @@ use crate::{ use super::{path::RangeOnPath, ApproxPoint}; -/// An approximation of a [`Curve`] -#[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct CurveApprox { - /// The points that approximate the curve - pub points: Vec>, -} - -impl CurveApprox { - /// Create an empty instance of `CurveApprox` - pub fn empty() -> Self { - Self { points: Vec::new() } - } - - /// Add points to the approximation - pub fn with_points( - mut self, - points: impl IntoIterator>, - ) -> Self { - self.points.extend(points); - self - } -} - /// A cache for results of an approximation #[derive(Default)] pub struct CurveCache { diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 68ce4f163..bc0eb1757 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -12,7 +12,7 @@ use crate::{ }; use super::{ - curve::{CurveApprox, CurveCache, GlobalCurveApprox}, + curve::{CurveCache, GlobalCurveApprox}, path::RangeOnPath, Approx, ApproxPoint, Tolerance, }; @@ -57,8 +57,10 @@ impl Approx for (&Handle, &Surface) { } }; - CurveApprox::empty().with_points( - global_curve_approx.points.into_iter().map(|point| { + global_curve_approx + .points + .into_iter() + .map(|point| { let point_surface = half_edge .curve() .path() @@ -69,8 +71,8 @@ impl Approx for (&Handle, &Surface) { half_edge.curve().clone(), point.local_form, )) - }), - ) + }) + .collect() }; HalfEdgeApprox { @@ -87,7 +89,7 @@ pub struct HalfEdgeApprox { pub first: ApproxPoint<2>, /// The approximation of the edge's curve - pub curve_approx: CurveApprox, + pub curve_approx: Vec>, } impl HalfEdgeApprox { @@ -96,7 +98,7 @@ impl HalfEdgeApprox { let mut points = Vec::new(); points.push(self.first.clone()); - points.extend(self.curve_approx.points.clone()); + points.extend(self.curve_approx.clone()); points } @@ -193,8 +195,6 @@ mod tests { services::Services, }; - use super::CurveApprox; - #[test] fn approx_line_on_flat_surface() { let mut services = Services::new(); @@ -214,7 +214,7 @@ mod tests { let tolerance = 1.; let approx = (&half_edge, surface.deref()).approx(tolerance); - assert_eq!(approx.curve_approx, CurveApprox::empty()); + assert_eq!(approx.curve_approx, Vec::new()); } #[test] @@ -241,7 +241,7 @@ mod tests { let tolerance = 1.; let approx = (&half_edge, surface.deref()).approx(tolerance); - assert_eq!(approx.curve_approx, CurveApprox::empty()); + assert_eq!(approx.curve_approx, Vec::new()); } #[test] @@ -285,10 +285,7 @@ mod tests { ApproxPoint::new(point_surface, point_global) }) .collect::>(); - assert_eq!( - approx.curve_approx, - CurveApprox::empty().with_points(expected_approx) - ); + assert_eq!(approx.curve_approx, expected_approx); } #[test] @@ -321,9 +318,6 @@ mod tests { ApproxPoint::new(point_surface, point_global) }) .collect::>(); - assert_eq!( - approx.curve_approx, - CurveApprox::empty().with_points(expected_approx) - ); + assert_eq!(approx.curve_approx, expected_approx); } } From f28c2655ae2e0c94a07df533ed92a40ae06f04b7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:31:02 +0100 Subject: [PATCH 13/25] Remove `fmt::Display` impl for `HalfEdge` It doesn't seem to be used anywhere, and it's getting in the way of my ongoing cleanup work. --- crates/fj-kernel/src/objects/full/edge.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index c25653416..7ff6415a6 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -1,5 +1,3 @@ -use std::fmt; - use fj_interop::ext::ArrayExt; use fj_math::Point; @@ -95,16 +93,6 @@ impl HalfEdge { } } -impl fmt::Display for HalfEdge { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let [a, b] = self.boundary(); - write!(f, "edge from {a:?} to {b:?}")?; - write!(f, " on {:?}", self.global_form().curve())?; - - Ok(()) - } -} - /// An undirected edge, defined in global (3D) coordinates /// /// In contrast to [`HalfEdge`], `GlobalEdge` is undirected, meaning it has no From 087401bf2193fefe2044eadf989bcfa7038e37b8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:34:31 +0100 Subject: [PATCH 14/25] Move approximation caching code to `edge` module --- .../fj-kernel/src/algorithms/approx/curve.rs | 76 ------------------- .../fj-kernel/src/algorithms/approx/cycle.rs | 3 +- .../fj-kernel/src/algorithms/approx/edge.rs | 69 +++++++++++++++-- .../fj-kernel/src/algorithms/approx/face.rs | 2 +- crates/fj-kernel/src/algorithms/approx/mod.rs | 1 - .../fj-kernel/src/algorithms/approx/shell.rs | 2 +- .../fj-kernel/src/algorithms/approx/sketch.rs | 2 +- .../fj-kernel/src/algorithms/approx/solid.rs | 2 +- 8 files changed, 68 insertions(+), 89 deletions(-) delete mode 100644 crates/fj-kernel/src/algorithms/approx/curve.rs diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs deleted file mode 100644 index 1bf0bb0db..000000000 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ /dev/null @@ -1,76 +0,0 @@ -//! Curve approximation -//! -//! Since curves 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 std::collections::BTreeMap; - -use crate::{ - objects::GlobalCurve, - storage::{Handle, ObjectId}, -}; - -use super::{path::RangeOnPath, ApproxPoint}; - -/// A cache for results of an approximation -#[derive(Default)] -pub struct CurveCache { - inner: BTreeMap<(ObjectId, RangeOnPath), GlobalCurveApprox>, -} - -impl CurveCache { - /// Create an empty cache - pub fn new() -> Self { - Self::default() - } - - /// Insert the approximation of a [`GlobalCurve`] - pub fn insert( - &mut self, - handle: Handle, - range: RangeOnPath, - approx: GlobalCurveApprox, - ) -> GlobalCurveApprox { - self.inner.insert((handle.id(), range), approx.clone()); - approx - } - - /// Access the approximation for the given [`GlobalCurve`], if available - pub fn get( - &self, - handle: Handle, - range: RangeOnPath, - ) -> Option { - if let Some(approx) = self.inner.get(&(handle.id(), range)) { - return Some(approx.clone()); - } - if let Some(approx) = self.inner.get(&(handle.id(), range.reverse())) { - // If we have a cache entry for the reverse range, we need to use - // that too! - return Some(approx.clone().reverse()); - } - - None - } -} - -/// 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>, -} - -impl GlobalCurveApprox { - /// Reverse the order of the approximation - pub fn reverse(mut self) -> Self { - self.points.reverse(); - self - } -} diff --git a/crates/fj-kernel/src/algorithms/approx/cycle.rs b/crates/fj-kernel/src/algorithms/approx/cycle.rs index 02c404311..51b364830 100644 --- a/crates/fj-kernel/src/algorithms/approx/cycle.rs +++ b/crates/fj-kernel/src/algorithms/approx/cycle.rs @@ -7,7 +7,8 @@ use fj_math::Segment; use crate::objects::{Cycle, Surface}; use super::{ - curve::CurveCache, edge::HalfEdgeApprox, Approx, ApproxPoint, Tolerance, + edge::{CurveCache, HalfEdgeApprox}, + Approx, ApproxPoint, Tolerance, }; impl Approx for (&Cycle, &Surface) { diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index bc0eb1757..87a2aa3c0 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -5,17 +5,15 @@ //! approximations are usually used to build cycle approximations, and this way, //! the caller doesn't have to call with duplicate vertices. +use std::collections::BTreeMap; + use crate::{ geometry::path::{GlobalPath, SurfacePath}, - objects::{Curve, HalfEdge, Surface}, - storage::Handle, + objects::{Curve, GlobalCurve, HalfEdge, Surface}, + storage::{Handle, ObjectId}, }; -use super::{ - curve::{CurveCache, GlobalCurveApprox}, - path::RangeOnPath, - Approx, ApproxPoint, Tolerance, -}; +use super::{path::RangeOnPath, Approx, ApproxPoint, Tolerance}; impl Approx for (&Handle, &Surface) { type Approximation = HalfEdgeApprox; @@ -180,6 +178,63 @@ fn approx_edge( GlobalCurveApprox { points } } +/// A cache for results of an approximation +#[derive(Default)] +pub struct CurveCache { + inner: BTreeMap<(ObjectId, RangeOnPath), GlobalCurveApprox>, +} + +impl CurveCache { + /// Create an empty cache + pub fn new() -> Self { + Self::default() + } + + /// Insert the approximation of a [`GlobalCurve`] + pub fn insert( + &mut self, + handle: Handle, + range: RangeOnPath, + approx: GlobalCurveApprox, + ) -> GlobalCurveApprox { + self.inner.insert((handle.id(), range), approx.clone()); + approx + } + + /// Access the approximation for the given [`GlobalCurve`], if available + pub fn get( + &self, + handle: Handle, + range: RangeOnPath, + ) -> Option { + if let Some(approx) = self.inner.get(&(handle.id(), range)) { + return Some(approx.clone()); + } + if let Some(approx) = self.inner.get(&(handle.id(), range.reverse())) { + // If we have a cache entry for the reverse range, we need to use + // that too! + return Some(approx.clone().reverse()); + } + + None + } +} + +/// 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>, +} + +impl GlobalCurveApprox { + /// Reverse the order of the approximation + pub fn reverse(mut self) -> Self { + self.points.reverse(); + self + } +} + #[cfg(test)] mod tests { use std::{f64::consts::TAU, ops::Deref}; diff --git a/crates/fj-kernel/src/algorithms/approx/face.rs b/crates/fj-kernel/src/algorithms/approx/face.rs index aa99ea0a9..b81ff580b 100644 --- a/crates/fj-kernel/src/algorithms/approx/face.rs +++ b/crates/fj-kernel/src/algorithms/approx/face.rs @@ -12,7 +12,7 @@ use crate::{ }; use super::{ - curve::CurveCache, cycle::CycleApprox, Approx, ApproxPoint, Tolerance, + cycle::CycleApprox, edge::CurveCache, Approx, ApproxPoint, Tolerance, }; impl Approx for &FaceSet { diff --git a/crates/fj-kernel/src/algorithms/approx/mod.rs b/crates/fj-kernel/src/algorithms/approx/mod.rs index 85dcb1d0a..cc8ae0208 100644 --- a/crates/fj-kernel/src/algorithms/approx/mod.rs +++ b/crates/fj-kernel/src/algorithms/approx/mod.rs @@ -1,6 +1,5 @@ //! Approximation of objects -pub mod curve; pub mod cycle; pub mod edge; pub mod face; diff --git a/crates/fj-kernel/src/algorithms/approx/shell.rs b/crates/fj-kernel/src/algorithms/approx/shell.rs index f3262c1d2..942d13a0f 100644 --- a/crates/fj-kernel/src/algorithms/approx/shell.rs +++ b/crates/fj-kernel/src/algorithms/approx/shell.rs @@ -4,7 +4,7 @@ use std::collections::BTreeSet; use crate::objects::Shell; -use super::{curve::CurveCache, face::FaceApprox, Approx, Tolerance}; +use super::{edge::CurveCache, face::FaceApprox, Approx, Tolerance}; impl Approx for &Shell { type Approximation = BTreeSet; diff --git a/crates/fj-kernel/src/algorithms/approx/sketch.rs b/crates/fj-kernel/src/algorithms/approx/sketch.rs index 4cf8e89df..0510cc167 100644 --- a/crates/fj-kernel/src/algorithms/approx/sketch.rs +++ b/crates/fj-kernel/src/algorithms/approx/sketch.rs @@ -4,7 +4,7 @@ use std::collections::BTreeSet; use crate::objects::Sketch; -use super::{curve::CurveCache, face::FaceApprox, Approx, Tolerance}; +use super::{edge::CurveCache, face::FaceApprox, Approx, Tolerance}; impl Approx for &Sketch { type Approximation = BTreeSet; diff --git a/crates/fj-kernel/src/algorithms/approx/solid.rs b/crates/fj-kernel/src/algorithms/approx/solid.rs index 2077c5666..8995057c7 100644 --- a/crates/fj-kernel/src/algorithms/approx/solid.rs +++ b/crates/fj-kernel/src/algorithms/approx/solid.rs @@ -4,7 +4,7 @@ use std::collections::BTreeSet; use crate::objects::Solid; -use super::{curve::CurveCache, face::FaceApprox, Approx, Tolerance}; +use super::{edge::CurveCache, face::FaceApprox, Approx, Tolerance}; impl Approx for &Solid { type Approximation = BTreeSet; From e7d8b24b6b4e6426ca083796bde2663020857d97 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:36:56 +0100 Subject: [PATCH 15/25] Use `GlobalEdge` as approximation cache key --- crates/fj-kernel/src/algorithms/approx/edge.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 87a2aa3c0..5ef3a7082 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -9,7 +9,7 @@ use std::collections::BTreeMap; use crate::{ geometry::path::{GlobalPath, SurfacePath}, - objects::{Curve, GlobalCurve, HalfEdge, Surface}, + objects::{Curve, GlobalEdge, HalfEdge, Surface}, storage::{Handle, ObjectId}, }; @@ -37,7 +37,7 @@ impl Approx for (&Handle, &Surface) { let curve_approx = { let global_curve_approx = match cache - .get(half_edge.global_form().curve().clone(), range) + .get(half_edge.global_form().clone(), range) { Some(approx) => approx, None => { @@ -47,11 +47,7 @@ impl Approx for (&Handle, &Surface) { range, tolerance, ); - cache.insert( - half_edge.global_form().curve().clone(), - range, - approx, - ) + cache.insert(half_edge.global_form().clone(), range, approx) } }; @@ -193,7 +189,7 @@ impl CurveCache { /// Insert the approximation of a [`GlobalCurve`] pub fn insert( &mut self, - handle: Handle, + handle: Handle, range: RangeOnPath, approx: GlobalCurveApprox, ) -> GlobalCurveApprox { @@ -204,7 +200,7 @@ impl CurveCache { /// Access the approximation for the given [`GlobalCurve`], if available pub fn get( &self, - handle: Handle, + handle: Handle, range: RangeOnPath, ) -> Option { if let Some(approx) = self.inner.get(&(handle.id(), range)) { From 073531e5f3230184b7ad349a296df1c04175faaf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:37:55 +0100 Subject: [PATCH 16/25] Update name of `GlobalEdgeApprox` --- crates/fj-kernel/src/algorithms/approx/edge.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 5ef3a7082..84ead283e 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -103,7 +103,7 @@ fn approx_edge( surface: &Surface, range: RangeOnPath, tolerance: impl Into, -) -> GlobalCurveApprox { +) -> GlobalEdgeApprox { // 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. // @@ -171,13 +171,13 @@ fn approx_edge( ApproxPoint::new(point_curve, point_global) }) .collect(); - GlobalCurveApprox { points } + GlobalEdgeApprox { points } } /// A cache for results of an approximation #[derive(Default)] pub struct CurveCache { - inner: BTreeMap<(ObjectId, RangeOnPath), GlobalCurveApprox>, + inner: BTreeMap<(ObjectId, RangeOnPath), GlobalEdgeApprox>, } impl CurveCache { @@ -191,8 +191,8 @@ impl CurveCache { &mut self, handle: Handle, range: RangeOnPath, - approx: GlobalCurveApprox, - ) -> GlobalCurveApprox { + approx: GlobalEdgeApprox, + ) -> GlobalEdgeApprox { self.inner.insert((handle.id(), range), approx.clone()); approx } @@ -202,7 +202,7 @@ impl CurveCache { &self, handle: Handle, range: RangeOnPath, - ) -> Option { + ) -> Option { if let Some(approx) = self.inner.get(&(handle.id(), range)) { return Some(approx.clone()); } @@ -218,12 +218,12 @@ impl CurveCache { /// An approximation of a [`GlobalCurve`] #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct GlobalCurveApprox { +pub struct GlobalEdgeApprox { /// The points that approximate the curve pub points: Vec>, } -impl GlobalCurveApprox { +impl GlobalEdgeApprox { /// Reverse the order of the approximation pub fn reverse(mut self) -> Self { self.points.reverse(); From 5f2156cfa77885d8b03b33e1b9de97d40f61f041 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:39:24 +0100 Subject: [PATCH 17/25] Update name of `EdgeCache` --- crates/fj-kernel/src/algorithms/approx/cycle.rs | 4 ++-- crates/fj-kernel/src/algorithms/approx/edge.rs | 6 +++--- crates/fj-kernel/src/algorithms/approx/face.rs | 6 +++--- crates/fj-kernel/src/algorithms/approx/shell.rs | 4 ++-- crates/fj-kernel/src/algorithms/approx/sketch.rs | 4 ++-- crates/fj-kernel/src/algorithms/approx/solid.rs | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/cycle.rs b/crates/fj-kernel/src/algorithms/approx/cycle.rs index 51b364830..5b8b54da0 100644 --- a/crates/fj-kernel/src/algorithms/approx/cycle.rs +++ b/crates/fj-kernel/src/algorithms/approx/cycle.rs @@ -7,13 +7,13 @@ use fj_math::Segment; use crate::objects::{Cycle, Surface}; use super::{ - edge::{CurveCache, HalfEdgeApprox}, + edge::{EdgeCache, HalfEdgeApprox}, Approx, ApproxPoint, Tolerance, }; impl Approx for (&Cycle, &Surface) { type Approximation = CycleApprox; - type Cache = CurveCache; + type Cache = EdgeCache; fn approx_with_cache( self, diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 84ead283e..cebf72bfb 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -17,7 +17,7 @@ use super::{path::RangeOnPath, Approx, ApproxPoint, Tolerance}; impl Approx for (&Handle, &Surface) { type Approximation = HalfEdgeApprox; - type Cache = CurveCache; + type Cache = EdgeCache; fn approx_with_cache( self, @@ -176,11 +176,11 @@ fn approx_edge( /// A cache for results of an approximation #[derive(Default)] -pub struct CurveCache { +pub struct EdgeCache { inner: BTreeMap<(ObjectId, RangeOnPath), GlobalEdgeApprox>, } -impl CurveCache { +impl EdgeCache { /// Create an empty cache pub fn new() -> Self { Self::default() diff --git a/crates/fj-kernel/src/algorithms/approx/face.rs b/crates/fj-kernel/src/algorithms/approx/face.rs index b81ff580b..d9baff01c 100644 --- a/crates/fj-kernel/src/algorithms/approx/face.rs +++ b/crates/fj-kernel/src/algorithms/approx/face.rs @@ -12,12 +12,12 @@ use crate::{ }; use super::{ - cycle::CycleApprox, edge::CurveCache, Approx, ApproxPoint, Tolerance, + cycle::CycleApprox, edge::EdgeCache, Approx, ApproxPoint, Tolerance, }; impl Approx for &FaceSet { type Approximation = BTreeSet; - type Cache = CurveCache; + type Cache = EdgeCache; fn approx_with_cache( self, @@ -65,7 +65,7 @@ impl Approx for &FaceSet { impl Approx for &Face { type Approximation = FaceApprox; - type Cache = CurveCache; + type Cache = EdgeCache; fn approx_with_cache( self, diff --git a/crates/fj-kernel/src/algorithms/approx/shell.rs b/crates/fj-kernel/src/algorithms/approx/shell.rs index 942d13a0f..0b9e3601d 100644 --- a/crates/fj-kernel/src/algorithms/approx/shell.rs +++ b/crates/fj-kernel/src/algorithms/approx/shell.rs @@ -4,11 +4,11 @@ use std::collections::BTreeSet; use crate::objects::Shell; -use super::{edge::CurveCache, face::FaceApprox, Approx, Tolerance}; +use super::{edge::EdgeCache, face::FaceApprox, Approx, Tolerance}; impl Approx for &Shell { type Approximation = BTreeSet; - type Cache = CurveCache; + type Cache = EdgeCache; fn approx_with_cache( self, diff --git a/crates/fj-kernel/src/algorithms/approx/sketch.rs b/crates/fj-kernel/src/algorithms/approx/sketch.rs index 0510cc167..873257a01 100644 --- a/crates/fj-kernel/src/algorithms/approx/sketch.rs +++ b/crates/fj-kernel/src/algorithms/approx/sketch.rs @@ -4,11 +4,11 @@ use std::collections::BTreeSet; use crate::objects::Sketch; -use super::{edge::CurveCache, face::FaceApprox, Approx, Tolerance}; +use super::{edge::EdgeCache, face::FaceApprox, Approx, Tolerance}; impl Approx for &Sketch { type Approximation = BTreeSet; - type Cache = CurveCache; + type Cache = EdgeCache; fn approx_with_cache( self, diff --git a/crates/fj-kernel/src/algorithms/approx/solid.rs b/crates/fj-kernel/src/algorithms/approx/solid.rs index 8995057c7..4aff45ecb 100644 --- a/crates/fj-kernel/src/algorithms/approx/solid.rs +++ b/crates/fj-kernel/src/algorithms/approx/solid.rs @@ -4,11 +4,11 @@ use std::collections::BTreeSet; use crate::objects::Solid; -use super::{edge::CurveCache, face::FaceApprox, Approx, Tolerance}; +use super::{edge::EdgeCache, face::FaceApprox, Approx, Tolerance}; impl Approx for &Solid { type Approximation = BTreeSet; - type Cache = CurveCache; + type Cache = EdgeCache; fn approx_with_cache( self, From 69f5e092cab591d872b024f675aa2c5c1e5141d7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:40:30 +0100 Subject: [PATCH 18/25] Update name of struct field --- .../fj-kernel/src/algorithms/approx/edge.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index cebf72bfb..2636a5ed2 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -35,7 +35,7 @@ impl Approx for (&Handle, &Surface) { ) .with_source((half_edge.clone(), half_edge.boundary()[0])); - let curve_approx = { + let points = { let global_curve_approx = match cache .get(half_edge.global_form().clone(), range) { @@ -69,10 +69,7 @@ impl Approx for (&Handle, &Surface) { .collect() }; - HalfEdgeApprox { - first, - curve_approx, - } + HalfEdgeApprox { first, points } } } @@ -83,7 +80,7 @@ pub struct HalfEdgeApprox { pub first: ApproxPoint<2>, /// The approximation of the edge's curve - pub curve_approx: Vec>, + pub points: Vec>, } impl HalfEdgeApprox { @@ -92,7 +89,7 @@ impl HalfEdgeApprox { let mut points = Vec::new(); points.push(self.first.clone()); - points.extend(self.curve_approx.clone()); + points.extend(self.points.clone()); points } @@ -265,7 +262,7 @@ mod tests { let tolerance = 1.; let approx = (&half_edge, surface.deref()).approx(tolerance); - assert_eq!(approx.curve_approx, Vec::new()); + assert_eq!(approx.points, Vec::new()); } #[test] @@ -292,7 +289,7 @@ mod tests { let tolerance = 1.; let approx = (&half_edge, surface.deref()).approx(tolerance); - assert_eq!(approx.curve_approx, Vec::new()); + assert_eq!(approx.points, Vec::new()); } #[test] @@ -336,7 +333,7 @@ mod tests { ApproxPoint::new(point_surface, point_global) }) .collect::>(); - assert_eq!(approx.curve_approx, expected_approx); + assert_eq!(approx.points, expected_approx); } #[test] @@ -369,6 +366,6 @@ mod tests { ApproxPoint::new(point_surface, point_global) }) .collect::>(); - assert_eq!(approx.curve_approx, expected_approx); + assert_eq!(approx.points, expected_approx); } } From 52816b6c964ae150a2da2bd20ea08872d827274e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:41:15 +0100 Subject: [PATCH 19/25] Avoid useless allocation --- crates/fj-kernel/src/algorithms/approx/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 2636a5ed2..7ba30ba55 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -89,7 +89,7 @@ impl HalfEdgeApprox { let mut points = Vec::new(); points.push(self.first.clone()); - points.extend(self.points.clone()); + points.extend(self.points.iter().cloned()); points } From 6c717920d07dda1a21fc2ed39d55c255c7e0a2e7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:42:21 +0100 Subject: [PATCH 20/25] Simplify variable name --- crates/fj-kernel/src/algorithms/approx/edge.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 7ba30ba55..b43a881f0 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -36,8 +36,7 @@ impl Approx for (&Handle, &Surface) { .with_source((half_edge.clone(), half_edge.boundary()[0])); let points = { - let global_curve_approx = match cache - .get(half_edge.global_form().clone(), range) + let approx = match cache.get(half_edge.global_form().clone(), range) { Some(approx) => approx, None => { @@ -51,7 +50,7 @@ impl Approx for (&Handle, &Surface) { } }; - global_curve_approx + approx .points .into_iter() .map(|point| { From 2e8320699bea53f7a90d5df126cb9e9e9dfbc54c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:43:29 +0100 Subject: [PATCH 21/25] Update doc comments --- crates/fj-kernel/src/algorithms/approx/edge.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index b43a881f0..db834a594 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -78,7 +78,7 @@ pub struct HalfEdgeApprox { /// The point that approximates the first vertex of the edge pub first: ApproxPoint<2>, - /// The approximation of the edge's curve + /// The approximation of the edge pub points: Vec>, } @@ -182,7 +182,7 @@ impl EdgeCache { Self::default() } - /// Insert the approximation of a [`GlobalCurve`] + /// Insert the approximation of a [`GlobalEdge`] pub fn insert( &mut self, handle: Handle, @@ -193,7 +193,7 @@ impl EdgeCache { approx } - /// Access the approximation for the given [`GlobalCurve`], if available + /// Access the approximation for the given [`GlobalEdge`], if available pub fn get( &self, handle: Handle, @@ -212,10 +212,10 @@ impl EdgeCache { } } -/// An approximation of a [`GlobalCurve`] +/// An approximation of a [`GlobalEdge`] #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct GlobalEdgeApprox { - /// The points that approximate the curve + /// The points that approximate the edge pub points: Vec>, } From abfc6793740c877219992c4d42b56883dbcf5f51 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:45:54 +0100 Subject: [PATCH 22/25] Remove `GlobalCurve` reference from `GlobalEdge` --- .../fj-kernel/src/algorithms/sweep/vertex.rs | 7 ++----- .../fj-kernel/src/algorithms/transform/edge.rs | 6 +----- crates/fj-kernel/src/builder/edge.rs | 3 --- crates/fj-kernel/src/objects/full/edge.rs | 18 ++++-------------- crates/fj-kernel/src/partial/objects/edge.rs | 12 ++---------- 5 files changed, 9 insertions(+), 37 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index 4bf5f662a..05ae32dcd 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -2,7 +2,7 @@ use fj_math::Vector; use crate::{ insert::Insert, - objects::{GlobalCurve, GlobalEdge, GlobalVertex, Objects}, + objects::{GlobalEdge, GlobalVertex, Objects}, services::Service, storage::Handle, }; @@ -18,8 +18,6 @@ impl Sweep for Handle { cache: &mut SweepCache, objects: &mut Service, ) -> Self::Swept { - let curve = GlobalCurve.insert(objects); - let a = self.clone(); let b = cache .global_vertex @@ -30,8 +28,7 @@ impl Sweep for Handle { .clone(); let vertices = [a, b]; - let global_edge = - GlobalEdge::new(curve, vertices.clone()).insert(objects); + let global_edge = GlobalEdge::new(vertices.clone()).insert(objects); // The vertices of the returned `GlobalEdge` are in normalized order, // which means the order can't be relied upon by the caller. Return the diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 4c5b7b242..1b3be7b98 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -43,15 +43,11 @@ impl TransformObject for GlobalEdge { objects: &mut Service, cache: &mut TransformCache, ) -> Self { - let curve = self - .curve() - .clone() - .transform_with_cache(transform, objects, cache); let vertices = self.vertices().access_in_normalized_order().map(|vertex| { vertex.transform_with_cache(transform, objects, cache) }); - Self::new(curve, vertices) + Self::new(vertices) } } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index eacb8d1eb..a740408b3 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -223,9 +223,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { other: &Partial, surface: &SurfaceGeometry, ) { - let global_curve = other.read().global_form.read().curve.clone(); - self.global_form.write().curve = global_curve; - self.curve.write().path = other.read().curve.read().path.as_ref().and_then(|path| { // We have information about the other edge's surface available. diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 7ff6415a6..0f5025e13 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -2,8 +2,8 @@ use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ - objects::{Curve, GlobalCurve, GlobalVertex, SurfaceVertex}, - storage::{Handle, HandleWrapper}, + objects::{Curve, GlobalVertex, SurfaceVertex}, + storage::Handle, }; /// A directed edge, defined in a surface's 2D space @@ -104,7 +104,6 @@ impl HalfEdge { /// between [`HalfEdge`] and `GlobalEdge`. #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct GlobalEdge { - curve: HandleWrapper, vertices: VerticesInNormalizedOrder, } @@ -114,19 +113,10 @@ impl GlobalEdge { /// The order of `vertices` is irrelevant. Two `GlobalEdge`s with the same /// `curve` and `vertices` will end up being equal, regardless of the order /// of `vertices` here. - pub fn new( - curve: impl Into>, - vertices: [Handle; 2], - ) -> Self { - let curve = curve.into(); + pub fn new(vertices: [Handle; 2]) -> Self { let (vertices, _) = VerticesInNormalizedOrder::new(vertices); - Self { curve, vertices } - } - - /// Access the curve that defines the edge's geometry - pub fn curve(&self) -> &Handle { - &self.curve + Self { vertices } } /// Access the vertices that bound the edge on the curve diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index ada1c7338..ffaece70d 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -5,8 +5,7 @@ use fj_math::Point; use crate::{ objects::{ - Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, - SurfaceVertex, + Curve, GlobalEdge, GlobalVertex, HalfEdge, Objects, SurfaceVertex, }, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, @@ -84,7 +83,6 @@ impl Default for PartialHalfEdge { let global_form = Partial::from_partial(PartialGlobalEdge { vertices: global_vertices, - ..Default::default() }); Self { @@ -98,9 +96,6 @@ impl Default for PartialHalfEdge { /// A partial [`GlobalEdge`] #[derive(Clone, Debug, Default)] pub struct PartialGlobalEdge { - /// The curve that defines the edge's geometry - pub curve: Partial, - /// The vertices that bound the edge on the curve pub vertices: [Partial; 2], } @@ -113,7 +108,6 @@ impl PartialObject for PartialGlobalEdge { cache: &mut FullToPartialCache, ) -> Self { Self { - curve: Partial::from_full(global_edge.curve().clone(), cache), vertices: global_edge .vertices() .access_in_normalized_order() @@ -122,9 +116,7 @@ impl PartialObject for PartialGlobalEdge { } fn build(self, objects: &mut Service) -> Self::Full { - let curve = self.curve.build(objects); let vertices = self.vertices.map(|vertex| vertex.build(objects)); - - GlobalEdge::new(curve, vertices) + GlobalEdge::new(vertices) } } From a6fbe517138f9cabab66c0f08e712c264dcafb58 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:50:32 +0100 Subject: [PATCH 23/25] Remove outdated remark from doc comment --- crates/fj-kernel/src/geometry/path.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/geometry/path.rs b/crates/fj-kernel/src/geometry/path.rs index f84b143b2..c8de63351 100644 --- a/crates/fj-kernel/src/geometry/path.rs +++ b/crates/fj-kernel/src/geometry/path.rs @@ -6,10 +6,8 @@ //! //! This is a bit of an in-between module. It is closely associated with curves //! ([`Curve`]/[`GlobalCurve`]) and [`Surface`]s, but paths are not really -//! objects themselves, as logically speaking, they are owned and not referenced -//! (practically speaking, all objects are owned and not referenced, but that is -//! an implementation detail; see [#1021] for context on where things are -//! going). +//! objects themselves, as logically speaking, they are owned and not +//! referenced. //! //! On the other hand, the types in this module don't follow the general style //! of types in `fj-math`. @@ -20,7 +18,6 @@ //! [`Curve`]: crate::objects::Curve //! [`GlobalCurve`]: crate::objects::GlobalCurve //! [`Surface`]: crate::objects::Surface -//! [#1021]: https://github.com/hannobraun/Fornjot/issues/1021 use fj_math::{Circle, Line, Point, Scalar, Transform, Vector}; From e271693846f322b27b34a2dca7a33f55ed208f09 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:51:10 +0100 Subject: [PATCH 24/25] Don't mention `GlobalCurve` in doc comment It's no longer used and I'm about to remove it. --- crates/fj-kernel/src/geometry/path.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/geometry/path.rs b/crates/fj-kernel/src/geometry/path.rs index c8de63351..cf85a2d73 100644 --- a/crates/fj-kernel/src/geometry/path.rs +++ b/crates/fj-kernel/src/geometry/path.rs @@ -4,10 +4,9 @@ //! //! # Implementation Note //! -//! This is a bit of an in-between module. It is closely associated with curves -//! ([`Curve`]/[`GlobalCurve`]) and [`Surface`]s, but paths are not really -//! objects themselves, as logically speaking, they are owned and not -//! referenced. +//! This is a bit of an in-between module. It is closely associated with +//! [`Curve`] and [`Surface`]s, but paths are not really objects themselves, as +//! logically speaking, they are owned and not referenced. //! //! On the other hand, the types in this module don't follow the general style //! of types in `fj-math`. @@ -16,7 +15,6 @@ //! move to `fj-math`, maybe something else entirely will happen. //! //! [`Curve`]: crate::objects::Curve -//! [`GlobalCurve`]: crate::objects::GlobalCurve //! [`Surface`]: crate::objects::Surface use fj_math::{Circle, Line, Point, Scalar, Transform, Vector}; From d3d74884298437c9f4f61634c631d17c275fa5f8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 13:52:55 +0100 Subject: [PATCH 25/25] Remove unused code --- .../src/algorithms/transform/curve.rs | 17 +-------------- crates/fj-kernel/src/insert.rs | 5 ++--- crates/fj-kernel/src/objects/full/curve.rs | 4 ---- crates/fj-kernel/src/objects/mod.rs | 2 +- crates/fj-kernel/src/objects/object.rs | 5 ++--- crates/fj-kernel/src/objects/stores.rs | 7 ++----- crates/fj-kernel/src/partial/mod.rs | 2 +- crates/fj-kernel/src/partial/objects/curve.rs | 18 +--------------- crates/fj-kernel/src/partial/traits.rs | 1 - crates/fj-kernel/src/validate/curve.rs | 11 +--------- crates/fj-kernel/src/validate/edge.rs | 21 +------------------ 11 files changed, 12 insertions(+), 81 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/transform/curve.rs b/crates/fj-kernel/src/algorithms/transform/curve.rs index 27489c417..b12f0c1d8 100644 --- a/crates/fj-kernel/src/algorithms/transform/curve.rs +++ b/crates/fj-kernel/src/algorithms/transform/curve.rs @@ -1,7 +1,7 @@ use fj_math::Transform; use crate::{ - objects::{Curve, GlobalCurve, Objects}, + objects::{Curve, Objects}, services::Service, }; @@ -21,18 +21,3 @@ impl TransformObject for Curve { Self::new(path) } } - -impl TransformObject for GlobalCurve { - fn transform_with_cache( - self, - _: &Transform, - _: &mut Service, - _: &mut TransformCache, - ) -> Self { - // `GlobalCurve` doesn't contain any internal geometry. If it did, that - // would just be redundant with the geometry of other objects, and this - // other geometry is already being transformed by other implementations - // of this trait. - self - } -} diff --git a/crates/fj-kernel/src/insert.rs b/crates/fj-kernel/src/insert.rs index 98831ad34..8cd1d03bc 100644 --- a/crates/fj-kernel/src/insert.rs +++ b/crates/fj-kernel/src/insert.rs @@ -4,8 +4,8 @@ use crate::{ objects::{ - Curve, Cycle, Face, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, - Objects, Shell, Sketch, Solid, Surface, SurfaceVertex, + Curve, Cycle, Face, GlobalEdge, GlobalVertex, HalfEdge, Objects, Shell, + Sketch, Solid, Surface, SurfaceVertex, }, services::{Service, ServiceObjectsExt}, storage::Handle, @@ -37,7 +37,6 @@ impl_insert!( Curve, curves; Cycle, cycles; Face, faces; - GlobalCurve, global_curves; GlobalEdge, global_edges; GlobalVertex, global_vertices; HalfEdge, half_edges; diff --git a/crates/fj-kernel/src/objects/full/curve.rs b/crates/fj-kernel/src/objects/full/curve.rs index 4bce36c06..c291ce6ff 100644 --- a/crates/fj-kernel/src/objects/full/curve.rs +++ b/crates/fj-kernel/src/objects/full/curve.rs @@ -17,7 +17,3 @@ impl Curve { self.path } } - -/// A curve, defined in global (3D) coordinates -#[derive(Clone, Copy, Debug)] -pub struct GlobalCurve; diff --git a/crates/fj-kernel/src/objects/mod.rs b/crates/fj-kernel/src/objects/mod.rs index 9fe7e2f9d..e957c10ab 100644 --- a/crates/fj-kernel/src/objects/mod.rs +++ b/crates/fj-kernel/src/objects/mod.rs @@ -79,7 +79,7 @@ mod stores; pub use self::{ full::{ - curve::{Curve, GlobalCurve}, + curve::Curve, cycle::{Cycle, HalfEdgesOfCycle}, edge::{GlobalEdge, HalfEdge, VerticesInNormalizedOrder}, face::{Face, FaceSet, Handedness}, diff --git a/crates/fj-kernel/src/objects/object.rs b/crates/fj-kernel/src/objects/object.rs index 2bf627d87..869403ea3 100644 --- a/crates/fj-kernel/src/objects/object.rs +++ b/crates/fj-kernel/src/objects/object.rs @@ -2,8 +2,8 @@ use std::any::Any; use crate::{ objects::{ - Curve, Cycle, Face, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, - Objects, Shell, Sketch, Solid, Surface, SurfaceVertex, + Curve, Cycle, Face, GlobalEdge, GlobalVertex, HalfEdge, Objects, Shell, + Sketch, Solid, Surface, SurfaceVertex, }, storage::{Handle, ObjectId}, validate::{Validate, ValidationError}, @@ -111,7 +111,6 @@ object!( Curve, "curve", curves; Cycle, "cycle", cycles; Face, "face", faces; - GlobalCurve, "global curve", global_curves; GlobalEdge, "global edge", global_edges; GlobalVertex, "global vertex", global_vertices; HalfEdge, "half-edge", half_edges; diff --git a/crates/fj-kernel/src/objects/stores.rs b/crates/fj-kernel/src/objects/stores.rs index 73e3c7df8..bcb2dcd88 100644 --- a/crates/fj-kernel/src/objects/stores.rs +++ b/crates/fj-kernel/src/objects/stores.rs @@ -6,8 +6,8 @@ use crate::{ }; use super::{ - Curve, Cycle, Face, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Shell, - Sketch, Solid, Surface, SurfaceVertex, + Curve, Cycle, Face, GlobalEdge, GlobalVertex, HalfEdge, Shell, Sketch, + Solid, Surface, SurfaceVertex, }; /// The available object stores @@ -22,9 +22,6 @@ pub struct Objects { /// Store for [`Face`]s pub faces: Store, - /// Store for [`GlobalCurve`]s - pub global_curves: Store, - /// Store for [`GlobalEdge`]s pub global_edges: Store, diff --git a/crates/fj-kernel/src/partial/mod.rs b/crates/fj-kernel/src/partial/mod.rs index f6209de04..cb6469170 100644 --- a/crates/fj-kernel/src/partial/mod.rs +++ b/crates/fj-kernel/src/partial/mod.rs @@ -16,7 +16,7 @@ mod wrapper; pub use self::{ objects::{ - curve::{MaybeSurfacePath, PartialCurve, PartialGlobalCurve}, + curve::{MaybeSurfacePath, PartialCurve}, cycle::PartialCycle, edge::{PartialGlobalEdge, PartialHalfEdge}, face::PartialFace, diff --git a/crates/fj-kernel/src/partial/objects/curve.rs b/crates/fj-kernel/src/partial/objects/curve.rs index 7f3174d40..baa43c241 100644 --- a/crates/fj-kernel/src/partial/objects/curve.rs +++ b/crates/fj-kernel/src/partial/objects/curve.rs @@ -2,7 +2,7 @@ use fj_math::Scalar; use crate::{ geometry::path::SurfacePath, - objects::{Curve, GlobalCurve, Objects}, + objects::{Curve, Objects}, partial::{FullToPartialCache, PartialObject}, services::Service, }; @@ -61,19 +61,3 @@ impl From for MaybeSurfacePath { Self::Defined(path) } } - -/// A partial [`GlobalCurve`] -#[derive(Clone, Debug, Default)] -pub struct PartialGlobalCurve; - -impl PartialObject for PartialGlobalCurve { - type Full = GlobalCurve; - - fn from_full(_: &Self::Full, _: &mut FullToPartialCache) -> Self { - Self - } - - fn build(self, _: &mut Service) -> Self::Full { - GlobalCurve - } -} diff --git a/crates/fj-kernel/src/partial/traits.rs b/crates/fj-kernel/src/partial/traits.rs index 2b14305c3..33da816c3 100644 --- a/crates/fj-kernel/src/partial/traits.rs +++ b/crates/fj-kernel/src/partial/traits.rs @@ -36,7 +36,6 @@ impl_trait!( Curve, PartialCurve; Cycle, PartialCycle; Face, PartialFace; - GlobalCurve, PartialGlobalCurve; GlobalEdge, PartialGlobalEdge; GlobalVertex, PartialGlobalVertex; HalfEdge, PartialHalfEdge; diff --git a/crates/fj-kernel/src/validate/curve.rs b/crates/fj-kernel/src/validate/curve.rs index 250eaea47..2f40a7820 100644 --- a/crates/fj-kernel/src/validate/curve.rs +++ b/crates/fj-kernel/src/validate/curve.rs @@ -1,4 +1,4 @@ -use crate::objects::{Curve, GlobalCurve}; +use crate::objects::Curve; use super::{Validate, ValidationConfig, ValidationError}; @@ -10,12 +10,3 @@ impl Validate for Curve { ) { } } - -impl Validate for GlobalCurve { - fn validate_with_config( - &self, - _: &ValidationConfig, - _: &mut Vec, - ) { - } -} diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 224512fc9..811358843 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -1,7 +1,7 @@ use fj_math::{Point, Scalar}; use crate::{ - objects::{GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface}, + objects::{GlobalEdge, GlobalVertex, HalfEdge, Surface}, storage::Handle, }; @@ -30,25 +30,6 @@ impl Validate for GlobalEdge { /// [`HalfEdge`] validation failed #[derive(Clone, Debug, thiserror::Error)] pub enum HalfEdgeValidationError { - /// [`HalfEdge`]'s [`GlobalCurve`]s do not match - #[error( - "Global form of `HalfEdge`'s `Curve` does not match `GlobalCurve` of \n\ - the `HalfEdge`'s `GlobalEdge`\n\ - - `GlobalCurve` from `Curve`: {global_curve_from_curve:#?}\n\ - - `GlobalCurve` from `GlobalEdge`: {global_curve_from_global_form:#?}\n\ - - `HalfEdge`: {half_edge:#?}", - )] - GlobalCurveMismatch { - /// The [`GlobalCurve`] from the [`HalfEdge`]'s `Curve` - global_curve_from_curve: Handle, - - /// The [`GlobalCurve`] from the [`HalfEdge`]'s global form - global_curve_from_global_form: Handle, - - /// The half-edge - half_edge: HalfEdge, - }, - /// [`HalfEdge`]'s [`GlobalVertex`] objects do not match #[error( "Global forms of `HalfEdge` vertices do not match vertices of \n\