From 710ee5ead05d44152d2ef780769b44966f21179a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 12 Aug 2024 21:35:31 +0200 Subject: [PATCH 1/7] Match order of struct fields Use the same order where the struct fields are defined and where they are initialized. --- crates/fj-core/src/validation/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/validation/config.rs b/crates/fj-core/src/validation/config.rs index cf295ec4d..7e7064985 100644 --- a/crates/fj-core/src/validation/config.rs +++ b/crates/fj-core/src/validation/config.rs @@ -43,9 +43,9 @@ impl Default for ValidationConfig { fn default() -> Self { Self { panic_on_error: false, - distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm, tolerance: Tolerance::from_scalar(0.001) .expect("Tolerance provided is larger than zero"), + 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 From 7515515a99e3660c00104af7fab258ee195d2e00 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 12 Aug 2024 21:40:53 +0200 Subject: [PATCH 2/7] Add `ValidationConfig::from_tolerance` --- crates/fj-core/src/validation/config.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validation/config.rs b/crates/fj-core/src/validation/config.rs index 7e7064985..ace887ebc 100644 --- a/crates/fj-core/src/validation/config.rs +++ b/crates/fj-core/src/validation/config.rs @@ -39,12 +39,14 @@ pub struct ValidationConfig { pub identical_max_distance: Scalar, } -impl Default for ValidationConfig { - fn default() -> Self { +impl ValidationConfig { + /// Compute validation config from a tolerance value + pub fn from_tolerance(tolerance: impl Into) -> Self { + let tolerance = tolerance.into(); + Self { panic_on_error: false, - tolerance: Tolerance::from_scalar(0.001) - .expect("Tolerance provided is larger than zero"), + tolerance, distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm, // This value was chosen pretty arbitrarily. Seems small enough to @@ -55,3 +57,9 @@ impl Default for ValidationConfig { } } } + +impl Default for ValidationConfig { + fn default() -> Self { + Self::from_tolerance(0.001) + } +} From 86e9ad1ca6078a0b362c4b0a09a840e8a716282d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 12 Aug 2024 21:49:12 +0200 Subject: [PATCH 3/7] Prepare for follow-on change --- crates/fj-core/src/validation/config.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/fj-core/src/validation/config.rs b/crates/fj-core/src/validation/config.rs index ace887ebc..7dbc2f2b8 100644 --- a/crates/fj-core/src/validation/config.rs +++ b/crates/fj-core/src/validation/config.rs @@ -44,16 +44,16 @@ impl ValidationConfig { pub fn from_tolerance(tolerance: impl Into) -> Self { let tolerance = tolerance.into(); + // 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. + let identical_max_distance = Scalar::from_f64(5e-14); + Self { panic_on_error: false, tolerance, 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), + identical_max_distance, } } } From ba2c744cfe1bee043b043438686fd5f83e6293cf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 12 Aug 2024 21:49:42 +0200 Subject: [PATCH 4/7] Move code to improve consistency --- crates/fj-core/src/validation/config.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/validation/config.rs b/crates/fj-core/src/validation/config.rs index 7dbc2f2b8..29d04a045 100644 --- a/crates/fj-core/src/validation/config.rs +++ b/crates/fj-core/src/validation/config.rs @@ -49,10 +49,12 @@ impl ValidationConfig { // positives due to floating-point accuracy issues), we can adjust it. let identical_max_distance = Scalar::from_f64(5e-14); + let distinct_min_distance = Scalar::from_f64(5e-7); // 0.5 µm + Self { panic_on_error: false, tolerance, - distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm, + distinct_min_distance, identical_max_distance, } } From 682fc6b9dc090b365b6b9c44eb84dae18db21814 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 12 Aug 2024 21:52:50 +0200 Subject: [PATCH 5/7] Compute "dinstinct min" based on "identical max" --- crates/fj-core/src/validation/config.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/validation/config.rs b/crates/fj-core/src/validation/config.rs index 29d04a045..a0e17372d 100644 --- a/crates/fj-core/src/validation/config.rs +++ b/crates/fj-core/src/validation/config.rs @@ -49,7 +49,12 @@ impl ValidationConfig { // positives due to floating-point accuracy issues), we can adjust it. let identical_max_distance = Scalar::from_f64(5e-14); - let distinct_min_distance = Scalar::from_f64(5e-7); // 0.5 µm + // This value can't be smaller than `identical_max_distance`. Otherwise + // we can have distinct points that satisfy this constraint, but must be + // considered identical according to the other. + // + // This factor was chosen pretty arbitrarily and might need to be tuned. + let distinct_min_distance = identical_max_distance * 2.; Self { panic_on_error: false, From 8c78e3512a332eb16d4cd50679df3278282b7a3f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 12 Aug 2024 21:53:41 +0200 Subject: [PATCH 6/7] Compute "identical max" based on tolerance --- crates/fj-core/src/validation/config.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validation/config.rs b/crates/fj-core/src/validation/config.rs index a0e17372d..a28cadf1e 100644 --- a/crates/fj-core/src/validation/config.rs +++ b/crates/fj-core/src/validation/config.rs @@ -44,10 +44,9 @@ impl ValidationConfig { pub fn from_tolerance(tolerance: impl Into) -> Self { let tolerance = tolerance.into(); - // 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. - let identical_max_distance = Scalar::from_f64(5e-14); + // This value can't be smaller than the tolerance. If it is, we'll get + // validation errors everywhere, just from numerical noise. + let identical_max_distance = tolerance.inner() * 10.; // This value can't be smaller than `identical_max_distance`. Otherwise // we can have distinct points that satisfy this constraint, but must be From e7e500bd0ea09e6d9a31aa4729f22657951d726f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 12 Aug 2024 21:54:21 +0200 Subject: [PATCH 7/7] Update order of fields based on natural order Now we start with the most basic value, moving to the ones that are computed based on the more basic ones. --- crates/fj-core/src/validation/config.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/validation/config.rs b/crates/fj-core/src/validation/config.rs index a28cadf1e..dc763c24d 100644 --- a/crates/fj-core/src/validation/config.rs +++ b/crates/fj-core/src/validation/config.rs @@ -24,12 +24,6 @@ pub struct ValidationConfig { /// The tolerance value used for intermediate geometry representation pub tolerance: Tolerance, - /// The minimum distance between distinct objects - /// - /// Objects whose distance is less than the value defined in this field, are - /// considered identical. - pub distinct_min_distance: Scalar, - /// The maximum distance between identical objects /// /// Objects that are considered identical might still have a distance @@ -37,6 +31,12 @@ pub struct ValidationConfig { /// that distance is less than the one defined in this field, can not be /// considered identical. pub identical_max_distance: Scalar, + + /// The minimum distance between distinct objects + /// + /// Objects whose distance is less than the value defined in this field, are + /// considered identical. + pub distinct_min_distance: Scalar, } impl ValidationConfig { @@ -58,8 +58,8 @@ impl ValidationConfig { Self { panic_on_error: false, tolerance, - distinct_min_distance, identical_max_distance, + distinct_min_distance, } } }