From 4139128d0694f08f70f6a9b0d70278b715809a9c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 19 Oct 2023 12:01:42 +0200 Subject: [PATCH 1/7] Properly document validation infrastructure --- crates/fj-core/src/validate/mod.rs | 71 +++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index 2cd8a9c77..1bcf151bb 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -1,4 +1,73 @@ -//! Infrastructure for validating objects +//! # Infrastructure for validating objects +//! +//! ## Structure and Nomenclature +//! +//! **Validation** is the process of checking that objects meet specific +//! requirements. Each kind of object has its own set of requirements. +//! +//! An object that meets all the requirement for its kind is considered +//! **valid**. An object that does not meet all of them is considered +//! **invalid**. This results in a **validation error**, which is represented by +//! [`ValidationError`]. +//! +//! Every single requirement is checked by a dedicated function. These functions +//! are called **validation checks**. Validation checks are currently not +//! visible in the public API, but their structure is reflected in the variants +//! of the different enums that make up [`ValidationError`] (each validation +//! check produces one of the types of validation error, that these nested enums +//! represent). +//! +//! In principle, the absence of validation errors should guarantee, that an +//! object can be exported to an external file format without problems (which +//! falls under the purview of the [`fj-export`] crate). This has not yet been +//! achieved, as some validation checks are still missing. +//! +//! The [issue tracker] has open issues for some of those missing checks, but +//! others are not currently tracked (or not even known). Please feel free to +//! open a new issue (or comment on an existing one), if you encounter an object +//! that you believe should be invalid, but is not. +//! +//! +//! ## Use +//! +//! All objects implement the [`Validate`] trait, which users can use to +//! validate objects manually. This might be useful for debugging, but is +//! otherwise not recommended. +//! +//! Experience has shown, that stopping the construction of a shape on the first +//! validation failure can make it hard to understand what actually went wrong. +//! For that reason, objects are validated as they are constructed, but +//! validation errors are collected in the background, to be processed when the +//! whole shape has been finished. +//! +//! This is set up within the [`Services`] API, and validation errors result in +//! a panic, when the [`Services`] instance is dropped. Unless you want to +//! handle validation errors in a different way, you don't have to do anything +//! special to use the validation infrastructure. +//! +//! +//! ## Configuration +//! +//! Fornjot's geometry representation is set up to prioritize correctness, which +//! is achieved by making the relations between different objects *explicit*. +//! This means, for example, that coincident objects of the same type that don't +//! have the same *identity* are generally considered invalid. +//! +//! Coincidence checks must use a tolerance value to be useful, meaning objects +//! that are very close together can be considered coincident. What should be +//! considered "very close" is dependent on the scale that your model operates +//! on, and this fact is taken into account by allowing for configuration via +//! [`Validate::validate_with_config`] and [`ValidationConfig`]. +//! +//! However, it is currently not possible to define this configuration for the +//! background validation done by the [`Services`] API. If this is a problem for +//! you, please comment on this issue: +//! +//! +//! +//! [`fj-export`]: https://crates.io/crates/fj-export +//! [issue tracker]: https://github.com/hannobraun/fornjot/issues +//! [`Services`]: crate::services::Services mod curve; mod cycle; From bc14c0969fd4ad0fa747237dadb7ff04fef581d8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 19 Oct 2023 12:02:41 +0200 Subject: [PATCH 2/7] Rename shell validation checks for consistency Validation checks for all other objects start with `check`. --- crates/fj-core/src/validate/shell.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index fa39a129b..b928e731b 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -17,10 +17,10 @@ impl Validate for Shell { config: &ValidationConfig, errors: &mut Vec, ) { - ShellValidationError::validate_curve_coordinates(self, config, errors); - ShellValidationError::validate_edges_coincident(self, config, errors); - ShellValidationError::validate_watertight(self, config, errors); - ShellValidationError::validate_same_orientation(self, errors); + ShellValidationError::check_curve_coordinates(self, config, errors); + ShellValidationError::check_edges_coincident(self, config, errors); + ShellValidationError::check_watertight(self, config, errors); + ShellValidationError::check_same_orientation(self, errors); } } @@ -111,7 +111,7 @@ fn distances( } impl ShellValidationError { - fn validate_curve_coordinates( + fn check_curve_coordinates( shell: &Shell, config: &ValidationConfig, errors: &mut Vec, @@ -203,7 +203,7 @@ impl ShellValidationError { } } - fn validate_edges_coincident( + fn check_edges_coincident( shell: &Shell, config: &ValidationConfig, errors: &mut Vec, @@ -289,7 +289,7 @@ impl ShellValidationError { } } - fn validate_watertight( + fn check_watertight( shell: &Shell, _: &ValidationConfig, errors: &mut Vec, @@ -319,7 +319,7 @@ impl ShellValidationError { } } - fn validate_same_orientation( + fn check_same_orientation( shell: &Shell, errors: &mut Vec, ) { From 8ae6fc8d86c7dbe0cc1bcfeab2d950ecbd958b8c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 19 Oct 2023 12:05:30 +0200 Subject: [PATCH 3/7] Document `check_curve_coordinates` --- crates/fj-core/src/validate/shell.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index b928e731b..180fa3c02 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -111,6 +111,7 @@ fn distances( } impl ShellValidationError { + /// Check that local curve definitions that refer to the same curve match fn check_curve_coordinates( shell: &Shell, config: &ValidationConfig, From 2f4ed8f51440d58cbec7b61f11ccd2d7980ad176 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 19 Oct 2023 12:07:53 +0200 Subject: [PATCH 4/7] Update function name --- crates/fj-core/src/validate/shell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 180fa3c02..6d0da1749 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -18,7 +18,7 @@ impl Validate for Shell { errors: &mut Vec, ) { ShellValidationError::check_curve_coordinates(self, config, errors); - ShellValidationError::check_edges_coincident(self, config, errors); + ShellValidationError::check_half_edge_coincidence(self, config, errors); ShellValidationError::check_watertight(self, config, errors); ShellValidationError::check_same_orientation(self, errors); } @@ -204,7 +204,7 @@ impl ShellValidationError { } } - fn check_edges_coincident( + fn check_half_edge_coincidence( shell: &Shell, config: &ValidationConfig, errors: &mut Vec, From 586a9e8220e012ee1a8054ddd01623b91c0abf0a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 19 Oct 2023 12:09:01 +0200 Subject: [PATCH 5/7] Document `check_half_edge_coincidence` --- crates/fj-core/src/validate/shell.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 6d0da1749..e014cd089 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -204,6 +204,7 @@ impl ShellValidationError { } } + /// Check that identical half-edges are coincident, non-identical are not fn check_half_edge_coincidence( shell: &Shell, config: &ValidationConfig, From 476aad3aef58b1f6d28853a929efa4f81506c5ed Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 19 Oct 2023 12:11:11 +0200 Subject: [PATCH 6/7] Improve comments in `check_half_edge_coincidence` --- crates/fj-core/src/validate/shell.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index e014cd089..5afb27f48 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -245,8 +245,8 @@ impl ShellValidationError { match identical { true => { // All points on identical curves should be within - // identical_max_distance, so we shouldn't have any - // greater than the max + // `identical_max_distance`, so we shouldn't have any + // distances greater than that. if distances( edge_a.clone(), surface_a.clone(), @@ -268,7 +268,7 @@ impl ShellValidationError { } false => { // If all points on distinct curves are within - // distinct_min_distance, that's a problem. + // `distinct_min_distance`, that's a problem. if distances( edge_a.clone(), surface_a.clone(), From b65b7e72c2b39ecf4d9d0d6a87a7518185ff4e00 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 19 Oct 2023 12:12:49 +0200 Subject: [PATCH 7/7] Match order of validation errors and checks --- crates/fj-core/src/validate/shell.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 5afb27f48..550552743 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -35,10 +35,6 @@ pub enum ShellValidationError { )] CurveCoordinateSystemMismatch(Vec), - /// [`Shell`] is not watertight - #[error("Shell is not watertight")] - NotWatertight, - /// [`Shell`] contains edges that are coincident, but not identical #[error( "`Shell` contains `Edge`s that are coincident but refer to different \ @@ -70,6 +66,10 @@ pub enum ShellValidationError { surface_b: Handle, }, + /// [`Shell`] is not watertight + #[error("Shell is not watertight")] + NotWatertight, + /// [`Shell`] contains faces of mixed orientation (inwards and outwards) #[error("Shell has mixed face orientations")] MixedOrientations,