From 09fbd74ee18bbfd3ade8c2aeaf2936e13466f0c7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 9 Nov 2022 11:51:58 +0100 Subject: [PATCH 1/4] Remove uniqueness validation It is the only check left that runs on the old validation infrastructure, and I'm not sure it actually does something useful anymore, given the coverage provided by all the other checks. If it turns out it does, we can re-add some form of it later, based on the new validation infrastructure. --- crates/fj-kernel/src/validate/mod.rs | 69 +-------------------- crates/fj-kernel/src/validate/uniqueness.rs | 51 --------------- 2 files changed, 2 insertions(+), 118 deletions(-) delete mode 100644 crates/fj-kernel/src/validate/uniqueness.rs diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index f1a290243..953c5a43d 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -22,18 +22,16 @@ mod shell; mod sketch; mod solid; mod surface; -mod uniqueness; mod vertex; pub use self::{ cycle::CycleValidationError, edge::HalfEdgeValidationError, face::FaceValidationError, - uniqueness::UniquenessIssues, vertex::{SurfaceVertexValidationError, VertexValidationError}, }; -use std::{collections::HashSet, convert::Infallible, ops::Deref}; +use std::{convert::Infallible, ops::Deref}; use fj_math::Scalar; @@ -83,20 +81,8 @@ where { fn validate_with_config( self, - config: &ValidationConfig, + _: &ValidationConfig, ) -> Result, ValidationError> { - let mut global_vertices = HashSet::new(); - - for global_vertex in self.global_vertex_iter() { - uniqueness::validate_vertex( - global_vertex, - &global_vertices, - config.distinct_min_distance, - )?; - - global_vertices.insert(*global_vertex); - } - Ok(Validated(self)) } } @@ -175,10 +161,6 @@ impl Deref for Validated { #[allow(clippy::large_enum_variant)] #[derive(Debug, thiserror::Error)] pub enum ValidationError { - /// Uniqueness validation failed - #[error("Uniqueness validation failed")] - Uniqueness(#[from] UniquenessIssues), - /// `Cycle` validation error #[error(transparent)] Cycle(#[from] CycleValidationError), @@ -205,50 +187,3 @@ impl From for ValidationError { match infallible {} } } - -#[cfg(test)] -mod tests { - use fj_math::{Point, Scalar}; - - use crate::{ - objects::{GlobalVertex, Objects}, - validate::{Validate, ValidationConfig, ValidationError}, - }; - - #[test] - fn uniqueness_vertex() -> anyhow::Result<()> { - let objects = Objects::new(); - let mut shape = Vec::new(); - - let deviation = Scalar::from_f64(0.25); - - let a = Point::from([0., 0., 0.]); - - let mut b = a; - b.x += deviation; - - let config = ValidationConfig { - distinct_min_distance: deviation * 2., - ..ValidationConfig::default() - }; - - // Adding a vertex should work. - shape.push( - objects - .global_vertices - .insert(GlobalVertex::from_position(a)), - ); - shape.clone().validate_with_config(&config)?; - - // Adding a second vertex that is considered identical should fail. - shape.push( - objects - .global_vertices - .insert(GlobalVertex::from_position(b)), - ); - let result = shape.validate_with_config(&config); - assert!(matches!(result, Err(ValidationError::Uniqueness(_)))); - - Ok(()) - } -} diff --git a/crates/fj-kernel/src/validate/uniqueness.rs b/crates/fj-kernel/src/validate/uniqueness.rs deleted file mode 100644 index 516740d81..000000000 --- a/crates/fj-kernel/src/validate/uniqueness.rs +++ /dev/null @@ -1,51 +0,0 @@ -use std::{collections::HashSet, fmt}; - -use fj_math::Scalar; - -use crate::objects::GlobalVertex; - -pub fn validate_vertex( - vertex: &GlobalVertex, - vertices: &HashSet, - min_distance: Scalar, -) -> Result<(), UniquenessIssues> { - for existing in vertices { - if (existing.position() - vertex.position()).magnitude() < min_distance - { - return Err(UniquenessIssues { - duplicate_vertex: Some(*existing), - }); - } - } - - Ok(()) -} - -/// Uniqueness issues found during validation -/// -/// Used by [`ValidationError`]. -/// -/// # Implementation Note -/// -/// This struct doesn't carry any actual information, currently. Information -/// about the specific uniqueness issues found can be added as required. For -/// now, this struct exists to ease the error handling code. -/// -/// [`ValidationError`]: super::ValidationError -#[derive(Debug, Default, thiserror::Error)] -pub struct UniquenessIssues { - /// Duplicate vertex found - pub duplicate_vertex: Option, -} - -impl fmt::Display for UniquenessIssues { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - writeln!(f, "Uniqueness issues found:")?; - - if let Some(duplicate_vertex) = &self.duplicate_vertex { - writeln!(f, "- Duplicate vertex ({:?}", duplicate_vertex)?; - } - - Ok(()) - } -} From e9969290615d46c5a88cef0c37d2065befb42d13 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 9 Nov 2022 12:04:47 +0100 Subject: [PATCH 2/4] Remove old validation infrastructure --- crates/fj-kernel/src/validate/mod.rs | 75 +--------------------------- 1 file changed, 1 insertion(+), 74 deletions(-) diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 953c5a43d..4b4c94d9f 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -31,62 +31,10 @@ pub use self::{ vertex::{SurfaceVertexValidationError, VertexValidationError}, }; -use std::{convert::Infallible, ops::Deref}; +use std::convert::Infallible; use fj_math::Scalar; -use crate::iter::ObjectIters; - -/// Validate an object -pub trait Validate: Sized { - /// Validate the object using default configuration - /// - /// The following calls are equivalent: - /// ``` rust - /// # use fj_kernel::{ - /// # objects::{GlobalVertex, Objects}, - /// # validate::{Validate, ValidationConfig}, - /// # }; - /// # let objects = Objects::new(); - /// # let object = objects.global_vertices.insert( - /// # GlobalVertex::from_position([0., 0., 0.]) - /// # ); - /// object.validate(); - /// ``` - /// ``` rust - /// # use fj_kernel::{ - /// # objects::{GlobalVertex, Objects}, - /// # validate::{Validate, ValidationConfig}, - /// # }; - /// # let objects = Objects::new(); - /// # let object = objects.global_vertices.insert( - /// # GlobalVertex::from_position([0., 0., 0.]) - /// # ); - /// object.validate_with_config(&ValidationConfig::default()); - /// ``` - fn validate(self) -> Result, ValidationError> { - self.validate_with_config(&ValidationConfig::default()) - } - - /// Validate the object - fn validate_with_config( - self, - config: &ValidationConfig, - ) -> Result, ValidationError>; -} - -impl Validate for T -where - T: for<'r> ObjectIters<'r>, -{ - fn validate_with_config( - self, - _: &ValidationConfig, - ) -> Result, ValidationError> { - Ok(Validated(self)) - } -} - /// Validate an object pub trait Validate2: Sized { /// The error that validation of the implementing type can result in @@ -136,27 +84,6 @@ impl Default for ValidationConfig { } } -/// Wrapper around an object that indicates the object has been validated -/// -/// Returned by implementations of `Validate`. -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Validated(T); - -impl Validated { - /// Consume this instance of `Validated` and return the wrapped object - pub fn into_inner(self) -> T { - self.0 - } -} - -impl Deref for Validated { - type Target = T; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - /// An error that can occur during a validation #[allow(clippy::large_enum_variant)] #[derive(Debug, thiserror::Error)] From 899b401265a782435b13950f79a9071dd7018814 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 9 Nov 2022 12:05:18 +0100 Subject: [PATCH 3/4] Rename `Validate2` to `Validate` This completes the transition to the new validation infrastructure. --- crates/fj-kernel/src/objects/mod.rs | 2 +- crates/fj-kernel/src/validate/curve.rs | 6 +++--- crates/fj-kernel/src/validate/cycle.rs | 6 +++--- crates/fj-kernel/src/validate/edge.rs | 8 ++++---- crates/fj-kernel/src/validate/face.rs | 6 +++--- crates/fj-kernel/src/validate/mod.rs | 2 +- crates/fj-kernel/src/validate/shell.rs | 4 ++-- crates/fj-kernel/src/validate/sketch.rs | 4 ++-- crates/fj-kernel/src/validate/solid.rs | 4 ++-- crates/fj-kernel/src/validate/surface.rs | 4 ++-- crates/fj-kernel/src/validate/vertex.rs | 10 +++++----- 11 files changed, 28 insertions(+), 28 deletions(-) diff --git a/crates/fj-kernel/src/objects/mod.rs b/crates/fj-kernel/src/objects/mod.rs index c35a28972..d0406c0c9 100644 --- a/crates/fj-kernel/src/objects/mod.rs +++ b/crates/fj-kernel/src/objects/mod.rs @@ -104,7 +104,7 @@ use crate::{ storage::{Handle, Store}, validate::{ CycleValidationError, FaceValidationError, HalfEdgeValidationError, - SurfaceVertexValidationError, Validate2, VertexValidationError, + SurfaceVertexValidationError, Validate, VertexValidationError, }, }; diff --git a/crates/fj-kernel/src/validate/curve.rs b/crates/fj-kernel/src/validate/curve.rs index 95e680f8f..d7a8e6e87 100644 --- a/crates/fj-kernel/src/validate/curve.rs +++ b/crates/fj-kernel/src/validate/curve.rs @@ -2,9 +2,9 @@ use std::convert::Infallible; use crate::objects::{Curve, GlobalCurve}; -use super::{Validate2, ValidationConfig}; +use super::{Validate, ValidationConfig}; -impl Validate2 for Curve { +impl Validate for Curve { type Error = Infallible; fn validate_with_config( @@ -15,7 +15,7 @@ impl Validate2 for Curve { } } -impl Validate2 for GlobalCurve { +impl Validate for GlobalCurve { type Error = Infallible; fn validate_with_config( diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 22603cd9f..d20c74750 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -5,9 +5,9 @@ use crate::{ storage::Handle, }; -use super::{Validate2, ValidationConfig}; +use super::{Validate, ValidationConfig}; -impl Validate2 for Cycle { +impl Validate for Cycle { type Error = CycleValidationError; fn validate_with_config( @@ -69,7 +69,7 @@ mod tests { builder::{CycleBuilder, HalfEdgeBuilder, VertexBuilder}, objects::{Cycle, Objects}, partial::HasPartial, - validate::Validate2, + validate::Validate, }; #[test] diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 6db571ebd..ab105ea32 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -11,9 +11,9 @@ use crate::{ storage::Handle, }; -use super::{Validate2, ValidationConfig}; +use super::{Validate, ValidationConfig}; -impl Validate2 for HalfEdge { +impl Validate for HalfEdge { type Error = HalfEdgeValidationError; fn validate_with_config( @@ -33,7 +33,7 @@ impl Validate2 for HalfEdge { } } -impl Validate2 for GlobalEdge { +impl Validate for GlobalEdge { type Error = Infallible; fn validate_with_config( @@ -204,7 +204,7 @@ mod tests { builder::{HalfEdgeBuilder, VertexBuilder}, objects::{GlobalCurve, HalfEdge, Objects}, partial::HasPartial, - validate::Validate2, + validate::Validate, }; #[test] diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index e4881e784..166afb827 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -5,9 +5,9 @@ use crate::{ storage::Handle, }; -use super::{Validate2, ValidationConfig}; +use super::{Validate, ValidationConfig}; -impl Validate2 for Face { +impl Validate for Face { type Error = FaceValidationError; fn validate_with_config( @@ -108,7 +108,7 @@ mod tests { builder::CycleBuilder, objects::{Cycle, Face, Objects}, partial::HasPartial, - validate::Validate2, + validate::Validate, }; #[test] diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 4b4c94d9f..249deda32 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -36,7 +36,7 @@ use std::convert::Infallible; use fj_math::Scalar; /// Validate an object -pub trait Validate2: Sized { +pub trait Validate: Sized { /// The error that validation of the implementing type can result in type Error: Into; diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index e8a9756e6..e71626fad 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -2,9 +2,9 @@ use std::convert::Infallible; use crate::objects::Shell; -use super::{Validate2, ValidationConfig}; +use super::{Validate, ValidationConfig}; -impl Validate2 for Shell { +impl Validate for Shell { type Error = Infallible; fn validate_with_config( diff --git a/crates/fj-kernel/src/validate/sketch.rs b/crates/fj-kernel/src/validate/sketch.rs index 7b9ebc8d8..1a1896658 100644 --- a/crates/fj-kernel/src/validate/sketch.rs +++ b/crates/fj-kernel/src/validate/sketch.rs @@ -2,9 +2,9 @@ use std::convert::Infallible; use crate::objects::Sketch; -use super::{Validate2, ValidationConfig}; +use super::{Validate, ValidationConfig}; -impl Validate2 for Sketch { +impl Validate for Sketch { type Error = Infallible; fn validate_with_config( diff --git a/crates/fj-kernel/src/validate/solid.rs b/crates/fj-kernel/src/validate/solid.rs index b75954be2..9e484cb4b 100644 --- a/crates/fj-kernel/src/validate/solid.rs +++ b/crates/fj-kernel/src/validate/solid.rs @@ -2,9 +2,9 @@ use std::convert::Infallible; use crate::objects::Solid; -use super::{Validate2, ValidationConfig}; +use super::{Validate, ValidationConfig}; -impl Validate2 for Solid { +impl Validate for Solid { type Error = Infallible; fn validate_with_config( diff --git a/crates/fj-kernel/src/validate/surface.rs b/crates/fj-kernel/src/validate/surface.rs index 219bb8404..8c1998787 100644 --- a/crates/fj-kernel/src/validate/surface.rs +++ b/crates/fj-kernel/src/validate/surface.rs @@ -2,9 +2,9 @@ use std::convert::Infallible; use crate::objects::Surface; -use super::{Validate2, ValidationConfig}; +use super::{Validate, ValidationConfig}; -impl Validate2 for Surface { +impl Validate for Surface { type Error = Infallible; fn validate_with_config( diff --git a/crates/fj-kernel/src/validate/vertex.rs b/crates/fj-kernel/src/validate/vertex.rs index 3055f88fa..39d6d082d 100644 --- a/crates/fj-kernel/src/validate/vertex.rs +++ b/crates/fj-kernel/src/validate/vertex.rs @@ -7,9 +7,9 @@ use crate::{ storage::Handle, }; -use super::{Validate2, ValidationConfig}; +use super::{Validate, ValidationConfig}; -impl Validate2 for Vertex { +impl Validate for Vertex { type Error = VertexValidationError; fn validate_with_config( @@ -22,7 +22,7 @@ impl Validate2 for Vertex { } } -impl Validate2 for SurfaceVertex { +impl Validate for SurfaceVertex { type Error = SurfaceVertexValidationError; fn validate_with_config( @@ -34,7 +34,7 @@ impl Validate2 for SurfaceVertex { } } -impl Validate2 for GlobalVertex { +impl Validate for GlobalVertex { type Error = Infallible; fn validate_with_config( @@ -182,7 +182,7 @@ mod tests { builder::{CurveBuilder, SurfaceVertexBuilder}, objects::{Curve, GlobalVertex, Objects, SurfaceVertex, Vertex}, partial::HasPartial, - validate::Validate2, + validate::Validate, }; #[test] From c874a9cef216c422ab01abbf326c71fae556dc97 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 9 Nov 2022 11:54:40 +0100 Subject: [PATCH 4/4] Update documentation of `validate` --- crates/fj-kernel/src/validate/mod.rs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 249deda32..8bcf5eb84 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -1,18 +1,4 @@ -//! Infrastructure for validating shapes -//! -//! Validation enforces various constraints about shapes and the objects that -//! constitute them. These constraints fall into 4 categories: -//! -//! - **Coherence:** Local forms of objects must be consistent with their -//! canonical forms. -//! - **Geometric:** Comprises various object-specific constraints, for example -//! edges or faces might not be allowed to intersect. -//! - **Structural:** All other objects that an object references must be part -//! of the same shape. -//! - **Uniqueness:** Objects within a shape must be unique. -//! -//! Please note that not all of these validation categories are fully -//! implemented, as of this writing. +//! Infrastructure for validating objects mod curve; mod cycle; @@ -36,6 +22,8 @@ use std::convert::Infallible; use fj_math::Scalar; /// Validate an object +/// +/// This trait is used automatically when inserting an object into a store. pub trait Validate: Sized { /// The error that validation of the implementing type can result in type Error: Into;