diff --git a/crates/fj-kernel/src/algorithms/approx/edges.rs b/crates/fj-kernel/src/algorithms/approx/edges.rs index 538081b12..7efeabcfe 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,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.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); @@ -44,7 +37,7 @@ mod test { use crate::{ geometry, shape::{LocalForm, Shape}, - topology::Vertex, + topology::{Vertex, VerticesOfEdge}, }; #[test] @@ -59,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 = [ + 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); @@ -71,12 +64,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(()) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index 6396e32d5..a19c791d5 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -210,11 +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 - .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/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 d84e3c818..443cd4728 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::{ @@ -117,11 +117,9 @@ 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.map(|vertices| { - vertices.map(|vertex| { + let vertices = + self.vertices + .try_convert::<_, _, ValidationError>(|vertex| { let canonical = vertex.canonical(); let canonical = canonical.get().merge_into( Some(canonical), @@ -129,14 +127,12 @@ impl Object for Edge<3> { mapping, )?; Ok(LocalForm::new(*vertex.local(), canonical)) - }) - }); - let vertices = match vertices { - Some([a, b]) => Some([a?, b?]), - None => None, - }; + })?; - let merged = shape.get_handle_or_insert(Edge::new(curve, 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/shape/validate/geometric.rs b/crates/fj-kernel/src/shape/validate/geometric.rs index 508b85a7a..b75bc20a7 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 { - 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(()) } @@ -116,13 +115,11 @@ mod tests { .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]])? .get(); let edge = Edge { - vertices: edge.vertices.clone().map(|vertices| { - vertices.map(|vertex| { - LocalForm::new( - *vertex.local() + [deviation], - vertex.canonical(), - ) - }) + vertices: edge.vertices.map(|vertex| { + LocalForm::new( + *vertex.local() + [deviation], + vertex.canonical(), + ) }), ..edge }; diff --git a/crates/fj-kernel/src/shape/validate/structural.rs b/crates/fj-kernel/src/shape/validate/structural.rs index 83a3f6b2d..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 { - 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/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 0f3289ead..e7b516d6f 100644 --- a/crates/fj-kernel/src/topology/edge.rs +++ b/crates/fj-kernel/src/topology/edge.rs @@ -34,15 +34,12 @@ 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> { /// 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 } } @@ -68,6 +65,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 +86,82 @@ impl fmt::Display for Edge { Ok(()) } } + +/// The vertices that bound an edge +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct VerticesOfEdge(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)) + } + + /// Construct an instance of `VerticesOfEdge` without vertices + pub fn none() -> Self { + 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") + } + + /// Iterate over the vertices, if any + pub fn iter(&self) -> impl Iterator, Vertex>> { + 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 + 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 + F: FnMut(LocalForm, Vertex>) -> T, + { + 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) + } +} 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..adc40b7df 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -1,7 +1,7 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, - shape::{Handle, LocalForm, Shape, ValidationError, ValidationResult}, + shape::{Handle, Shape, ValidationError, ValidationResult}, topology::{Cycle, Edge, Face}, }; use fj_math::Aabb; @@ -103,18 +103,11 @@ fn add_cycle( let curve = if reverse { curve.reverse() } else { curve }; let curve = shape.insert(curve)?; - let vertices = edge.vertices.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, vertices))?; edges.push(edge);