From 0ab67b5e97be64b3c8631722e3810d96fdc6274d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Jun 2022 17:04:55 +0200 Subject: [PATCH 1/7] Move coherence validation to new infrastructure --- crates/fj-kernel/src/shape/mod.rs | 3 +-- crates/fj-kernel/src/shape/validate/mod.rs | 5 +---- .../{shape/validate => validation}/coherence.rs | 0 crates/fj-kernel/src/validation/mod.rs | 14 ++++++++++---- 4 files changed, 12 insertions(+), 10 deletions(-) rename crates/fj-kernel/src/{shape/validate => validation}/coherence.rs (100%) diff --git a/crates/fj-kernel/src/shape/mod.rs b/crates/fj-kernel/src/shape/mod.rs index f633b3df7..8867b825e 100644 --- a/crates/fj-kernel/src/shape/mod.rs +++ b/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, }, }; diff --git a/crates/fj-kernel/src/shape/validate/mod.rs b/crates/fj-kernel/src/shape/validate/mod.rs index 2e18185c6..1b128ea73 100644 --- a/crates/fj-kernel/src/shape/validate/mod.rs +++ b/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}, }; @@ -78,10 +76,9 @@ impl Validate for Edge<3> { &self, handle: Option<&Handle>, _: Scalar, - max_distance: Scalar, + _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { - coherence::validate_edge(self, max_distance)?; structural::validate_edge(self, stores)?; uniqueness::validate_edge(self, handle, &stores.edges)?; diff --git a/crates/fj-kernel/src/shape/validate/coherence.rs b/crates/fj-kernel/src/validation/coherence.rs similarity index 100% rename from crates/fj-kernel/src/shape/validate/coherence.rs rename to crates/fj-kernel/src/validation/coherence.rs diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index c4b52f5c0..c130a35fb 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/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, ValidationError> { + for edge in shape.edges() { + coherence::validate_edge(&edge.get(), config.identical_max_distance)?; + } + Ok(Validated(shape)) } From 4827bc95853e6203c03e35a94d9b026c01ac691f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Jun 2022 17:07:45 +0200 Subject: [PATCH 2/7] Remove unused trait method argument --- crates/fj-kernel/src/shape/api.rs | 13 ++----------- crates/fj-kernel/src/shape/update.rs | 14 +------------- crates/fj-kernel/src/shape/validate/mod.rs | 7 ------- 3 files changed, 3 insertions(+), 31 deletions(-) diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index dc82f6e82..347507454 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -96,12 +96,7 @@ 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.identical_max_distance, - &self.stores, - )?; + object.validate(None, self.distinct_min_distance, &self.stores)?; let handle = self.stores.get::().insert(object); Ok(handle) } @@ -191,11 +186,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 diff --git a/crates/fj-kernel/src/shape/update.rs b/crates/fj-kernel/src/shape/update.rs index 9651a1a5b..aa74b8e3a 100644 --- a/crates/fj-kernel/src/shape/update.rs +++ b/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,7 +48,6 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, - self.max_distance, self.stores, )?; } @@ -62,7 +55,6 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, - self.max_distance, self.stores, )?; } @@ -70,7 +62,6 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, - self.max_distance, self.stores, )?; } @@ -78,7 +69,6 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, - self.max_distance, self.stores, )?; } @@ -86,7 +76,6 @@ impl<'r> Update<'r> { object.get().validate( Some(&object), self.min_distance, - self.max_distance, self.stores, )?; } @@ -94,7 +83,6 @@ 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 1b128ea73..56e2ea342 100644 --- a/crates/fj-kernel/src/shape/validate/mod.rs +++ b/crates/fj-kernel/src/shape/validate/mod.rs @@ -20,7 +20,6 @@ pub trait Validate { &self, handle: Option<&Handle>, min_distance: Scalar, - max_distance: Scalar, stores: &Stores, ) -> Result<(), ValidationError> where @@ -32,7 +31,6 @@ impl Validate for Curve<3> { &self, _: Option<&Handle>, _: Scalar, - _: Scalar, _: &Stores, ) -> Result<(), ValidationError> { Ok(()) @@ -44,7 +42,6 @@ impl Validate for Surface { &self, _: Option<&Handle>, _: Scalar, - _: Scalar, _: &Stores, ) -> Result<(), ValidationError> { Ok(()) @@ -62,7 +59,6 @@ impl Validate for Vertex { &self, handle: Option<&Handle>, _: Scalar, - _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { uniqueness::validate_vertex(self, handle, &stores.vertices)?; @@ -76,7 +72,6 @@ impl Validate for Edge<3> { &self, handle: Option<&Handle>, _: Scalar, - _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { structural::validate_edge(self, stores)?; @@ -99,7 +94,6 @@ impl Validate for Cycle<3> { &self, _: Option<&Handle>, _: Scalar, - _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { structural::validate_cycle(self, stores)?; @@ -112,7 +106,6 @@ impl Validate for Face { &self, _: Option<&Handle>, _: Scalar, - _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { structural::validate_face(self, stores)?; From 13c1fc64fbba852e660ccd5ece482572e0a6effb Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Jun 2022 17:09:55 +0200 Subject: [PATCH 3/7] Add comment --- crates/fj-kernel/src/validation/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index c130a35fb..22ec66001 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -71,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), } } From 3df2877f3bf6070c1a46e8a9e6d1698de7b7958b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Jun 2022 17:10:18 +0200 Subject: [PATCH 4/7] Remove unused code --- crates/fj-kernel/src/shape/api.rs | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index 347507454..0579733d8 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/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,17 +69,6 @@ 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 c43ffcf82441e859c41d1b9bc5438117362e0c97 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Jun 2022 17:12:33 +0200 Subject: [PATCH 5/7] Move test --- crates/fj-kernel/src/validation/coherence.rs | 34 -------------------- crates/fj-kernel/src/validation/mod.rs | 34 ++++++++++++++++++++ 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/crates/fj-kernel/src/validation/coherence.rs b/crates/fj-kernel/src/validation/coherence.rs index 1d5461648..fa7a56bef 100644 --- a/crates/fj-kernel/src/validation/coherence.rs +++ b/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(()) - } -} diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 22ec66001..93666c2b7 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -175,3 +175,37 @@ impl ValidationError { false } } + +#[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::coherence::validate_edge(&edge, deviation * 2.).is_ok()); + assert!(super::coherence::validate_edge(&edge, deviation / 2.).is_err()); + + Ok(()) + } +} From b82b6639839902537384f52956903cbdb05015dd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Jun 2022 13:50:51 +0200 Subject: [PATCH 6/7] Redefine test in terms of public API --- crates/fj-kernel/src/validation/mod.rs | 51 +++++++++++++++++++------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 93666c2b7..870f9e7a9 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -183,28 +183,51 @@ mod tests { use crate::{ objects::Edge, shape::{LocalForm, Shape}, + validation::Config, }; #[test] fn validate_edge() -> 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); - 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::coherence::validate_edge(&edge, deviation * 2.).is_ok()); - assert!(super::coherence::validate_edge(&edge, deviation / 2.).is_err()); + 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(()) } From 4a12a6461a0c56c982d735061a825ebe5c56ccf7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Jun 2022 13:51:25 +0200 Subject: [PATCH 7/7] Make test name more specific --- crates/fj-kernel/src/validation/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 870f9e7a9..c10757307 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -187,7 +187,7 @@ mod tests { }; #[test] - fn validate_edge() -> anyhow::Result<()> { + 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.]])?