From c18667539735f195c1626b43f945160fb86034f2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 13:18:46 +0100 Subject: [PATCH 01/11] Merge imports --- crates/fj-kernel/src/validate/shell.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 109c6f452..988acd61c 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -174,15 +174,13 @@ impl ShellValidationError { #[cfg(test)] mod tests { - use crate::assert_contains_err; - use crate::insert::Insert; - - use crate::validate::shell::ShellValidationError; use crate::{ + assert_contains_err, builder::{CycleBuilder, FaceBuilder}, + insert::Insert, objects::Shell, services::Services, - validate::{Validate, ValidationError}, + validate::{shell::ShellValidationError, Validate, ValidationError}, }; #[test] From f212de520548dfbc2b8a7e9ccf163a504b89dd89 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 13:19:50 +0100 Subject: [PATCH 02/11] Fix typo in error message --- crates/fj-kernel/src/validate/shell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 988acd61c..8ab11378e 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -29,7 +29,7 @@ pub enum ShellValidationError { NotWatertight, /// [`Shell`] contains half_edges that are coincident, but refer to different global_edges #[error( - "Shell contains HalfEdges that are coinciendent but refer to different GlobalEdges\n + "Shell contains HalfEdges that are coincident but refer to different GlobalEdges\n Edge 1: {0:#?} Edge 2: {1:#?} " From f711c81f48ec5f4add448822179b5187070032a2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 15:37:20 +0100 Subject: [PATCH 03/11] Improve error messages --- crates/fj-kernel/src/validate/shell.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 8ab11378e..ce17c73c5 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -29,18 +29,17 @@ pub enum ShellValidationError { NotWatertight, /// [`Shell`] contains half_edges that are coincident, but refer to different global_edges #[error( - "Shell contains HalfEdges that are coincident but refer to different GlobalEdges\n - Edge 1: {0:#?} - Edge 2: {1:#?} - " + "`Shell` contains `HalfEdge`s that are coincident but refer to \ + different `GlobalEdge`s\n\ + Edge 1: {0:#?}\n\ + Edge 2: {1:#?}" )] CoincidentEdgesNotIdentical(Handle, Handle), /// [`Shell`] contains half_edges that are identical, but do not coincide #[error( - "Shell contains HalfEdges that are identical but do not coincide\n - Edge 1: {0:#?} - Edge 2: {1:#?} - " + "Shell contains HalfEdges that are identical but do not coincide\n\ + Edge 1: {0:#?}\n\ + Edge 2: {1:#?}" )] IdenticalEdgesNotCoincident(Handle, Handle), } From 8f4f0c51e5e26be7dc5b32cdb931a820f9ac3054 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 15:37:39 +0100 Subject: [PATCH 04/11] Improve formatting --- crates/fj-kernel/src/validate/shell.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index ce17c73c5..922ec7996 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -27,6 +27,7 @@ pub enum ShellValidationError { /// [`Shell`] contains global_edges not referred to by two half_edges #[error("Shell is not watertight")] NotWatertight, + /// [`Shell`] contains half_edges that are coincident, but refer to different global_edges #[error( "`Shell` contains `HalfEdge`s that are coincident but refer to \ @@ -35,6 +36,7 @@ pub enum ShellValidationError { Edge 2: {1:#?}" )] CoincidentEdgesNotIdentical(Handle, Handle), + /// [`Shell`] contains half_edges that are identical, but do not coincide #[error( "Shell contains HalfEdges that are identical but do not coincide\n\ From e9aa257365f8946939f6991e8f9ff19298e2336e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 15:40:00 +0100 Subject: [PATCH 05/11] Remove redundant error information --- crates/fj-kernel/src/validate/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 7d155d3d3..25c119d68 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -98,23 +98,23 @@ impl Default for ValidationConfig { #[derive(Clone, Debug, thiserror::Error)] pub enum ValidationError { /// `Face` validation error - #[error("`Face` validation error:\n{0}")] + #[error("`Face` validation error\n")] Face(#[from] FaceValidationError), /// `HalfEdge` validation error - #[error("`HalfEdge` validation error:\n{0}")] + #[error("`HalfEdge` validation error\n")] HalfEdge(#[from] HalfEdgeValidationError), /// `Cycle` validation error - #[error("`Cycle` validation error:\n{0}")] + #[error("`Cycle` validation error\n")] Cycle(#[from] CycleValidationError), /// `Shell` validation error - #[error("`Shell` validation error:\n{0}")] + #[error("`Shell` validation error\n")] Shell(#[from] ShellValidationError), /// `Solid` validation error - #[error("`Solid` validation error:\n{0}")] + #[error("`Solid` validation error\n")] Solid(#[from] SolidValidationError), } From 159a2473feccdce9192868f718389492724958af Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 15:41:06 +0100 Subject: [PATCH 06/11] Move code to restore alphabetical ordering --- crates/fj-kernel/src/validate/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 25c119d68..40cd15cbe 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -97,6 +97,10 @@ impl Default for ValidationConfig { /// An error that can occur during a validation #[derive(Clone, Debug, thiserror::Error)] pub enum ValidationError { + /// `Cycle` validation error + #[error("`Cycle` validation error\n")] + Cycle(#[from] CycleValidationError), + /// `Face` validation error #[error("`Face` validation error\n")] Face(#[from] FaceValidationError), @@ -105,10 +109,6 @@ pub enum ValidationError { #[error("`HalfEdge` validation error\n")] HalfEdge(#[from] HalfEdgeValidationError), - /// `Cycle` validation error - #[error("`Cycle` validation error\n")] - Cycle(#[from] CycleValidationError), - /// `Shell` validation error #[error("`Shell` validation error\n")] Shell(#[from] ShellValidationError), From 053a42c993ee483c7e6ee0702d3e472064b9d348 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 16:33:05 +0100 Subject: [PATCH 07/11] Refactor code to prepare for follow-on change --- crates/fj-kernel/src/validate/shell.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 922ec7996..5813ca636 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -40,10 +40,16 @@ pub enum ShellValidationError { /// [`Shell`] contains half_edges that are identical, but do not coincide #[error( "Shell contains HalfEdges that are identical but do not coincide\n\ - Edge 1: {0:#?}\n\ - Edge 2: {1:#?}" + Edge 1: {edge_1:#?}\n\ + Edge 2: {edge_2:#?}" )] - IdenticalEdgesNotCoincident(Handle, Handle), + IdenticalEdgesNotCoincident { + /// The first edge + edge_1: Handle, + + /// The second edge + edge_2: Handle, + }, } /// Sample two edges at various (currently 3) points in 3D along them. @@ -121,10 +127,10 @@ impl ShellValidationError { .any(|d| d > config.identical_max_distance) { errors.push( - Self::IdenticalEdgesNotCoincident( - edge.0.clone(), - other_edge.0.clone(), - ) + Self::IdenticalEdgesNotCoincident { + edge_1: edge.0.clone(), + edge_2: other_edge.0.clone(), + } .into(), ) } From 53db5a7b3b895822d19b2959f8399a34603a8d45 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 16:35:34 +0100 Subject: [PATCH 08/11] Make variable name more explicit --- crates/fj-kernel/src/validate/shell.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 5813ca636..c12816474 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -100,21 +100,22 @@ impl ShellValidationError { config: &ValidationConfig, errors: &mut Vec, ) { - let faces: Vec<(Handle, SurfaceGeometry)> = shell - .faces() - .into_iter() - .flat_map(|face| { - face.all_cycles() - .flat_map(|cycle| cycle.half_edges().cloned()) - .zip(repeat(face.surface().geometry())) - }) - .collect(); + let edges_and_surfaces: Vec<(Handle, SurfaceGeometry)> = + shell + .faces() + .into_iter() + .flat_map(|face| { + face.all_cycles() + .flat_map(|cycle| cycle.half_edges().cloned()) + .zip(repeat(face.surface().geometry())) + }) + .collect(); // This is O(N^2) which isn't great, but we can't use a HashMap since we // need to deal with float inaccuracies. Maybe we could use some smarter // data-structure like an octree. - for edge in &faces { - for other_edge in &faces { + for edge in &edges_and_surfaces { + for other_edge in &edges_and_surfaces { let id = edge.0.global_form().id(); let other_id = other_edge.0.global_form().id(); let identical = id == other_id; From 43244ed74ed4048dad533550fcd8552fc3a9fedb Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 16:38:14 +0100 Subject: [PATCH 09/11] Remove redundant type ascription --- crates/fj-kernel/src/validate/shell.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index c12816474..f9acf9f5f 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -100,16 +100,15 @@ impl ShellValidationError { config: &ValidationConfig, errors: &mut Vec, ) { - let edges_and_surfaces: Vec<(Handle, SurfaceGeometry)> = - shell - .faces() - .into_iter() - .flat_map(|face| { - face.all_cycles() - .flat_map(|cycle| cycle.half_edges().cloned()) - .zip(repeat(face.surface().geometry())) - }) - .collect(); + let edges_and_surfaces: Vec<_> = shell + .faces() + .into_iter() + .flat_map(|face| { + face.all_cycles() + .flat_map(|cycle| cycle.half_edges().cloned()) + .zip(repeat(face.surface().geometry())) + }) + .collect(); // This is O(N^2) which isn't great, but we can't use a HashMap since we // need to deal with float inaccuracies. Maybe we could use some smarter From 53bda52fac206e1277314406b2c2000a2b8a1d7c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 16:39:17 +0100 Subject: [PATCH 10/11] Refactor code to prepare for follow-on change --- crates/fj-kernel/src/validate/shell.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index f9acf9f5f..4d6e7f1de 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -4,7 +4,7 @@ use fj_math::{Point, Scalar}; use crate::{ geometry::surface::SurfaceGeometry, - objects::{HalfEdge, Shell}, + objects::{HalfEdge, Shell, Surface}, storage::{Handle, ObjectId}, }; @@ -57,8 +57,8 @@ pub enum ShellValidationError { /// Returns an [`Iterator`] of the distance at each sample. fn distances( config: &ValidationConfig, - (edge1, surface1): (Handle, SurfaceGeometry), - (edge2, surface2): (Handle, SurfaceGeometry), + (edge1, surface1): (Handle, Handle), + (edge2, surface2): (Handle, Handle), ) -> impl Iterator { fn sample( percent: f64, @@ -71,8 +71,8 @@ fn distances( } // Check whether start positions do not match. If they don't treat second edge as flipped - let flip = sample(0.0, (&edge1, surface1)) - .distance_to(&sample(0.0, (&edge2, surface2))) + let flip = sample(0.0, (&edge1, surface1.geometry())) + .distance_to(&sample(0.0, (&edge2, surface2.geometry()))) > config.identical_max_distance; // Three samples (start, middle, end), are enough to detect weather lines @@ -84,10 +84,10 @@ fn distances( let mut distances = Vec::new(); for i in 0..sample_count { let percent = i as f64 * step; - let sample1 = sample(percent, (&edge1, surface1)); + let sample1 = sample(percent, (&edge1, surface1.geometry())); let sample2 = sample( if flip { 1.0 - percent } else { percent }, - (&edge2, surface2), + (&edge2, surface2.geometry()), ); distances.push(sample1.distance_to(&sample2)) } @@ -106,7 +106,7 @@ impl ShellValidationError { .flat_map(|face| { face.all_cycles() .flat_map(|cycle| cycle.half_edges().cloned()) - .zip(repeat(face.surface().geometry())) + .zip(repeat(face.surface().clone())) }) .collect(); From 98986128c8c1c7524dc12868eb73589ba3dd4ea5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 16:40:51 +0100 Subject: [PATCH 11/11] Add more information to error message --- crates/fj-kernel/src/validate/shell.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 4d6e7f1de..46a9e186e 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -41,14 +41,22 @@ pub enum ShellValidationError { #[error( "Shell contains HalfEdges that are identical but do not coincide\n\ Edge 1: {edge_1:#?}\n\ - Edge 2: {edge_2:#?}" + Surface for edge 1: {surface_1:#?}\n\ + Edge 2: {edge_2:#?}\n\ + Surface for edge 2: {surface_2:#?}" )] IdenticalEdgesNotCoincident { /// The first edge edge_1: Handle, + /// The surface that the first edge is on + surface_1: Handle, + /// The second edge edge_2: Handle, + + /// The surface that the second edge is on + surface_2: Handle, }, } @@ -129,7 +137,9 @@ impl ShellValidationError { errors.push( Self::IdenticalEdgesNotCoincident { edge_1: edge.0.clone(), + surface_1: edge.1.clone(), edge_2: other_edge.0.clone(), + surface_2: other_edge.1.clone(), } .into(), )