From f5699f6164a487b45ad74e7c7360672af6966197 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Jan 2023 17:20:25 +0100 Subject: [PATCH 1/5] Improve error message --- crates/fj-kernel/src/validate/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 91b5d6001..cacac64d0 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -95,7 +95,7 @@ pub enum HalfEdgeValidationError { /// [`HalfEdge`]'s vertices are coincident #[error( - "Vertices on curve are coincident\n\ + "Vertices of `HalfEdge` on curve are coincident\n\ - Position of back vertex: {back_position:?}\n\ - Position of front vertex: {front_position:?}" )] From fec78d128b9fc36c96b58ae56e182f94bca389ad Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Jan 2023 17:21:32 +0100 Subject: [PATCH 2/5] Add more information to `HalfEdgeValidationError` --- crates/fj-kernel/src/validate/edge.rs | 28 +++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index cacac64d0..182269566 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -51,7 +51,8 @@ pub enum HalfEdgeValidationError { #[error( "`HalfEdge` vertices are not defined on the same `Curve`\n\ - `Curve` of back vertex: {back_curve:#?}\n\ - - `Curve` of front vertex: {front_curve:#?}" + - `Curve` of front vertex: {front_curve:#?}\n\ + - `HalfEdge`: {half_edge:#?}" )] CurveMismatch { /// The curve of the [`HalfEdge`]'s back vertex @@ -59,6 +60,9 @@ pub enum HalfEdgeValidationError { /// The curve of the [`HalfEdge`]'s front vertex front_curve: Handle, + + /// The half-edge + half_edge: HalfEdge, }, /// [`HalfEdge`]'s [`GlobalCurve`]s do not match @@ -66,7 +70,8 @@ pub enum HalfEdgeValidationError { "Global form of `HalfEdge`'s `Curve` does not match `GlobalCurve` of \n\ the `HalfEdge`'s `GlobalEdge`\n\ - `GlobalCurve` from `Curve`: {global_curve_from_curve:#?}\n\ - - `GlobalCurve` from `GlobalEdge`: {global_curve_from_global_form:#?}", + - `GlobalCurve` from `GlobalEdge`: {global_curve_from_global_form:#?}\n\ + - `HalfEdge`: {half_edge:#?}", )] GlobalCurveMismatch { /// The [`GlobalCurve`] from the [`HalfEdge`]'s [`Curve`] @@ -74,6 +79,9 @@ pub enum HalfEdgeValidationError { /// The [`GlobalCurve`] from the [`HalfEdge`]'s global form global_curve_from_global_form: Handle, + + /// The half-edge + half_edge: HalfEdge, }, /// [`HalfEdge`]'s [`GlobalVertex`] objects do not match @@ -83,7 +91,8 @@ pub enum HalfEdgeValidationError { - `GlobalVertex` objects from `Vertex` objects: \ {global_vertices_from_vertices:#?}\n\ - `GlobalVertex` objects from `GlobalEdge`: \ - {global_vertices_from_global_form:#?}" + {global_vertices_from_global_form:#?}\n\ + - `HalfEdge`: {half_edge:#?}" )] GlobalVertexMismatch { /// The [`GlobalVertex`] from the [`HalfEdge`]'s vertices @@ -91,13 +100,17 @@ pub enum HalfEdgeValidationError { /// The [`GlobalCurve`] from the [`HalfEdge`]'s global form global_vertices_from_global_form: [Handle; 2], + + /// The half-edge + half_edge: HalfEdge, }, /// [`HalfEdge`]'s vertices are coincident #[error( "Vertices of `HalfEdge` on curve are coincident\n\ - Position of back vertex: {back_position:?}\n\ - - Position of front vertex: {front_position:?}" + - Position of front vertex: {front_position:?}\n\ + - `HalfEdge`: {half_edge:#?}" )] VerticesAreCoincident { /// The position of the back vertex @@ -108,6 +121,9 @@ pub enum HalfEdgeValidationError { /// The distance between the two vertices distance: Scalar, + + /// The half-edge + half_edge: HalfEdge, }, } @@ -120,6 +136,7 @@ impl HalfEdgeValidationError { return Err(Self::CurveMismatch { back_curve: back_curve.clone(), front_curve: front_curve.clone(), + half_edge: half_edge.clone(), }); } @@ -135,6 +152,7 @@ impl HalfEdgeValidationError { global_curve_from_curve: global_curve_from_curve.clone(), global_curve_from_global_form: global_curve_from_global_form .clone(), + half_edge: half_edge.clone(), }); } @@ -168,6 +186,7 @@ impl HalfEdgeValidationError { return Err(Self::GlobalVertexMismatch { global_vertices_from_vertices, global_vertices_from_global_form, + half_edge: half_edge.clone(), }); } @@ -188,6 +207,7 @@ impl HalfEdgeValidationError { back_position, front_position, distance, + half_edge: half_edge.clone(), }); } From 6cc572a7bb8847b96876377a9900022f5e6a040c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Jan 2023 17:23:11 +0100 Subject: [PATCH 3/5] Improve output of validation tests --- crates/fj-kernel/src/validate/cycle.rs | 6 ++++-- crates/fj-kernel/src/validate/edge.rs | 24 ++++++++++++++++-------- crates/fj-kernel/src/validate/face.rs | 12 ++++++++---- crates/fj-kernel/src/validate/vertex.rs | 18 ++++++++++++------ 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 1fd6b2fb8..6b5cbd1fd 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -74,7 +74,7 @@ mod tests { }; #[test] - fn cycle_half_edge_connections() { + fn cycle_half_edge_connections() -> anyhow::Result<()> { let mut services = Services::new(); let valid = { @@ -109,7 +109,9 @@ mod tests { Cycle::new(half_edges) }; - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } } diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 182269566..474522283 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -230,7 +230,7 @@ mod tests { }; #[test] - fn half_edge_curve_mismatch() { + fn half_edge_curve_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); let valid = { @@ -254,12 +254,14 @@ mod tests { HalfEdge::new(vertices, valid.global_form().clone()) }; - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } #[test] - fn half_edge_global_curve_mismatch() { + fn half_edge_global_curve_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); let valid = { @@ -278,12 +280,14 @@ mod tests { tmp.build(&mut services.objects) }); - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } #[test] - fn half_edge_global_vertex_mismatch() { + fn half_edge_global_vertex_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); let valid = { @@ -308,12 +312,14 @@ mod tests { tmp.build(&mut services.objects) }); - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } #[test] - fn half_edge_vertices_are_coincident() { + fn half_edge_vertices_are_coincident() -> anyhow::Result<()> { let mut services = Services::new(); let valid = { @@ -343,7 +349,9 @@ mod tests { valid.global_form().clone(), ); - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } } diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 0effcee19..d94bd4325 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -114,7 +114,7 @@ mod tests { }; #[test] - fn face_surface_mismatch() { + fn face_surface_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); let surface = services.objects.surfaces.xy_plane(); @@ -149,12 +149,14 @@ mod tests { Face::new(valid.exterior().clone(), interiors, valid.color()) }; - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } #[test] - fn face_invalid_interior_winding() { + fn face_invalid_interior_winding() -> anyhow::Result<()> { let mut services = Services::new(); let surface = services.objects.surfaces.xy_plane(); @@ -184,7 +186,9 @@ mod tests { Face::new(valid.exterior().clone(), interiors, valid.color()) }; - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } } diff --git a/crates/fj-kernel/src/validate/vertex.rs b/crates/fj-kernel/src/validate/vertex.rs index f8a18caaa..6fa016363 100644 --- a/crates/fj-kernel/src/validate/vertex.rs +++ b/crates/fj-kernel/src/validate/vertex.rs @@ -192,7 +192,7 @@ mod tests { }; #[test] - fn vertex_surface_mismatch() { + fn vertex_surface_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); let surface = Partial::from(services.objects.surfaces.xy_plane()); @@ -220,12 +220,14 @@ mod tests { Vertex::new(valid.position(), valid.curve().clone(), surface_form) }; - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } #[test] - fn vertex_position_mismatch() { + fn vertex_position_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); let valid = { @@ -255,12 +257,14 @@ mod tests { Vertex::new(valid.position(), valid.curve().clone(), surface_form) }; - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } #[test] - fn surface_vertex_position_mismatch() { + fn surface_vertex_position_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); let valid = PartialSurfaceVertex { @@ -275,7 +279,9 @@ mod tests { GlobalVertex::new([1., 0., 0.]).insert(&mut services.objects), ); - assert!(valid.validate().is_ok()); + valid.validate()?; assert!(invalid.validate().is_err()); + + Ok(()) } } From b5e895ef8c38c63f475b4ade01013670d776ed0e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Jan 2023 17:27:33 +0100 Subject: [PATCH 4/5] Remove unnecessary `#[allow(...)]`s in `fj-kernel` --- crates/fj-kernel/src/algorithms/intersect/face_point.rs | 1 - crates/fj-kernel/src/algorithms/intersect/ray_face.rs | 1 - crates/fj-kernel/src/validate/mod.rs | 1 - 3 files changed, 3 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/face_point.rs b/crates/fj-kernel/src/algorithms/intersect/face_point.rs index c7192eb2e..465f260b2 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -116,7 +116,6 @@ impl Intersect for (&Handle, &Point<2>) { } /// The intersection between a face and a point -#[allow(clippy::large_enum_variant)] #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub enum FacePointIntersection { /// The point is inside of the face diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index 3391e7a18..21cb23157 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -125,7 +125,6 @@ impl Intersect for (&HorizontalRayToTheRight<3>, &Handle) { /// A hit between a ray and a face #[derive(Clone, Debug, Eq, PartialEq)] -#[allow(clippy::large_enum_variant)] pub enum RayFaceIntersection { /// The ray hits the face itself RayHitsFace, diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index f3118e29d..4cb607f72 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -73,7 +73,6 @@ impl Default for ValidationConfig { } /// An error that can occur during a validation -#[allow(clippy::large_enum_variant)] #[derive(Clone, Debug, thiserror::Error)] pub enum ValidationError { /// `Cycle` validation error From 70f99b8f35661519e049a4122a0bbeabfc16a388 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 6 Jan 2023 17:38:25 +0100 Subject: [PATCH 5/5] Improve formatting --- crates/fj-kernel/src/partial/objects/edge.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 4cf3f5eb8..7aaeacf5b 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -82,7 +82,6 @@ impl PartialObject for PartialHalfEdge { impl Default for PartialHalfEdge { fn default() -> Self { let curve = Partial::::new(); - let vertices = array::from_fn(|_| { Partial::from_partial(PartialVertex { curve: curve.clone(),