Merge pull request #2368 from hannobraun/validation

Port rest of reference counting validation checks to new infrastructure

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

@@ -3,8 +3,8 @@ use std::iter::repeat;
 use crate::{
     geometry::Geometry,
     storage::Handle,
-    topology::{Solid, Vertex},
+    topology::{Cycle, Face, HalfEdge, Region, Shell, Solid, Vertex},
-    validation::checks::ReferenceCounter,
+    validation::{checks::MultipleReferencesToObject, ValidationCheck},
 };
 use fj_math::Point;
 
@@ -17,8 +17,31 @@ impl Validate for Solid {
         errors: &mut Vec<ValidationError>,
         geometry: &Geometry,
     ) {
+        errors.extend(
+            MultipleReferencesToObject::<Face, Shell>::check(
+                self, geometry, config,
+            )
+            .map(Into::into),
+        );
+        errors.extend(
+            MultipleReferencesToObject::<Region, Face>::check(
+                self, geometry, config,
+            )
+            .map(Into::into),
+        );
+        errors.extend(
+            MultipleReferencesToObject::<Cycle, Region>::check(
+                self, geometry, config,
+            )
+            .map(Into::into),
+        );
+        errors.extend(
+            MultipleReferencesToObject::<HalfEdge, Cycle>::check(
+                self, geometry, config,
+            )
+            .map(Into::into),
+        );
         SolidValidationError::check_vertices(self, geometry, config, errors);
-        SolidValidationError::check_object_references(self, config, errors);
     }
 }
 
@@ -134,236 +157,4 @@ impl SolidValidationError {
             }
         }
     }
