diff --git a/crates/fj-core/src/validate/cycle.rs b/crates/fj-core/src/validate/cycle.rs index bc9ed4ddf..854a9adba 100644 --- a/crates/fj-core/src/validate/cycle.rs +++ b/crates/fj-core/src/validate/cycle.rs @@ -1,10 +1,7 @@ use crate::{ geometry::Geometry, topology::Cycle, - validation::{ - checks::AdjacentHalfEdgesNotConnected, ValidationCheck, - ValidationConfig, ValidationError, - }, + validation::{ValidationConfig, ValidationError}, }; use super::Validate; @@ -12,13 +9,9 @@ use super::Validate; impl Validate for Cycle { fn validate( &self, - config: &ValidationConfig, - errors: &mut Vec, - geometry: &Geometry, + _: &ValidationConfig, + _: &mut Vec, + _: &Geometry, ) { - errors.extend( - AdjacentHalfEdgesNotConnected::check(self, geometry, config) - .map(Into::into), - ); } } diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs index 4ffd9d9c0..a168dff8a 100644 --- a/crates/fj-core/src/validate/face.rs +++ b/crates/fj-core/src/validate/face.rs @@ -2,7 +2,10 @@ use crate::{ geometry::Geometry, topology::Face, validation::{ - checks::{FaceHasNoBoundary, InteriorCycleHasInvalidWinding}, + checks::{ + AdjacentHalfEdgesNotConnected, FaceHasNoBoundary, + InteriorCycleHasInvalidWinding, + }, ValidationCheck, ValidationConfig, ValidationError, }, }; @@ -16,6 +19,10 @@ impl Validate for Face { errors: &mut Vec, geometry: &Geometry, ) { + errors.extend( + AdjacentHalfEdgesNotConnected::check(self, geometry, config) + .map(Into::into), + ); errors.extend( FaceHasNoBoundary::check(self, geometry, config).map(Into::into), ); diff --git a/crates/fj-core/src/validate/sketch.rs b/crates/fj-core/src/validate/sketch.rs index 39170e10f..b8231bd09 100644 --- a/crates/fj-core/src/validate/sketch.rs +++ b/crates/fj-core/src/validate/sketch.rs @@ -1,8 +1,13 @@ -use crate::geometry::Geometry; -use crate::{storage::Handle, topology::Cycle}; -use crate::{topology::Sketch, validate_references}; use fj_math::Winding; +use crate::{ + geometry::Geometry, + storage::Handle, + topology::{Cycle, Sketch}, + validate_references, + validation::{checks::AdjacentHalfEdgesNotConnected, ValidationCheck}, +}; + use super::{ references::{ReferenceCountError, ReferenceCounter}, Validate, ValidationConfig, ValidationError, @@ -15,6 +20,10 @@ impl Validate for Sketch { errors: &mut Vec, geometry: &Geometry, ) { + errors.extend( + AdjacentHalfEdgesNotConnected::check(self, geometry, config) + .map(Into::into), + ); SketchValidationError::check_object_references(self, config, errors); SketchValidationError::check_exterior_cycles( self, geometry, config, errors, diff --git a/crates/fj-core/src/validation/checks/face_boundary.rs b/crates/fj-core/src/validation/checks/face_boundary.rs index 3f48cc731..737ebae1c 100644 --- a/crates/fj-core/src/validation/checks/face_boundary.rs +++ b/crates/fj-core/src/validation/checks/face_boundary.rs @@ -17,11 +17,11 @@ use crate::{ pub struct FaceHasNoBoundary {} impl ValidationCheck for FaceHasNoBoundary { - fn check( - object: &Face, - _: &Geometry, - _: &ValidationConfig, - ) -> impl Iterator { + fn check<'r>( + object: &'r Face, + _: &'r Geometry, + _: &'r ValidationConfig, + ) -> impl Iterator + 'r { let error = if object.region().exterior().half_edges().is_empty() { Some(FaceHasNoBoundary {}) } else { diff --git a/crates/fj-core/src/validation/checks/face_winding.rs b/crates/fj-core/src/validation/checks/face_winding.rs index af8baf7a9..fb2273bce 100644 --- a/crates/fj-core/src/validation/checks/face_winding.rs +++ b/crates/fj-core/src/validation/checks/face_winding.rs @@ -37,11 +37,11 @@ pub struct InteriorCycleHasInvalidWinding { } impl ValidationCheck for InteriorCycleHasInvalidWinding { - fn check( - object: &Face, - geometry: &Geometry, - _: &ValidationConfig, - ) -> impl Iterator { + fn check<'r>( + object: &'r Face, + geometry: &'r Geometry, + _: &'r ValidationConfig, + ) -> impl Iterator + 'r { object.region().interiors().iter().filter_map(|interior| { let exterior = object.region().exterior(); diff --git a/crates/fj-core/src/validation/checks/half_edge_connection.rs b/crates/fj-core/src/validation/checks/half_edge_connection.rs index 554799b23..489b42608 100644 --- a/crates/fj-core/src/validation/checks/half_edge_connection.rs +++ b/crates/fj-core/src/validation/checks/half_edge_connection.rs @@ -3,7 +3,7 @@ use fj_math::{Point, Scalar}; use crate::{ geometry::Geometry, storage::Handle, - topology::{Cycle, HalfEdge}, + topology::{Cycle, Face, HalfEdge, Region, Sketch}, validation::{validation_check::ValidationCheck, ValidationConfig}, }; @@ -57,50 +57,82 @@ pub struct AdjacentHalfEdgesNotConnected { pub unconnected_half_edges: [Handle; 2], } -impl ValidationCheck for AdjacentHalfEdgesNotConnected { - fn check( - object: &Cycle, - geometry: &Geometry, - config: &ValidationConfig, - ) -> impl Iterator { - object.half_edges().pairs().filter_map(|(first, second)| { - let end_pos_of_first_half_edge = { - let [_, end] = geometry.of_half_edge(first).boundary.inner; - geometry - .of_half_edge(first) - .path - .point_from_path_coords(end) - }; - let start_pos_of_second_half_edge = - geometry.of_half_edge(second).start_position(); - - let distance_between_positions = (end_pos_of_first_half_edge - - start_pos_of_second_half_edge) - .magnitude(); - - if distance_between_positions > config.identical_max_distance { - return Some(AdjacentHalfEdgesNotConnected { - end_pos_of_first_half_edge, - start_pos_of_second_half_edge, - distance_between_positions, - unconnected_half_edges: [first.clone(), second.clone()], - }); - } - - None - }) +impl ValidationCheck for AdjacentHalfEdgesNotConnected { + fn check<'r>( + object: &'r Face, + geometry: &'r Geometry, + config: &'r ValidationConfig, + ) -> impl Iterator + 'r { + check_region(object.region(), geometry, config) } } +impl ValidationCheck for AdjacentHalfEdgesNotConnected { + fn check<'r>( + object: &'r Sketch, + geometry: &'r Geometry, + config: &'r ValidationConfig, + ) -> impl Iterator + 'r { + object + .regions() + .iter() + .flat_map(|region| check_region(region, geometry, config)) + } +} + +fn check_region<'r>( + region: &'r Region, + geometry: &'r Geometry, + config: &'r ValidationConfig, +) -> impl Iterator + 'r { + [region.exterior()] + .into_iter() + .chain(region.interiors()) + .flat_map(|cycle| check_cycle(cycle, geometry, config)) +} + +fn check_cycle<'r>( + cycle: &'r Cycle, + geometry: &'r Geometry, + config: &'r ValidationConfig, +) -> impl Iterator + 'r { + cycle.half_edges().pairs().filter_map(|(first, second)| { + let end_pos_of_first_half_edge = { + let [_, end] = geometry.of_half_edge(first).boundary.inner; + geometry + .of_half_edge(first) + .path + .point_from_path_coords(end) + }; + let start_pos_of_second_half_edge = + geometry.of_half_edge(second).start_position(); + + let distance_between_positions = (end_pos_of_first_half_edge + - start_pos_of_second_half_edge) + .magnitude(); + + if distance_between_positions > config.identical_max_distance { + return Some(AdjacentHalfEdgesNotConnected { + end_pos_of_first_half_edge, + start_pos_of_second_half_edge, + distance_between_positions, + unconnected_half_edges: [first.clone(), second.clone()], + }); + } + + None + }) +} + #[cfg(test)] mod tests { use crate::{ operations::{ - build::{BuildCycle, BuildHalfEdge}, - update::UpdateCycle, + build::{BuildFace, BuildHalfEdge}, + update::{UpdateCycle, UpdateFace, UpdateRegion}, }, - topology::{Cycle, HalfEdge}, + topology::{Face, HalfEdge}, validation::ValidationCheck, Core, }; @@ -111,16 +143,36 @@ mod tests { fn adjacent_half_edges_not_connected() -> anyhow::Result<()> { let mut core = Core::new(); - let valid = Cycle::polygon([[0., 0.], [1., 0.], [1., 1.]], &mut core); + // We're only testing for `Face` here, not `Sketch`. Should be fine, as + // most of the code is shared. + let valid = Face::polygon( + core.layers.topology.surfaces.space_2d(), + [[0., 0.], [1., 0.], [1., 1.]], + &mut core, + ); AdjacentHalfEdgesNotConnected::check_and_return_first_error( &valid, &core.layers.geometry, )?; - let invalid = valid.update_half_edge( - valid.half_edges().first(), - |_, core| { - [HalfEdge::line_segment([[0., 0.], [2., 0.]], None, core)] + let invalid = valid.update_region( + |region, core| { + region.update_exterior( + |cycle, core| { + cycle.update_half_edge( + cycle.half_edges().first(), + |_, core| { + [HalfEdge::line_segment( + [[0., 0.], [2., 0.]], + None, + core, + )] + }, + core, + ) + }, + core, + ) }, &mut core, ); diff --git a/crates/fj-core/src/validation/validation_check.rs b/crates/fj-core/src/validation/validation_check.rs index 154536993..99922a7a1 100644 --- a/crates/fj-core/src/validation/validation_check.rs +++ b/crates/fj-core/src/validation/validation_check.rs @@ -10,11 +10,11 @@ use super::ValidationConfig; /// to. `Self` is the object, while `T` identifies the validation check. pub trait ValidationCheck: Sized { /// Run the validation check on the implementing object - fn check( - object: &T, - geometry: &Geometry, - config: &ValidationConfig, - ) -> impl Iterator; + fn check<'r>( + object: &'r T, + geometry: &'r Geometry, + config: &'r ValidationConfig, + ) -> impl Iterator + 'r; /// Convenience method to run the check return the first error ///