From 55aefd643ffd74d8b9bb3154f480f864a8aa2876 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 23 May 2022 16:36:29 +0200 Subject: [PATCH 1/7] Add `GeometricIssues` --- crates/fj-kernel/src/shape/mod.rs | 3 ++- crates/fj-kernel/src/shape/validate/geometric.rs | 16 ++++++++++++++++ crates/fj-kernel/src/shape/validate/mod.rs | 8 ++++++-- 3 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 crates/fj-kernel/src/shape/validate/geometric.rs diff --git a/crates/fj-kernel/src/shape/mod.rs b/crates/fj-kernel/src/shape/mod.rs index 77dea4ff6..1147cc7e3 100644 --- a/crates/fj-kernel/src/shape/mod.rs +++ b/crates/fj-kernel/src/shape/mod.rs @@ -18,6 +18,7 @@ pub use self::{ stores::{Handle, Iter}, update::Update, validate::{ - StructuralIssues, UniquenessIssues, ValidationError, ValidationResult, + GeometricIssues, StructuralIssues, UniquenessIssues, ValidationError, + ValidationResult, }, }; diff --git a/crates/fj-kernel/src/shape/validate/geometric.rs b/crates/fj-kernel/src/shape/validate/geometric.rs new file mode 100644 index 000000000..e0cb4c67d --- /dev/null +++ b/crates/fj-kernel/src/shape/validate/geometric.rs @@ -0,0 +1,16 @@ +use std::fmt; + +/// Geometric issues found during validation +/// +/// Used by [`ValidationError`]. +/// +/// [`ValidationError`]: super::ValidationError +#[derive(Debug, Default, thiserror::Error)] +pub struct GeometricIssues; + +impl fmt::Display for GeometricIssues { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Geometric issues found")?; + Ok(()) + } +} diff --git a/crates/fj-kernel/src/shape/validate/mod.rs b/crates/fj-kernel/src/shape/validate/mod.rs index 0c2c9a6f4..a2e06cde0 100644 --- a/crates/fj-kernel/src/shape/validate/mod.rs +++ b/crates/fj-kernel/src/shape/validate/mod.rs @@ -1,7 +1,11 @@ +mod geometric; mod structural; mod uniqueness; -pub use self::{structural::StructuralIssues, uniqueness::UniquenessIssues}; +pub use self::{ + geometric::GeometricIssues, structural::StructuralIssues, + uniqueness::UniquenessIssues, +}; use fj_math::{Point, Scalar}; @@ -132,7 +136,7 @@ pub enum ValidationError { /// object are upheld. For example, edges or faces might not be allowed to /// intersect. #[error("Geometric validation failed")] - Geometric, + Geometric(#[from] GeometricIssues), /// Structural validation failed /// From 4b711f305dc4cf3c194c9485caa2a68842536457 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 May 2022 13:30:22 +0200 Subject: [PATCH 2/7] Make test-only method generally available --- crates/fj-kernel/src/shape/api.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index ca144dcb9..bf9748e8f 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -47,7 +47,6 @@ impl Shape { /// /// This functionality should be exposed to models, eventually. For now it's /// just used in unit tests. - #[cfg(test)] pub fn with_distinct_min_distance( mut self, distinct_min_distance: impl Into, From 4b3e1c4e569cf0e75ed99dd4ea68c2c33db4c9f6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 May 2022 13:37:38 +0200 Subject: [PATCH 3/7] Improve wording --- crates/fj-kernel/src/shape/api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index bf9748e8f..0040b8b11 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -39,7 +39,7 @@ impl Shape { } } - /// Override the minimum distance of distinct objects + /// Override the minimum distance between distinct objects /// /// Used for vertex validation, to determine whether vertices are unique. /// From 76c9098a15cf509b4d135c4a4f6a3a4916ee3ea7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 May 2022 13:37:48 +0200 Subject: [PATCH 4/7] Remove obsolete implementation note --- crates/fj-kernel/src/shape/api.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index 0040b8b11..118cf8bda 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -42,11 +42,6 @@ impl Shape { /// Override the minimum distance between distinct objects /// /// Used for vertex validation, to determine whether vertices are unique. - /// - /// # Implementation note - /// - /// This functionality should be exposed to models, eventually. For now it's - /// just used in unit tests. pub fn with_distinct_min_distance( mut self, distinct_min_distance: impl Into, From aafeed0f0ad6ca695a18c725a4096c588442640f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 May 2022 13:38:07 +0200 Subject: [PATCH 5/7] Add configuration value to `Shape` It will soon be used for geometrical validation. --- crates/fj-kernel/src/shape/api.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index 118cf8bda..32a49ee5d 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -14,6 +14,8 @@ use super::{ #[derive(Clone, Debug)] pub struct Shape { distinct_min_distance: Scalar, + identical_max_distance: Scalar, + stores: Stores, } @@ -26,6 +28,16 @@ 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-16), + stores: Stores { points: Store::new(), curves: Store::new(), @@ -50,6 +62,17 @@ 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, + ) -> 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 From 33f19f6d587b57dcfa53a87dc62d1ce7b6cc9a2f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 May 2022 14:36:15 +0200 Subject: [PATCH 6/7] Pass max distance into validation methods --- crates/fj-kernel/src/shape/api.rs | 13 +++++++++++-- crates/fj-kernel/src/shape/update.rs | 15 ++++++++++++++- crates/fj-kernel/src/shape/validate/mod.rs | 8 ++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index 32a49ee5d..3ec6674f3 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -78,7 +78,12 @@ impl 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(&mut self, object: T) -> ValidationResult { - object.validate(None, self.distinct_min_distance, &self.stores)?; + object.validate( + None, + self.distinct_min_distance, + self.identical_max_distance, + &self.stores, + )?; let handle = self.stores.get::().insert(object); Ok(handle) } @@ -171,7 +176,11 @@ 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, &mut self.stores) + Update::new( + self.distinct_min_distance, + self.identical_max_distance, + &mut self.stores, + ) } /// Clone the shape diff --git a/crates/fj-kernel/src/shape/update.rs b/crates/fj-kernel/src/shape/update.rs index ae6dcbe66..ea8c09a7b 100644 --- a/crates/fj-kernel/src/shape/update.rs +++ b/crates/fj-kernel/src/shape/update.rs @@ -8,14 +8,20 @@ use super::{stores::Stores, validate::Validate as _, Object, ValidationError}; #[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, stores: &'r mut Stores) -> Self { + pub(super) fn new( + min_distance: Scalar, + max_distance: Scalar, + stores: &'r mut Stores, + ) -> Self { Self { min_distance, + max_distance, stores, executed: false, } @@ -46,6 +52,7 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, + self.max_distance, self.stores, )?; } @@ -53,6 +60,7 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, + self.max_distance, self.stores, )?; } @@ -60,6 +68,7 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, + self.max_distance, self.stores, )?; } @@ -67,6 +76,7 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, + self.max_distance, self.stores, )?; } @@ -74,6 +84,7 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, + self.max_distance, self.stores, )?; } @@ -81,6 +92,7 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, + self.max_distance, self.stores, )?; } @@ -88,6 +100,7 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, + self.max_distance, self.stores, )?; } diff --git a/crates/fj-kernel/src/shape/validate/mod.rs b/crates/fj-kernel/src/shape/validate/mod.rs index a2e06cde0..fbae39e5c 100644 --- a/crates/fj-kernel/src/shape/validate/mod.rs +++ b/crates/fj-kernel/src/shape/validate/mod.rs @@ -21,6 +21,7 @@ pub trait Validate { &self, handle: Option<&Handle>, min_distance: Scalar, + max_distance: Scalar, stores: &Stores, ) -> Result<(), ValidationError> where @@ -32,6 +33,7 @@ impl Validate for Point<3> { &self, _: Option<&Handle>, _: Scalar, + _: Scalar, _: &Stores, ) -> Result<(), ValidationError> { Ok(()) @@ -43,6 +45,7 @@ impl Validate for Curve<3> { &self, _: Option<&Handle>, _: Scalar, + _: Scalar, _: &Stores, ) -> Result<(), ValidationError> { Ok(()) @@ -54,6 +57,7 @@ impl Validate for Surface { &self, _: Option<&Handle>, _: Scalar, + _: Scalar, _: &Stores, ) -> Result<(), ValidationError> { Ok(()) @@ -71,6 +75,7 @@ impl Validate for Vertex { &self, handle: Option<&Handle>, min_distance: Scalar, + _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { structural::validate_vertex(self, stores)?; @@ -85,6 +90,7 @@ impl Validate for Edge<3> { &self, _: Option<&Handle>, _: Scalar, + _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { structural::validate_edge(self, stores)?; @@ -105,6 +111,7 @@ impl Validate for Cycle<3> { &self, _: Option<&Handle>, _: Scalar, + _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { structural::validate_cycle(self, stores)?; @@ -117,6 +124,7 @@ impl Validate for Face { &self, _: Option<&Handle>, _: Scalar, + _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { structural::validate_face(self, stores)?; From 6c3143ada8ba803ad4461b6efc25d08ad272f872 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 May 2022 14:37:36 +0200 Subject: [PATCH 7/7] Implement geometric validation of edge vertices --- .../fj-kernel/src/shape/validate/geometric.rs | 66 +++++++++++++++++++ crates/fj-kernel/src/shape/validate/mod.rs | 4 +- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/shape/validate/geometric.rs b/crates/fj-kernel/src/shape/validate/geometric.rs index e0cb4c67d..a704a4a45 100644 --- a/crates/fj-kernel/src/shape/validate/geometric.rs +++ b/crates/fj-kernel/src/shape/validate/geometric.rs @@ -1,5 +1,35 @@ use std::fmt; +use fj_math::Scalar; + +use crate::topology::Edge; + +pub fn validate_edge( + edge: &Edge<3>, + max_distance: impl Into, +) -> Result<(), GeometricIssues> { + let max_distance = max_distance.into(); + + // Validate that the local and canonical forms of the vertices match. As a + // side effect, this also happens to validate that the canonical forms of + // the vertices lie on the curve. + if let Some(vertices) = &edge.vertices { + for vertex in vertices { + let local_point = + edge.curve().point_from_curve_coords(*vertex.local()); + + let distance = + (local_point - vertex.canonical().get().point()).magnitude(); + + if distance > max_distance { + return Err(GeometricIssues); + } + } + } + + Ok(()) +} + /// Geometric issues found during validation /// /// Used by [`ValidationError`]. @@ -14,3 +44,39 @@ impl fmt::Display for GeometricIssues { Ok(()) } } + +#[cfg(test)] +mod tests { + use fj_math::Scalar; + + use crate::{ + shape::{LocalForm, Shape}, + topology::Edge, + }; + + #[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.clone().map(|vertices| { + 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(()) + } +} diff --git a/crates/fj-kernel/src/shape/validate/mod.rs b/crates/fj-kernel/src/shape/validate/mod.rs index fbae39e5c..4e380a970 100644 --- a/crates/fj-kernel/src/shape/validate/mod.rs +++ b/crates/fj-kernel/src/shape/validate/mod.rs @@ -90,10 +90,12 @@ impl Validate for Edge<3> { &self, _: Option<&Handle>, _: Scalar, - _: Scalar, + max_distance: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { + geometric::validate_edge(self, max_distance)?; structural::validate_edge(self, stores)?; + Ok(()) } }