From 08aa1f69ed7cabeabb7eabb854146099e36d756c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:18:52 +0200 Subject: [PATCH 1/9] Rename argument to make it more specific --- .../checks/coincident_half_edges_are_not_siblings.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index 86a59fc8a..92eee47ce 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -169,7 +169,7 @@ impl ValidationCheck for CoincidentHalfEdgesAreNotSiblings { /// /// Returns an [`Iterator`] of the distance at each sample. fn distances( - edge_a: Handle, + half_edge_a: Handle, surface_a: &SurfaceGeom, edge_b: Handle, surface_b: &SurfaceGeom, @@ -198,7 +198,7 @@ fn distances( let mut distances = Vec::new(); for i in 0..sample_count { let percent = i as f64 * step; - let sample1 = sample(percent, (&edge_a, surface_a), geometry); + let sample1 = sample(percent, (&half_edge_a, surface_a), geometry); let sample2 = sample(1.0 - percent, (&edge_b, surface_b), geometry); distances.push(sample1.distance_to(&sample2)) } From 3825599792d950296d69f42b62fad3e3b8f5b466 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:19:15 +0200 Subject: [PATCH 2/9] Rename argument to make it more specific --- .../checks/coincident_half_edges_are_not_siblings.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index 92eee47ce..8980393b6 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -171,7 +171,7 @@ impl ValidationCheck for CoincidentHalfEdgesAreNotSiblings { fn distances( half_edge_a: Handle, surface_a: &SurfaceGeom, - edge_b: Handle, + half_edge_b: Handle, surface_b: &SurfaceGeom, geometry: &Geometry, ) -> impl Iterator { @@ -199,7 +199,8 @@ fn distances( for i in 0..sample_count { let percent = i as f64 * step; let sample1 = sample(percent, (&half_edge_a, surface_a), geometry); - let sample2 = sample(1.0 - percent, (&edge_b, surface_b), geometry); + let sample2 = + sample(1.0 - percent, (&half_edge_b, surface_b), geometry); distances.push(sample1.distance_to(&sample2)) } distances.into_iter() From c9bfce1ca926115b9c3612bdb920a4728a93a72f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:19:31 +0200 Subject: [PATCH 3/9] Rename argument to make it more specific --- .../checks/coincident_half_edges_are_not_siblings.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index 8980393b6..bf111458c 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -177,13 +177,13 @@ fn distances( ) -> impl Iterator { fn sample( percent: f64, - (edge, surface): (&Handle, &SurfaceGeom), + (half_edge, surface): (&Handle, &SurfaceGeom), geometry: &Geometry, ) -> Point<3> { - let [start, end] = geometry.of_half_edge(edge).boundary.inner; + let [start, end] = geometry.of_half_edge(half_edge).boundary.inner; let path_coords = start + (end - start) * percent; let surface_coords = geometry - .of_half_edge(edge) + .of_half_edge(half_edge) .path .point_from_path_coords(path_coords); surface.point_from_surface_coords(surface_coords) From 2043b40c9ebdc4131e80b250f86a500834e35d64 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:21:03 +0200 Subject: [PATCH 4/9] Prepare for follow-on change --- .../checks/coincident_half_edges_are_not_siblings.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index bf111458c..49d05aa15 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -8,7 +8,7 @@ use crate::{ AllHalfEdgesWithSurface, BoundingVerticesOfHalfEdge, SiblingOfHalfEdge, }, storage::Handle, - topology::{Curve, HalfEdge, Shell, Vertex}, + topology::{Curve, HalfEdge, Shell, Surface, Vertex}, validation::ValidationCheck, }; @@ -128,7 +128,7 @@ impl ValidationCheck for CoincidentHalfEdgesAreNotSiblings { // `distinct_min_distance`, that's a problem. if distances( half_edge_a.clone(), - geometry.of_surface(surface_a), + surface_a, half_edge_b.clone(), geometry.of_surface(surface_b), geometry, @@ -170,7 +170,7 @@ impl ValidationCheck for CoincidentHalfEdgesAreNotSiblings { /// Returns an [`Iterator`] of the distance at each sample. fn distances( half_edge_a: Handle, - surface_a: &SurfaceGeom, + surface_a: &Handle, half_edge_b: Handle, surface_b: &SurfaceGeom, geometry: &Geometry, @@ -198,7 +198,11 @@ fn distances( let mut distances = Vec::new(); for i in 0..sample_count { let percent = i as f64 * step; - let sample1 = sample(percent, (&half_edge_a, surface_a), geometry); + let sample1 = sample( + percent, + (&half_edge_a, geometry.of_surface(surface_a)), + geometry, + ); let sample2 = sample(1.0 - percent, (&half_edge_b, surface_b), geometry); distances.push(sample1.distance_to(&sample2)) From 536187536a86ab09b72a019c4b31d8792efc19fc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:21:46 +0200 Subject: [PATCH 5/9] Prepare for follow-on change --- .../checks/coincident_half_edges_are_not_siblings.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index 49d05aa15..ba1b560dd 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -130,7 +130,7 @@ impl ValidationCheck for CoincidentHalfEdgesAreNotSiblings { half_edge_a.clone(), surface_a, half_edge_b.clone(), - geometry.of_surface(surface_b), + surface_b, geometry, ) .all(|d| d < config.distinct_min_distance) @@ -172,7 +172,7 @@ fn distances( half_edge_a: Handle, surface_a: &Handle, half_edge_b: Handle, - surface_b: &SurfaceGeom, + surface_b: &Handle, geometry: &Geometry, ) -> impl Iterator { fn sample( @@ -203,8 +203,11 @@ fn distances( (&half_edge_a, geometry.of_surface(surface_a)), geometry, ); - let sample2 = - sample(1.0 - percent, (&half_edge_b, surface_b), geometry); + let sample2 = sample( + 1.0 - percent, + (&half_edge_b, geometry.of_surface(surface_b)), + geometry, + ); distances.push(sample1.distance_to(&sample2)) } distances.into_iter() From 9e85c9862787ea4c44c3425672e4e6d33963448a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:22:49 +0200 Subject: [PATCH 6/9] Simplify function signature --- .../checks/coincident_half_edges_are_not_siblings.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index ba1b560dd..6d90fa34d 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -177,7 +177,8 @@ fn distances( ) -> impl Iterator { fn sample( percent: f64, - (half_edge, surface): (&Handle, &SurfaceGeom), + half_edge: &Handle, + surface: &SurfaceGeom, geometry: &Geometry, ) -> Point<3> { let [start, end] = geometry.of_half_edge(half_edge).boundary.inner; @@ -200,12 +201,14 @@ fn distances( let percent = i as f64 * step; let sample1 = sample( percent, - (&half_edge_a, geometry.of_surface(surface_a)), + &half_edge_a, + geometry.of_surface(surface_a), geometry, ); let sample2 = sample( 1.0 - percent, - (&half_edge_b, geometry.of_surface(surface_b)), + &half_edge_b, + geometry.of_surface(surface_b), geometry, ); distances.push(sample1.distance_to(&sample2)) From 845b0578f8f6726ca9b7b2f6e2a0f4376876e7d6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:23:33 +0200 Subject: [PATCH 7/9] Consolidate redundant calls --- .../coincident_half_edges_are_not_siblings.rs | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index 6d90fa34d..3b0e9c978 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -3,7 +3,7 @@ use std::fmt; use fj_math::{Point, Scalar}; use crate::{ - geometry::{CurveBoundary, Geometry, SurfaceGeom}, + geometry::{CurveBoundary, Geometry}, queries::{ AllHalfEdgesWithSurface, BoundingVerticesOfHalfEdge, SiblingOfHalfEdge, }, @@ -178,7 +178,7 @@ fn distances( fn sample( percent: f64, half_edge: &Handle, - surface: &SurfaceGeom, + surface: &Handle, geometry: &Geometry, ) -> Point<3> { let [start, end] = geometry.of_half_edge(half_edge).boundary.inner; @@ -187,7 +187,9 @@ fn distances( .of_half_edge(half_edge) .path .point_from_path_coords(path_coords); - surface.point_from_surface_coords(surface_coords) + geometry + .of_surface(surface) + .point_from_surface_coords(surface_coords) } // Three samples (start, middle, end), are enough to detect weather lines @@ -199,18 +201,8 @@ fn distances( let mut distances = Vec::new(); for i in 0..sample_count { let percent = i as f64 * step; - let sample1 = sample( - percent, - &half_edge_a, - geometry.of_surface(surface_a), - geometry, - ); - let sample2 = sample( - 1.0 - percent, - &half_edge_b, - geometry.of_surface(surface_b), - geometry, - ); + let sample1 = sample(percent, &half_edge_a, surface_a, geometry); + let sample2 = sample(1.0 - percent, &half_edge_b, surface_b, geometry); distances.push(sample1.distance_to(&sample2)) } distances.into_iter() From 30e5c0ba7249322a461d282851b5aa2d4ac8739b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:27:09 +0200 Subject: [PATCH 8/9] Prepare for follow-on change --- .../checks/coincident_half_edges_are_not_siblings.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index 3b0e9c978..1a81d4794 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -183,10 +183,8 @@ fn distances( ) -> Point<3> { let [start, end] = geometry.of_half_edge(half_edge).boundary.inner; let path_coords = start + (end - start) * percent; - let surface_coords = geometry - .of_half_edge(half_edge) - .path - .point_from_path_coords(path_coords); + let path = geometry.of_half_edge(half_edge).path; + let surface_coords = path.point_from_path_coords(path_coords); geometry .of_surface(surface) .point_from_surface_coords(surface_coords) From d43a206d8c1c3e288e92563db964109bf55f5c45 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:30:08 +0200 Subject: [PATCH 9/9] Remove redundant check in test --- .../src/validation/checks/multiple_references.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/crates/fj-core/src/validation/checks/multiple_references.rs b/crates/fj-core/src/validation/checks/multiple_references.rs index 0a6365ec2..4c82774e8 100644 --- a/crates/fj-core/src/validation/checks/multiple_references.rs +++ b/crates/fj-core/src/validation/checks/multiple_references.rs @@ -179,7 +179,6 @@ impl ReferenceCounter { #[cfg(test)] mod tests { use crate::{ - assert_contains_err, operations::{ build::{BuildShell, BuildSketch, BuildSolid}, update::{ @@ -188,11 +187,7 @@ mod tests { }, }, topology::{Cycle, Face, HalfEdge, Region, Shell, Sketch, Solid}, - validate::Validate, - validation::{ - checks::MultipleReferencesToObject, ValidationCheck, - ValidationError, - }, + validation::{checks::MultipleReferencesToObject, ValidationCheck}, Core, }; @@ -412,12 +407,6 @@ mod tests { &core.layers.geometry, ).is_err()); - assert_contains_err!( - core, - invalid, - ValidationError::MultipleReferencesToCycle(_) - ); - // Ignore remaining validation errors. let _ = core.layers.validation.take_errors();