diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs index 5318b2e24..86c91de4f 100644 --- a/crates/fj-core/src/validate/face.rs +++ b/crates/fj-core/src/validate/face.rs @@ -1,11 +1,9 @@ -use fj_math::Winding; - use crate::{ geometry::Geometry, objects::Face, validation::{ - checks::FaceHasNoBoundary, ValidationCheck, ValidationConfig, - ValidationError, + checks::{FaceHasNoBoundary, InteriorCycleHasInvalidWinding}, + ValidationCheck, ValidationConfig, ValidationError, }, }; @@ -21,143 +19,9 @@ impl Validate for Face { errors.extend( FaceHasNoBoundary::check(self, geometry, config).map(Into::into), ); - FaceValidationError::check_interior_winding(self, geometry, errors); - } -} - -/// [`Face`] validation error -#[derive(Clone, Debug, thiserror::Error)] -pub enum FaceValidationError { - /// Interior of [`Face`] has invalid winding; must be opposite of exterior - #[error( - "Interior of `Face` has invalid winding; must be opposite of exterior\n\ - - Winding of exterior cycle: {exterior_winding:#?}\n\ - - Winding of interior cycle: {interior_winding:#?}\n\ - - `Face`: {face:#?}" - )] - InvalidInteriorWinding { - /// The winding of the [`Face`]'s exterior cycle - exterior_winding: Winding, - - /// The winding of the invalid interior cycle - interior_winding: Winding, - - /// The face - face: Face, - }, -} - -impl FaceValidationError { - fn check_interior_winding( - face: &Face, - geometry: &Geometry, - errors: &mut Vec, - ) { - if face.region().exterior().half_edges().is_empty() { - // Can't determine winding, if the cycle has no edges. Sounds like a - // job for a different validation check. - return; - } - - let exterior_winding = face.region().exterior().winding(geometry); - - for interior in face.region().interiors() { - if interior.half_edges().is_empty() { - // Can't determine winding, if the cycle has no edges. Sounds - // like a job for a different validation check. - continue; - } - let interior_winding = interior.winding(geometry); - - if exterior_winding == interior_winding { - errors.push( - Self::InvalidInteriorWinding { - exterior_winding, - interior_winding, - face: face.clone(), - } - .into(), - ); - } - } - } -} - -#[cfg(test)] -mod tests { - use crate::{ - assert_contains_err, - objects::{Cycle, Face, Region}, - operations::{ - build::{BuildCycle, BuildFace}, - derive::DeriveFrom, - insert::Insert, - reverse::Reverse, - update::{UpdateFace, UpdateRegion}, - }, - validate::{FaceValidationError, Validate}, - validation::ValidationError, - Core, - }; - - #[test] - fn interior_winding() -> anyhow::Result<()> { - let mut core = Core::new(); - - let valid = - Face::unbound(core.layers.objects.surfaces.xy_plane(), &mut core) - .update_region( - |region, core| { - region - .update_exterior( - |_, core| { - Cycle::polygon( - [[0., 0.], [3., 0.], [0., 3.]], - core, - ) - }, - core, - ) - .add_interiors( - [Cycle::polygon( - [[1., 1.], [1., 2.], [2., 1.]], - core, - )], - core, - ) - }, - &mut core, - ); - let invalid = { - let interiors = valid - .region() - .interiors() - .iter() - .cloned() - .map(|cycle| { - cycle - .reverse(&mut core) - .insert(&mut core) - .derive_from(&cycle, &mut core) - }) - .collect::>(); - - let region = - Region::new(valid.region().exterior().clone(), interiors) - .insert(&mut core); - - Face::new(valid.surface().clone(), region) - }; - - valid.validate_and_return_first_error(&core.layers.geometry)?; - assert_contains_err!( - core, - invalid, - ValidationError::Face( - FaceValidationError::InvalidInteriorWinding { .. } - ) + errors.extend( + InteriorCycleHasInvalidWinding::check(self, geometry, config) + .map(Into::into), ); - - Ok(()) } } diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index 9f77067a3..e0103a2a9 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -79,9 +79,8 @@ use crate::{ }; pub use self::{ - edge::EdgeValidationError, face::FaceValidationError, - shell::ShellValidationError, sketch::SketchValidationError, - solid::SolidValidationError, + edge::EdgeValidationError, shell::ShellValidationError, + sketch::SketchValidationError, solid::SolidValidationError, }; /// Assert that some object has a validation error which matches a specific diff --git a/crates/fj-core/src/validation/checks/face_winding.rs b/crates/fj-core/src/validation/checks/face_winding.rs new file mode 100644 index 000000000..8811f34c6 --- /dev/null +++ b/crates/fj-core/src/validation/checks/face_winding.rs @@ -0,0 +1,136 @@ +use fj_math::Winding; + +use crate::{ + objects::{Cycle, Face}, + storage::Handle, + validation::ValidationCheck, +}; + +/// Interior [`Cycle`] of [`Face`] has invalid winding +/// +/// The winding of a face's exterior cycle is part of what defines the +/// orientation of that face. The winding of the interior cycle has no such +/// meaning attached to it, but it can't be arbitrary either. Triangulation, for +/// example, might need to assume that it is the opposite of the exterior +/// winding. +/// +/// This validation check ensures just that: that the winding of the interior +/// cycles of a face is the opposite of the winding of that face's exterior +/// cycle. +#[derive(Clone, Debug, thiserror::Error)] +#[error( + "Interior of `Face` has invalid winding; must be opposite of exterior\n\ + - Winding of exterior cycle: {exterior_winding:#?}\n\ + - Interior cycle with invalid winding: {interior_cycle:#?}\n\ + - Winding of invalid interior cycle: {interior_winding:#?}" +)] +pub struct InteriorCycleHasInvalidWinding { + /// The winding of the [`Face`]'s exterior cycle + pub exterior_winding: Winding, + + /// The interior cycle with invalid winding + pub interior_cycle: Handle, + + /// The winding of the invalid interior cycle + pub interior_winding: Winding, +} + +impl ValidationCheck for InteriorCycleHasInvalidWinding { + fn check( + object: &Face, + geometry: &crate::geometry::Geometry, + _: &crate::validation::ValidationConfig, + ) -> impl Iterator { + object.region().interiors().iter().filter_map(|interior| { + let exterior = object.region().exterior(); + + if exterior.half_edges().is_empty() + || interior.half_edges().is_empty() + { + // Can't determine winding, if the cycle has no edges. Sounds + // like a job for a different validation check. + return None; + } + + let exterior_winding = exterior.winding(geometry); + let interior_winding = interior.winding(geometry); + + if exterior_winding == interior_winding { + return Some(InteriorCycleHasInvalidWinding { + exterior_winding, + interior_cycle: interior.clone(), + interior_winding, + }); + } + + None + }) + } +} + +#[cfg(test)] +mod tests { + use crate::{ + objects::{Cycle, Face, Region}, + operations::{ + build::{BuildCycle, BuildFace}, + derive::DeriveFrom, + insert::Insert, + reverse::Reverse, + update::{UpdateFace, UpdateRegion}, + }, + validation::{checks::InteriorCycleHasInvalidWinding, ValidationCheck}, + Core, + }; + + #[test] + fn interior_winding() -> anyhow::Result<()> { + let mut core = Core::new(); + + let valid = Face::polygon( + core.layers.objects.surfaces.xy_plane(), + [[0., 0.], [3., 0.], [0., 3.]], + &mut core, + ) + .update_region( + |region, core| { + region.add_interiors( + [Cycle::polygon([[1., 1.], [1., 2.], [2., 1.]], core)], + core, + ) + }, + &mut core, + ); + InteriorCycleHasInvalidWinding::check_and_return_first_error( + &valid, + &core.layers.geometry, + )?; + + let invalid = { + let interiors = valid + .region() + .interiors() + .iter() + .cloned() + .map(|cycle| { + cycle + .reverse(&mut core) + .insert(&mut core) + .derive_from(&cycle, &mut core) + }) + .collect::>(); + + let region = + Region::new(valid.region().exterior().clone(), interiors) + .insert(&mut core); + + Face::new(valid.surface().clone(), region) + }; + InteriorCycleHasInvalidWinding::check_and_expect_one_error( + &invalid, + &core.layers.geometry, + ); + + Ok(()) + } +} diff --git a/crates/fj-core/src/validation/checks/mod.rs b/crates/fj-core/src/validation/checks/mod.rs index b60f27d36..33f3e8c22 100644 --- a/crates/fj-core/src/validation/checks/mod.rs +++ b/crates/fj-core/src/validation/checks/mod.rs @@ -3,9 +3,11 @@ //! See documentation of [parent module](super) for more information. mod face_boundary; +mod face_winding; mod half_edge_connection; pub use self::{ face_boundary::FaceHasNoBoundary, + face_winding::InteriorCycleHasInvalidWinding, half_edge_connection::AdjacentHalfEdgesNotConnected, }; diff --git a/crates/fj-core/src/validation/error.rs b/crates/fj-core/src/validation/error.rs index 4c0992669..247b49d28 100644 --- a/crates/fj-core/src/validation/error.rs +++ b/crates/fj-core/src/validation/error.rs @@ -1,11 +1,14 @@ use std::{convert::Infallible, fmt}; use crate::validate::{ - EdgeValidationError, FaceValidationError, ShellValidationError, - SketchValidationError, SolidValidationError, + EdgeValidationError, ShellValidationError, SketchValidationError, + SolidValidationError, }; -use super::checks::{AdjacentHalfEdgesNotConnected, FaceHasNoBoundary}; +use super::checks::{ + AdjacentHalfEdgesNotConnected, FaceHasNoBoundary, + InteriorCycleHasInvalidWinding, +}; /// An error that can occur during a validation #[derive(Clone, Debug, thiserror::Error)] @@ -18,14 +21,14 @@ pub enum ValidationError { #[error(transparent)] FaceHasNoBoundary(#[from] FaceHasNoBoundary), + /// Interior cycle has invalid winding + #[error(transparent)] + InteriorCycleHasInvalidWinding(#[from] InteriorCycleHasInvalidWinding), + /// `Edge` validation error #[error("`Edge` validation error")] Edge(#[from] EdgeValidationError), - /// `Face` validation error - #[error("`Face` validation error")] - Face(#[from] FaceValidationError), - /// `Shell` validation error #[error("`Shell` validation error")] Shell(#[from] ShellValidationError),