diff --git a/crates/fj-kernel/src/algorithms/reverse/edge.rs b/crates/fj-kernel/src/algorithms/reverse/edge.rs index adf0b4237..f8bcb5f27 100644 --- a/crates/fj-kernel/src/algorithms/reverse/edge.rs +++ b/crates/fj-kernel/src/algorithms/reverse/edge.rs @@ -1,4 +1,4 @@ -use crate::objects::{GlobalEdge, HalfEdge}; +use crate::objects::HalfEdge; use super::Reverse; @@ -12,18 +12,7 @@ impl Reverse for HalfEdge { HalfEdge::new( self.curve().clone(), vertices, - self.global_form().clone().reverse(), + self.global_form().clone(), ) } } - -impl Reverse for GlobalEdge { - fn reverse(self) -> Self { - let vertices = { - let &[a, b] = self.vertices(); - [b, a] - }; - - GlobalEdge::new(self.curve().clone(), vertices) - } -} diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index eb36cef85..73a083e53 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -53,7 +53,8 @@ impl Sweep for (Vertex, Surface) { // With that out of the way, let's start by creating the `GlobalEdge`, // as that is the most straight-forward part of this operations, and // we're going to need it soon anyway. - let edge_global = vertex.global_form().sweep(path, stores); + let (edge_global, vertices_global) = + vertex.global_form().sweep(path, stores); // Next, let's compute the surface coordinates of the two vertices of // the output `Edge`, as we're going to need these for the rest of this @@ -87,15 +88,13 @@ impl Sweep for (Vertex, Surface) { // And now the vertices. Again, nothing wild here. let vertices = { - let vertices_global = edge_global.vertices(); - // Can be cleaned up, once `zip` is stable: // https://doc.rust-lang.org/std/primitive.array.html#method.zip let [a_surface, b_surface] = points_surface; let [a_global, b_global] = vertices_global; let vertices_surface = [(a_surface, a_global), (b_surface, b_global)].map( - |(point_surface, &vertex_global)| { + |(point_surface, vertex_global)| { SurfaceVertex::new( point_surface, surface, @@ -110,7 +109,7 @@ impl Sweep for (Vertex, Surface) { let [a_global, b_global] = vertices_global; let vertices = [(a_surface, a_global), (b_surface, b_global)]; - vertices.map(|(vertex_surface, &vertex_global)| { + vertices.map(|(vertex_surface, vertex_global)| { Vertex::new( [vertex_surface.position().v], curve.clone(), @@ -127,7 +126,7 @@ impl Sweep for (Vertex, Surface) { } impl Sweep for GlobalVertex { - type Swept = GlobalEdge; + type Swept = (GlobalEdge, [GlobalVertex; 2]); fn sweep(self, path: impl Into>, stores: &Stores) -> Self::Swept { let curve = GlobalCurve::new(stores); @@ -135,7 +134,13 @@ impl Sweep for GlobalVertex { let a = self; let b = GlobalVertex::from_position(self.position() + path.into()); - GlobalEdge::new(curve, [a, b]) + let vertices = [a, b]; + let global_edge = GlobalEdge::new(curve, vertices); + + // The vertices of the returned `GlobalEdge` are in normalized order, + // which means the order can't be relied upon by the caller. Return the + // ordered vertices in addition. + (global_edge, vertices) } } diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 03dbb2f01..4ff8324a3 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -38,7 +38,7 @@ impl TransformObject for GlobalEdge { fn transform(self, transform: &Transform, stores: &Stores) -> Self { let curve = self.curve().clone().transform(transform, stores); let vertices = self - .vertices() + .vertices_in_normalized_order() .map(|vertex| vertex.transform(transform, stores)); Self::new(curve, vertices) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index b3bce8e1b..143041d1e 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -52,8 +52,10 @@ impl HalfEdge { the half-edge's global form" ); assert_eq!( - &vertices.clone().map(|vertex| *vertex.global_form()), - global_form.vertices(), + &normalize_vertex_order( + vertices.clone().map(|vertex| *vertex.global_form()) + ), + global_form.vertices_in_normalized_order(), "The global forms of a half-edge's vertices must match the \ vertices of the half-edge's global form" ); @@ -108,6 +110,11 @@ impl fmt::Display for HalfEdge { } /// An edge, defined in global (3D) coordinates +/// +/// In contract to [`HalfEdge`], `GlobalEdge` is undirected, meaning it has no +/// defined direction, and its vertices have no defined order. This means it can +/// be used to determine whether two [`HalfEdge`]s map to the same `GlobalEdge`, +/// regardless of their direction. #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct GlobalEdge { curve: HandleWrapper, @@ -116,11 +123,16 @@ pub struct GlobalEdge { impl GlobalEdge { /// Create a new instance + /// + /// 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: [GlobalVertex; 2], ) -> Self { let curve = curve.into(); + let vertices = normalize_vertex_order(vertices); Self { curve, vertices } } @@ -135,10 +147,47 @@ impl GlobalEdge { /// Access the vertices that bound the edge on the curve /// - /// An edge has either two bounding vertices or none. The latter is possible - /// if the edge's curve is continuous (i.e. connects to itself), and defines - /// the whole edge. - pub fn vertices(&self) -> &[GlobalVertex; 2] { + /// As the name indicates, the order of the returned vertices is normalized + /// and might not match the order of the vertices that were passed to + /// [`GlobalEdge::new`]. You must not rely on the vertices being in any + /// specific order. + pub fn vertices_in_normalized_order(&self) -> &[GlobalVertex; 2] { &self.vertices } } + +fn normalize_vertex_order([a, b]: [GlobalVertex; 2]) -> [GlobalVertex; 2] { + if a < b { + [a, b] + } else { + [b, a] + } +} + +#[cfg(test)] +mod tests { + use pretty_assertions::assert_eq; + + use crate::{objects::Surface, partial::HasPartial, stores::Stores}; + + use super::HalfEdge; + + #[test] + fn global_edge_equality() { + let stores = Stores::new(); + + let surface = Surface::xy_plane(); + + let a = [0., 0.]; + let b = [1., 0.]; + + let a_to_b = HalfEdge::partial() + .as_line_segment_from_points(surface, [a, b]) + .build(&stores); + let b_to_a = HalfEdge::partial() + .as_line_segment_from_points(surface, [b, a]) + .build(&stores); + + assert_eq!(a_to_b.global_form(), b_to_a.global_form()); + } +} diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index d884e6020..ea5591ff9 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -254,7 +254,7 @@ impl From<&GlobalEdge> for PartialGlobalEdge { fn from(global_edge: &GlobalEdge) -> Self { Self { curve: Some(global_edge.curve().clone().into()), - vertices: Some(*global_edge.vertices()), + vertices: Some(*global_edge.vertices_in_normalized_order()), } } }