-
-    fn check_object_references(
-        solid: &Solid,
-        _config: &ValidationConfig,
-        errors: &mut Vec<ValidationError>,
-    ) {
-        let mut faces = ReferenceCounter::new();
-        let mut regions = ReferenceCounter::new();
-        let mut cycles = ReferenceCounter::new();
-        let mut half_edges = ReferenceCounter::new();
-
-        solid.shells().iter().for_each(|s| {
-            s.faces().into_iter().for_each(|f| {
-                faces.count(f.clone(), s.clone());
-                regions.count(f.region().clone(), f.clone());
-                f.region().all_cycles().for_each(|c| {
-                    cycles.count(c.clone(), f.region().clone());
-                    c.half_edges().into_iter().for_each(|e| {
-                        half_edges.count(e.clone(), c.clone());
-                    })
-                })
-            })
-        });
-
-        errors.extend(faces.multiples().map(Into::into));
-        errors.extend(regions.multiples().map(Into::into));
-        errors.extend(cycles.multiples().map(Into::into));
-        errors.extend(half_edges.multiples().map(Into::into));
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use crate::{
-        assert_contains_err,
-        geometry::GlobalPath,
-        operations::{
-            build::{BuildFace, BuildHalfEdge, BuildSurface},
-            insert::Insert,
-        },
-        topology::{Cycle, Face, HalfEdge, Region, Shell, Solid, Surface},
-        validate::{Validate, ValidationError},
-        Core,
-    };
-
-    #[test]
-    fn should_find_face_multiple_references() -> anyhow::Result<()> {
-        let mut core = Core::new();
-
-        let surface = Surface::from_uv(
-            GlobalPath::circle_from_radius(1.),
-            [0., 1., 1.],
-            &mut core,
-        );
-
-        let shared_face = Face::new(
-            surface.clone(),
-            Region::new(
-                Cycle::new(vec![HalfEdge::circle(
-                    [0., 0.],
-                    1.,
-                    surface,
-                    &mut core,
-                )])
-                .insert(&mut core),
-                vec![],
-            )
-            .insert(&mut core),
-        )
-        .insert(&mut core);
-
-        let invalid_solid = Solid::new(vec![
-            Shell::new(vec![shared_face.clone()]).insert(&mut core),
-            Shell::new(vec![
-                shared_face,
-                Face::triangle(
-                    [[0., 0., 0.], [1., 0., 0.], [1., 1., 0.]],
-                    &mut core,
-                )
-                .insert(&mut core)
-                .face,
-            ])
-            .insert(&mut core),
-        ])
-        .insert(&mut core);
-
-        assert_contains_err!(
-            core,
-            invalid_solid,
-            ValidationError::MultipleReferencesToFace(_)
-        );
-
-        let valid_solid = Solid::new(vec![]).insert(&mut core);
-        valid_solid.validate_and_return_first_error(&core.layers.geometry)?;
-
-        // Ignore remaining validation errors.
-        let _ = core.layers.validation.take_errors();
-
-        Ok(())
-    }
-
-    #[test]
-    fn should_find_region_multiple_references() -> anyhow::Result<()> {
-        let mut core = Core::new();
-
-        let surface = Surface::from_uv(
-            GlobalPath::circle_from_radius(1.),
-            [0., 0., 1.],
-            &mut core,
-        );
-
-        let shared_region = Region::new(
-            Cycle::new(vec![HalfEdge::circle(
-                [0., 0.],
-                1.,
-                surface.clone(),
-                &mut core,
-            )])
-            .insert(&mut core),
-            vec![],
-        )
-        .insert(&mut core);
-
-        let invalid_solid = Solid::new(vec![Shell::new(vec![
-            Face::new(surface.clone(), shared_region.clone()).insert(&mut core),
-            Face::new(surface, shared_region.clone()).insert(&mut core),
-        ])
-        .insert(&mut core)])
-        .insert(&mut core);
-
-        assert_contains_err!(
-            core,
-            invalid_solid,
-            ValidationError::MultipleReferencesToRegion(_)
-        );
-
-        let valid_solid = Solid::new(vec![]).insert(&mut core);
-        valid_solid.validate_and_return_first_error(&core.layers.geometry)?;
-
-        // Ignore remaining validation errors.
-        let _ = core.layers.validation.take_errors();
-
-        Ok(())
-    }
-
-    #[test]
-    fn should_find_cycle_multiple_references() -> anyhow::Result<()> {
-        let mut core = Core::new();
-
-        let surface = Surface::from_uv(
-            GlobalPath::circle_from_radius(1.),
-            [0., 0., 1.],
-            &mut core,
-        );
-
-        let shared_cycle = Cycle::new(vec![HalfEdge::circle(
-            [0., 0.],
-            1.,
-            surface.clone(),
-            &mut core,
-        )])
-        .insert(&mut core);
-
-        let invalid_solid = Solid::new(vec![Shell::new(vec![
-            Face::new(
-                surface.clone(),
-                Region::new(shared_cycle.clone(), vec![]).insert(&mut core),
-            )
-            .insert(&mut core),
-            Face::new(
-                surface,
-                Region::new(shared_cycle, vec![]).insert(&mut core),
-            )
-            .insert(&mut core),
-        ])
-        .insert(&mut core)])
-        .insert(&mut core);
-
-        assert_contains_err!(
-            core,
-            invalid_solid,
-            ValidationError::MultipleReferencesToCycle(_)
-        );
-
-        let valid_solid = Solid::new(vec![]).insert(&mut core);
-        valid_solid.validate_and_return_first_error(&core.layers.geometry)?;
-
-        // Ignore remaining validation errors.
-        let _ = core.layers.validation.take_errors();
-
-        Ok(())
-    }
-
-    #[test]
-    fn should_find_half_edge_multiple_references() -> anyhow::Result<()> {
-        let mut core = Core::new();
-
-        let surface = Surface::from_uv(
-            GlobalPath::circle_from_radius(1.),
-            [0., 0., 1.],
-            &mut core,
-        );
-
-        let shared_edge =
-            HalfEdge::circle([0., 0.], 1., surface.clone(), &mut core);
-
-        let invalid_solid = Solid::new(vec![Shell::new(vec![Face::new(
-            surface,
-            Region::new(
-                Cycle::new(vec![shared_edge.clone()]).insert(&mut core),
-                vec![Cycle::new(vec![shared_edge.clone()]).insert(&mut core)],
-            )
-            .insert(&mut core),
-        )
-        .insert(&mut core)])
-        .insert(&mut core)])
-        .insert(&mut core);
-
-        assert_contains_err!(
-            core,
-            invalid_solid,
-            ValidationError::MultipleReferencesToHalfEdge(_)
-        );
-
-        let valid_solid = Solid::new(vec![]).insert(&mut core);
-        valid_solid.validate_and_return_first_error(&core.layers.geometry)?;
-
-        // Ignore remaining validation errors.
-        let _ = core.layers.validation.take_errors();
-
-        Ok(())
-    }
 }

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

