Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly document validation infrastructure #2061

Merged
merged 7 commits into from
Oct 19, 2023
Merged

Properly document validation infrastructure #2061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4139128
Properly document validation infrastructure
hannobraun Oct 19, 2023
bc14c09
Rename shell validation checks for consistency
hannobraun Oct 19, 2023
8ae6fc8
Document `check_curve_coordinates`
hannobraun Oct 19, 2023
2f4ed8f
Update function name
hannobraun Oct 19, 2023
586a9e8
Document `check_half_edge_coincidence`
hannobraun Oct 19, 2023
476aad3
Improve comments in `check_half_edge_coincidence`
hannobraun Oct 19, 2023
b65b7e7
Match order of validation errors and checks
hannobraun Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 70 additions & 1 deletion crates/fj-core/src/validate/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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:
//! <https://github.com/hannobraun/fornjot/issues/2060>
//!
//!
//! [`fj-export`]: https://crates.io/crates/fj-export
//! [issue tracker]: https://github.com/hannobraun/fornjot/issues
//! [`Services`]: crate::services::Services

mod curve;
mod cycle;
Expand Down
32 changes: 17 additions & 15 deletions crates/fj-core/src/validate/shell.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ impl Validate for Shell {
config: &ValidationConfig,
errors: &mut Vec<ValidationError>,
) {
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_half_edge_coincidence(self, config, errors);
ShellValidationError::check_watertight(self, config, errors);
ShellValidationError::check_same_orientation(self, errors);
}
}

Expand All @@ -35,10 +35,6 @@ pub enum ShellValidationError {
)]
CurveCoordinateSystemMismatch(Vec<CurveCoordinateSystemMismatch>),

/// [`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 \
Expand Down Expand Up @@ -70,6 +66,10 @@ pub enum ShellValidationError {
surface_b: Handle<Surface>,
},

/// [`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,
Expand Down Expand Up @@ -111,7 +111,8 @@ fn distances(
}

impl ShellValidationError {
fn validate_curve_coordinates(
/// Check that local curve definitions that refer to the same curve match
fn check_curve_coordinates(
shell: &Shell,
config: &ValidationConfig,
errors: &mut Vec<ValidationError>,
Expand Down Expand Up @@ -203,7 +204,8 @@ impl ShellValidationError {
}
}

fn validate_edges_coincident(
/// Check that identical half-edges are coincident, non-identical are not
fn check_half_edge_coincidence(
shell: &Shell,
config: &ValidationConfig,
errors: &mut Vec<ValidationError>,
Expand Down Expand Up @@ -243,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(),
Expand All @@ -266,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(),
Expand All @@ -289,7 +291,7 @@ impl ShellValidationError {
}
}

fn validate_watertight(
fn check_watertight(
shell: &Shell,
_: &ValidationConfig,
errors: &mut Vec<ValidationError>,
Expand Down Expand Up @@ -319,7 +321,7 @@ impl ShellValidationError {
}
}

fn validate_same_orientation(
fn check_same_orientation(
shell: &Shell,
errors: &mut Vec<ValidationError>,
) {
Expand Down