From 6cbca41cee504eca5ee0bae01381a8f5659ceebc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 15:59:10 +0200 Subject: [PATCH 01/13] Add `VerticesOfEdge` This new struct will be the basis for a more convenient API for manipulating those vertices. --- crates/fj-kernel/src/algorithms/approx/cycles.rs | 2 +- crates/fj-kernel/src/algorithms/sweep.rs | 1 + crates/fj-kernel/src/shape/object.rs | 2 +- crates/fj-kernel/src/shape/validate/geometric.rs | 8 ++++---- crates/fj-kernel/src/shape/validate/structural.rs | 2 +- crates/fj-kernel/src/topology/edge.rs | 12 ++++++++++-- crates/fj-kernel/src/topology/mod.rs | 2 +- crates/fj-operations/src/difference_2d.rs | 2 +- 8 files changed, 20 insertions(+), 11 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/cycles.rs b/crates/fj-kernel/src/algorithms/approx/cycles.rs index 9cba6c1cf..8d630a981 100644 --- a/crates/fj-kernel/src/algorithms/approx/cycles.rs +++ b/crates/fj-kernel/src/algorithms/approx/cycles.rs @@ -22,7 +22,7 @@ impl CycleApprox { for edge in cycle.edges() { let mut edge_points = Vec::new(); approx_curve(&edge.curve(), tolerance, &mut edge_points); - approximate_edge(edge.vertices, &mut edge_points); + approximate_edge(edge.vertices.0, &mut edge_points); points.extend(edge_points); } diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index 6396e32d5..449343120 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -213,6 +213,7 @@ fn create_side_edges( let [vertices_bottom, vertices_top] = [edge_bottom, edge_top].map(|edge| { edge.get() .vertices + .0 .expect("Expected vertices on non-continuous edge") }); diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index d84e3c818..77db0cb15 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -120,7 +120,7 @@ impl Object for Edge<3> { // Can be cleaned up using `try_map`, once that is stable: // https://doc.rust-lang.org/std/primitive.array.html#method.try_map let vertices: Option<[Result<_, ValidationError>; 2]> = - self.vertices.map(|vertices| { + self.vertices.0.map(|vertices| { vertices.map(|vertex| { let canonical = vertex.canonical(); let canonical = canonical.get().merge_into( diff --git a/crates/fj-kernel/src/shape/validate/geometric.rs b/crates/fj-kernel/src/shape/validate/geometric.rs index 508b85a7a..d9d747b4c 100644 --- a/crates/fj-kernel/src/shape/validate/geometric.rs +++ b/crates/fj-kernel/src/shape/validate/geometric.rs @@ -13,7 +13,7 @@ pub fn validate_edge( // Validate that the local and canonical forms of the vertices match. As a // side effect, this also happens to validate that the canonical forms of // the vertices lie on the curve. - if let Some(vertices) = &edge.vertices { + if let Some(vertices) = &edge.vertices.0 { let mut edge_vertex_mismatches = Vec::new(); for vertex in vertices { @@ -103,7 +103,7 @@ mod tests { use crate::{ shape::{LocalForm, Shape}, - topology::Edge, + topology::{Edge, VerticesOfEdge}, }; #[test] @@ -116,14 +116,14 @@ mod tests { .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]])? .get(); let edge = Edge { - vertices: edge.vertices.clone().map(|vertices| { + vertices: VerticesOfEdge(edge.vertices.0.clone().map(|vertices| { vertices.map(|vertex| { LocalForm::new( *vertex.local() + [deviation], vertex.canonical(), ) }) - }), + })), ..edge }; assert!(super::validate_edge(&edge, deviation * 2.).is_ok()); diff --git a/crates/fj-kernel/src/shape/validate/structural.rs b/crates/fj-kernel/src/shape/validate/structural.rs index 83a3f6b2d..e71c17fec 100644 --- a/crates/fj-kernel/src/shape/validate/structural.rs +++ b/crates/fj-kernel/src/shape/validate/structural.rs @@ -32,7 +32,7 @@ pub fn validate_edge( if !stores.curves.contains(&edge.curve.canonical()) { missing_curve = Some(edge.curve.canonical()); } - for vertices in &edge.vertices { + for vertices in &edge.vertices.0 { for vertex in vertices { if !stores.vertices.contains(&vertex.canonical()) { missing_vertices.insert(vertex.canonical().clone()); diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index 0f3289ead..a71629648 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -34,7 +34,7 @@ pub struct Edge { /// /// If there are no such vertices, that means that both the curve and the /// edge are continuous (i.e. connected to themselves). - pub vertices: Option<[LocalForm, Vertex>; 2]>, + pub vertices: VerticesOfEdge, } impl Edge<3> { @@ -44,7 +44,10 @@ impl Edge<3> { vertices: Option<[LocalForm, Vertex>; 2]>, ) -> Self { let curve = LocalForm::canonical_only(curve); - Self { curve, vertices } + Self { + curve, + vertices: VerticesOfEdge(vertices), + } } /// Build an edge using the [`EdgeBuilder`] API @@ -68,6 +71,7 @@ impl Edge { /// [`Handle`]s. pub fn vertices(&self) -> Option<[Vertex; 2]> { self.vertices + .0 .as_ref() .map(|[a, b]| [a.canonical().get(), b.canonical().get()]) } @@ -88,3 +92,7 @@ impl fmt::Display for Edge { Ok(()) } } + +/// The vertices that bound an edge +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct VerticesOfEdge(pub Option<[LocalForm, Vertex>; 2]>); diff --git a/crates/fj-kernel/src/topology/mod.rs b/crates/fj-kernel/src/topology/mod.rs index 2e45cae62..c2b70833d 100644 --- a/crates/fj-kernel/src/topology/mod.rs +++ b/crates/fj-kernel/src/topology/mod.rs @@ -25,7 +25,7 @@ mod vertex; pub use self::{ builder::{CycleBuilder, EdgeBuilder, FaceBuilder, VertexBuilder}, cycle::Cycle, - edge::Edge, + edge::{Edge, VerticesOfEdge}, face::{CyclesInFace, Face}, vertex::Vertex, }; diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 4cf300e0b..eb15718f4 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -103,7 +103,7 @@ fn add_cycle( let curve = if reverse { curve.reverse() } else { curve }; let curve = shape.insert(curve)?; - let vertices = edge.vertices.clone().map(|[a, b]| { + let vertices = edge.vertices.0.clone().map(|[a, b]| { if reverse { // Switch `a` and `b`, but make sure the local forms are still // correct, after we reversed the curve above. From e1d593123a9b3a340df1a1451b9b515ccddf497e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 16:01:33 +0200 Subject: [PATCH 02/13] Simplify function argument --- .../fj-kernel/src/algorithms/approx/cycles.rs | 2 +- .../fj-kernel/src/algorithms/approx/edges.rs | 18 ++++++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/cycles.rs b/crates/fj-kernel/src/algorithms/approx/cycles.rs index 8d630a981..9cba6c1cf 100644 --- a/crates/fj-kernel/src/algorithms/approx/cycles.rs +++ b/crates/fj-kernel/src/algorithms/approx/cycles.rs @@ -22,7 +22,7 @@ impl CycleApprox { for edge in cycle.edges() { let mut edge_points = Vec::new(); approx_curve(&edge.curve(), tolerance, &mut edge_points); - approximate_edge(edge.vertices.0, &mut edge_points); + approximate_edge(edge.vertices, &mut edge_points); points.extend(edge_points); } diff --git a/crates/fj-kernel/src/algorithms/approx/edges.rs b/crates/fj-kernel/src/algorithms/approx/edges.rs index 538081b12..03c6b0c65 100644 --- a/crates/fj-kernel/src/algorithms/approx/edges.rs +++ b/crates/fj-kernel/src/algorithms/approx/edges.rs @@ -1,9 +1,7 @@ -use fj_math::Point; - -use crate::{geometry, shape::LocalForm, topology::Vertex}; +use crate::{geometry, topology::VerticesOfEdge}; pub fn approximate_edge( - vertices: Option<[LocalForm, Vertex>; 2]>, + vertices: VerticesOfEdge, points: &mut Vec>, ) { // Insert the exact vertices of this edge into the approximation. This means @@ -14,7 +12,7 @@ pub fn approximate_edge( // would lead to bugs in the approximation, as points that should refer to // the same vertex would be understood to refer to very close, but distinct // vertices. - let vertices = vertices.map(|vertices| { + let vertices = vertices.0.map(|vertices| { vertices.map(|vertex| { geometry::Point::new( *vertex.local(), @@ -44,7 +42,7 @@ mod test { use crate::{ geometry, shape::{LocalForm, Shape}, - topology::Vertex, + topology::{Vertex, VerticesOfEdge}, }; #[test] @@ -59,10 +57,10 @@ mod test { let v1 = Vertex::builder(&mut shape).build_from_point(a)?; let v2 = Vertex::builder(&mut shape).build_from_point(d)?; - let vertices = [ + let vertices = VerticesOfEdge(Some([ LocalForm::new(Point::from([0.]), v1), LocalForm::new(Point::from([1.]), v2), - ]; + ])); let a = geometry::Point::new([0.0], a); let b = geometry::Point::new([0.25], b); @@ -71,12 +69,12 @@ mod test { // Regular edge let mut points = vec![b, c]; - super::approximate_edge(Some(vertices), &mut points); + super::approximate_edge(vertices, &mut points); assert_eq!(points, vec![a, b, c, d]); // Continuous edge let mut points = vec![b, c]; - super::approximate_edge(None, &mut points); + super::approximate_edge(VerticesOfEdge(None), &mut points); assert_eq!(points, vec![b, c, b]); Ok(()) From 564e20f2446572e223e380c72adfbd4b68e1a5eb Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 16:09:05 +0200 Subject: [PATCH 03/13] Add `VerticesOfEdge::convert` --- .../fj-kernel/src/algorithms/approx/edges.rs | 9 ++------- crates/fj-kernel/src/shape/object.rs | 18 ++++++++---------- .../fj-kernel/src/shape/validate/geometric.rs | 12 +++++------- crates/fj-kernel/src/topology/edge.rs | 10 ++++++++++ 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edges.rs b/crates/fj-kernel/src/algorithms/approx/edges.rs index 03c6b0c65..30b07c064 100644 --- a/crates/fj-kernel/src/algorithms/approx/edges.rs +++ b/crates/fj-kernel/src/algorithms/approx/edges.rs @@ -12,13 +12,8 @@ pub fn approximate_edge( // would lead to bugs in the approximation, as points that should refer to // the same vertex would be understood to refer to very close, but distinct // vertices. - let vertices = vertices.0.map(|vertices| { - vertices.map(|vertex| { - geometry::Point::new( - *vertex.local(), - vertex.canonical().get().point(), - ) - }) + let vertices = vertices.convert(|vertex| { + geometry::Point::new(*vertex.local(), vertex.canonical().get().point()) }); if let Some([a, b]) = vertices { points.insert(0, a); diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index 77db0cb15..30c17ac42 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -120,16 +120,14 @@ impl Object for Edge<3> { // Can be cleaned up using `try_map`, once that is stable: // https://doc.rust-lang.org/std/primitive.array.html#method.try_map let vertices: Option<[Result<_, ValidationError>; 2]> = - self.vertices.0.map(|vertices| { - vertices.map(|vertex| { - let canonical = vertex.canonical(); - let canonical = canonical.get().merge_into( - Some(canonical), - shape, - mapping, - )?; - Ok(LocalForm::new(*vertex.local(), canonical)) - }) + self.vertices.convert(|vertex| { + let canonical = vertex.canonical(); + let canonical = canonical.get().merge_into( + Some(canonical), + shape, + mapping, + )?; + Ok(LocalForm::new(*vertex.local(), canonical)) }); let vertices = match vertices { Some([a, b]) => Some([a?, b?]), diff --git a/crates/fj-kernel/src/shape/validate/geometric.rs b/crates/fj-kernel/src/shape/validate/geometric.rs index d9d747b4c..da132b847 100644 --- a/crates/fj-kernel/src/shape/validate/geometric.rs +++ b/crates/fj-kernel/src/shape/validate/geometric.rs @@ -116,13 +116,11 @@ mod tests { .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]])? .get(); let edge = Edge { - vertices: VerticesOfEdge(edge.vertices.0.clone().map(|vertices| { - vertices.map(|vertex| { - LocalForm::new( - *vertex.local() + [deviation], - vertex.canonical(), - ) - }) + vertices: VerticesOfEdge(edge.vertices.convert(|vertex| { + LocalForm::new( + *vertex.local() + [deviation], + vertex.canonical(), + ) })), ..edge }; diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index a71629648..970048af2 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -96,3 +96,13 @@ impl fmt::Display for Edge { /// The vertices that bound an edge #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct VerticesOfEdge(pub Option<[LocalForm, Vertex>; 2]>); + +impl VerticesOfEdge { + /// Convert each vertex using the provided function + pub fn convert(self, f: F) -> Option<[T; 2]> + where + F: FnMut(LocalForm, Vertex>) -> T, + { + self.0.map(|vertices| vertices.map(f)) + } +} From 5e0a07a1b7f6217f211333b6e2adc63e8f70e6b5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 16:25:18 +0200 Subject: [PATCH 04/13] Add `VerticesOfEdge::try_convert` --- crates/fj-kernel/src/shape/object.rs | 27 +++++++++++---------------- crates/fj-kernel/src/topology/edge.rs | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index 30c17ac42..a9a937cb0 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -117,22 +117,17 @@ impl Object for Edge<3> { mapping, )?; - // Can be cleaned up using `try_map`, once that is stable: - // https://doc.rust-lang.org/std/primitive.array.html#method.try_map - let vertices: Option<[Result<_, ValidationError>; 2]> = - self.vertices.convert(|vertex| { - let canonical = vertex.canonical(); - let canonical = canonical.get().merge_into( - Some(canonical), - shape, - mapping, - )?; - Ok(LocalForm::new(*vertex.local(), canonical)) - }); - let vertices = match vertices { - Some([a, b]) => Some([a?, b?]), - None => None, - }; + let vertices = + self.vertices + .try_convert::<_, _, ValidationError>(|vertex| { + let canonical = vertex.canonical(); + let canonical = canonical.get().merge_into( + Some(canonical), + shape, + mapping, + )?; + Ok(LocalForm::new(*vertex.local(), canonical)) + })?; let merged = shape.get_handle_or_insert(Edge::new(curve, vertices))?; diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index 970048af2..21a47512b 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -105,4 +105,20 @@ impl VerticesOfEdge { { self.0.map(|vertices| vertices.map(f)) } + + /// Convert each vertex using the provided fallible function + pub fn try_convert(self, f: F) -> Result, E> + where + F: FnMut(LocalForm, Vertex>) -> Result, + { + // Can be cleaned up using `try_map`, once that is stable: + // https://doc.rust-lang.org/std/primitive.array.html#method.try_map + let vertices: Option<[Result<_, E>; 2]> = self.convert(f); + let vertices = match vertices { + Some([a, b]) => Some([a?, b?]), + None => None, + }; + + Ok(vertices) + } } From b9d3aa1ac6db846f7906a2bd1d499654d30c70c3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 16:35:42 +0200 Subject: [PATCH 05/13] Add `VerticesOfEdge::from_vertices` --- crates/fj-kernel/src/algorithms/approx/edges.rs | 4 ++-- crates/fj-kernel/src/topology/edge.rs | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edges.rs b/crates/fj-kernel/src/algorithms/approx/edges.rs index 30b07c064..c05ded393 100644 --- a/crates/fj-kernel/src/algorithms/approx/edges.rs +++ b/crates/fj-kernel/src/algorithms/approx/edges.rs @@ -52,10 +52,10 @@ mod test { let v1 = Vertex::builder(&mut shape).build_from_point(a)?; let v2 = Vertex::builder(&mut shape).build_from_point(d)?; - let vertices = VerticesOfEdge(Some([ + let vertices = VerticesOfEdge::from_vertices([ LocalForm::new(Point::from([0.]), v1), LocalForm::new(Point::from([1.]), v2), - ])); + ]); let a = geometry::Point::new([0.0], a); let b = geometry::Point::new([0.25], b); diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index 21a47512b..1002fc46e 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -98,6 +98,11 @@ impl fmt::Display for Edge { pub struct VerticesOfEdge(pub Option<[LocalForm, Vertex>; 2]>); impl VerticesOfEdge { + /// Construct an instance of `VerticesOfEdge` from two vertices + pub fn from_vertices(vertices: [LocalForm, Vertex>; 2]) -> Self { + Self(Some(vertices)) + } + /// Convert each vertex using the provided function pub fn convert(self, f: F) -> Option<[T; 2]> where From e2224d1827e06ce95b3cb4dd99e1822f21116320 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 16:36:51 +0200 Subject: [PATCH 06/13] Add `VerticesOfEdge::none` --- crates/fj-kernel/src/algorithms/approx/edges.rs | 2 +- crates/fj-kernel/src/topology/edge.rs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edges.rs b/crates/fj-kernel/src/algorithms/approx/edges.rs index c05ded393..7efeabcfe 100644 --- a/crates/fj-kernel/src/algorithms/approx/edges.rs +++ b/crates/fj-kernel/src/algorithms/approx/edges.rs @@ -69,7 +69,7 @@ mod test { // Continuous edge let mut points = vec![b, c]; - super::approximate_edge(VerticesOfEdge(None), &mut points); + super::approximate_edge(VerticesOfEdge::none(), &mut points); assert_eq!(points, vec![b, c, b]); Ok(()) diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index 1002fc46e..4f3005311 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -103,6 +103,11 @@ impl VerticesOfEdge { Self(Some(vertices)) } + /// Construct an instance of `VerticesOfEdge` without vertices + pub fn none() -> Self { + Self(None) + } + /// Convert each vertex using the provided function pub fn convert(self, f: F) -> Option<[T; 2]> where From 58aeb7a37cc26403826c52cec76396232fefbc7b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 16:40:01 +0200 Subject: [PATCH 07/13] Add `VerticesOfEdge::expect_vertices` --- crates/fj-kernel/src/algorithms/sweep.rs | 8 ++------ crates/fj-kernel/src/topology/edge.rs | 9 +++++++++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index 449343120..a19c791d5 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -210,12 +210,8 @@ fn create_side_edges( ) -> Result<[Handle>; 2], ValidationError> { // Can't panic. We already ruled out the "continuous edge" case above, so // these edges must have vertices. - let [vertices_bottom, vertices_top] = [edge_bottom, edge_top].map(|edge| { - edge.get() - .vertices - .0 - .expect("Expected vertices on non-continuous edge") - }); + let [vertices_bottom, vertices_top] = [edge_bottom, edge_top] + .map(|edge| edge.get().vertices.expect_vertices()); // Can be simplified, once `zip` is stabilized: // https://doc.rust-lang.org/std/primitive.array.html#method.zip diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index 4f3005311..fe3806b9f 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -108,6 +108,15 @@ impl VerticesOfEdge { Self(None) } + /// Access the two vertices + /// + /// # Panics + /// + /// Panics, if the edge has no vertices. + pub fn expect_vertices(self) -> [LocalForm, Vertex>; 2] { + self.0.expect("Expected edge to have vertices") + } + /// Convert each vertex using the provided function pub fn convert(self, f: F) -> Option<[T; 2]> where From 22ada8b567e1eb9124e0410bc04b702751e0d829 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 16:44:12 +0200 Subject: [PATCH 08/13] Add `VerticesOfEdge::iter` --- .../fj-kernel/src/shape/validate/geometric.rs | 41 +++++++++---------- .../src/shape/validate/structural.rs | 8 ++-- crates/fj-kernel/src/topology/edge.rs | 5 +++ 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/crates/fj-kernel/src/shape/validate/geometric.rs b/crates/fj-kernel/src/shape/validate/geometric.rs index da132b847..ff3803707 100644 --- a/crates/fj-kernel/src/shape/validate/geometric.rs +++ b/crates/fj-kernel/src/shape/validate/geometric.rs @@ -13,32 +13,31 @@ pub fn validate_edge( // Validate that the local and canonical forms of the vertices match. As a // side effect, this also happens to validate that the canonical forms of // the vertices lie on the curve. - if let Some(vertices) = &edge.vertices.0 { - let mut edge_vertex_mismatches = Vec::new(); - - for vertex in vertices { - let local = *vertex.local(); - let local_3d = edge.curve().point_from_curve_coords(local); - let canonical = vertex.canonical().get().point(); - let distance = (local_3d - canonical).magnitude(); - - if distance > max_distance { - edge_vertex_mismatches.push(EdgeVertexMismatch { - local, - local_3d, - canonical, - distance, - }); - } - } - if !edge_vertex_mismatches.is_empty() { - return Err(GeometricIssues { - edge_vertex_mismatches, + let mut edge_vertex_mismatches = Vec::new(); + + for vertex in edge.vertices.iter() { + let local = *vertex.local(); + let local_3d = edge.curve().point_from_curve_coords(local); + let canonical = vertex.canonical().get().point(); + let distance = (local_3d - canonical).magnitude(); + + if distance > max_distance { + edge_vertex_mismatches.push(EdgeVertexMismatch { + local, + local_3d, + canonical, + distance, }); } } + if !edge_vertex_mismatches.is_empty() { + return Err(GeometricIssues { + edge_vertex_mismatches, + }); + } + Ok(()) } diff --git a/crates/fj-kernel/src/shape/validate/structural.rs b/crates/fj-kernel/src/shape/validate/structural.rs index e71c17fec..006f5658d 100644 --- a/crates/fj-kernel/src/shape/validate/structural.rs +++ b/crates/fj-kernel/src/shape/validate/structural.rs @@ -32,11 +32,9 @@ pub fn validate_edge( if !stores.curves.contains(&edge.curve.canonical()) { missing_curve = Some(edge.curve.canonical()); } - for vertices in &edge.vertices.0 { - for vertex in vertices { - if !stores.vertices.contains(&vertex.canonical()) { - missing_vertices.insert(vertex.canonical().clone()); - } + for vertex in edge.vertices.iter() { + if !stores.vertices.contains(&vertex.canonical()) { + missing_vertices.insert(vertex.canonical().clone()); } } diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index fe3806b9f..bd65867c1 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -117,6 +117,11 @@ impl VerticesOfEdge { self.0.expect("Expected edge to have vertices") } + /// Iterate over the vertices, if any + pub fn iter(&self) -> impl Iterator, Vertex>> { + self.0.iter().flatten() + } + /// Convert each vertex using the provided function pub fn convert(self, f: F) -> Option<[T; 2]> where From 698078b4994f11a930586e63600f0b6847022cc8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 16:53:56 +0200 Subject: [PATCH 09/13] Add `VerticesOfEdge::map` --- crates/fj-kernel/src/shape/validate/geometric.rs | 6 +++--- crates/fj-kernel/src/topology/edge.rs | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/shape/validate/geometric.rs b/crates/fj-kernel/src/shape/validate/geometric.rs index ff3803707..b75bc20a7 100644 --- a/crates/fj-kernel/src/shape/validate/geometric.rs +++ b/crates/fj-kernel/src/shape/validate/geometric.rs @@ -102,7 +102,7 @@ mod tests { use crate::{ shape::{LocalForm, Shape}, - topology::{Edge, VerticesOfEdge}, + topology::Edge, }; #[test] @@ -115,12 +115,12 @@ mod tests { .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]])? .get(); let edge = Edge { - vertices: VerticesOfEdge(edge.vertices.convert(|vertex| { + vertices: edge.vertices.map(|vertex| { LocalForm::new( *vertex.local() + [deviation], vertex.canonical(), ) - })), + }), ..edge }; assert!(super::validate_edge(&edge, deviation * 2.).is_ok()); diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index bd65867c1..99c11ba9b 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -122,6 +122,14 @@ impl VerticesOfEdge { self.0.iter().flatten() } + /// Map each vertex using the provided function + pub fn map(self, f: F) -> Self + where + F: FnMut(LocalForm, Vertex>) -> LocalForm, Vertex>, + { + Self(self.convert(f)) + } + /// Convert each vertex using the provided function pub fn convert(self, f: F) -> Option<[T; 2]> where From 2bb34c5e3a17dbdfe2e6566d95cc795f0f7cef67 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 17:07:09 +0200 Subject: [PATCH 10/13] Simplify method parameters --- crates/fj-kernel/src/shape/api.rs | 12 ++++++++---- crates/fj-kernel/src/shape/object.rs | 5 +++-- crates/fj-kernel/src/topology/builder.rs | 11 ++++++++--- crates/fj-kernel/src/topology/edge.rs | 10 ++-------- crates/fj-operations/src/difference_2d.rs | 4 ++-- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index 83f261ddc..4b5bd1e80 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -313,7 +313,7 @@ mod tests { use crate::{ geometry::{Curve, Surface}, shape::{Handle, LocalForm, Shape, ValidationError, ValidationResult}, - topology::{Cycle, Edge, Face, Vertex}, + topology::{Cycle, Edge, Face, Vertex, VerticesOfEdge}, }; const MIN_DISTANCE: f64 = 5e-7; @@ -340,7 +340,7 @@ mod tests { assert!(shape.get_handle(&surface.get()).as_ref() == Some(&surface)); let vertex = Vertex { point }; - let edge = Edge::new(curve, None); + let edge = Edge::new(curve, VerticesOfEdge::none()); assert!(shape.get_handle(&vertex).is_none()); assert!(shape.get_handle(&edge).is_none()); @@ -429,7 +429,10 @@ mod tests { // Shouldn't work. Nothing has been added to `shape`. let err = shape - .insert(Edge::new(curve.clone(), Some([a.clone(), b.clone()]))) + .insert(Edge::new( + curve.clone(), + VerticesOfEdge::from_vertices([a.clone(), b.clone()]), + )) .unwrap_err(); assert!(err.missing_curve(&curve)); assert!(err.missing_vertex(&a.canonical())); @@ -443,7 +446,8 @@ mod tests { let b = LocalForm::new(Point::from([2.]), b); // Everything has been added to `shape` now. Should work! - shape.insert(Edge::new(curve, Some([a, b])))?; + shape + .insert(Edge::new(curve, VerticesOfEdge::from_vertices([a, b])))?; Ok(()) } diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index a9a937cb0..7db484dc8 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -2,7 +2,7 @@ use fj_math::Point; use crate::{ geometry::{Curve, Surface}, - topology::{Cycle, Edge, Face, Vertex}, + topology::{Cycle, Edge, Face, Vertex, VerticesOfEdge}, }; use super::{ @@ -129,7 +129,8 @@ impl Object for Edge<3> { Ok(LocalForm::new(*vertex.local(), canonical)) })?; - let merged = shape.get_handle_or_insert(Edge::new(curve, vertices))?; + let merged = shape + .get_handle_or_insert(Edge::new(curve, VerticesOfEdge(vertices)))?; if let Some(handle) = handle { mapping.edges.insert(handle, merged.clone()); diff --git a/crates/fj-kernel/src/topology/builder.rs b/crates/fj-kernel/src/topology/builder.rs index 234f75187..2be312d9c 100644 --- a/crates/fj-kernel/src/topology/builder.rs +++ b/crates/fj-kernel/src/topology/builder.rs @@ -5,7 +5,7 @@ use crate::{ shape::{Handle, LocalForm, Shape, ValidationResult}, }; -use super::{Cycle, Edge, Face, Vertex}; +use super::{Cycle, Edge, Face, Vertex, VerticesOfEdge}; /// API for building a [`Vertex`] #[must_use] @@ -53,7 +53,9 @@ impl<'r> EdgeBuilder<'r> { a: Vector::from([radius, Scalar::ZERO, Scalar::ZERO]), b: Vector::from([Scalar::ZERO, radius, Scalar::ZERO]), }))?; - let edge = self.shape.insert(Edge::new(curve, None))?; + let edge = self + .shape + .insert(Edge::new(curve, VerticesOfEdge::none()))?; Ok(edge) } @@ -93,7 +95,10 @@ impl<'r> EdgeBuilder<'r> { LocalForm::new(Point::from([1.]), b), ]; - let edge = self.shape.insert(Edge::new(curve, Some(vertices)))?; + let edge = self.shape.insert(Edge::new( + curve, + VerticesOfEdge::from_vertices(vertices), + ))?; Ok(edge) } diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index 99c11ba9b..a341e767b 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -39,15 +39,9 @@ pub struct Edge { impl Edge<3> { /// Construct an instance of `Edge` - pub fn new( - curve: Handle>, - vertices: Option<[LocalForm, Vertex>; 2]>, - ) -> Self { + pub fn new(curve: Handle>, vertices: VerticesOfEdge) -> Self { let curve = LocalForm::canonical_only(curve); - Self { - curve, - vertices: VerticesOfEdge(vertices), - } + Self { curve, vertices } } /// Build an edge using the [`EdgeBuilder`] API diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index eb15718f4..e7a69d3f6 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -2,7 +2,7 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, shape::{Handle, LocalForm, Shape, ValidationError, ValidationResult}, - topology::{Cycle, Edge, Face}, + topology::{Cycle, Edge, Face, VerticesOfEdge}, }; use fj_math::Aabb; @@ -116,7 +116,7 @@ fn add_cycle( } }); - let edge = shape.merge(Edge::new(curve, vertices))?; + let edge = shape.merge(Edge::new(curve, VerticesOfEdge(vertices)))?; edges.push(edge); } From d8d1445bb16e45b18c4e46d0022821bda66476eb Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 17:08:42 +0200 Subject: [PATCH 11/13] Add `VerticesOfEdge::new` --- crates/fj-kernel/src/shape/object.rs | 6 ++++-- crates/fj-kernel/src/topology/edge.rs | 5 +++++ crates/fj-operations/src/difference_2d.rs | 3 ++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index 7db484dc8..443cd4728 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -129,8 +129,10 @@ impl Object for Edge<3> { Ok(LocalForm::new(*vertex.local(), canonical)) })?; - let merged = shape - .get_handle_or_insert(Edge::new(curve, VerticesOfEdge(vertices)))?; + let merged = shape.get_handle_or_insert(Edge::new( + curve, + VerticesOfEdge::new(vertices), + ))?; if let Some(handle) = handle { mapping.edges.insert(handle, merged.clone()); diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index a341e767b..81a4c6c94 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -92,6 +92,11 @@ impl fmt::Display for Edge { pub struct VerticesOfEdge(pub Option<[LocalForm, Vertex>; 2]>); impl VerticesOfEdge { + /// Construct an instance of `VerticesOfEdge` from zero or two vertices + pub fn new(vertices: Option<[LocalForm, Vertex>; 2]>) -> Self { + Self(vertices) + } + /// Construct an instance of `VerticesOfEdge` from two vertices pub fn from_vertices(vertices: [LocalForm, Vertex>; 2]) -> Self { Self(Some(vertices)) diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index e7a69d3f6..5ac5f8cd6 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -116,7 +116,8 @@ fn add_cycle( } }); - let edge = shape.merge(Edge::new(curve, VerticesOfEdge(vertices)))?; + let edge = + shape.merge(Edge::new(curve, VerticesOfEdge::new(vertices)))?; edges.push(edge); } From 57bccc081d5537dbfd6c76cc2144fdd8d887ceea Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 17:10:09 +0200 Subject: [PATCH 12/13] Add `VerticesOfEdge::reverse` --- crates/fj-kernel/src/topology/edge.rs | 12 ++++++++++++ crates/fj-operations/src/difference_2d.rs | 24 ++++++++--------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index 81a4c6c94..bfc568bf9 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -121,6 +121,18 @@ impl VerticesOfEdge { self.0.iter().flatten() } + /// Reverse the order of vertices + /// + /// Makes sure that the local coordinates are still correct. + pub fn reverse(self) -> Self { + Self(self.0.map(|[a, b]| { + [ + LocalForm::new(-(*b.local()), b.canonical()), + LocalForm::new(-(*a.local()), a.canonical()), + ] + })) + } + /// Map each vertex using the provided function pub fn map(self, f: F) -> Self where diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 5ac5f8cd6..adc40b7df 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -1,8 +1,8 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, - shape::{Handle, LocalForm, Shape, ValidationError, ValidationResult}, - topology::{Cycle, Edge, Face, VerticesOfEdge}, + shape::{Handle, Shape, ValidationError, ValidationResult}, + topology::{Cycle, Edge, Face}, }; use fj_math::Aabb; @@ -103,21 +103,13 @@ fn add_cycle( let curve = if reverse { curve.reverse() } else { curve }; let curve = shape.insert(curve)?; - let vertices = edge.vertices.0.clone().map(|[a, b]| { - if reverse { - // Switch `a` and `b`, but make sure the local forms are still - // correct, after we reversed the curve above. - [ - LocalForm::new(-(*b.local()), b.canonical()), - LocalForm::new(-(*a.local()), a.canonical()), - ] - } else { - [a, b] - } - }); + let vertices = if reverse { + edge.vertices.reverse() + } else { + edge.vertices + }; - let edge = - shape.merge(Edge::new(curve, VerticesOfEdge::new(vertices)))?; + let edge = shape.merge(Edge::new(curve, vertices))?; edges.push(edge); } From b8b4946017ea9c6d392c1020ba7bdd79f42c0b45 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Jun 2022 17:10:23 +0200 Subject: [PATCH 13/13] Remove unncessary `pub` --- crates/fj-kernel/src/topology/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/topology/edge.rs b/crates/fj-kernel/src/topology/edge.rs index bfc568bf9..e7b516d6f 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -89,7 +89,7 @@ impl fmt::Display for Edge { /// The vertices that bound an edge #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct VerticesOfEdge(pub Option<[LocalForm, Vertex>; 2]>); +pub struct VerticesOfEdge(Option<[LocalForm, Vertex>; 2]>); impl VerticesOfEdge { /// Construct an instance of `VerticesOfEdge` from zero or two vertices