From f2bf1ede7534e10d853f0d48e5ca6fbf993e3e69 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:36:14 +0200 Subject: [PATCH 01/17] Add `HalfEdgeHasNoSibling` --- crates/fj-core/src/validate/shell.rs | 17 ++++++++++------- .../validation/checks/half_edge_siblings.rs | 18 ++++++++++++++++++ crates/fj-core/src/validation/checks/mod.rs | 2 ++ 3 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 crates/fj-core/src/validation/checks/half_edge_siblings.rs diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 20a4c966a..e54800f30 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -9,7 +9,10 @@ use crate::{ }, storage::Handle, topology::{Curve, HalfEdge, Shell, Vertex}, - validation::{checks::CurveGeometryMismatch, ValidationCheck}, + validation::{ + checks::{CurveGeometryMismatch, HalfEdgeHasNoSibling}, + ValidationCheck, + }, }; use super::{Validate, ValidationConfig, ValidationError}; @@ -36,11 +39,8 @@ impl Validate for Shell { #[derive(Clone, Debug, thiserror::Error)] pub enum ShellValidationError { /// [`Shell`] contains a half-edge that is not part of a pair - #[error("Half-edge has no sibling: {half_edge:#?}")] - HalfEdgeHasNoSibling { - /// The half-edge that has no sibling - half_edge: Handle, - }, + #[error("Half-edge has no sibling: {:#?}", .0.half_edge)] + HalfEdgeHasNoSibling(HalfEdgeHasNoSibling), /// [`Shell`] contains half-edges that are coincident, but aren't siblings #[error( @@ -114,7 +114,10 @@ impl ShellValidationError { } for half_edge in unmatched_half_edges.into_values().cloned() { - errors.push(Self::HalfEdgeHasNoSibling { half_edge }.into()); + errors.push( + Self::HalfEdgeHasNoSibling(HalfEdgeHasNoSibling { half_edge }) + .into(), + ); } } diff --git a/crates/fj-core/src/validation/checks/half_edge_siblings.rs b/crates/fj-core/src/validation/checks/half_edge_siblings.rs new file mode 100644 index 000000000..71e06b226 --- /dev/null +++ b/crates/fj-core/src/validation/checks/half_edge_siblings.rs @@ -0,0 +1,18 @@ +use crate::{storage::Handle, topology::HalfEdge}; + +/// A [`Shell`] contains a [`HalfEdge`] without a sibling +/// +/// Half-edges that are coincident must reference the same curve. This makes +/// those half-edges siblings. +/// +/// In a shell, every half-edge must have a sibling. If that is not the case, +/// this is a sign of either of the following: +/// - That the shell is not closed, meaning it has some kind of hole. +/// - If the shell is closed, that its topological object graph is not valid. +/// +/// [`Shell`]: crate::topology::Shell +#[derive(Clone, Debug)] +pub struct HalfEdgeHasNoSibling { + /// The half-edge that does not have a sibling + pub half_edge: Handle, +} diff --git a/crates/fj-core/src/validation/checks/mod.rs b/crates/fj-core/src/validation/checks/mod.rs index ec0b68aa5..c66e63357 100644 --- a/crates/fj-core/src/validation/checks/mod.rs +++ b/crates/fj-core/src/validation/checks/mod.rs @@ -6,10 +6,12 @@ mod curve_geometry_mismatch; mod face_boundary; mod face_winding; mod half_edge_connection; +mod half_edge_siblings; pub use self::{ curve_geometry_mismatch::CurveGeometryMismatch, face_boundary::FaceHasNoBoundary, face_winding::InteriorCycleHasInvalidWinding, half_edge_connection::AdjacentHalfEdgesNotConnected, + half_edge_siblings::HalfEdgeHasNoSibling, }; From a06a8c0102095e7cde476eeaf5ffab701ba9a756 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:37:31 +0200 Subject: [PATCH 02/17] Derive `Error` for `HalfEdgeHasNoSibling` --- crates/fj-core/src/validate/shell.rs | 2 +- crates/fj-core/src/validation/checks/half_edge_siblings.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index e54800f30..ec72df05e 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -39,7 +39,7 @@ impl Validate for Shell { #[derive(Clone, Debug, thiserror::Error)] pub enum ShellValidationError { /// [`Shell`] contains a half-edge that is not part of a pair - #[error("Half-edge has no sibling: {:#?}", .0.half_edge)] + #[error(transparent)] HalfEdgeHasNoSibling(HalfEdgeHasNoSibling), /// [`Shell`] contains half-edges that are coincident, but aren't siblings diff --git a/crates/fj-core/src/validation/checks/half_edge_siblings.rs b/crates/fj-core/src/validation/checks/half_edge_siblings.rs index 71e06b226..180206944 100644 --- a/crates/fj-core/src/validation/checks/half_edge_siblings.rs +++ b/crates/fj-core/src/validation/checks/half_edge_siblings.rs @@ -11,7 +11,8 @@ use crate::{storage::Handle, topology::HalfEdge}; /// - If the shell is closed, that its topological object graph is not valid. /// /// [`Shell`]: crate::topology::Shell -#[derive(Clone, Debug)] +#[derive(Clone, Debug, thiserror::Error)] +#[error("Half-edge has no sibling: {half_edge:#?}")] pub struct HalfEdgeHasNoSibling { /// The half-edge that does not have a sibling pub half_edge: Handle, From 3ddc250bc8109d549748a2fc38e8e16446d965b4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:40:56 +0200 Subject: [PATCH 03/17] Refactor to prepare for follow-on change --- crates/fj-core/src/validate/shell.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index ec72df05e..1b539465b 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -113,12 +113,17 @@ impl ShellValidationError { } } - for half_edge in unmatched_half_edges.into_values().cloned() { - errors.push( - Self::HalfEdgeHasNoSibling(HalfEdgeHasNoSibling { half_edge }) + unmatched_half_edges + .into_values() + .cloned() + .for_each(|half_edge| { + errors.push( + Self::HalfEdgeHasNoSibling(HalfEdgeHasNoSibling { + half_edge, + }) .into(), - ); - } + ); + }); } /// Check that non-sibling half-edges are not coincident From b69691e523a8d1f20090d20fdf64d993c1c29771 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:41:32 +0200 Subject: [PATCH 04/17] Refactor to prepare for follow-on change --- crates/fj-core/src/validate/shell.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 1b539465b..2681e9530 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -116,13 +116,12 @@ impl ShellValidationError { unmatched_half_edges .into_values() .cloned() - .for_each(|half_edge| { - errors.push( - Self::HalfEdgeHasNoSibling(HalfEdgeHasNoSibling { - half_edge, - }) - .into(), - ); + .map(|half_edge| { + Self::HalfEdgeHasNoSibling(HalfEdgeHasNoSibling { half_edge }) + .into() + }) + .for_each(|err| { + errors.push(err); }); } From aeb5ac8131a5840b65f0d7e56f30c5d4cfecf3f5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:42:01 +0200 Subject: [PATCH 05/17] Refactor to prepare for follow-on change --- crates/fj-core/src/validate/shell.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 2681e9530..bc49752bd 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -113,16 +113,12 @@ impl ShellValidationError { } } - unmatched_half_edges - .into_values() - .cloned() - .map(|half_edge| { + errors.extend(unmatched_half_edges.into_values().cloned().map( + |half_edge| { Self::HalfEdgeHasNoSibling(HalfEdgeHasNoSibling { half_edge }) .into() - }) - .for_each(|err| { - errors.push(err); - }); + }, + )); } /// Check that non-sibling half-edges are not coincident From 43b924ed69ff6d4fffe173427948314585f87486 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:43:43 +0200 Subject: [PATCH 06/17] Add lifetime to prepare for follow-on change --- 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 bc49752bd..863322b3c 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -72,10 +72,10 @@ pub enum ShellValidationError { impl ShellValidationError { /// Check that each half-edge is part of a pair - fn check_half_edge_pairs( - shell: &Shell, - geometry: &Geometry, - errors: &mut Vec, + fn check_half_edge_pairs<'r>( + shell: &'r Shell, + geometry: &'r Geometry, + errors: &'r mut Vec, ) { let mut unmatched_half_edges = BTreeMap::new(); From 187a943e18196e9f8bd8d81bad9fa6cbeb96d592 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:45:12 +0200 Subject: [PATCH 07/17] Refactor to prepare for follow-on change --- crates/fj-core/src/validate/shell.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 863322b3c..3ad4bdee6 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -28,7 +28,9 @@ impl Validate for Shell { CurveGeometryMismatch::check(self, geometry, config) .map(Into::into), ); - ShellValidationError::check_half_edge_pairs(self, geometry, errors); + errors.extend(ShellValidationError::check_half_edge_pairs( + self, geometry, + )); ShellValidationError::check_half_edge_coincidence( self, geometry, config, errors, ); @@ -75,8 +77,7 @@ impl ShellValidationError { fn check_half_edge_pairs<'r>( shell: &'r Shell, geometry: &'r Geometry, - errors: &'r mut Vec, - ) { + ) -> impl Iterator + 'r { let mut unmatched_half_edges = BTreeMap::new(); for face in shell.faces() { @@ -113,12 +114,13 @@ impl ShellValidationError { } } - errors.extend(unmatched_half_edges.into_values().cloned().map( - |half_edge| { + unmatched_half_edges + .into_values() + .cloned() + .map(|half_edge| { Self::HalfEdgeHasNoSibling(HalfEdgeHasNoSibling { half_edge }) .into() - }, - )); + }) } /// Check that non-sibling half-edges are not coincident From 8c2a1eea69a127262043962d32ecdb6e02f93f0c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:45:59 +0200 Subject: [PATCH 08/17] Refactor to prepare for follow-on change --- crates/fj-core/src/validate/shell.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 3ad4bdee6..52e60533f 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -28,9 +28,10 @@ impl Validate for Shell { CurveGeometryMismatch::check(self, geometry, config) .map(Into::into), ); - errors.extend(ShellValidationError::check_half_edge_pairs( - self, geometry, - )); + errors.extend( + ShellValidationError::check_half_edge_pairs(self, geometry) + .map(Into::into), + ); ShellValidationError::check_half_edge_coincidence( self, geometry, config, errors, ); @@ -77,7 +78,7 @@ impl ShellValidationError { fn check_half_edge_pairs<'r>( shell: &'r Shell, geometry: &'r Geometry, - ) -> impl Iterator + 'r { + ) -> impl Iterator + 'r { let mut unmatched_half_edges = BTreeMap::new(); for face in shell.faces() { @@ -119,7 +120,6 @@ impl ShellValidationError { .cloned() .map(|half_edge| { Self::HalfEdgeHasNoSibling(HalfEdgeHasNoSibling { half_edge }) - .into() }) } From 0aef67f9ce4b41fc3f32530b99cbf509e79046cc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:48:44 +0200 Subject: [PATCH 09/17] Refactor to prepare for follow-on change --- crates/fj-core/src/validate/shell.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 52e60533f..41632cfd0 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -30,6 +30,7 @@ impl Validate for Shell { ); errors.extend( ShellValidationError::check_half_edge_pairs(self, geometry) + .map(ShellValidationError::HalfEdgeHasNoSibling) .map(Into::into), ); ShellValidationError::check_half_edge_coincidence( @@ -78,7 +79,7 @@ impl ShellValidationError { fn check_half_edge_pairs<'r>( shell: &'r Shell, geometry: &'r Geometry, - ) -> impl Iterator + 'r { + ) -> impl Iterator + 'r { let mut unmatched_half_edges = BTreeMap::new(); for face in shell.faces() { @@ -118,9 +119,7 @@ impl ShellValidationError { unmatched_half_edges .into_values() .cloned() - .map(|half_edge| { - Self::HalfEdgeHasNoSibling(HalfEdgeHasNoSibling { half_edge }) - }) + .map(|half_edge| HalfEdgeHasNoSibling { half_edge }) } /// Check that non-sibling half-edges are not coincident From 03e0c320fabffd67a22776132adc718420b9c88f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:49:28 +0200 Subject: [PATCH 10/17] Rename argument to prepare for follow-on change --- 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 41632cfd0..b4da23679 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -77,12 +77,12 @@ pub enum ShellValidationError { impl ShellValidationError { /// Check that each half-edge is part of a pair fn check_half_edge_pairs<'r>( - shell: &'r Shell, + object: &'r Shell, geometry: &'r Geometry, ) -> impl Iterator + 'r { let mut unmatched_half_edges = BTreeMap::new(); - for face in shell.faces() { + for face in object.faces() { for cycle in face.region().all_cycles() { for half_edge in cycle.half_edges() { let curve = half_edge.curve().clone(); @@ -102,7 +102,7 @@ impl ShellValidationError { // currently looking at. Let's make sure the logic // we use here to determine that matches the // "official" definition. - assert!(shell + assert!(object .are_siblings(half_edge, sibling, geometry)); } None => { From aca0bc75d5926b70c4ef3f8946fe5059fb4a82b4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:51:10 +0200 Subject: [PATCH 11/17] Pass argument to prepare for follow-on change --- crates/fj-core/src/validate/shell.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index b4da23679..d458e1fe1 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -29,7 +29,7 @@ impl Validate for Shell { .map(Into::into), ); errors.extend( - ShellValidationError::check_half_edge_pairs(self, geometry) + ShellValidationError::check_half_edge_pairs(self, geometry, config) .map(ShellValidationError::HalfEdgeHasNoSibling) .map(Into::into), ); @@ -79,6 +79,7 @@ impl ShellValidationError { fn check_half_edge_pairs<'r>( object: &'r Shell, geometry: &'r Geometry, + _: &'r ValidationConfig, ) -> impl Iterator + 'r { let mut unmatched_half_edges = BTreeMap::new(); From 6f092467e75f6c6f629a793b9fc691fea4946111 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:52:19 +0200 Subject: [PATCH 12/17] Impl `ValidationCheck` for `HalfEdgeHasNoSibling` --- crates/fj-core/src/validate/shell.rs | 45 +------------- .../validation/checks/half_edge_siblings.rs | 61 ++++++++++++++++++- 2 files changed, 61 insertions(+), 45 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index d458e1fe1..bf9967129 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, fmt}; +use std::fmt; use fj_math::{Point, Scalar}; @@ -79,48 +79,9 @@ impl ShellValidationError { fn check_half_edge_pairs<'r>( object: &'r Shell, geometry: &'r Geometry, - _: &'r ValidationConfig, + config: &'r ValidationConfig, ) -> impl Iterator + 'r { - let mut unmatched_half_edges = BTreeMap::new(); - - for face in object.faces() { - for cycle in face.region().all_cycles() { - for half_edge in cycle.half_edges() { - let curve = half_edge.curve().clone(); - let boundary = geometry.of_half_edge(half_edge).boundary; - let vertices = - cycle.bounding_vertices_of_half_edge(half_edge).expect( - "`half_edge` came from `cycle`, must exist there", - ); - - let key = (curve.clone(), boundary, vertices.clone()); - let key_reversed = - (curve, boundary.reverse(), vertices.reverse()); - - 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!(object - .are_siblings(half_edge, sibling, geometry)); - } - 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); - } - } - } - } - } - - unmatched_half_edges - .into_values() - .cloned() - .map(|half_edge| HalfEdgeHasNoSibling { half_edge }) + HalfEdgeHasNoSibling::check(object, geometry, config) } /// Check that non-sibling half-edges are not coincident diff --git a/crates/fj-core/src/validation/checks/half_edge_siblings.rs b/crates/fj-core/src/validation/checks/half_edge_siblings.rs index 180206944..29caccd4d 100644 --- a/crates/fj-core/src/validation/checks/half_edge_siblings.rs +++ b/crates/fj-core/src/validation/checks/half_edge_siblings.rs @@ -1,4 +1,12 @@ -use crate::{storage::Handle, topology::HalfEdge}; +use std::collections::BTreeMap; + +use crate::{ + geometry::Geometry, + queries::{BoundingVerticesOfHalfEdge, SiblingOfHalfEdge}, + storage::Handle, + topology::{HalfEdge, Shell}, + validation::{ValidationCheck, ValidationConfig}, +}; /// A [`Shell`] contains a [`HalfEdge`] without a sibling /// @@ -9,11 +17,58 @@ use crate::{storage::Handle, topology::HalfEdge}; /// this is a sign of either of the following: /// - That the shell is not closed, meaning it has some kind of hole. /// - If the shell is closed, that its topological object graph is not valid. -/// -/// [`Shell`]: crate::topology::Shell #[derive(Clone, Debug, thiserror::Error)] #[error("Half-edge has no sibling: {half_edge:#?}")] pub struct HalfEdgeHasNoSibling { /// The half-edge that does not have a sibling pub half_edge: Handle, } + +impl ValidationCheck for HalfEdgeHasNoSibling { + fn check<'r>( + object: &'r Shell, + geometry: &'r Geometry, + _: &'r ValidationConfig, + ) -> impl Iterator + 'r { + let mut unmatched_half_edges = BTreeMap::new(); + + for face in object.faces() { + for cycle in face.region().all_cycles() { + for half_edge in cycle.half_edges() { + let curve = half_edge.curve().clone(); + let boundary = geometry.of_half_edge(half_edge).boundary; + let vertices = + cycle.bounding_vertices_of_half_edge(half_edge).expect( + "`half_edge` came from `cycle`, must exist there", + ); + + let key = (curve.clone(), boundary, vertices.clone()); + let key_reversed = + (curve, boundary.reverse(), vertices.reverse()); + + 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!(object + .are_siblings(half_edge, sibling, geometry)); + } + 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); + } + } + } + } + } + + unmatched_half_edges + .into_values() + .cloned() + .map(|half_edge| HalfEdgeHasNoSibling { half_edge }) + } +} From 7885e699ba06cff3ef3316214db38d88a9f17ba2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:52:54 +0200 Subject: [PATCH 13/17] Inline redundant function --- crates/fj-core/src/validate/shell.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index bf9967129..cd0a2e360 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -29,7 +29,7 @@ impl Validate for Shell { .map(Into::into), ); errors.extend( - ShellValidationError::check_half_edge_pairs(self, geometry, config) + HalfEdgeHasNoSibling::check(self, geometry, config) .map(ShellValidationError::HalfEdgeHasNoSibling) .map(Into::into), ); @@ -75,15 +75,6 @@ pub enum ShellValidationError { } impl ShellValidationError { - /// Check that each half-edge is part of a pair - fn check_half_edge_pairs<'r>( - object: &'r Shell, - geometry: &'r Geometry, - config: &'r ValidationConfig, - ) -> impl Iterator + 'r { - HalfEdgeHasNoSibling::check(object, geometry, config) - } - /// Check that non-sibling half-edges are not coincident fn check_half_edge_coincidence( shell: &Shell, From f165c25faae1a4aa1bde60d7cc3da5a62c304832 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:54:36 +0200 Subject: [PATCH 14/17] Move `HalfEdgeHasNoSibling` to `ValidationError` --- crates/fj-core/src/validate/shell.rs | 12 ++---------- crates/fj-core/src/validation/error.rs | 6 +++++- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index cd0a2e360..7da8ed10e 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -29,9 +29,7 @@ impl Validate for Shell { .map(Into::into), ); errors.extend( - HalfEdgeHasNoSibling::check(self, geometry, config) - .map(ShellValidationError::HalfEdgeHasNoSibling) - .map(Into::into), + HalfEdgeHasNoSibling::check(self, geometry, config).map(Into::into), ); ShellValidationError::check_half_edge_coincidence( self, geometry, config, errors, @@ -42,10 +40,6 @@ impl Validate for Shell { /// [`Shell`] validation failed #[derive(Clone, Debug, thiserror::Error)] pub enum ShellValidationError { - /// [`Shell`] contains a half-edge that is not part of a pair - #[error(transparent)] - HalfEdgeHasNoSibling(HalfEdgeHasNoSibling), - /// [`Shell`] contains half-edges that are coincident, but aren't siblings #[error( "`Shell` contains `HalfEdge`s that are coincident but are not \ @@ -293,9 +287,7 @@ mod tests { assert_contains_err!( core, invalid, - ValidationError::Shell( - ShellValidationError::HalfEdgeHasNoSibling { .. } - ) + ValidationError::HalfEdgeHasNoSibling { .. } ); Ok(()) diff --git a/crates/fj-core/src/validation/error.rs b/crates/fj-core/src/validation/error.rs index 8fb15cbee..071317f64 100644 --- a/crates/fj-core/src/validation/error.rs +++ b/crates/fj-core/src/validation/error.rs @@ -6,7 +6,7 @@ use crate::validate::{ use super::checks::{ AdjacentHalfEdgesNotConnected, CurveGeometryMismatch, FaceHasNoBoundary, - InteriorCycleHasInvalidWinding, + HalfEdgeHasNoSibling, InteriorCycleHasInvalidWinding, }; /// An error that can occur during a validation @@ -24,6 +24,10 @@ pub enum ValidationError { #[error(transparent)] FaceHasNoBoundary(#[from] FaceHasNoBoundary), + /// Half-edge has no sibling + #[error(transparent)] + HalfEdgeHasNoSibling(#[from] HalfEdgeHasNoSibling), + /// Interior cycle has invalid winding #[error(transparent)] InteriorCycleHasInvalidWinding(#[from] InteriorCycleHasInvalidWinding), From ad0680467569e9f14dc40f9e6ece9b59710f3021 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:56:33 +0200 Subject: [PATCH 15/17] Move test closer to code under test --- crates/fj-core/src/validate/shell.rs | 22 ------------ .../validation/checks/half_edge_siblings.rs | 34 +++++++++++++++++++ 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 7da8ed10e..1bc9e3657 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -271,28 +271,6 @@ mod tests { Core, }; - #[test] - fn half_edge_has_no_sibling() -> anyhow::Result<()> { - let mut core = Core::new(); - - let valid = Shell::tetrahedron( - [[0., 0., 0.], [0., 1., 0.], [1., 0., 0.], [0., 0., 1.]], - &mut core, - ); - let invalid = valid.shell.remove_face(&valid.abc.face); - - valid - .shell - .validate_and_return_first_error(&core.layers.geometry)?; - assert_contains_err!( - core, - invalid, - ValidationError::HalfEdgeHasNoSibling { .. } - ); - - Ok(()) - } - #[test] fn coincident_half_edges_are_not_siblings() -> anyhow::Result<()> { let mut core = Core::new(); diff --git a/crates/fj-core/src/validation/checks/half_edge_siblings.rs b/crates/fj-core/src/validation/checks/half_edge_siblings.rs index 29caccd4d..33411e74d 100644 --- a/crates/fj-core/src/validation/checks/half_edge_siblings.rs +++ b/crates/fj-core/src/validation/checks/half_edge_siblings.rs @@ -72,3 +72,37 @@ impl ValidationCheck for HalfEdgeHasNoSibling { .map(|half_edge| HalfEdgeHasNoSibling { half_edge }) } } + +#[cfg(test)] +mod tests { + use crate::{ + assert_contains_err, + operations::{build::BuildShell, update::UpdateShell}, + topology::Shell, + validate::Validate, + validation::ValidationError, + Core, + }; + + #[test] + fn half_edge_has_no_sibling() -> anyhow::Result<()> { + let mut core = Core::new(); + + let valid = Shell::tetrahedron( + [[0., 0., 0.], [0., 1., 0.], [1., 0., 0.], [0., 0., 1.]], + &mut core, + ); + let invalid = valid.shell.remove_face(&valid.abc.face); + + valid + .shell + .validate_and_return_first_error(&core.layers.geometry)?; + assert_contains_err!( + core, + invalid, + ValidationError::HalfEdgeHasNoSibling { .. } + ); + + Ok(()) + } +} From e1178590b9767fb579eda07559e18f3ccaf07b1d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:57:54 +0200 Subject: [PATCH 16/17] Refactor to increase consistency --- .../src/validation/checks/half_edge_siblings.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validation/checks/half_edge_siblings.rs b/crates/fj-core/src/validation/checks/half_edge_siblings.rs index 33411e74d..96757c7e0 100644 --- a/crates/fj-core/src/validation/checks/half_edge_siblings.rs +++ b/crates/fj-core/src/validation/checks/half_edge_siblings.rs @@ -80,7 +80,9 @@ mod tests { operations::{build::BuildShell, update::UpdateShell}, topology::Shell, validate::Validate, - validation::ValidationError, + validation::{ + checks::HalfEdgeHasNoSibling, ValidationCheck, ValidationError, + }, Core, }; @@ -92,11 +94,13 @@ mod tests { [[0., 0., 0.], [0., 1., 0.], [1., 0., 0.], [0., 0., 1.]], &mut core, ); + HalfEdgeHasNoSibling::check_and_return_first_error( + &valid.shell, + &core.layers.geometry, + )?; + let invalid = valid.shell.remove_face(&valid.abc.face); - valid - .shell - .validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( core, invalid, From 7af79bd7a0062979d673f153af48c57ad8cf1096 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 May 2024 21:59:27 +0200 Subject: [PATCH 17/17] Refactor to increase consistency --- .../src/validation/checks/half_edge_siblings.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/crates/fj-core/src/validation/checks/half_edge_siblings.rs b/crates/fj-core/src/validation/checks/half_edge_siblings.rs index 96757c7e0..4b5dd5d1a 100644 --- a/crates/fj-core/src/validation/checks/half_edge_siblings.rs +++ b/crates/fj-core/src/validation/checks/half_edge_siblings.rs @@ -76,13 +76,9 @@ impl ValidationCheck for HalfEdgeHasNoSibling { #[cfg(test)] mod tests { use crate::{ - assert_contains_err, operations::{build::BuildShell, update::UpdateShell}, topology::Shell, - validate::Validate, - validation::{ - checks::HalfEdgeHasNoSibling, ValidationCheck, ValidationError, - }, + validation::{checks::HalfEdgeHasNoSibling, ValidationCheck}, Core, }; @@ -100,12 +96,11 @@ mod tests { )?; let invalid = valid.shell.remove_face(&valid.abc.face); - - assert_contains_err!( - core, - invalid, - ValidationError::HalfEdgeHasNoSibling { .. } - ); + assert!(HalfEdgeHasNoSibling::check_and_return_first_error( + &invalid, + &core.layers.geometry, + ) + .is_err()); Ok(()) }