Merge pull request #706 from hannobraun/validation

Port coherence validation to new infrastructure

crates/fj-kernel/src/shape/api.rs

@@ -14,7 +14,6 @@ use super::{
 #[derive(Clone, Debug)]
 pub struct Shape {
     distinct_min_distance: Scalar,
-    identical_max_distance: Scalar,
 
     stores: Stores,
 }
@@ -28,16 +27,6 @@ impl Shape {
             // be `const` yet.
             distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm
 
-            // This value was chosen pretty arbitrarily. Seems small enough to
-            // catch errors. If it turns out it's too small (because it produces
-            // false positives due to floating-point accuracy issues), we can
-            // adjust it.
-            //
-            // This should be defined in an associated constant, so API users
-            // can see what the default is. Unfortunately, `Scalar::from_f64`
-            // can't be `const` yet.
-            identical_max_distance: Scalar::from_f64(5e-14),
-
             stores: Stores {
                 curves: Store::new(),
                 surfaces: Store::new(),
@@ -80,28 +69,12 @@ impl Shape {
         self
     }
 
-    /// Override the maximum distance between objects considered identical
-    ///
-    /// Used for geometric validation.
-    pub fn with_identical_max_distance(
-        mut self,
-        identical_max_distance: impl Into<Scalar>,
-    ) -> Self {
-        self.identical_max_distance = identical_max_distance.into();
-        self
-    }
-
     /// Insert an object into the shape
     ///
     /// Validates the object, and returns an error if it is not valid. See the
     /// documentation of each object for validation requirements.
     pub fn insert<T: Object>(&mut self, object: T) -> ValidationResult<T> {
-        object.validate(
-            None,
-            self.distinct_min_distance,
-            self.identical_max_distance,
-            &self.stores,
-        )?;
+        object.validate(None, self.distinct_min_distance, &self.stores)?;
         let handle = self.stores.get::<T>().insert(object);
         Ok(handle)
     }
@@ -191,11 +164,7 @@ impl Shape {
     /// Returns [`Update`], and API that can be used to update objects in the
     /// shape.
     pub fn update(&mut self) -> Update {
-        Update::new(
-            self.distinct_min_distance,
-            self.identical_max_distance,
-            &mut self.stores,
-        )
+        Update::new(self.distinct_min_distance, &mut self.stores)
     }
 
     /// Clone the shape

crates/fj-kernel/src/shape/mod.rs

@@ -18,7 +18,6 @@ pub use self::{
     stores::{Handle, Iter},
     update::Update,
     validate::{
-        CoherenceIssues, CoherenceMismatch, DuplicateEdge, StructuralIssues,
-        UniquenessIssues, ValidationResult,
+        DuplicateEdge, StructuralIssues, UniquenessIssues, ValidationResult,
     },
 };

crates/fj-kernel/src/shape/update.rs

@@ -10,20 +10,14 @@ use super::{stores::Stores, validate::Validate as _, Object};
 #[must_use]
 pub struct Update<'r> {
     min_distance: Scalar,
-    max_distance: Scalar,
     stores: &'r mut Stores,
     executed: bool,
 }
 
 impl<'r> Update<'r> {
-    pub(super) fn new(
-        min_distance: Scalar,
-        max_distance: Scalar,
-        stores: &'r mut Stores,
-    ) -> Self {
+    pub(super) fn new(min_distance: Scalar, stores: &'r mut Stores) -> Self {
         Self {
             min_distance,
-            max_distance,
             stores,
             executed: false,
         }
@@ -54,47 +48,41 @@ impl<'r> Update<'r> {
                 object.get().validate(
                     Some(&object),
                     self.min_distance,
-                    self.max_distance,
                     self.stores,
                 )?;
             }
             for object in self.stores.surfaces.iter() {
                 object.get().validate(
                     Some(&object),
                     self.min_distance,
-                    self.max_distance,
                     self.stores,
                 )?;
             }
             for object in self.stores.vertices.iter() {
                 object.get().validate(
                     Some(&object),
                     self.min_distance,
-                    self.max_distance,
                     self.stores,
                 )?;
             }
             for object in self.stores.edges.iter() {
                 object.get().validate(
                     Some(&object),
                     self.min_distance,
-                    self.max_distance,
                     self.stores,
                 )?;
             }
             for object in self.stores.cycles.iter() {
                 object.get().validate(
                     Some(&object),
                     self.min_distance,
-                    self.max_distance,
                     self.stores,
                 )?;
             }
             for object in self.stores.faces.iter() {
                 object.get().validate(
                     Some(&object),
                     self.min_distance,
-                    self.max_distance,
                     self.stores,
                 )?;
             }

crates/fj-kernel/src/shape/validate/mod.rs

@@ -1,9 +1,7 @@
-mod coherence;
 mod structural;
 mod uniqueness;
 
 pub use self::{
-    coherence::{CoherenceIssues, CoherenceMismatch},
     structural::StructuralIssues,
     uniqueness::{DuplicateEdge, UniquenessIssues},
 };
@@ -22,7 +20,6 @@ pub trait Validate {
         &self,
         handle: Option<&Handle<Self>>,
         min_distance: Scalar,
-        max_distance: Scalar,
         stores: &Stores,
     ) -> Result<(), ValidationError>
     where
@@ -34,7 +31,6 @@ impl Validate for Curve<3> {
         &self,
         _: Option<&Handle<Self>>,
         _: Scalar,
-        _: Scalar,
         _: &Stores,
     ) -> Result<(), ValidationError> {
         Ok(())
@@ -46,7 +42,6 @@ impl Validate for Surface {
         &self,
         _: Option<&Handle<Self>>,
         _: Scalar,
-        _: Scalar,
         _: &Stores,
     ) -> Result<(), ValidationError> {
         Ok(())
@@ -64,7 +59,6 @@ impl Validate for Vertex {
         &self,
         handle: Option<&Handle<Self>>,
         _: Scalar,
-        _: Scalar,
         stores: &Stores,
     ) -> Result<(), ValidationError> {
         uniqueness::validate_vertex(self, handle, &stores.vertices)?;
@@ -78,10 +72,8 @@ impl Validate for Edge<3> {
         &self,
         handle: Option<&Handle<Self>>,
         _: Scalar,
-        max_distance: Scalar,
         stores: &Stores,
     ) -> Result<(), ValidationError> {
-        coherence::validate_edge(self, max_distance)?;
         structural::validate_edge(self, stores)?;
         uniqueness::validate_edge(self, handle, &stores.edges)?;
 
@@ -102,7 +94,6 @@ impl Validate for Cycle<3> {
         &self,
         _: Option<&Handle<Self>>,
         _: Scalar,
-        _: Scalar,
         stores: &Stores,
     ) -> Result<(), ValidationError> {
         structural::validate_cycle(self, stores)?;
@@ -115,7 +106,6 @@ impl Validate for Face {
         &self,
         _: Option<&Handle<Self>>,
         _: Scalar,
-        _: Scalar,
         stores: &Stores,
     ) -> Result<(), ValidationError> {
         structural::validate_face(self, stores)?;

crates/fj-kernel/src/shape/validate/coherence.rs → crates/fj-kernel/src/validation/coherence.rs

@@ -95,37 +95,3 @@ where
         )
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use fj_math::Scalar;
-
-    use crate::{
-        objects::Edge,
-        shape::{LocalForm, Shape},
-    };
-
-    #[test]
-    fn validate_edge() -> anyhow::Result<()> {
-        let mut shape = Shape::new();
-
-        let deviation = Scalar::from_f64(0.25);
-
-        let edge = Edge::builder(&mut shape)
-            .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]])?
-            .get();
-        let edge = Edge {
-            vertices: edge.vertices.map(|vertex| {
-                LocalForm::new(
-                    *vertex.local() + [deviation],
-                    vertex.canonical(),
-                )
-            }),
-            ..edge
-        };
-        assert!(super::validate_edge(&edge, deviation * 2.).is_ok());
-        assert!(super::validate_edge(&edge, deviation / 2.).is_err());
-
-        Ok(())
-    }
-}

crates/fj-kernel/src/validation/mod.rs

@@ -24,22 +24,28 @@
 //!
 //! [`Shape`]: crate::shape::Shape
 
+mod coherence;
+
+pub use self::coherence::{CoherenceIssues, CoherenceMismatch};
+
 use std::ops::Deref;
 
 use fj_math::Scalar;
 
 use crate::{
     objects::{Curve, Cycle, Edge, Surface, Vertex},
-    shape::{
-        CoherenceIssues, Handle, Shape, StructuralIssues, UniquenessIssues,
-    },
+    shape::{Handle, Shape, StructuralIssues, UniquenessIssues},
 };
 
 /// Validate the given [`Shape`]
 pub fn validate(
     shape: Shape,
-    _: &Config,
+    config: &Config,
 ) -> Result<Validated<Shape>, ValidationError> {
+    for edge in shape.edges() {
+        coherence::validate_edge(&edge.get(), config.identical_max_distance)?;
+    }
+
     Ok(Validated(shape))
 }
 
@@ -65,6 +71,11 @@ impl Default for Config {
     fn default() -> Self {
         Self {
             distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm,
+
+            // This value was chosen pretty arbitrarily. Seems small enough to
+            // catch errors. If it turns out it's too small (because it produces
+            // false positives due to floating-point accuracy issues), we can
+            // adjust it.
             identical_max_distance: Scalar::from_f64(5e-14),
         }
     }
@@ -164,3 +175,60 @@ impl ValidationError {
         false
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use fj_math::Scalar;
+
+    use crate::{
+        objects::Edge,
+        shape::{LocalForm, Shape},
+        validation::Config,
+    };
+
+    #[test]
+    fn edge_coherence() -> anyhow::Result<()> {
+        let mut shape = Shape::new();
+        Edge::builder(&mut shape)
+            .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]])?
+            .get();
+
+        let deviation = Scalar::from_f64(0.25);
+
+        shape
+            .update()
+            .update_all(|edge: &mut Edge<3>| {
+                let original = edge.clone();
+                *edge = Edge {
+                    vertices: original.vertices.map(|vertex| {
+                        LocalForm::new(
+                            *vertex.local() + [deviation],
+                            vertex.canonical(),
+                        )
+                    }),
+                    ..original
+                }
+            })
+            .validate()?;
+
+        let result = super::validate(
+            shape.clone(),
+            &Config {
+                identical_max_distance: deviation * 2.,
+                ..Config::default()
+            },
+        );
+        assert!(result.is_ok());
+
+        let result = super::validate(
+            shape,
+            &Config {
+                identical_max_distance: deviation / 2.,
+                ..Config::default()
+            },
+        );
+        assert!(result.is_err());
+
+        Ok(())
+    }
+}