diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 20a4c966a..1bc9e3657 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}; @@ -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}; @@ -25,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( + HalfEdgeHasNoSibling::check(self, geometry, config).map(Into::into), + ); ShellValidationError::check_half_edge_coincidence( self, geometry, config, errors, ); @@ -35,13 +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("Half-edge has no sibling: {half_edge:#?}")] - HalfEdgeHasNoSibling { - /// The half-edge that has no sibling - half_edge: Handle, - }, - /// [`Shell`] contains half-edges that are coincident, but aren't siblings #[error( "`Shell` contains `HalfEdge`s that are coincident but are not \ @@ -71,53 +69,6 @@ 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, - ) { - let mut unmatched_half_edges = BTreeMap::new(); - - for face in shell.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!(shell - .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); - } - } - } - } - } - - for half_edge in unmatched_half_edges.into_values().cloned() { - errors.push(Self::HalfEdgeHasNoSibling { half_edge }.into()); - } - } - /// Check that non-sibling half-edges are not coincident fn check_half_edge_coincidence( shell: &Shell, @@ -320,30 +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::Shell( - ShellValidationError::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 new file mode 100644 index 000000000..4b5dd5d1a --- /dev/null +++ b/crates/fj-core/src/validation/checks/half_edge_siblings.rs @@ -0,0 +1,107 @@ +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 +/// +/// 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. +#[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 }) + } +} + +#[cfg(test)] +mod tests { + use crate::{ + operations::{build::BuildShell, update::UpdateShell}, + topology::Shell, + validation::{checks::HalfEdgeHasNoSibling, ValidationCheck}, + 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, + ); + HalfEdgeHasNoSibling::check_and_return_first_error( + &valid.shell, + &core.layers.geometry, + )?; + + let invalid = valid.shell.remove_face(&valid.abc.face); + assert!(HalfEdgeHasNoSibling::check_and_return_first_error( + &invalid, + &core.layers.geometry, + ) + .is_err()); + + Ok(()) + } +} 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, }; 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),