@@ -17,5 +17,5 @@ pub use self::{
     face_winding::InteriorCycleHasInvalidWinding,
     half_edge_connection::AdjacentHalfEdgesNotConnected,
     half_edge_has_no_sibling::HalfEdgeHasNoSibling,
-    multiple_references::{MultipleReferencesToObject, ReferenceCounter},
+    multiple_references::MultipleReferencesToObject,
 };

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

@@ -1,9 +1,10 @@
 use std::{any::type_name_of_val, collections::HashMap, fmt};
 
 use crate::{
+    geometry::Geometry,
     storage::Handle,
-    topology::{Cycle, HalfEdge, Region, Sketch},
+    topology::{Cycle, Face, HalfEdge, Region, Shell, Sketch, Solid},
-    validation::ValidationCheck,
+    validation::{ValidationCheck, ValidationConfig},
 };
 
 /// Object that should be exclusively owned by another, is not
@@ -37,8 +38,8 @@ where
 impl ValidationCheck<Sketch> for MultipleReferencesToObject<Cycle, Region> {
     fn check<'r>(
         object: &'r Sketch,
-        _: &'r crate::geometry::Geometry,
+        _: &'r Geometry,
-        _: &'r crate::validation::ValidationConfig,
+        _: &'r ValidationConfig,
     ) -> impl Iterator<Item = Self> + 'r {
         let mut cycles = ReferenceCounter::new();
 
@@ -55,8 +56,8 @@ impl ValidationCheck<Sketch> for MultipleReferencesToObject<Cycle, Region> {
 impl ValidationCheck<Sketch> for MultipleReferencesToObject<HalfEdge, Cycle> {
     fn check<'r>(
         object: &'r Sketch,
-        _: &'r crate::geometry::Geometry,
+        _: &'r Geometry,
-        _: &'r crate::validation::ValidationConfig,
+        _: &'r ValidationConfig,
     ) -> impl Iterator<Item = Self> + 'r {
         let mut half_edges = ReferenceCounter::new();
 
@@ -72,17 +73,87 @@ impl ValidationCheck<Sketch> for MultipleReferencesToObject<HalfEdge, Cycle> {
     }
 }
 
-// Warnings are temporarily silenced, until this struct can be made private.
+impl ValidationCheck<Solid> for MultipleReferencesToObject<Face, Shell> {
-// This can happen once this validation check has been fully ported from the old
+    fn check<'r>(
-// infrastructure.
+        object: &'r Solid,
-#[allow(missing_docs)]
+        _: &'r Geometry,
-#[derive(Default)]
+        _: &'r ValidationConfig,
-pub struct ReferenceCounter<T, U>(HashMap<Handle<T>, Vec<Handle<U>>>);
+    ) -> impl Iterator<Item = Self> + 'r {
+        let mut faces = ReferenceCounter::new();
+
+        for shell in object.shells() {
+            for face in shell.faces() {
+                faces.count(face.clone(), shell.clone());
+            }
+        }
+
+        faces.multiples()
+    }
+}
+
+impl ValidationCheck<Solid> for MultipleReferencesToObject<Region, Face> {
+    fn check<'r>(
+        object: &'r Solid,
+        _: &'r Geometry,
+        _: &'r ValidationConfig,
+    ) -> impl Iterator<Item = Self> + 'r {
+        let mut regions = ReferenceCounter::new();
+
+        for shell in object.shells() {
+            for face in shell.faces() {
+                regions.count(face.region().clone(), face.clone());
+            }
+        }
+
+        regions.multiples()
+    }
+}
+
+impl ValidationCheck<Solid> for MultipleReferencesToObject<Cycle, Region> {
+    fn check<'r>(
+        object: &'r Solid,
+        _: &'r Geometry,
+        _: &'r ValidationConfig,
+    ) -> impl Iterator<Item = Self> + 'r {
+        let mut cycles = ReferenceCounter::new();
+
+        for shell in object.shells() {
+            for face in shell.faces() {
+                for cycle in face.region().all_cycles() {
+                    cycles.count(cycle.clone(), face.region().clone());
+                }
+            }
+        }
+
+        cycles.multiples()
+    }
+}
+
+impl ValidationCheck<Solid> for MultipleReferencesToObject<HalfEdge, Cycle> {
+    fn check<'r>(
+        object: &'r Solid,
+        _: &'r Geometry,
+        _: &'r ValidationConfig,
+    ) -> impl Iterator<Item = Self> + 'r {
+        let mut half_edges = ReferenceCounter::new();
+
+        for shell in object.shells() {
+            for face in shell.faces() {
+                for cycle in face.region().all_cycles() {
+                    for half_edge in cycle.half_edges() {
+                        half_edges.count(half_edge.clone(), cycle.clone());
+                    }
+                }
+            }
+        }
+
+        half_edges.multiples()
+    }
+}
+
+#[derive(Default)]
+struct ReferenceCounter<T, U>(HashMap<Handle<T>, Vec<Handle<U>>>);
 
