From 23e9f97ac406cf185195e87afc194c4ccaecd730 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 11 Aug 2023 12:24:14 +0200 Subject: [PATCH 1/4] Refactor to prepare for follow-on change --- crates/fj-core/src/geometry/boundary.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 8a6eef11a..c8eed0a16 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -38,10 +38,10 @@ impl CurveBoundary { #[must_use] pub fn normalize(self) -> (Self, bool) { let [a, b] = &self.inner; - if a > b { - (self.reverse(), true) - } else { + if a <= b { (self, false) + } else { + (self.reverse(), true) } } } From bf60afae7d5780454ea2dc8d5648d1da0524820d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 11 Aug 2023 12:33:18 +0200 Subject: [PATCH 2/4] Add `CurveBoundary::is_normalized` --- crates/fj-core/src/geometry/boundary.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index c8eed0a16..42542a6b8 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -28,6 +28,15 @@ impl CurveBoundary { Self { inner: [b, a] } } + /// Indicate whether the boundary is normalized + /// + /// If the boundary is normalized, its bounding elements are in a defined + /// order, and calling `normalize` will return an identical instance. + pub fn is_normalized(&self) -> bool { + let [a, b] = &self.inner; + a <= b + } + /// Normalize the boundary /// /// Returns a new instance of this struct, which has the bounding elements @@ -37,8 +46,7 @@ impl CurveBoundary { /// This can be used to compare a boundary while disregarding its direction. #[must_use] pub fn normalize(self) -> (Self, bool) { - let [a, b] = &self.inner; - if a <= b { + if self.is_normalized() { (self, false) } else { (self.reverse(), true) From 5fb76e8a02f78e6e613fbf564b07008d4743c3d2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 11 Aug 2023 12:51:27 +0200 Subject: [PATCH 3/4] Simplify `CurveBoundary::normalize` The additional `bool` return value was confusing and easy to misuse. Callers that need this information can use `is_normalized`, which should result in clearer (if a bit more verbose) code. --- crates/fj-core/src/geometry/boundary.rs | 6 +++--- crates/fj-core/src/validate/shell.rs | 7 ++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 42542a6b8..26040bbd8 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -45,11 +45,11 @@ impl CurveBoundary { /// /// This can be used to compare a boundary while disregarding its direction. #[must_use] - pub fn normalize(self) -> (Self, bool) { + pub fn normalize(self) -> Self { if self.is_normalized() { - (self, false) + self } else { - (self.reverse(), true) + self.reverse() } } } diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index c87b92542..eb1f1f328 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -235,7 +235,6 @@ impl ShellValidationError { .bounding_vertices_of_edge(edge) .expect("Expected edge to be part of shell") .normalize() - .0 }; bounding_vertices_of(edge_a) @@ -315,8 +314,7 @@ impl ShellValidationError { .expect( "Cycle should provide bounds of its own half-edge", ) - .normalize() - .0; + .normalize(); let edge = (curve, bounding_vertices); @@ -396,8 +394,7 @@ impl ShellValidationError { .expect( "Just got edge from this cycle; must be part of it", ) - .normalize() - .0; + .normalize(); edges_by_coincidence .entry((curve, boundary)) From 3fee65dbd2ec3442bd39548199d21ccdd1447cb3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 11 Aug 2023 13:02:50 +0200 Subject: [PATCH 4/4] Update doc comment --- crates/fj-core/src/geometry/boundary.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/geometry/boundary.rs b/crates/fj-core/src/geometry/boundary.rs index 26040bbd8..85bb6fe0e 100644 --- a/crates/fj-core/src/geometry/boundary.rs +++ b/crates/fj-core/src/geometry/boundary.rs @@ -40,10 +40,8 @@ impl CurveBoundary { /// Normalize the boundary /// /// Returns a new instance of this struct, which has the bounding elements - /// in a defined order, alongside a boolean that indicates whether the new - /// instance is different from the original one. - /// - /// This can be used to compare a boundary while disregarding its direction. + /// in a defined order. This can be used to compare boundaries while + /// disregarding their direction. #[must_use] pub fn normalize(self) -> Self { if self.is_normalized() {