From 8fc8c5d9081e7c1ab3a905e891c55f1a32a112e7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 10:50:24 +0200 Subject: [PATCH 01/15] Update order of methods to improve clarity Now the first methods each access a field, while the only other method that doesn't comes after. --- crates/fj-core/src/objects/kinds/edge.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/fj-core/src/objects/kinds/edge.rs b/crates/fj-core/src/objects/kinds/edge.rs index 4ebc15d57..9261ec6ef 100644 --- a/crates/fj-core/src/objects/kinds/edge.rs +++ b/crates/fj-core/src/objects/kinds/edge.rs @@ -67,16 +67,6 @@ impl HalfEdge { self.boundary } - /// Compute the surface position where the edge starts - pub fn start_position(&self) -> Point<2> { - // Computing the surface position from the curve position is fine. - // `Edge` "owns" its start position. There is no competing code that - // could compute the surface position from slightly different data. - - let [start, _] = self.boundary.inner; - self.path.point_from_path_coords(start) - } - /// Access the curve of the edge pub fn curve(&self) -> &Handle { &self.curve @@ -86,4 +76,14 @@ impl HalfEdge { pub fn start_vertex(&self) -> &Handle { &self.start_vertex } + + /// Compute the surface position where the edge starts + pub fn start_position(&self) -> Point<2> { + // Computing the surface position from the curve position is fine. + // `Edge` "owns" its start position. There is no competing code that + // could compute the surface position from slightly different data. + + let [start, _] = self.boundary.inner; + self.path.point_from_path_coords(start) + } } From f12ae5747435293e9cc3f5126d6430586b662758 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:01:52 +0200 Subject: [PATCH 02/15] Add `Shell::are_siblings` --- crates/fj-core/src/objects/kinds/shell.rs | 27 ++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/objects/kinds/shell.rs b/crates/fj-core/src/objects/kinds/shell.rs index c905cf70b..261783517 100644 --- a/crates/fj-core/src/objects/kinds/shell.rs +++ b/crates/fj-core/src/objects/kinds/shell.rs @@ -1,5 +1,6 @@ use crate::{ - objects::{handles::Handles, Face}, + objects::{handles::Handles, Face, HalfEdge}, + queries::BoundingVerticesOfHalfEdge, storage::Handle, }; @@ -21,4 +22,28 @@ impl Shell { pub fn faces(&self) -> &Handles { &self.faces } + + /// Indicate whether the provided half-edges are siblings + pub fn are_siblings( + &self, + a: &Handle, + b: &Handle, + ) -> bool { + let same_curve = a.curve().id() == b.curve().id(); + let same_boundary = a.boundary() == b.boundary().reverse(); + let same_vertices = { + let Some(a_vertices) = self.bounding_vertices_of_half_edge(a) + else { + return false; + }; + let Some(b_vertices) = self.bounding_vertices_of_half_edge(b) + else { + return false; + }; + + a_vertices == b_vertices.reverse() + }; + + same_curve && same_boundary && same_vertices + } } From c5d5db94ea18fa3ad9e9e5283fef3f320ae24cd6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:02:06 +0200 Subject: [PATCH 03/15] Refactor to increase robustness --- crates/fj-core/src/validate/shell.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 616a14066..ea77eeee0 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -225,11 +225,20 @@ impl ShellValidationError { let key_reversed = (curve, boundary.reverse(), vertices.reverse()); - let sibling_not_seen_yet = - unmatched_half_edges.remove(&key_reversed).is_none(); - - if sibling_not_seen_yet { - unmatched_half_edges.insert(key, half_edge.clone()); + match unmatched_half_edges.remove(&key_reversed) { + Some(sibling) => { + // This must be the sibling of the half-edge we're + // currently looking at. Let's make sure the logic + // we use here to determine that matches the + // "official" definition. + assert!(shell.are_siblings(half_edge, &sibling)); + } + None => { + // If this half-edge has a sibling, we haven't seen + // it yet. Let's store this half-edge then, in case + // we come across the sibling later. + unmatched_half_edges.insert(key, half_edge.clone()); + } } } } From 0bfe94720965a15d68257f65d8e2e1bdc79d13d6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:02:42 +0200 Subject: [PATCH 04/15] Refactor to avoid unnecessary `clone`s --- crates/fj-core/src/validate/shell.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index ea77eeee0..7726bfb48 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -231,20 +231,20 @@ impl ShellValidationError { // currently looking at. Let's make sure the logic // we use here to determine that matches the // "official" definition. - assert!(shell.are_siblings(half_edge, &sibling)); + assert!(shell.are_siblings(half_edge, sibling)); } None => { // If this half-edge has a sibling, we haven't seen // it yet. Let's store this half-edge then, in case // we come across the sibling later. - unmatched_half_edges.insert(key, half_edge.clone()); + unmatched_half_edges.insert(key, half_edge); } } } } } - for half_edge in unmatched_half_edges.into_values() { + for half_edge in unmatched_half_edges.into_values().cloned() { errors.push(Self::HalfEdgeHasNoSibling { half_edge }.into()); } } From ab09a386df33aabfdefa38eb3e8e96dbc09ba732 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:06:17 +0200 Subject: [PATCH 05/15] Update variable name --- crates/fj-core/src/validate/shell.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 7726bfb48..07262a0c9 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -261,16 +261,16 @@ impl ShellValidationError { // This is O(N^2) which isn't great, but we can't use a HashMap since we // need to deal with float inaccuracies. Maybe we could use some smarter // data-structure like an octree. - for (edge_a, surface_a) in &edges_and_surfaces { + for (half_edge_a, surface_a) in &edges_and_surfaces { for (edge_b, surface_b) in &edges_and_surfaces { // No need to check an edge against itself. - if edge_a.id() == edge_b.id() { + if half_edge_a.id() == edge_b.id() { continue; } let identical = { let on_same_curve = - edge_a.curve().id() == edge_b.curve().id(); + half_edge_a.curve().id() == edge_b.curve().id(); let have_same_boundary = { let bounding_vertices_of = |edge| { @@ -280,7 +280,7 @@ impl ShellValidationError { .normalize() }; - bounding_vertices_of(edge_a) + bounding_vertices_of(half_edge_a) == bounding_vertices_of(edge_b) }; @@ -293,7 +293,7 @@ impl ShellValidationError { // `identical_max_distance`, so we shouldn't have any // distances greater than that. if distances( - edge_a.clone(), + half_edge_a.clone(), surface_a.clone(), edge_b.clone(), surface_b.clone(), @@ -302,7 +302,7 @@ impl ShellValidationError { { errors.push( Self::IdenticalEdgesNotCoincident { - half_edge_a: edge_a.clone(), + half_edge_a: half_edge_a.clone(), surface_a: surface_a.clone(), half_edge_b: edge_b.clone(), surface_b: surface_b.clone(), @@ -315,7 +315,7 @@ impl ShellValidationError { // If all points on distinct curves are within // `distinct_min_distance`, that's a problem. if distances( - edge_a.clone(), + half_edge_a.clone(), surface_a.clone(), edge_b.clone(), surface_b.clone(), @@ -324,7 +324,7 @@ impl ShellValidationError { { errors.push( Self::CoincidentEdgesNotIdentical( - edge_a.clone(), + half_edge_a.clone(), edge_b.clone(), ) .into(), From e1bf5e5b0c80072d732226901e18bcda1f0ab775 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:06:43 +0200 Subject: [PATCH 06/15] Update variable name --- crates/fj-core/src/validate/shell.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 07262a0c9..419fb0731 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -262,15 +262,15 @@ impl ShellValidationError { // need to deal with float inaccuracies. Maybe we could use some smarter // data-structure like an octree. for (half_edge_a, surface_a) in &edges_and_surfaces { - for (edge_b, surface_b) in &edges_and_surfaces { + for (half_edge_b, surface_b) in &edges_and_surfaces { // No need to check an edge against itself. - if half_edge_a.id() == edge_b.id() { + if half_edge_a.id() == half_edge_b.id() { continue; } let identical = { let on_same_curve = - half_edge_a.curve().id() == edge_b.curve().id(); + half_edge_a.curve().id() == half_edge_b.curve().id(); let have_same_boundary = { let bounding_vertices_of = |edge| { @@ -281,7 +281,7 @@ impl ShellValidationError { }; bounding_vertices_of(half_edge_a) - == bounding_vertices_of(edge_b) + == bounding_vertices_of(half_edge_b) }; on_same_curve && have_same_boundary @@ -295,7 +295,7 @@ impl ShellValidationError { if distances( half_edge_a.clone(), surface_a.clone(), - edge_b.clone(), + half_edge_b.clone(), surface_b.clone(), ) .any(|d| d > config.identical_max_distance) @@ -304,7 +304,7 @@ impl ShellValidationError { Self::IdenticalEdgesNotCoincident { half_edge_a: half_edge_a.clone(), surface_a: surface_a.clone(), - half_edge_b: edge_b.clone(), + half_edge_b: half_edge_b.clone(), surface_b: surface_b.clone(), } .into(), @@ -317,7 +317,7 @@ impl ShellValidationError { if distances( half_edge_a.clone(), surface_a.clone(), - edge_b.clone(), + half_edge_b.clone(), surface_b.clone(), ) .all(|d| d < config.distinct_min_distance) @@ -325,7 +325,7 @@ impl ShellValidationError { errors.push( Self::CoincidentEdgesNotIdentical( half_edge_a.clone(), - edge_b.clone(), + half_edge_b.clone(), ) .into(), ) From 835cc649a69f015f0e7eafc18b2cd5c3bb0445ee Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:07:42 +0200 Subject: [PATCH 07/15] Update comment --- crates/fj-core/src/validate/shell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 419fb0731..436e42036 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -263,7 +263,7 @@ impl ShellValidationError { // data-structure like an octree. for (half_edge_a, surface_a) in &edges_and_surfaces { for (half_edge_b, surface_b) in &edges_and_surfaces { - // No need to check an edge against itself. + // No need to check a half-edge against itself. if half_edge_a.id() == half_edge_b.id() { continue; } From b950f4b057ff8fa2abf11ad1f3b5a4c901da210e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:09:02 +0200 Subject: [PATCH 08/15] Simplify `check_half_edge_coincidence` --- crates/fj-core/src/validate/shell.rs | 81 ++++++++-------------------- 1 file changed, 21 insertions(+), 60 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 436e42036..22ee2a6f2 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -268,69 +268,30 @@ impl ShellValidationError { continue; } - let identical = { - let on_same_curve = - half_edge_a.curve().id() == half_edge_b.curve().id(); - - let have_same_boundary = { - let bounding_vertices_of = |edge| { - shell - .bounding_vertices_of_half_edge(edge) - .expect("Expected edge to be part of shell") - .normalize() - }; - - bounding_vertices_of(half_edge_a) - == bounding_vertices_of(half_edge_b) - }; - - on_same_curve && have_same_boundary - }; - - match identical { - true => { - // All points on identical curves should be within - // `identical_max_distance`, so we shouldn't have any - // distances greater than that. - if distances( - half_edge_a.clone(), - surface_a.clone(), - half_edge_b.clone(), - surface_b.clone(), - ) - .any(|d| d > config.identical_max_distance) - { - errors.push( - Self::IdenticalEdgesNotCoincident { - half_edge_a: half_edge_a.clone(), - surface_a: surface_a.clone(), - half_edge_b: half_edge_b.clone(), - surface_b: surface_b.clone(), - } - .into(), - ) - } - } - false => { - // If all points on distinct curves are within - // `distinct_min_distance`, that's a problem. - if distances( + if shell.are_siblings(half_edge_a, half_edge_b) { + // If the half-edges are siblings, they are allowed to be + // coincident. Must be, in fact. There's another validation + // check that takes care of that. + continue; + } + + // If all points on distinct curves are within + // `distinct_min_distance`, that's a problem. + if distances( + half_edge_a.clone(), + surface_a.clone(), + half_edge_b.clone(), + surface_b.clone(), + ) + .all(|d| d < config.distinct_min_distance) + { + errors.push( + Self::CoincidentEdgesNotIdentical( half_edge_a.clone(), - surface_a.clone(), half_edge_b.clone(), - surface_b.clone(), ) - .all(|d| d < config.distinct_min_distance) - { - errors.push( - Self::CoincidentEdgesNotIdentical( - half_edge_a.clone(), - half_edge_b.clone(), - ) - .into(), - ) - } - } + .into(), + ) } } } From c7c3f0b20c99c9c9310527293cd98afdeb827cf3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:16:55 +0200 Subject: [PATCH 09/15] Remove unused error variant --- crates/fj-core/src/validate/shell.rs | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 22ee2a6f2..e881d9f9c 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -51,28 +51,6 @@ pub enum ShellValidationError { )] CoincidentEdgesNotIdentical(Handle, Handle), - /// [`Shell`] contains edges that are identical, but do not coincide - #[error( - "Shell contains `Edge`s that are identical but do not coincide\n\ - Edge 1: {half_edge_a:#?}\n\ - Surface for edge 1: {surface_a:#?}\n\ - Edge 2: {half_edge_b:#?}\n\ - Surface for edge 2: {surface_b:#?}" - )] - IdenticalEdgesNotCoincident { - /// The first edge - half_edge_a: Handle, - - /// The surface that the first edge is on - surface_a: Handle, - - /// The second edge - half_edge_b: Handle, - - /// The surface that the second edge is on - surface_b: Handle, - }, - /// [`Shell`] contains faces of mixed orientation (inwards and outwards) #[error("Shell has mixed face orientations")] MixedOrientations, From ac8870434a97055bd4217658a536bfc750754809 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:15:04 +0200 Subject: [PATCH 10/15] Update wording of error variant --- crates/fj-core/src/validate/shell.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index e881d9f9c..d9e0bf833 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -49,7 +49,7 @@ pub enum ShellValidationError { Edge 1: {0:#?}\n\ Edge 2: {1:#?}" )] - CoincidentEdgesNotIdentical(Handle, Handle), + CoincidentHalfEdgesAreNotSiblings(Handle, Handle), /// [`Shell`] contains faces of mixed orientation (inwards and outwards) #[error("Shell has mixed face orientations")] @@ -264,7 +264,7 @@ impl ShellValidationError { .all(|d| d < config.distinct_min_distance) { errors.push( - Self::CoincidentEdgesNotIdentical( + Self::CoincidentHalfEdgesAreNotSiblings( half_edge_a.clone(), half_edge_b.clone(), ) @@ -434,7 +434,7 @@ mod tests { assert_contains_err!( invalid, ValidationError::Shell( - ShellValidationError::CoincidentEdgesNotIdentical(..) + ShellValidationError::CoincidentHalfEdgesAreNotSiblings(..) ) ); From c8b07568c07650472ff0fed4848a573107f0d20c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:16:33 +0200 Subject: [PATCH 11/15] Update test name --- crates/fj-core/src/validate/shell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index d9e0bf833..98a959853 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -402,7 +402,7 @@ mod tests { } #[test] - fn coincident_edges_not_identical() -> anyhow::Result<()> { + fn coincident_half_edges_are_not_siblings() -> anyhow::Result<()> { let mut services = Services::new(); let valid = Shell::tetrahedron( From 2e2ed2dcbb97b9e763c226f9f619a81e616fe32a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:12:16 +0200 Subject: [PATCH 12/15] Remove redundant validation check Its functionality has been subsumed by the new half-edge pair check. --- crates/fj-core/src/validate/shell.rs | 70 +--------------------------- 1 file changed, 2 insertions(+), 68 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 98a959853..73c90c613 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -20,7 +20,6 @@ impl Validate for Shell { ShellValidationError::check_curve_coordinates(self, config, errors); ShellValidationError::check_half_edge_pairs(self, errors); ShellValidationError::check_half_edge_coincidence(self, config, errors); - ShellValidationError::check_same_orientation(self, errors); } } @@ -50,10 +49,6 @@ pub enum ShellValidationError { Edge 2: {1:#?}" )] CoincidentHalfEdgesAreNotSiblings(Handle, Handle), - - /// [`Shell`] contains faces of mixed orientation (inwards and outwards) - #[error("Shell has mixed face orientations")] - MixedOrientations, } /// Sample two edges at various (currently 3) points in 3D along them. @@ -274,46 +269,6 @@ impl ShellValidationError { } } } - - fn check_same_orientation( - shell: &Shell, - errors: &mut Vec, - ) { - let mut edges_by_coincidence = BTreeMap::new(); - - for face in shell.faces() { - for cycle in face.region().all_cycles() { - for edge in cycle.half_edges() { - let curve = HandleWrapper::from(edge.curve().clone()); - let boundary = cycle - .bounding_vertices_of_half_edge(edge) - .expect( - "Just got edge from this cycle; must be part of it", - ) - .normalize(); - - edges_by_coincidence - .entry((curve, boundary)) - .or_insert(Vec::new()) - .push(edge.clone()); - } - } - } - - for (_, edges) in edges_by_coincidence { - let mut edges = edges.into_iter(); - - // We should have exactly two coincident edges here. This is - // verified in a different validation check, so let's just silently - // do nothing here, if that isn't the case. - if let (Some(a), Some(b)) = (edges.next(), edges.next()) { - if a.boundary().reverse() != b.boundary() { - errors.push(Self::MixedOrientations.into()); - return; - } - } - } - } } #[derive(Clone, Debug)] @@ -332,8 +287,8 @@ mod tests { assert_contains_err, objects::{Curve, Shell}, operations::{ - BuildShell, Insert, Reverse, UpdateCycle, UpdateFace, - UpdateHalfEdge, UpdateRegion, UpdateShell, + BuildShell, Insert, UpdateCycle, UpdateFace, UpdateHalfEdge, + UpdateRegion, UpdateShell, }, services::Services, validate::{shell::ShellValidationError, Validate, ValidationError}, @@ -440,25 +395,4 @@ mod tests { Ok(()) } - - #[test] - fn mixed_orientations() -> anyhow::Result<()> { - let mut services = Services::new(); - - let valid = Shell::tetrahedron( - [[0., 0., 0.], [0., 1., 0.], [1., 0., 0.], [0., 0., 1.]], - &mut services, - ); - let invalid = valid.shell.update_face(&valid.abc.face, |face| { - face.reverse(&mut services).insert(&mut services) - }); - - valid.shell.validate_and_return_first_error()?; - assert_contains_err!( - invalid, - ValidationError::Shell(ShellValidationError::MixedOrientations) - ); - - Ok(()) - } } From 9cdb633a739b1cda67a001b8859bbaf53f9b7987 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:21:58 +0200 Subject: [PATCH 13/15] Update documentation of `ShellValidationError` --- crates/fj-core/src/validate/shell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 73c90c613..2e4c7da0e 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -41,7 +41,7 @@ pub enum ShellValidationError { half_edge: Handle, }, - /// [`Shell`] contains edges that are coincident, but not identical + /// [`Shell`] contains half-edges that are coincident, but aren't siblings #[error( "`Shell` contains `Edge`s that are coincident but refer to different \ `Curve`s\n\ @@ -222,7 +222,7 @@ impl ShellValidationError { } } - /// Check that identical half-edges are coincident, non-identical are not + /// Check that non-sibling half-edges are not coincident fn check_half_edge_coincidence( shell: &Shell, config: &ValidationConfig, From d1fef8665ff7c9e0aca801aaccf0e0c11718f970 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:23:51 +0200 Subject: [PATCH 14/15] Update error message --- crates/fj-core/src/validate/shell.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 2e4c7da0e..046f38fab 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -43,10 +43,10 @@ pub enum ShellValidationError { /// [`Shell`] contains half-edges that are coincident, but aren't siblings #[error( - "`Shell` contains `Edge`s that are coincident but refer to different \ - `Curve`s\n\ - Edge 1: {0:#?}\n\ - Edge 2: {1:#?}" + "`Shell` contains `HalfEdge`s that are coincident but are not \ + siblings\n\ + Half-edge 1: {0:#?}\n\ + Half-edge 2: {1:#?}" )] CoincidentHalfEdgesAreNotSiblings(Handle, Handle), } From 02a8ece98889414159eae74f713006ddca92933f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 20 Oct 2023 11:24:17 +0200 Subject: [PATCH 15/15] Update order of code In its old position, the function separated the definition of `ShellValidationError` from its impl block. Plus, in other parts of the code base, functions tend to be defined *after* they are used. --- crates/fj-core/src/validate/shell.rs | 70 ++++++++++++++-------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 046f38fab..571aaf40b 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -51,41 +51,6 @@ pub enum ShellValidationError { CoincidentHalfEdgesAreNotSiblings(Handle, Handle), } -/// Sample two edges at various (currently 3) points in 3D along them. -/// -/// Returns an [`Iterator`] of the distance at each sample. -fn distances( - edge_a: Handle, - surface_a: Handle, - edge_b: Handle, - surface_b: Handle, -) -> impl Iterator { - fn sample( - percent: f64, - (edge, surface): (&Handle, SurfaceGeometry), - ) -> Point<3> { - let [start, end] = edge.boundary().inner; - let path_coords = start + (end - start) * percent; - let surface_coords = edge.path().point_from_path_coords(path_coords); - surface.point_from_surface_coords(surface_coords) - } - - // Three samples (start, middle, end), are enough to detect weather lines - // and circles match. If we were to add more complicated curves, this might - // need to change. - let sample_count = 3; - let step = 1.0 / (sample_count as f64 - 1.0); - - let mut distances = Vec::new(); - for i in 0..sample_count { - let percent = i as f64 * step; - let sample1 = sample(percent, (&edge_a, surface_a.geometry())); - let sample2 = sample(1.0 - percent, (&edge_b, surface_b.geometry())); - distances.push(sample1.distance_to(&sample2)) - } - distances.into_iter() -} - impl ShellValidationError { /// Check that local curve definitions that refer to the same curve match fn check_curve_coordinates( @@ -281,6 +246,41 @@ pub struct CurveCoordinateSystemMismatch { pub distance: Scalar, } +/// Sample two edges at various (currently 3) points in 3D along them. +/// +/// Returns an [`Iterator`] of the distance at each sample. +fn distances( + edge_a: Handle, + surface_a: Handle, + edge_b: Handle, + surface_b: Handle, +) -> impl Iterator { + fn sample( + percent: f64, + (edge, surface): (&Handle, SurfaceGeometry), + ) -> Point<3> { + let [start, end] = edge.boundary().inner; + let path_coords = start + (end - start) * percent; + let surface_coords = edge.path().point_from_path_coords(path_coords); + surface.point_from_surface_coords(surface_coords) + } + + // Three samples (start, middle, end), are enough to detect weather lines + // and circles match. If we were to add more complicated curves, this might + // need to change. + let sample_count = 3; + let step = 1.0 / (sample_count as f64 - 1.0); + + let mut distances = Vec::new(); + for i in 0..sample_count { + let percent = i as f64 * step; + let sample1 = sample(percent, (&edge_a, surface_a.geometry())); + let sample2 = sample(1.0 - percent, (&edge_b, surface_b.geometry())); + distances.push(sample1.distance_to(&sample2)) + } + distances.into_iter() +} + #[cfg(test)] mod tests { use crate::{