Merge pull request #2322 from hannobraun/validation

Move half-edge connection check from `Cycle` to `Face`/`Sketch`

crates/fj-core/src/validate/cycle.rs

@@ -1,10 +1,7 @@
 use crate::{
     geometry::Geometry,
     topology::Cycle,
-    validation::{
+    validation::{ValidationConfig, ValidationError},
-        checks::AdjacentHalfEdgesNotConnected, ValidationCheck,
-        ValidationConfig, ValidationError,
-    },
 };
 
 use super::Validate;
@@ -12,13 +9,9 @@ use super::Validate;
 impl Validate for Cycle {
     fn validate(
         &self,
-        config: &ValidationConfig,
+        _: &ValidationConfig,
-        errors: &mut Vec<ValidationError>,
+        _: &mut Vec<ValidationError>,
-        geometry: &Geometry,
+        _: &Geometry,
     ) {
-        errors.extend(
-            AdjacentHalfEdgesNotConnected::check(self, geometry, config)
-                .map(Into::into),
-        );
     }
 }

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<ValidationError>,
         geometry: &Geometry,
     ) {
+        errors.extend(
+            AdjacentHalfEdgesNotConnected::check(self, geometry, config)
+                .map(Into::into),
+        );
         errors.extend(
             FaceHasNoBoundary::check(self, geometry, config).map(Into::into),
         );

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<ValidationError>,
         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,

crates/fj-core/src/validation/checks/face_boundary.rs

@@ -17,11 +17,11 @@ use crate::{
 pub struct FaceHasNoBoundary {}
 
 impl ValidationCheck<Face> for FaceHasNoBoundary {
-    fn check(
+    fn check<'r>(
-        object: &Face,
+        object: &'r Face,
-        _: &Geometry,
+        _: &'r Geometry,
-        _: &ValidationConfig,
+        _: &'r ValidationConfig,
-    ) -> impl Iterator<Item = Self> {
+    ) -> impl Iterator<Item = Self> + 'r {
         let error = if object.region().exterior().half_edges().is_empty() {
             Some(FaceHasNoBoundary {})
         } else {

crates/fj-core/src/validation/checks/face_winding.rs

@@ -37,11 +37,11 @@ pub struct InteriorCycleHasInvalidWinding {
 }
 
 impl ValidationCheck<Face> for InteriorCycleHasInvalidWinding {
-    fn check(
+    fn check<'r>(
-        object: &Face,
+        object: &'r Face,
-        geometry: &Geometry,
+        geometry: &'r Geometry,
-        _: &ValidationConfig,
+        _: &'r ValidationConfig,
-    ) -> impl Iterator<Item = Self> {
+    ) -> impl Iterator<Item = Self> + 'r {
         object.region().interiors().iter().filter_map(|interior| {
             let exterior = object.region().exterior();
 

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<HalfEdge>; 2],
 }
 
-impl ValidationCheck<Cycle> for AdjacentHalfEdgesNotConnected {
+impl ValidationCheck<Face> for AdjacentHalfEdgesNotConnected {
-    fn check(
+    fn check<'r>(
-        object: &Cycle,
+        object: &'r Face,
-        geometry: &Geometry,
+        geometry: &'r Geometry,
-        config: &ValidationConfig,
+        config: &'r ValidationConfig,
-    ) -> impl Iterator<Item = Self> {
+    ) -> impl Iterator<Item = Self> + 'r {
-        object.half_edges().pairs().filter_map(|(first, second)| {
+        check_region(object.region(), geometry, config)
-            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<Sketch> for AdjacentHalfEdgesNotConnected {
+    fn check<'r>(
+        object: &'r Sketch,
+        geometry: &'r Geometry,
+        config: &'r ValidationConfig,
+    ) -> impl Iterator<Item = Self> + '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<Item = AdjacentHalfEdgesNotConnected> + '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<Item = AdjacentHalfEdgesNotConnected> + '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},
+            build::{BuildFace, BuildHalfEdge},
-            update::UpdateCycle,
+            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(
+        let invalid = valid.update_region(
-            valid.half_edges().first(),
+            |region, core| {
-            |_, core| {
+                region.update_exterior(
-                [HalfEdge::line_segment([[0., 0.], [2., 0.]], None, core)]
+                    |cycle, core| {
+                        cycle.update_half_edge(
+                            cycle.half_edges().first(),
+                            |_, core| {
+                                [HalfEdge::line_segment(
+                                    [[0., 0.], [2., 0.]],
+                                    None,
+                                    core,
+                                )]
+                            },
+                            core,
+                        )
+                    },
+                    core,
+                )
             },
             &mut core,
         );

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<T>: Sized {
     /// Run the validation check on the implementing object
-    fn check(
+    fn check<'r>(
-        object: &T,
+        object: &'r T,
-        geometry: &Geometry,
+        geometry: &'r Geometry,
-        config: &ValidationConfig,
+        config: &'r ValidationConfig,
-    ) -> impl Iterator<Item = Self>;
+    ) -> impl Iterator<Item = Self> + 'r;
 
     /// Convenience method to run the check return the first error
     ///