From 0780d7981a9d4d286f3181e33eb2d41d2a7c7b3c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 7 Oct 2022 13:43:21 +0200 Subject: [PATCH 1/4] Add `VerticesInNormalizedOrder` I intend to use this to make handling of vertices in normalized order easier and more flexible. --- crates/fj-kernel/src/objects/edge.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 50234a954..1ef4f4fcb 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -104,7 +104,7 @@ impl fmt::Display for HalfEdge { #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct GlobalEdge { curve: HandleWrapper, - vertices: [Handle; 2], + vertices: VerticesInNormalizedOrder, } impl GlobalEdge { @@ -118,7 +118,8 @@ impl GlobalEdge { vertices: [Handle; 2], ) -> Self { let curve = curve.into(); - let vertices = normalize_vertex_order(vertices); + let vertices = + VerticesInNormalizedOrder(normalize_vertex_order(vertices)); Self { curve, vertices } } @@ -138,10 +139,19 @@ impl GlobalEdge { /// [`GlobalEdge::new`]. You must not rely on the vertices being in any /// specific order. pub fn vertices_in_normalized_order(&self) -> &[Handle; 2] { - &self.vertices + &self.vertices.0 } } +/// The vertices of a [`GlobalEdge`] +/// +/// Since [`GlobalEdge`] is the single global representation of an edge in +/// global space, it must normalize the order of its vertices. Otherwise, it is +/// possible to construct two [`GlobalEdge`] instances that are meant to +/// represent the same edge, but aren't equal. +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct VerticesInNormalizedOrder([Handle; 2]); + fn normalize_vertex_order( [a, b]: [Handle; 2], ) -> [Handle; 2] { From d5db87e2d94d26163ff4b368ce163e5b61f85f3c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 7 Oct 2022 13:45:47 +0200 Subject: [PATCH 2/4] Convert function into constructor --- crates/fj-kernel/src/objects/edge.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 1ef4f4fcb..91ccc769a 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -47,9 +47,10 @@ impl HalfEdge { the half-edge's global form" ); assert_eq!( - &normalize_vertex_order( + &VerticesInNormalizedOrder::new( [&a, &b].map(|vertex| vertex.global_form().clone()) - ), + ) + .0, 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" @@ -118,8 +119,7 @@ impl GlobalEdge { vertices: [Handle; 2], ) -> Self { let curve = curve.into(); - let vertices = - VerticesInNormalizedOrder(normalize_vertex_order(vertices)); + let vertices = VerticesInNormalizedOrder::new(vertices); Self { curve, vertices } } @@ -152,13 +152,13 @@ impl GlobalEdge { #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct VerticesInNormalizedOrder([Handle; 2]); -fn normalize_vertex_order( - [a, b]: [Handle; 2], -) -> [Handle; 2] { - if a < b { - [a, b] - } else { - [b, a] +impl VerticesInNormalizedOrder { + /// Construct a new instance of `VerticesInNormalizedOrder` + /// + /// The provided vertices can be in any order. + pub fn new([a, b]: [Handle; 2]) -> Self { + let vertices = if a < b { [a, b] } else { [b, a] }; + Self(vertices) } } From d1de5f40f7bddf3bdd17c3ca4d1223c5b46cf8a0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 7 Oct 2022 13:53:16 +0200 Subject: [PATCH 3/4] Return `VerticesInNormalizedOrder` --- crates/fj-kernel/src/objects/edge.rs | 14 ++++++++++---- crates/fj-kernel/src/objects/mod.rs | 2 +- crates/fj-kernel/src/partial/objects/edge.rs | 7 ++++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 91ccc769a..ed5e12bea 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -49,8 +49,7 @@ impl HalfEdge { assert_eq!( &VerticesInNormalizedOrder::new( [&a, &b].map(|vertex| vertex.global_form().clone()) - ) - .0, + ), 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" @@ -138,8 +137,8 @@ impl GlobalEdge { /// 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) -> &[Handle; 2] { - &self.vertices.0 + pub fn vertices_in_normalized_order(&self) -> &VerticesInNormalizedOrder { + &self.vertices } } @@ -160,6 +159,13 @@ impl VerticesInNormalizedOrder { let vertices = if a < b { [a, b] } else { [b, a] }; Self(vertices) } + + /// Access the vertices + /// + /// The vertices in the returned array will be in normalized order. + pub fn access_in_normalized_order(&self) -> &[Handle; 2] { + &self.0 + } } #[cfg(test)] diff --git a/crates/fj-kernel/src/objects/mod.rs b/crates/fj-kernel/src/objects/mod.rs index 0a1ee6035..8f137b1ff 100644 --- a/crates/fj-kernel/src/objects/mod.rs +++ b/crates/fj-kernel/src/objects/mod.rs @@ -87,7 +87,7 @@ mod vertex; pub use self::{ curve::{Curve, GlobalCurve}, cycle::Cycle, - edge::{GlobalEdge, HalfEdge}, + edge::{GlobalEdge, HalfEdge, VerticesInNormalizedOrder}, face::{Face, Faces, Handedness}, shell::Shell, sketch::Sketch, diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 7430f4a0f..494b827b1 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -288,7 +288,12 @@ impl From<&GlobalEdge> for PartialGlobalEdge { fn from(global_edge: &GlobalEdge) -> Self { Self { curve: Some(global_edge.curve().clone().into()), - vertices: Some(global_edge.vertices_in_normalized_order().clone()), + vertices: Some( + global_edge + .vertices_in_normalized_order() + .access_in_normalized_order() + .clone(), + ), } } } From 92bb328256097fce239244e5cc7f0686b9fe1b58 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 7 Oct 2022 13:54:06 +0200 Subject: [PATCH 4/4] Rename `GlobalEdge::vertices_in_normalized_order` Now that the return type does the heavy lifting (in preventing any misunderstandings about the order of the vertices), the simpler name is appropriate again. --- crates/fj-kernel/src/objects/edge.rs | 4 ++-- crates/fj-kernel/src/partial/objects/edge.rs | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index ed5e12bea..a472c577a 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -50,7 +50,7 @@ impl HalfEdge { &VerticesInNormalizedOrder::new( [&a, &b].map(|vertex| vertex.global_form().clone()) ), - global_form.vertices_in_normalized_order(), + global_form.vertices(), "The global forms of a half-edge's vertices must match the \ vertices of the half-edge's global form" ); @@ -137,7 +137,7 @@ impl GlobalEdge { /// 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) -> &VerticesInNormalizedOrder { + pub fn vertices(&self) -> &VerticesInNormalizedOrder { &self.vertices } } diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 494b827b1..3036720fb 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -289,10 +289,7 @@ impl From<&GlobalEdge> for PartialGlobalEdge { Self { curve: Some(global_edge.curve().clone().into()), vertices: Some( - global_edge - .vertices_in_normalized_order() - .access_in_normalized_order() - .clone(), + global_edge.vertices().access_in_normalized_order().clone(), ), } }