From 1a3b160881a7287cb218e71911cfd630b2f0c39b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 22 Feb 2024 13:59:23 +0100 Subject: [PATCH 1/4] Add `ValidationCheck` trait --- crates/fj-core/src/validation/mod.rs | 2 + .../src/validation/validation_check.rs | 53 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 crates/fj-core/src/validation/validation_check.rs diff --git a/crates/fj-core/src/validation/mod.rs b/crates/fj-core/src/validation/mod.rs index b5c3f415c..e0726bab8 100644 --- a/crates/fj-core/src/validation/mod.rs +++ b/crates/fj-core/src/validation/mod.rs @@ -23,9 +23,11 @@ mod config; mod error; mod validation; +mod validation_check; pub use self::{ config::ValidationConfig, error::{ValidationError, ValidationErrors}, validation::Validation, + validation_check::ValidationCheck, }; diff --git a/crates/fj-core/src/validation/validation_check.rs b/crates/fj-core/src/validation/validation_check.rs new file mode 100644 index 000000000..5e30c92a5 --- /dev/null +++ b/crates/fj-core/src/validation/validation_check.rs @@ -0,0 +1,53 @@ +use std::fmt::Display; + +use super::ValidationConfig; + +/// Run a specific validation check on an object +/// +/// This trait is implemented once per validation check and object it applies +/// to. `Self` is the object, while `T` identifies the validation check. +pub trait ValidationCheck { + /// Run the validation check on the implementing object + fn check(&self, config: &ValidationConfig) -> impl Iterator; + + /// Convenience method to run the check return the first error + /// + /// This method is designed for convenience over flexibility (it is intended + /// for use in unit tests), and thus always uses the default configuration. + fn check_and_return_first_error(&self) -> Result<(), T> { + let errors = + self.check(&ValidationConfig::default()).collect::>(); + + if let Some(err) = errors.into_iter().next() { + return Err(err); + } + + Ok(()) + } + + /// Convenience method to run the check and expect one error + /// + /// This method is designed for convenience over flexibility (it is intended + /// for use in unit tests), and thus always uses the default configuration. + fn check_and_expect_one_error(&self) + where + T: Display, + { + let config = ValidationConfig::default(); + let mut errors = self.check(&config).peekable(); + + errors + .next() + .expect("Expected one validation error; none found"); + + if errors.peek().is_some() { + println!("Unexpected validation errors:"); + + for err in errors { + println!("{err}"); + } + + panic!("Expected only one validation error") + } + } +} From bb720fd5798a5ace6b040949a0c8758e11dc1c5f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 23 Feb 2024 10:14:29 +0100 Subject: [PATCH 2/4] Add `validation::checks` --- crates/fj-core/src/validation/checks/mod.rs | 3 +++ crates/fj-core/src/validation/mod.rs | 2 ++ 2 files changed, 5 insertions(+) create mode 100644 crates/fj-core/src/validation/checks/mod.rs diff --git a/crates/fj-core/src/validation/checks/mod.rs b/crates/fj-core/src/validation/checks/mod.rs new file mode 100644 index 000000000..0512298b2 --- /dev/null +++ b/crates/fj-core/src/validation/checks/mod.rs @@ -0,0 +1,3 @@ +//! All validation checks +//! +//! See documentation of [parent module](super) for more information. diff --git a/crates/fj-core/src/validation/mod.rs b/crates/fj-core/src/validation/mod.rs index e0726bab8..dbe9aed4e 100644 --- a/crates/fj-core/src/validation/mod.rs +++ b/crates/fj-core/src/validation/mod.rs @@ -25,6 +25,8 @@ mod error; mod validation; mod validation_check; +pub mod checks; + pub use self::{ config::ValidationConfig, error::{ValidationError, ValidationErrors}, From 7e58d2787e7fa73d08cc7c563203b68d04679ce0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 23 Feb 2024 11:07:19 +0100 Subject: [PATCH 3/4] Add `AdjacentHalfEdgesNotConnected` --- .../validation/checks/half_edge_connection.rs | 101 ++++++++++++++++++ crates/fj-core/src/validation/checks/mod.rs | 4 + 2 files changed, 105 insertions(+) create mode 100644 crates/fj-core/src/validation/checks/half_edge_connection.rs diff --git a/crates/fj-core/src/validation/checks/half_edge_connection.rs b/crates/fj-core/src/validation/checks/half_edge_connection.rs new file mode 100644 index 000000000..296f5ac86 --- /dev/null +++ b/crates/fj-core/src/validation/checks/half_edge_connection.rs @@ -0,0 +1,101 @@ +use fj_math::{Point, Scalar}; + +use crate::{ + objects::{Cycle, HalfEdge}, + storage::Handle, + validation::{validation_check::ValidationCheck, ValidationConfig}, +}; + +/// Adjacent [`HalfEdge`]s in [`Cycle`] are not connected +/// +/// Each [`HalfEdge`] only references its start vertex. The end vertex is always +/// assumed to be the start vertex of the next [`HalfEdge`] in the cycle. This +/// part of the definition carries no redundancy, and thus doesn't need to be +/// subject to a validation check. +/// +/// However, the *position* of that shared vertex is redundantly defined in both +/// [`HalfEdge`]s. This check verifies that both positions are the same. +#[derive(Clone, Debug, thiserror::Error)] +#[error( + "Adjacent `HalfEdge`s in `Cycle` are not connected\n\ + - End position of first `HalfEdge`: {end_pos_of_first_half_edge:?}\n\ + - Start position of second `HalfEdge`: {start_pos_of_second_half_edge:?}\n\ + - Distance between vertices: {distance_between_positions}\n\ + - The unconnected `HalfEdge`s: {unconnected_half_edges:#?}" +)] +pub struct AdjacentHalfEdgesNotConnected { + /// The end position of the first [`HalfEdge`] + pub end_pos_of_first_half_edge: Point<2>, + + /// The start position of the second [`HalfEdge`] + pub start_pos_of_second_half_edge: Point<2>, + + /// The distance between the two positions + pub distance_between_positions: Scalar, + + /// The edges + pub unconnected_half_edges: [Handle; 2], +} + +impl ValidationCheck for Cycle { + fn check( + &self, + config: &ValidationConfig, + ) -> impl Iterator { + self.half_edges().pairs().filter_map(|(first, second)| { + let end_pos_of_first_half_edge = { + let [_, end] = first.boundary().inner; + first.path().point_from_path_coords(end) + }; + let start_pos_of_second_half_edge = second.start_position(); + + let distance_between_positions = (end_pos_of_first_half_edge + - start_pos_of_second_half_edge) + .magnitude(); + + if distance_between_positions > config.identical_max_distance { + return Some(AdjacentHalfEdgesNotConnected { + end_pos_of_first_half_edge, + start_pos_of_second_half_edge, + distance_between_positions, + unconnected_half_edges: [first.clone(), second.clone()], + }); + } + + None + }) + } +} + +#[cfg(test)] +mod tests { + + use crate::{ + objects::{Cycle, HalfEdge}, + operations::{ + build::{BuildCycle, BuildHalfEdge}, + update::UpdateCycle, + }, + validation::ValidationCheck, + Core, + }; + + #[test] + fn adjacent_half_edges_connected() -> anyhow::Result<()> { + let mut core = Core::new(); + + let valid = Cycle::polygon([[0., 0.], [1., 0.], [1., 1.]], &mut core); + valid.check_and_return_first_error()?; + + let invalid = valid.update_half_edge( + valid.half_edges().first(), + |_, core| { + [HalfEdge::line_segment([[0., 0.], [2., 0.]], None, core)] + }, + &mut core, + ); + invalid.check_and_expect_one_error(); + + Ok(()) + } +} diff --git a/crates/fj-core/src/validation/checks/mod.rs b/crates/fj-core/src/validation/checks/mod.rs index 0512298b2..ab9e9debd 100644 --- a/crates/fj-core/src/validation/checks/mod.rs +++ b/crates/fj-core/src/validation/checks/mod.rs @@ -1,3 +1,7 @@ //! All validation checks //! //! See documentation of [parent module](super) for more information. + +mod half_edge_connection; + +pub use self::half_edge_connection::AdjacentHalfEdgesNotConnected; From 1773519bd944eb65cd486402c9537f86dfec39be Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 23 Feb 2024 11:14:39 +0100 Subject: [PATCH 4/4] Replace old-style cycle validation check --- crates/fj-core/src/validate/cycle.rs | 109 +------------------------ crates/fj-core/src/validate/mod.rs | 6 +- crates/fj-core/src/validation/error.rs | 12 +-- 3 files changed, 13 insertions(+), 114 deletions(-) diff --git a/crates/fj-core/src/validate/cycle.rs b/crates/fj-core/src/validate/cycle.rs index 035d46781..a1d628605 100644 --- a/crates/fj-core/src/validate/cycle.rs +++ b/crates/fj-core/src/validate/cycle.rs @@ -1,9 +1,6 @@ -use fj_math::{Point, Scalar}; - use crate::{ - objects::{Cycle, HalfEdge}, - storage::Handle, - validation::{ValidationConfig, ValidationError}, + objects::Cycle, + validation::{ValidationCheck, ValidationConfig, ValidationError}, }; use super::Validate; @@ -14,106 +11,6 @@ impl Validate for Cycle { config: &ValidationConfig, errors: &mut Vec, ) { - CycleValidationError::check_half_edge_connections(self, config, errors); - } -} - -/// [`Cycle`] validation failed -#[derive(Clone, Debug, thiserror::Error)] -pub enum CycleValidationError { - /// [`Cycle`]'s edges are not connected - #[error( - "Adjacent `HalfEdge`s are not connected\n\ - - End position of first `HalfEdge`: {end_of_first:?}\n\ - - Start position of second `HalfEdge`: {start_of_second:?}\n\ - - Distance between vertices: {distance}\n\ - - `HalfEdge`s: {half_edges:#?}" - )] - HalfEdgesNotConnected { - /// The end position of the first [`HalfEdge`] - end_of_first: Point<2>, - - /// The start position of the second [`HalfEdge`] - start_of_second: Point<2>, - - /// The distance between the two vertices - distance: Scalar, - - /// The edges - half_edges: [Handle; 2], - }, -} - -impl CycleValidationError { - fn check_half_edge_connections( - cycle: &Cycle, - config: &ValidationConfig, - errors: &mut Vec, - ) { - for (first, second) in cycle.half_edges().pairs() { - let end_of_first = { - let [_, end] = first.boundary().inner; - first.path().point_from_path_coords(end) - }; - let start_of_second = second.start_position(); - - let distance = (end_of_first - start_of_second).magnitude(); - - if distance > config.identical_max_distance { - errors.push( - Self::HalfEdgesNotConnected { - end_of_first, - start_of_second, - distance, - half_edges: [first.clone(), second.clone()], - } - .into(), - ); - } - } - } -} - -#[cfg(test)] -mod tests { - - use crate::{ - assert_contains_err, - objects::{Cycle, HalfEdge}, - operations::{ - build::{BuildCycle, BuildHalfEdge}, - update::UpdateCycle, - }, - validate::{cycle::CycleValidationError, Validate}, - validation::ValidationError, - Core, - }; - - #[test] - fn edges_connected() -> anyhow::Result<()> { - let mut core = Core::new(); - - let valid = - Cycle::polygon([[0.0, 0.0], [1.0, 0.0], [1.0, 1.0]], &mut core); - - valid.validate_and_return_first_error()?; - - let disconnected = { - let edges = [ - HalfEdge::line_segment([[0., 0.], [1., 0.]], None, &mut core), - HalfEdge::line_segment([[0., 0.], [1., 0.]], None, &mut core), - ]; - - Cycle::empty().add_half_edges(edges, &mut core) - }; - - assert_contains_err!( - disconnected, - ValidationError::Cycle( - CycleValidationError::HalfEdgesNotConnected { .. } - ) - ); - - Ok(()) + errors.extend(self.check(config).map(Into::into)); } } diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index 295ca5224..2c3b3d56c 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -76,9 +76,9 @@ mod vertex; use crate::validation::{ValidationConfig, ValidationError}; pub use self::{ - cycle::CycleValidationError, edge::EdgeValidationError, - face::FaceValidationError, shell::ShellValidationError, - sketch::SketchValidationError, solid::SolidValidationError, + edge::EdgeValidationError, face::FaceValidationError, + shell::ShellValidationError, sketch::SketchValidationError, + solid::SolidValidationError, }; /// Assert that some object has a validation error which matches a specific diff --git a/crates/fj-core/src/validation/error.rs b/crates/fj-core/src/validation/error.rs index 01d28103a..353ac6409 100644 --- a/crates/fj-core/src/validation/error.rs +++ b/crates/fj-core/src/validation/error.rs @@ -1,16 +1,18 @@ use std::{convert::Infallible, fmt}; use crate::validate::{ - CycleValidationError, EdgeValidationError, FaceValidationError, - ShellValidationError, SketchValidationError, SolidValidationError, + EdgeValidationError, FaceValidationError, ShellValidationError, + SketchValidationError, SolidValidationError, }; +use super::checks::AdjacentHalfEdgesNotConnected; + /// An error that can occur during a validation #[derive(Clone, Debug, thiserror::Error)] pub enum ValidationError { - /// `Cycle` validation error - #[error("`Cycle` validation error")] - Cycle(#[from] CycleValidationError), + /// `HalfEdge`s in `Cycle` not connected + #[error(transparent)] + HalfEdgesInCycleNotConnected(#[from] AdjacentHalfEdgesNotConnected), /// `Edge` validation error #[error("`Edge` validation error")]