-// Warnings are temporarily silenced, until this struct can be made private.
-// This can happen once this validation check has been fully ported from the old
-// infrastructure.
-#[allow(missing_docs)]
 impl<T, U> ReferenceCounter<T, U> {
     pub fn new() -> Self {
         Self(HashMap::new())
@@ -108,17 +179,25 @@ impl<T, U> ReferenceCounter<T, U> {
 #[cfg(test)]
 mod tests {
     use crate::{
+        assert_contains_err,
         operations::{
-            build::BuildSketch,
+            build::{BuildShell, BuildSketch, BuildSolid},
-            update::{UpdateRegion, UpdateSketch},
+            update::{
+                UpdateCycle, UpdateFace, UpdateRegion, UpdateShell,
+                UpdateSketch, UpdateSolid,
+            },
+        },
+        topology::{Cycle, Face, HalfEdge, Region, Shell, Sketch, Solid},
+        validate::Validate,
+        validation::{
+            checks::MultipleReferencesToObject, ValidationCheck,
+            ValidationError,
         },
-        topology::{Cycle, HalfEdge, Region, Sketch},
-        validation::{checks::MultipleReferencesToObject, ValidationCheck},
         Core,
     };
 
     #[test]
-    fn multiple_references_to_cycle() -> anyhow::Result<()> {
+    fn multiple_references_to_cycle_within_sketch() -> anyhow::Result<()> {
         let mut core = Core::new();
 
         let valid = Sketch::circle([0., 0.], 1., &mut core);
@@ -146,7 +225,7 @@ mod tests {
     }
 
     #[test]
-    fn multiple_references_to_half_edge() -> anyhow::Result<()> {
+    fn multiple_references_to_half_edge_within_sketch() -> anyhow::Result<()> {
         let mut core = Core::new();
 
         let valid = Sketch::polygon([[0., 0.], [1., 1.], [0., 1.]], &mut core);
@@ -183,4 +262,232 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn multiple_references_to_face_within_solid() -> anyhow::Result<()> {
+        let mut core = Core::new();
+
+        let valid = Solid::tetrahedron(
+            [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]],
+            &mut core,
+        );
+        MultipleReferencesToObject::<
+            Face,
+            Shell
+        >::check_and_return_first_error(
+            &valid.solid,
+            &core.layers.geometry,
+        )?;
+
+        let invalid = valid.solid.add_shells(
+            {
+                let shell = Shell::tetrahedron(
+                    [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]],
+                    &mut core,
+                )
+                .shell;
+
+                [shell.update_face(
+                    shell.faces().first(),
+                    |_, _| {
+                        [valid.solid.shells().first().faces().first().clone()]
+                    },
+                    &mut core,
+                )]
+            },
+            &mut core,
+        );
+        assert!(MultipleReferencesToObject::<
+            Face,
+            Shell
+        >::check_and_return_first_error(
+            &invalid,
+            &core.layers.geometry,
+        ).is_err());
+
+        // Ignore remaining validation errors.
+        let _ = core.layers.validation.take_errors();
+
+        Ok(())
+    }
+
+    #[test]
+    fn multiple_references_to_region_within_solid() -> anyhow::Result<()> {
+        let mut core = Core::new();
+
+        let valid = Solid::tetrahedron(
+            [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]],
+            &mut core,
+        );
+        MultipleReferencesToObject::<
+            Region,
+            Face
+        >::check_and_return_first_error(
+            &valid.solid,
+            &core.layers.geometry,
+        )?;
+
+        let invalid = valid.solid.update_shell(
+            valid.solid.shells().first(),
+            |shell, core| {
+                [shell.update_face(
+                    shell.faces().first(),
+                    |face, core| {
+                        [face.update_region(
+                            |_, _| {
+                                shell.faces().nth(1).unwrap().region().clone()
+                            },
+                            core,
+                        )]
+                    },
+                    core,
+                )]
+            },
+            &mut core,
+        );
+        assert!(MultipleReferencesToObject::<
+            Region,
+            Face
+        >::check_and_return_first_error(
+            &invalid,
+            &core.layers.geometry,
+        ).is_err());
+
+        // Ignore remaining validation errors.
+        let _ = core.layers.validation.take_errors();
+
+        Ok(())
+    }
+
+    #[test]
+    fn multiple_references_to_cycle_within_solid() -> anyhow::Result<()> {
+        let mut core = Core::new();
+
+        let valid = Solid::tetrahedron(
+            [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]],
+            &mut core,
+        );
+        MultipleReferencesToObject::<
+            Cycle,
+            Region
+        >::check_and_return_first_error(
+            &valid.solid,
+            &core.layers.geometry,
+        )?;
+
+        let invalid = valid.solid.update_shell(
+            valid.solid.shells().first(),
+            |shell, core| {
+                [shell.update_face(
+                    shell.faces().first(),
+                    |face, core| {
+                        [face.update_region(
+                            |region, core| {
+                                region.update_exterior(
+                                    |_, _| {
+                                        shell
+                                            .faces()
+                                            .nth(1)
+                                            .unwrap()
+                                            .region()
+                                            .exterior()
+                                            .clone()
+                                    },
+                                    core,
+                                )
+                            },
+                            core,
+                        )]
+                    },
+                    core,
+                )]
+            },
+            &mut core,
+        );
+        assert!(MultipleReferencesToObject::<
+            Cycle,
+            Region
+        >::check_and_return_first_error(
+            &invalid,
+            &core.layers.geometry,
+        ).is_err());
+
+        assert_contains_err!(
+            core,
+            invalid,
+            ValidationError::MultipleReferencesToCycle(_)
+        );
+
+        // Ignore remaining validation errors.
+        let _ = core.layers.validation.take_errors();
+
+        Ok(())
+    }
+
+    #[test]
+    fn multiple_references_to_half_edge_within_solid() -> anyhow::Result<()> {
+        let mut core = Core::new();
+
+        let valid = Solid::tetrahedron(
+            [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]],
+            &mut core,
+        );
+        MultipleReferencesToObject::<
+            HalfEdge,
+            Cycle
+        >::check_and_return_first_error(
+            &valid.solid,
+            &core.layers.geometry,
+        )?;
+
+        let invalid = valid.solid.update_shell(
+            valid.solid.shells().first(),
+            |shell, core| {
+                [shell.update_face(
+                    shell.faces().first(),
+                    |face, core| {
+                        [face.update_region(
+                            |region, core| {
+                                region.update_exterior(
+                                    |cycle, core| {
+                                        cycle.update_half_edge(
+                                            cycle.half_edges().first(),
+                                            |_, _| {
+                                                [shell
+                                                    .faces()
+                                                    .nth(1)
+                                                    .unwrap()
+                                                    .region()
+                                                    .exterior()
+                                                    .half_edges()
+                                                    .first()
+                                                    .clone()]
+                                            },
+                                            core,
+                                        )
+                                    },
+                                    core,
+                                )
+                            },
+                            core,
+                        )]
+                    },
+                    core,
+                )]
+            },
+            &mut core,
+        );
+        assert!(MultipleReferencesToObject::<
+            HalfEdge,
+            Cycle
+        >::check_and_return_first_error(
+            &invalid,
+            &core.layers.geometry,
+        ).is_err());
+
+        // Ignore remaining validation errors.
+        let _ = core.layers.validation.take_errors();
+
+        Ok(())
+    }
 }