From c15a51a1135a045ed10e7ca19564d11734b6f7c5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 09:52:16 +0200 Subject: [PATCH 01/14] Update doc comment --- crates/fj-core/src/geometry/boundary.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index c38a02563..4819e2266 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -9,6 +9,8 @@ pub struct BoundaryOnCurve { impl BoundaryOnCurve { /// Reverse the direction of the boundary + /// + /// Returns a new instance of this struct, which has its direction reversed. pub fn reverse(self) -> Self { let [a, b] = self.inner; Self { inner: [b, a] } From d01feb69057ca4ebeac70d8e934f8be5bc3e9299 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 09:53:00 +0200 Subject: [PATCH 02/14] Mark `BoundaryOnCurve::reverse` as `#[must_use]` This can help prevent mistakes, since the method does not modify the existing instance. --- crates/fj-core/src/geometry/boundary.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 4819e2266..ea12bf386 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -11,6 +11,7 @@ impl BoundaryOnCurve { /// Reverse the direction of the boundary /// /// Returns a new instance of this struct, which has its direction reversed. + #[must_use] pub fn reverse(self) -> Self { let [a, b] = self.inner; Self { inner: [b, a] } From eca9926650a0d1a4c4b9c499d01591355b55f68a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 09:56:23 +0200 Subject: [PATCH 03/14] Make `BoundaryOnCurve` generic This is preparation for merging `BoundingVertices` into it. --- crates/fj-core/src/algorithms/approx/edge.rs | 8 ++++---- crates/fj-core/src/algorithms/approx/path.rs | 10 +++++----- crates/fj-core/src/geometry/boundary.rs | 8 ++++---- crates/fj-core/src/objects/kinds/edge.rs | 6 +++--- crates/fj-core/src/operations/build/edge.rs | 2 +- crates/fj-core/src/operations/join/cycle.rs | 5 +++-- 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index de5f1e9d8..c8cc73c63 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -141,7 +141,7 @@ impl HalfEdgeApprox { fn approx_edge( path: &SurfacePath, surface: &Surface, - boundary: BoundaryOnCurve, + boundary: BoundaryOnCurve>, tolerance: impl Into, ) -> GlobalEdgeApprox { // There are different cases of varying complexity. Circles are the hard @@ -218,7 +218,7 @@ fn approx_edge( #[derive(Default)] pub struct EdgeCache { edge_approx: BTreeMap< - (HandleWrapper, BoundaryOnCurve), + (HandleWrapper, BoundaryOnCurve>), GlobalEdgeApprox, >, vertex_approx: BTreeMap, Point<3>>, @@ -234,7 +234,7 @@ impl EdgeCache { fn get_edge( &self, handle: Handle, - boundary: BoundaryOnCurve, + boundary: BoundaryOnCurve>, ) -> Option { if let Some(approx) = self.edge_approx.get(&(handle.clone().into(), boundary)) @@ -256,7 +256,7 @@ impl EdgeCache { fn insert_edge( &mut self, handle: Handle, - boundary: BoundaryOnCurve, + boundary: BoundaryOnCurve>, approx: GlobalEdgeApprox, ) -> GlobalEdgeApprox { self.edge_approx diff --git a/crates/fj-core/src/algorithms/approx/path.rs b/crates/fj-core/src/algorithms/approx/path.rs index b06415248..96c0bd5dc 100644 --- a/crates/fj-core/src/algorithms/approx/path.rs +++ b/crates/fj-core/src/algorithms/approx/path.rs @@ -36,7 +36,7 @@ use crate::geometry::{BoundaryOnCurve, GlobalPath, SurfacePath}; use super::{Approx, Tolerance}; -impl Approx for (&SurfacePath, BoundaryOnCurve) { +impl Approx for (&SurfacePath, BoundaryOnCurve>) { type Approximation = Vec<(Point<1>, Point<2>)>; type Cache = (); @@ -56,7 +56,7 @@ impl Approx for (&SurfacePath, BoundaryOnCurve) { } } -impl Approx for (GlobalPath, BoundaryOnCurve) { +impl Approx for (GlobalPath, BoundaryOnCurve>) { type Approximation = Vec<(Point<1>, Point<3>)>; type Cache = (); @@ -82,7 +82,7 @@ impl Approx for (GlobalPath, BoundaryOnCurve) { /// from the circle. fn approx_circle( circle: &Circle, - boundary: impl Into, + boundary: impl Into>>, tolerance: Tolerance, ) -> Vec<(Point<1>, Point)> { let boundary = boundary.into(); @@ -127,7 +127,7 @@ impl PathApproxParams { pub fn points( &self, - boundary: impl Into, + boundary: impl Into>>, ) -> impl Iterator> + '_ { let boundary = boundary.into(); @@ -220,7 +220,7 @@ mod tests { test_path([[TAU - 2.], [0.]], [2., 1.]); fn test_path( - boundary: impl Into, + boundary: impl Into>>, expected_coords: impl IntoIterator>, ) { // Choose radius and tolerance such, that we need 4 vertices to diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index ea12bf386..6418d77ee 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -2,12 +2,12 @@ use fj_math::Point; /// A boundary on a curve #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct BoundaryOnCurve { +pub struct BoundaryOnCurve { /// The raw representation of the boundary - pub inner: [Point<1>; 2], + pub inner: [T; 2], } -impl BoundaryOnCurve { +impl BoundaryOnCurve { /// Reverse the direction of the boundary /// /// Returns a new instance of this struct, which has its direction reversed. @@ -18,7 +18,7 @@ impl BoundaryOnCurve { } } -impl From<[T; 2]> for BoundaryOnCurve +impl From<[T; 2]> for BoundaryOnCurve> where T: Into>, { diff --git a/crates/fj-core/src/objects/kinds/edge.rs b/crates/fj-core/src/objects/kinds/edge.rs index 153f82ac2..fe4264a8b 100644 --- a/crates/fj-core/src/objects/kinds/edge.rs +++ b/crates/fj-core/src/objects/kinds/edge.rs @@ -41,7 +41,7 @@ use crate::{ #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { path: SurfacePath, - boundary: BoundaryOnCurve, + boundary: BoundaryOnCurve>, curve: HandleWrapper, start_vertex: HandleWrapper, global_form: HandleWrapper, @@ -51,7 +51,7 @@ impl HalfEdge { /// Create an instance of `HalfEdge` pub fn new( path: SurfacePath, - boundary: impl Into, + boundary: impl Into>>, curve: Handle, start_vertex: Handle, global_form: Handle, @@ -71,7 +71,7 @@ impl HalfEdge { } /// Access the boundary points of the half-edge on the curve - pub fn boundary(&self) -> BoundaryOnCurve { + pub fn boundary(&self) -> BoundaryOnCurve> { self.boundary } diff --git a/crates/fj-core/src/operations/build/edge.rs b/crates/fj-core/src/operations/build/edge.rs index 3552ada11..2548d860d 100644 --- a/crates/fj-core/src/operations/build/edge.rs +++ b/crates/fj-core/src/operations/build/edge.rs @@ -13,7 +13,7 @@ pub trait BuildHalfEdge { /// Create a half-edge that is not joined to another fn unjoined( path: SurfacePath, - boundary: impl Into, + boundary: impl Into>>, services: &mut Services, ) -> HalfEdge { let curve = Curve::new().insert(services); diff --git a/crates/fj-core/src/operations/join/cycle.rs b/crates/fj-core/src/operations/join/cycle.rs index 316880f52..b0e3a7dcb 100644 --- a/crates/fj-core/src/operations/join/cycle.rs +++ b/crates/fj-core/src/operations/join/cycle.rs @@ -1,5 +1,6 @@ use std::ops::RangeInclusive; +use fj_math::Point; use itertools::Itertools; use crate::{ @@ -17,7 +18,7 @@ pub trait JoinCycle { fn add_joined_edges(&self, edges: Es, services: &mut Services) -> Self where Es: IntoIterator< - Item = (Handle, SurfacePath, BoundaryOnCurve), + Item = (Handle, SurfacePath, BoundaryOnCurve>), >, Es::IntoIter: Clone + ExactSizeIterator; @@ -64,7 +65,7 @@ impl JoinCycle for Cycle { fn add_joined_edges(&self, edges: Es, services: &mut Services) -> Self where Es: IntoIterator< - Item = (Handle, SurfacePath, BoundaryOnCurve), + Item = (Handle, SurfacePath, BoundaryOnCurve>), >, Es::IntoIter: Clone + ExactSizeIterator, { From 91375e5aafc0aafdbaa22e4e73cc0e5b97f4725a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 09:58:00 +0200 Subject: [PATCH 04/14] Make `From` implementation more general --- crates/fj-core/src/geometry/boundary.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 6418d77ee..a1e92bc63 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -1,5 +1,3 @@ -use fj_math::Point; - /// A boundary on a curve #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct BoundaryOnCurve { @@ -18,11 +16,11 @@ impl BoundaryOnCurve { } } -impl From<[T; 2]> for BoundaryOnCurve> +impl From<[S; 2]> for BoundaryOnCurve where - T: Into>, + S: Into, { - fn from(boundary: [T; 2]) -> Self { + fn from(boundary: [S; 2]) -> Self { let inner = boundary.map(Into::into); Self { inner } } From e9e2a8e42df479a13e7b4bb0bf2fcf910421f910 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 09:59:24 +0200 Subject: [PATCH 05/14] Update doc comment --- crates/fj-core/src/geometry/boundary.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index a1e92bc63..62bf32003 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -1,4 +1,8 @@ /// A boundary on a curve +/// +/// This struct is generic, because different situations require different +/// representations of a boundary. In some cases, curve coordinates are enough, +/// in other cases, vertices are required, and sometimes you need both. #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct BoundaryOnCurve { /// The raw representation of the boundary From d50cea5d49fa2db6c669877bcb686a9400fe1bd8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 10:03:11 +0200 Subject: [PATCH 06/14] Add `BoundaryOnCurve::normalize` This is preparation for merging `BoundingVertices` into `BoundaryOnCurve`. --- crates/fj-core/src/geometry/boundary.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 62bf32003..71c003e8e 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -18,6 +18,20 @@ impl BoundaryOnCurve { let [a, b] = self.inner; Self { inner: [b, a] } } + + /// Normalize the boundary + /// + /// Returns a new instance of this struct, which has the bounding elements + /// in a defined order. This can be used to compare a boundary while + /// disregarding its direction. + #[must_use] + pub fn normalize(mut self) -> Self + where + T: Ord, + { + self.inner.sort(); + self + } } impl From<[S; 2]> for BoundaryOnCurve From 4fb9f5d2168e0b30118aaeef85be8cd18d93eb34 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 10:05:10 +0200 Subject: [PATCH 07/14] Replace `BoundingVertices` with `BoundaryOnCurve` --- crates/fj-core/src/queries.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/fj-core/src/queries.rs b/crates/fj-core/src/queries.rs index a0116aa36..ac1320d9c 100644 --- a/crates/fj-core/src/queries.rs +++ b/crates/fj-core/src/queries.rs @@ -10,9 +10,9 @@ //! them for various objects that have the information to answer the query. use crate::{ - geometry::BoundingVertices, - objects::{Cycle, Face, HalfEdge, Region, Shell}, - storage::Handle, + geometry::BoundaryOnCurve, + objects::{Cycle, Face, HalfEdge, Region, Shell, Vertex}, + storage::{Handle, HandleWrapper}, }; /// Determine the bounding vertices of an edge @@ -24,18 +24,18 @@ pub trait BoundingVerticesOfEdge { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option; + ) -> Option>>; } impl BoundingVerticesOfEdge for Cycle { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option { + ) -> Option>> { let start = edge.start_vertex().clone(); let end = self.half_edge_after(edge)?.start_vertex().clone(); - Some(BoundingVertices::from([start, end])) + Some(BoundaryOnCurve::from([start, end])) } } @@ -43,7 +43,7 @@ impl BoundingVerticesOfEdge for Region { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option { + ) -> Option>> { for cycle in self.all_cycles() { if let Some(vertices) = cycle.bounding_vertices_of_edge(edge) { return Some(vertices); @@ -58,7 +58,7 @@ impl BoundingVerticesOfEdge for Face { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option { + ) -> Option>> { self.region().bounding_vertices_of_edge(edge) } } @@ -67,7 +67,7 @@ impl BoundingVerticesOfEdge for Shell { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option { + ) -> Option>> { for face in self.faces() { if let Some(vertices) = face.bounding_vertices_of_edge(edge) { return Some(vertices); From 07dd7948dd3e8827ff9f988ad7bdcacb6170c69e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 10:06:33 +0200 Subject: [PATCH 08/14] Remove `BoundingVertices` --- .../fj-core/src/geometry/bounding_vertices.rs | 31 ------------------- crates/fj-core/src/geometry/mod.rs | 2 -- 2 files changed, 33 deletions(-) delete mode 100644 crates/fj-core/src/geometry/bounding_vertices.rs diff --git a/crates/fj-core/src/geometry/bounding_vertices.rs b/crates/fj-core/src/geometry/bounding_vertices.rs deleted file mode 100644 index fd6afb6d4..000000000 --- a/crates/fj-core/src/geometry/bounding_vertices.rs +++ /dev/null @@ -1,31 +0,0 @@ -use crate::{ - objects::Vertex, - storage::{Handle, HandleWrapper}, -}; - -/// The bounding vertices of an edge -#[derive(Debug, Eq, PartialEq, Ord, PartialOrd)] -pub struct BoundingVertices { - /// The bounding vertices - pub inner: [HandleWrapper; 2], -} - -impl BoundingVertices { - /// Normalize the bounding vertices - /// - /// Returns a new instance of this struct, which has the vertices in a - /// defined order. This can be used to compare bounding vertices while - /// disregarding their order. - pub fn normalize(mut self) -> Self { - self.inner.sort(); - self - } -} - -impl From<[Handle; 2]> for BoundingVertices { - fn from(vertices: [Handle; 2]) -> Self { - Self { - inner: vertices.map(Into::into), - } - } -} diff --git a/crates/fj-core/src/geometry/mod.rs b/crates/fj-core/src/geometry/mod.rs index 732dbbb7f..cb5a1cb36 100644 --- a/crates/fj-core/src/geometry/mod.rs +++ b/crates/fj-core/src/geometry/mod.rs @@ -1,13 +1,11 @@ //! Types that are tied to objects, but aren't objects themselves mod boundary; -mod bounding_vertices; mod path; mod surface; pub use self::{ boundary::BoundaryOnCurve, - bounding_vertices::BoundingVertices, path::{GlobalPath, SurfacePath}, surface::SurfaceGeometry, }; From 2b571b9c137e0b686d0dff81aa2d2ae1e9c5a746 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 10:11:10 +0200 Subject: [PATCH 09/14] Rename `BoundaryOnCurve` to `CurveBoundary` The new name seems a bit simpler, but no less expressive. --- crates/fj-core/src/algorithms/approx/edge.rs | 18 +++++++++--------- crates/fj-core/src/algorithms/approx/path.rs | 14 +++++++------- crates/fj-core/src/geometry/boundary.rs | 6 +++--- crates/fj-core/src/geometry/mod.rs | 2 +- crates/fj-core/src/objects/kinds/edge.rs | 8 ++++---- crates/fj-core/src/operations/build/edge.rs | 4 ++-- crates/fj-core/src/operations/join/cycle.rs | 6 +++--- crates/fj-core/src/queries.rs | 14 +++++++------- 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index c8cc73c63..a66d02e78 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -10,7 +10,7 @@ use std::collections::BTreeMap; use fj_math::Point; use crate::{ - geometry::{BoundaryOnCurve, GlobalPath, SurfacePath}, + geometry::{CurveBoundary, GlobalPath, SurfacePath}, objects::{GlobalEdge, HalfEdge, Surface, Vertex}, storage::{Handle, HandleWrapper}, }; @@ -141,7 +141,7 @@ impl HalfEdgeApprox { fn approx_edge( path: &SurfacePath, surface: &Surface, - boundary: BoundaryOnCurve>, + boundary: CurveBoundary>, tolerance: impl Into, ) -> GlobalEdgeApprox { // There are different cases of varying complexity. Circles are the hard @@ -185,7 +185,7 @@ fn approx_edge( } (SurfacePath::Line(line), _) => { let range_u = - BoundaryOnCurve::from(boundary.inner.map(|point_curve| { + CurveBoundary::from(boundary.inner.map(|point_curve| { [path.point_from_path_coords(point_curve).u] })); @@ -218,7 +218,7 @@ fn approx_edge( #[derive(Default)] pub struct EdgeCache { edge_approx: BTreeMap< - (HandleWrapper, BoundaryOnCurve>), + (HandleWrapper, CurveBoundary>), GlobalEdgeApprox, >, vertex_approx: BTreeMap, Point<3>>, @@ -234,7 +234,7 @@ impl EdgeCache { fn get_edge( &self, handle: Handle, - boundary: BoundaryOnCurve>, + boundary: CurveBoundary>, ) -> Option { if let Some(approx) = self.edge_approx.get(&(handle.clone().into(), boundary)) @@ -256,7 +256,7 @@ impl EdgeCache { fn insert_edge( &mut self, handle: Handle, - boundary: BoundaryOnCurve>, + boundary: CurveBoundary>, approx: GlobalEdgeApprox, ) -> GlobalEdgeApprox { self.edge_approx @@ -302,7 +302,7 @@ mod tests { use crate::{ algorithms::approx::{Approx, ApproxPoint}, - geometry::{BoundaryOnCurve, GlobalPath, SurfaceGeometry}, + geometry::{CurveBoundary, GlobalPath, SurfaceGeometry}, objects::{HalfEdge, Surface}, operations::BuildHalfEdge, services::Services, @@ -344,7 +344,7 @@ mod tests { let mut services = Services::new(); let path = GlobalPath::circle_from_radius(1.); - let boundary = BoundaryOnCurve::from([[0.], [TAU]]); + let boundary = CurveBoundary::from([[0.], [TAU]]); let surface = Surface::new(SurfaceGeometry { u: path, @@ -384,7 +384,7 @@ mod tests { let approx = (&half_edge, surface.deref()).approx(tolerance); let expected_approx = - (&half_edge.path(), BoundaryOnCurve::from([[0.], [TAU]])) + (&half_edge.path(), CurveBoundary::from([[0.], [TAU]])) .approx(tolerance) .into_iter() .map(|(_, point_surface)| { diff --git a/crates/fj-core/src/algorithms/approx/path.rs b/crates/fj-core/src/algorithms/approx/path.rs index 96c0bd5dc..ad55f9eeb 100644 --- a/crates/fj-core/src/algorithms/approx/path.rs +++ b/crates/fj-core/src/algorithms/approx/path.rs @@ -32,11 +32,11 @@ use std::iter; use fj_math::{Circle, Point, Scalar, Sign}; -use crate::geometry::{BoundaryOnCurve, GlobalPath, SurfacePath}; +use crate::geometry::{CurveBoundary, GlobalPath, SurfacePath}; use super::{Approx, Tolerance}; -impl Approx for (&SurfacePath, BoundaryOnCurve>) { +impl Approx for (&SurfacePath, CurveBoundary>) { type Approximation = Vec<(Point<1>, Point<2>)>; type Cache = (); @@ -56,7 +56,7 @@ impl Approx for (&SurfacePath, BoundaryOnCurve>) { } } -impl Approx for (GlobalPath, BoundaryOnCurve>) { +impl Approx for (GlobalPath, CurveBoundary>) { type Approximation = Vec<(Point<1>, Point<3>)>; type Cache = (); @@ -82,7 +82,7 @@ impl Approx for (GlobalPath, BoundaryOnCurve>) { /// from the circle. fn approx_circle( circle: &Circle, - boundary: impl Into>>, + boundary: impl Into>>, tolerance: Tolerance, ) -> Vec<(Point<1>, Point)> { let boundary = boundary.into(); @@ -127,7 +127,7 @@ impl PathApproxParams { pub fn points( &self, - boundary: impl Into>>, + boundary: impl Into>>, ) -> impl Iterator> + '_ { let boundary = boundary.into(); @@ -170,7 +170,7 @@ mod tests { use fj_math::{Circle, Point, Scalar}; - use crate::algorithms::approx::{path::BoundaryOnCurve, Tolerance}; + use crate::algorithms::approx::{path::CurveBoundary, Tolerance}; use super::PathApproxParams; @@ -220,7 +220,7 @@ mod tests { test_path([[TAU - 2.], [0.]], [2., 1.]); fn test_path( - boundary: impl Into>>, + boundary: impl Into>>, expected_coords: impl IntoIterator>, ) { // Choose radius and tolerance such, that we need 4 vertices to diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 71c003e8e..206121f09 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -4,12 +4,12 @@ /// representations of a boundary. In some cases, curve coordinates are enough, /// in other cases, vertices are required, and sometimes you need both. #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct BoundaryOnCurve { +pub struct CurveBoundary { /// The raw representation of the boundary pub inner: [T; 2], } -impl BoundaryOnCurve { +impl CurveBoundary { /// Reverse the direction of the boundary /// /// Returns a new instance of this struct, which has its direction reversed. @@ -34,7 +34,7 @@ impl BoundaryOnCurve { } } -impl From<[S; 2]> for BoundaryOnCurve +impl From<[S; 2]> for CurveBoundary where S: Into, { diff --git a/crates/fj-core/src/geometry/mod.rs b/crates/fj-core/src/geometry/mod.rs index cb5a1cb36..0269b5fda 100644 --- a/crates/fj-core/src/geometry/mod.rs +++ b/crates/fj-core/src/geometry/mod.rs @@ -5,7 +5,7 @@ mod path; mod surface; pub use self::{ - boundary::BoundaryOnCurve, + boundary::CurveBoundary, path::{GlobalPath, SurfacePath}, surface::SurfaceGeometry, }; diff --git a/crates/fj-core/src/objects/kinds/edge.rs b/crates/fj-core/src/objects/kinds/edge.rs index fe4264a8b..419e2b2b6 100644 --- a/crates/fj-core/src/objects/kinds/edge.rs +++ b/crates/fj-core/src/objects/kinds/edge.rs @@ -1,7 +1,7 @@ use fj_math::Point; use crate::{ - geometry::{BoundaryOnCurve, SurfacePath}, + geometry::{CurveBoundary, SurfacePath}, objects::{Curve, Vertex}, storage::{Handle, HandleWrapper}, }; @@ -41,7 +41,7 @@ use crate::{ #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { path: SurfacePath, - boundary: BoundaryOnCurve>, + boundary: CurveBoundary>, curve: HandleWrapper, start_vertex: HandleWrapper, global_form: HandleWrapper, @@ -51,7 +51,7 @@ impl HalfEdge { /// Create an instance of `HalfEdge` pub fn new( path: SurfacePath, - boundary: impl Into>>, + boundary: impl Into>>, curve: Handle, start_vertex: Handle, global_form: Handle, @@ -71,7 +71,7 @@ impl HalfEdge { } /// Access the boundary points of the half-edge on the curve - pub fn boundary(&self) -> BoundaryOnCurve> { + pub fn boundary(&self) -> CurveBoundary> { self.boundary } diff --git a/crates/fj-core/src/operations/build/edge.rs b/crates/fj-core/src/operations/build/edge.rs index 2548d860d..0ffe35cff 100644 --- a/crates/fj-core/src/operations/build/edge.rs +++ b/crates/fj-core/src/operations/build/edge.rs @@ -2,7 +2,7 @@ use fj_interop::ext::ArrayExt; use fj_math::{Arc, Point, Scalar}; use crate::{ - geometry::{BoundaryOnCurve, SurfacePath}, + geometry::{CurveBoundary, SurfacePath}, objects::{Curve, GlobalEdge, HalfEdge, Vertex}, operations::Insert, services::Services, @@ -13,7 +13,7 @@ pub trait BuildHalfEdge { /// Create a half-edge that is not joined to another fn unjoined( path: SurfacePath, - boundary: impl Into>>, + boundary: impl Into>>, services: &mut Services, ) -> HalfEdge { let curve = Curve::new().insert(services); diff --git a/crates/fj-core/src/operations/join/cycle.rs b/crates/fj-core/src/operations/join/cycle.rs index b0e3a7dcb..44e5bbda6 100644 --- a/crates/fj-core/src/operations/join/cycle.rs +++ b/crates/fj-core/src/operations/join/cycle.rs @@ -4,7 +4,7 @@ use fj_math::Point; use itertools::Itertools; use crate::{ - geometry::{BoundaryOnCurve, SurfacePath}, + geometry::{CurveBoundary, SurfacePath}, objects::{Cycle, HalfEdge}, operations::{BuildHalfEdge, Insert, UpdateCycle, UpdateHalfEdge}, services::Services, @@ -18,7 +18,7 @@ pub trait JoinCycle { fn add_joined_edges(&self, edges: Es, services: &mut Services) -> Self where Es: IntoIterator< - Item = (Handle, SurfacePath, BoundaryOnCurve>), + Item = (Handle, SurfacePath, CurveBoundary>), >, Es::IntoIter: Clone + ExactSizeIterator; @@ -65,7 +65,7 @@ impl JoinCycle for Cycle { fn add_joined_edges(&self, edges: Es, services: &mut Services) -> Self where Es: IntoIterator< - Item = (Handle, SurfacePath, BoundaryOnCurve>), + Item = (Handle, SurfacePath, CurveBoundary>), >, Es::IntoIter: Clone + ExactSizeIterator, { diff --git a/crates/fj-core/src/queries.rs b/crates/fj-core/src/queries.rs index ac1320d9c..b58893027 100644 --- a/crates/fj-core/src/queries.rs +++ b/crates/fj-core/src/queries.rs @@ -10,7 +10,7 @@ //! them for various objects that have the information to answer the query. use crate::{ - geometry::BoundaryOnCurve, + geometry::CurveBoundary, objects::{Cycle, Face, HalfEdge, Region, Shell, Vertex}, storage::{Handle, HandleWrapper}, }; @@ -24,18 +24,18 @@ pub trait BoundingVerticesOfEdge { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>>; + ) -> Option>>; } impl BoundingVerticesOfEdge for Cycle { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>> { + ) -> Option>> { let start = edge.start_vertex().clone(); let end = self.half_edge_after(edge)?.start_vertex().clone(); - Some(BoundaryOnCurve::from([start, end])) + Some(CurveBoundary::from([start, end])) } } @@ -43,7 +43,7 @@ impl BoundingVerticesOfEdge for Region { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>> { + ) -> Option>> { for cycle in self.all_cycles() { if let Some(vertices) = cycle.bounding_vertices_of_edge(edge) { return Some(vertices); @@ -58,7 +58,7 @@ impl BoundingVerticesOfEdge for Face { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>> { + ) -> Option>> { self.region().bounding_vertices_of_edge(edge) } } @@ -67,7 +67,7 @@ impl BoundingVerticesOfEdge for Shell { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>> { + ) -> Option>> { for face in self.faces() { if let Some(vertices) = face.bounding_vertices_of_edge(edge) { return Some(vertices); From 151fba6045f0ba9378963d06b4df85312ec0173f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 10:14:24 +0200 Subject: [PATCH 10/14] Add trait bound for `CurveBoundary` type parameter This is preparation for making `CurveBoundary` more convenient to specify, by preventing implementation details from leaking into the type signature. --- crates/fj-core/src/geometry/boundary.rs | 34 ++++++++++++++++++++----- crates/fj-core/src/geometry/mod.rs | 2 +- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 206121f09..a97f94292 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -1,15 +1,19 @@ +use fj_math::Point; + +use crate::{objects::Vertex, storage::HandleWrapper}; + /// A boundary on a curve /// /// This struct is generic, because different situations require different /// representations of a boundary. In some cases, curve coordinates are enough, /// in other cases, vertices are required, and sometimes you need both. #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct CurveBoundary { +pub struct CurveBoundary { /// The raw representation of the boundary - pub inner: [T; 2], + pub inner: [T::Repr; 2], } -impl CurveBoundary { +impl CurveBoundary { /// Reverse the direction of the boundary /// /// Returns a new instance of this struct, which has its direction reversed. @@ -27,19 +31,37 @@ impl CurveBoundary { #[must_use] pub fn normalize(mut self) -> Self where - T: Ord, + T::Repr: Ord, { self.inner.sort(); self } } -impl From<[S; 2]> for CurveBoundary +impl From<[S; 2]> for CurveBoundary where - S: Into, + S: Into, { fn from(boundary: [S; 2]) -> Self { let inner = boundary.map(Into::into); Self { inner } } } + +/// An element of a curve boundary +/// +/// Used for the type parameter of [`CurveBoundary`]. +pub trait CurveBoundaryElement { + /// The representation the curve boundary element + /// + /// This is the actual data stored in [`CurveBoundary`]. + type Repr; +} + +impl CurveBoundaryElement for Point<1> { + type Repr = Self; +} + +impl CurveBoundaryElement for HandleWrapper { + type Repr = Self; +} diff --git a/crates/fj-core/src/geometry/mod.rs b/crates/fj-core/src/geometry/mod.rs index 0269b5fda..0ed948703 100644 --- a/crates/fj-core/src/geometry/mod.rs +++ b/crates/fj-core/src/geometry/mod.rs @@ -5,7 +5,7 @@ mod path; mod surface; pub use self::{ - boundary::CurveBoundary, + boundary::{CurveBoundary, CurveBoundaryElement}, path::{GlobalPath, SurfacePath}, surface::SurfaceGeometry, }; From 6522f10cc89cac83cb3ce73c89cb778a213c9578 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 10:16:44 +0200 Subject: [PATCH 11/14] Move trait bound to more convenient place --- crates/fj-core/src/geometry/boundary.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index a97f94292..941a7f37e 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -29,10 +29,7 @@ impl CurveBoundary { /// in a defined order. This can be used to compare a boundary while /// disregarding its direction. #[must_use] - pub fn normalize(mut self) -> Self - where - T::Repr: Ord, - { + pub fn normalize(mut self) -> Self { self.inner.sort(); self } @@ -55,7 +52,7 @@ pub trait CurveBoundaryElement { /// The representation the curve boundary element /// /// This is the actual data stored in [`CurveBoundary`]. - type Repr; + type Repr: Ord; } impl CurveBoundaryElement for Point<1> { From 12ef92382bced99b856536a9af48ddd308b9583a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 10:26:02 +0200 Subject: [PATCH 12/14] Add trait bounds to `CurveBoundaryElement::Repr` This is preparation for improving some of the trait implementations on `CurveBoundary`. --- crates/fj-core/src/geometry/boundary.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 941a7f37e..af0f16e4e 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -1,3 +1,5 @@ +use std::hash::Hash; + use fj_math::Point; use crate::{objects::Vertex, storage::HandleWrapper}; @@ -52,7 +54,7 @@ pub trait CurveBoundaryElement { /// The representation the curve boundary element /// /// This is the actual data stored in [`CurveBoundary`]. - type Repr: Ord; + type Repr: Eq + Hash + Ord; } impl CurveBoundaryElement for Point<1> { From f121cb8f5cacd2afd9fc5120284b8e241822ab45 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 10:27:13 +0200 Subject: [PATCH 13/14] Implement some traits on `CurveBoundary` manually Deriving doesn't work well with the associated type. It decides whether it should derive based on `T`, but whether it actually *can* derive is based on `T::Repr`. --- crates/fj-core/src/geometry/boundary.rs | 33 +++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index af0f16e4e..14f5fe20f 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -1,4 +1,7 @@ -use std::hash::Hash; +use std::{ + cmp::Ordering, + hash::{Hash, Hasher}, +}; use fj_math::Point; @@ -9,7 +12,7 @@ use crate::{objects::Vertex, storage::HandleWrapper}; /// This struct is generic, because different situations require different /// representations of a boundary. In some cases, curve coordinates are enough, /// in other cases, vertices are required, and sometimes you need both. -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +#[derive(Clone, Copy, Debug)] pub struct CurveBoundary { /// The raw representation of the boundary pub inner: [T::Repr; 2], @@ -37,6 +40,14 @@ impl CurveBoundary { } } +impl Eq for CurveBoundary {} + +impl PartialEq for CurveBoundary { + fn eq(&self, other: &Self) -> bool { + self.inner == other.inner + } +} + impl From<[S; 2]> for CurveBoundary where S: Into, @@ -47,6 +58,24 @@ where } } +impl Hash for CurveBoundary { + fn hash(&self, state: &mut H) { + self.inner.hash(state); + } +} + +impl Ord for CurveBoundary { + fn cmp(&self, other: &Self) -> Ordering { + self.inner.cmp(&other.inner) + } +} + +impl PartialOrd for CurveBoundary { + fn partial_cmp(&self, other: &Self) -> Option { + self.inner.partial_cmp(&other.inner) + } +} + /// An element of a curve boundary /// /// Used for the type parameter of [`CurveBoundary`]. From 147840054ab5520d2654770527f6d3c35e2ab70d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jul 2023 10:29:42 +0200 Subject: [PATCH 14/14] Don't leak `CurveBoundary` implementation details --- crates/fj-core/src/geometry/boundary.rs | 4 ++-- crates/fj-core/src/queries.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 14f5fe20f..bb5ed9935 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -90,6 +90,6 @@ impl CurveBoundaryElement for Point<1> { type Repr = Self; } -impl CurveBoundaryElement for HandleWrapper { - type Repr = Self; +impl CurveBoundaryElement for Vertex { + type Repr = HandleWrapper; } diff --git a/crates/fj-core/src/queries.rs b/crates/fj-core/src/queries.rs index b58893027..e4d880b1c 100644 --- a/crates/fj-core/src/queries.rs +++ b/crates/fj-core/src/queries.rs @@ -12,7 +12,7 @@ use crate::{ geometry::CurveBoundary, objects::{Cycle, Face, HalfEdge, Region, Shell, Vertex}, - storage::{Handle, HandleWrapper}, + storage::Handle, }; /// Determine the bounding vertices of an edge @@ -24,14 +24,14 @@ pub trait BoundingVerticesOfEdge { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>>; + ) -> Option>; } impl BoundingVerticesOfEdge for Cycle { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>> { + ) -> Option> { let start = edge.start_vertex().clone(); let end = self.half_edge_after(edge)?.start_vertex().clone(); @@ -43,7 +43,7 @@ impl BoundingVerticesOfEdge for Region { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>> { + ) -> Option> { for cycle in self.all_cycles() { if let Some(vertices) = cycle.bounding_vertices_of_edge(edge) { return Some(vertices); @@ -58,7 +58,7 @@ impl BoundingVerticesOfEdge for Face { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>> { + ) -> Option> { self.region().bounding_vertices_of_edge(edge) } } @@ -67,7 +67,7 @@ impl BoundingVerticesOfEdge for Shell { fn bounding_vertices_of_edge( &self, edge: &Handle, - ) -> Option>> { + ) -> Option> { for face in self.faces() { if let Some(vertices) = face.bounding_vertices_of_edge(edge) { return Some(vertices);