From a158f244edb854b79eae87c22beaa3512005415a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 27 Mar 2024 12:06:59 +0100 Subject: [PATCH 1/4] Don't implicitly copy `HalfEdgeGeometry` --- crates/fj-core/src/algorithms/approx/edge.rs | 13 +++---------- crates/fj-core/src/geometry/geometry.rs | 3 +-- crates/fj-core/src/operations/build/half_edge.rs | 2 +- crates/fj-core/src/operations/build/shell.rs | 3 ++- crates/fj-core/src/operations/holes.rs | 6 +++--- crates/fj-core/src/operations/join/cycle.rs | 6 ++++-- crates/fj-core/src/operations/reverse/cycle.rs | 2 +- crates/fj-core/src/operations/reverse/edge.rs | 2 +- crates/fj-core/src/operations/split/edge.rs | 2 +- crates/fj-core/src/operations/split/face.rs | 2 +- crates/fj-core/src/operations/split/half_edge.rs | 2 +- crates/fj-core/src/operations/sweep/cycle.rs | 2 +- crates/fj-core/src/operations/sweep/half_edge.rs | 4 ++-- crates/fj-core/src/operations/transform/edge.rs | 2 +- crates/fj-core/src/validate/shell.rs | 5 +++-- 15 files changed, 26 insertions(+), 30 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 5ccdc130f..6cff4cecb 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -48,16 +48,9 @@ impl Approx for (&Handle, &Handle) { let first = ApproxPoint::new(start_position_surface, start_position); let rest = { - let approx = ( - half_edge.curve(), - &geometry.of_half_edge(half_edge), - surface, - ) - .approx_with_cache( - tolerance, - &mut cache.curve, - geometry, - ); + let approx = + (half_edge.curve(), geometry.of_half_edge(half_edge), surface) + .approx_with_cache(tolerance, &mut cache.curve, geometry); approx.points.into_iter().map(|point| { let point_surface = geometry diff --git a/crates/fj-core/src/geometry/geometry.rs b/crates/fj-core/src/geometry/geometry.rs index 914ab79db..6d5f1a694 100644 --- a/crates/fj-core/src/geometry/geometry.rs +++ b/crates/fj-core/src/geometry/geometry.rs @@ -80,10 +80,9 @@ impl Geometry { pub fn of_half_edge( &self, half_edge: &Handle, - ) -> HalfEdgeGeometry { + ) -> &HalfEdgeGeometry { self.half_edge .get(half_edge) - .copied() .expect("Expected geometry of half-edge to be defined") } diff --git a/crates/fj-core/src/operations/build/half_edge.rs b/crates/fj-core/src/operations/build/half_edge.rs index 4c9722b5a..2d853d144 100644 --- a/crates/fj-core/src/operations/build/half_edge.rs +++ b/crates/fj-core/src/operations/build/half_edge.rs @@ -29,7 +29,7 @@ pub trait BuildHalfEdge { start_vertex: Handle, core: &mut Core, ) -> Handle { - let mut geometry = core.layers.geometry.of_half_edge(sibling); + let mut geometry = *core.layers.geometry.of_half_edge(sibling); geometry.boundary = geometry.boundary.reverse(); HalfEdge::new(sibling.curve().clone(), start_vertex) diff --git a/crates/fj-core/src/operations/build/shell.rs b/crates/fj-core/src/operations/build/shell.rs index 54c57a4ac..6f9e31ea6 100644 --- a/crates/fj-core/src/operations/build/shell.rs +++ b/crates/fj-core/src/operations/build/shell.rs @@ -102,7 +102,8 @@ pub trait BuildShell { .update_curve(|_, _| curve, core) .insert(core) .set_geometry( - core.layers + *core + .layers .geometry .of_half_edge(&half_edge), &mut core.layers.geometry, diff --git a/crates/fj-core/src/operations/holes.rs b/crates/fj-core/src/operations/holes.rs index d479ddc12..1e7b6f64a 100644 --- a/crates/fj-core/src/operations/holes.rs +++ b/crates/fj-core/src/operations/holes.rs @@ -68,7 +68,7 @@ impl AddHole for Shell { [Cycle::empty().add_joined_edges( [( entry.clone(), - core.layers.geometry.of_half_edge(&entry), + *core.layers.geometry.of_half_edge(&entry), )], core, )], @@ -138,7 +138,7 @@ impl AddHole for Shell { [Cycle::empty().add_joined_edges( [( entry.clone(), - core.layers.geometry.of_half_edge(&entry), + *core.layers.geometry.of_half_edge(&entry), )], core, )], @@ -159,7 +159,7 @@ impl AddHole for Shell { [Cycle::empty().add_joined_edges( [( exit.clone(), - core.layers.geometry.of_half_edge(exit), + *core.layers.geometry.of_half_edge(exit), )], core, )], diff --git a/crates/fj-core/src/operations/join/cycle.rs b/crates/fj-core/src/operations/join/cycle.rs index f745474e2..df0fda151 100644 --- a/crates/fj-core/src/operations/join/cycle.rs +++ b/crates/fj-core/src/operations/join/cycle.rs @@ -137,7 +137,8 @@ impl JoinCycle for Cycle { ) .insert(core) .set_geometry( - core.layers + *core + .layers .geometry .of_half_edge(half_edge), &mut core.layers.geometry, @@ -155,7 +156,8 @@ impl JoinCycle for Cycle { ) .insert(core) .set_geometry( - core.layers + *core + .layers .geometry .of_half_edge(half_edge), &mut core.layers.geometry, diff --git a/crates/fj-core/src/operations/reverse/cycle.rs b/crates/fj-core/src/operations/reverse/cycle.rs index 420dc0528..4f764593d 100644 --- a/crates/fj-core/src/operations/reverse/cycle.rs +++ b/crates/fj-core/src/operations/reverse/cycle.rs @@ -14,7 +14,7 @@ impl Reverse for Cycle { .half_edges() .pairs() .map(|(current, next)| { - let mut geometry = core.layers.geometry.of_half_edge(current); + let mut geometry = *core.layers.geometry.of_half_edge(current); geometry.boundary = geometry.boundary.reverse(); HalfEdge::new( diff --git a/crates/fj-core/src/operations/reverse/edge.rs b/crates/fj-core/src/operations/reverse/edge.rs index 5b4226cd5..d0ef5fab8 100644 --- a/crates/fj-core/src/operations/reverse/edge.rs +++ b/crates/fj-core/src/operations/reverse/edge.rs @@ -9,7 +9,7 @@ use super::ReverseCurveCoordinateSystems; impl ReverseCurveCoordinateSystems for Handle { fn reverse_curve_coordinate_systems(&self, core: &mut Core) -> Self { - let mut geometry = core.layers.geometry.of_half_edge(self); + let mut geometry = *core.layers.geometry.of_half_edge(self); geometry.path = geometry.path.reverse(); geometry.boundary = geometry.boundary.reverse(); diff --git a/crates/fj-core/src/operations/split/edge.rs b/crates/fj-core/src/operations/split/edge.rs index 9aa42f995..5551f6ed8 100644 --- a/crates/fj-core/src/operations/split/edge.rs +++ b/crates/fj-core/src/operations/split/edge.rs @@ -51,7 +51,7 @@ impl SplitEdge for Shell { ) .insert(core) .set_geometry( - core.layers.geometry.of_half_edge(&sibling_b), + *core.layers.geometry.of_half_edge(&sibling_b), &mut core.layers.geometry, ); diff --git a/crates/fj-core/src/operations/split/face.rs b/crates/fj-core/src/operations/split/face.rs index d46fe570e..9f2b15e23 100644 --- a/crates/fj-core/src/operations/split/face.rs +++ b/crates/fj-core/src/operations/split/face.rs @@ -115,7 +115,7 @@ impl SplitFace for Shell { .update_start_vertex(|_, _| b.start_vertex().clone(), core) .insert(core) .set_geometry( - core.layers.geometry.of_half_edge(&half_edge), + *core.layers.geometry.of_half_edge(&half_edge), &mut core.layers.geometry, ) }; diff --git a/crates/fj-core/src/operations/split/half_edge.rs b/crates/fj-core/src/operations/split/half_edge.rs index d1dd5caaf..11f9aaa45 100644 --- a/crates/fj-core/src/operations/split/half_edge.rs +++ b/crates/fj-core/src/operations/split/half_edge.rs @@ -42,7 +42,7 @@ impl SplitHalfEdge for Handle { ) -> [Handle; 2] { let point = point.into(); - let geometry = core.layers.geometry.of_half_edge(self); + let geometry = *core.layers.geometry.of_half_edge(self); let [start, end] = geometry.boundary.inner; let a = diff --git a/crates/fj-core/src/operations/sweep/cycle.rs b/crates/fj-core/src/operations/sweep/cycle.rs index 30fedb345..0b7b1ffef 100644 --- a/crates/fj-core/src/operations/sweep/cycle.rs +++ b/crates/fj-core/src/operations/sweep/cycle.rs @@ -77,7 +77,7 @@ impl SweepCycle for Cycle { top_edges.push(( top_edge, - core.layers.geometry.of_half_edge(bottom_half_edge), + *core.layers.geometry.of_half_edge(bottom_half_edge), )); } diff --git a/crates/fj-core/src/operations/sweep/half_edge.rs b/crates/fj-core/src/operations/sweep/half_edge.rs index 49f657c54..27e85b24a 100644 --- a/crates/fj-core/src/operations/sweep/half_edge.rs +++ b/crates/fj-core/src/operations/sweep/half_edge.rs @@ -58,7 +58,7 @@ impl SweepHalfEdge for Handle { ) -> (Face, Handle) { let path = path.into(); - let geometry = core.layers.geometry.of_half_edge(self); + let geometry = *core.layers.geometry.of_half_edge(self); let surface = geometry.path.sweep_surface_path( &core.layers.geometry.of_surface(&surface), path, @@ -134,7 +134,7 @@ impl SweepHalfEdge for Handle { }; half_edge.insert(core).set_geometry( - core.layers.geometry.of_half_edge(&line_segment), + *core.layers.geometry.of_half_edge(&line_segment), &mut core.layers.geometry, ) }; diff --git a/crates/fj-core/src/operations/transform/edge.rs b/crates/fj-core/src/operations/transform/edge.rs index cdbbe8a28..ba3fa4370 100644 --- a/crates/fj-core/src/operations/transform/edge.rs +++ b/crates/fj-core/src/operations/transform/edge.rs @@ -26,7 +26,7 @@ impl TransformObject for Handle { core.layers.geometry.define_half_edge( half_edge.clone(), - core.layers.geometry.of_half_edge(self), + *core.layers.geometry.of_half_edge(self), ); half_edge diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 7b181d4b5..3b02e429b 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -453,7 +453,7 @@ mod tests { cycle.update_half_edge( cycle.half_edges().nth_circular(0), |half_edge, core| { - let mut geometry = core + let mut geometry = *core .layers .geometry .of_half_edge(half_edge); @@ -546,7 +546,8 @@ mod tests { ) .insert(core) .set_geometry( - core.layers + *core + .layers .geometry .of_half_edge(half_edge), &mut core.layers.geometry, From 1a35324b5c3f392615355f9008863202088768b3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 27 Mar 2024 12:08:37 +0100 Subject: [PATCH 2/4] Make variable name more explicit --- crates/fj-core/src/operations/sweep/half_edge.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/half_edge.rs b/crates/fj-core/src/operations/sweep/half_edge.rs index 27e85b24a..2120ae47e 100644 --- a/crates/fj-core/src/operations/sweep/half_edge.rs +++ b/crates/fj-core/src/operations/sweep/half_edge.rs @@ -58,8 +58,8 @@ impl SweepHalfEdge for Handle { ) -> (Face, Handle) { let path = path.into(); - let geometry = *core.layers.geometry.of_half_edge(self); - let surface = geometry.path.sweep_surface_path( + let half_edge_geom = *core.layers.geometry.of_half_edge(self); + let surface = half_edge_geom.path.sweep_surface_path( &core.layers.geometry.of_surface(&surface), path, core, @@ -85,7 +85,7 @@ impl SweepHalfEdge for Handle { // Let's figure out the surface coordinates of the edge vertices. let surface_points = { - let [a, b] = geometry.boundary.inner; + let [a, b] = half_edge_geom.boundary.inner; [ [a.t, Scalar::ZERO], @@ -103,7 +103,7 @@ impl SweepHalfEdge for Handle { // Now, the boundaries of each edge. let boundaries = { - let [a, b] = geometry.boundary.inner; + let [a, b] = half_edge_geom.boundary.inner; let [c, d] = [0., 1.].map(|coord| Point::from([coord])); [[a, b], [c, d], [b, a], [d, c]] From 0fde6449da8331ddfd9457c14f623b98758db92e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 27 Mar 2024 12:09:40 +0100 Subject: [PATCH 3/4] Refactor to prepare for follow-on change --- crates/fj-core/src/operations/sweep/half_edge.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/half_edge.rs b/crates/fj-core/src/operations/sweep/half_edge.rs index 2120ae47e..3c44d0399 100644 --- a/crates/fj-core/src/operations/sweep/half_edge.rs +++ b/crates/fj-core/src/operations/sweep/half_edge.rs @@ -59,11 +59,11 @@ impl SweepHalfEdge for Handle { let path = path.into(); let half_edge_geom = *core.layers.geometry.of_half_edge(self); - let surface = half_edge_geom.path.sweep_surface_path( - &core.layers.geometry.of_surface(&surface), - path, - core, - ); + let surface_geom = core.layers.geometry.of_surface(&surface); + let surface = + half_edge_geom + .path + .sweep_surface_path(&surface_geom, path, core); // Next, we need to define the boundaries of the face. Let's start with // the global vertices and edges. From 54b7332eba2e2d906cd71eff10a295da0480e2d9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 27 Mar 2024 12:10:29 +0100 Subject: [PATCH 4/4] Don't implicitly copy `SurfaceGeometry` --- crates/fj-core/src/algorithms/approx/curve.rs | 2 +- crates/fj-core/src/geometry/geometry.rs | 9 ++++----- crates/fj-core/src/operations/sweep/half_edge.rs | 2 +- crates/fj-core/src/validate/shell.rs | 12 ++++++------ 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve.rs b/crates/fj-core/src/algorithms/approx/curve.rs index 8cb0106a0..1585d6492 100644 --- a/crates/fj-core/src/algorithms/approx/curve.rs +++ b/crates/fj-core/src/algorithms/approx/curve.rs @@ -32,7 +32,7 @@ impl Approx for (&Handle, &HalfEdgeGeometry, &Handle) { None => { let approx = approx_curve( &half_edge.path, - &geometry.of_surface(surface), + geometry.of_surface(surface), half_edge.boundary, tolerance, geometry, diff --git a/crates/fj-core/src/geometry/geometry.rs b/crates/fj-core/src/geometry/geometry.rs index 6d5f1a694..e57989910 100644 --- a/crates/fj-core/src/geometry/geometry.rs +++ b/crates/fj-core/src/geometry/geometry.rs @@ -91,25 +91,24 @@ impl Geometry { /// ## Panics /// /// Panics, if the geometry of the surface is not defined. - pub fn of_surface(&self, surface: &Handle) -> SurfaceGeometry { + pub fn of_surface(&self, surface: &Handle) -> &SurfaceGeometry { self.surface .get(surface) - .copied() .expect("Expected geometry of surface to be defined") } /// Access the geometry of the xy-plane - pub fn xy_plane(&self) -> SurfaceGeometry { + pub fn xy_plane(&self) -> &SurfaceGeometry { self.of_surface(&self.xy_plane) } /// Access the geometry of the xz-plane - pub fn xz_plane(&self) -> SurfaceGeometry { + pub fn xz_plane(&self) -> &SurfaceGeometry { self.of_surface(&self.xz_plane) } /// Access the geometry of the yz-plane - pub fn yz_plane(&self) -> SurfaceGeometry { + pub fn yz_plane(&self) -> &SurfaceGeometry { self.of_surface(&self.yz_plane) } } diff --git a/crates/fj-core/src/operations/sweep/half_edge.rs b/crates/fj-core/src/operations/sweep/half_edge.rs index 3c44d0399..bb2b37368 100644 --- a/crates/fj-core/src/operations/sweep/half_edge.rs +++ b/crates/fj-core/src/operations/sweep/half_edge.rs @@ -59,7 +59,7 @@ impl SweepHalfEdge for Handle { let path = path.into(); let half_edge_geom = *core.layers.geometry.of_half_edge(self); - let surface_geom = core.layers.geometry.of_surface(&surface); + let surface_geom = *core.layers.geometry.of_surface(&surface); let surface = half_edge_geom .path diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 3b02e429b..7264babfd 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -150,18 +150,18 @@ impl ShellValidationError { compare_curve_coords( edge_a, - &geometry.of_surface(surface_a), + geometry.of_surface(surface_a), edge_b, - &geometry.of_surface(surface_b), + geometry.of_surface(surface_b), geometry, config, &mut mismatches, ); compare_curve_coords( edge_b, - &geometry.of_surface(surface_b), + geometry.of_surface(surface_b), edge_a, - &geometry.of_surface(surface_a), + geometry.of_surface(surface_a), geometry, config, &mut mismatches, @@ -254,9 +254,9 @@ impl ShellValidationError { // `distinct_min_distance`, that's a problem. if distances( half_edge_a.clone(), - &geometry.of_surface(surface_a), + geometry.of_surface(surface_a), half_edge_b.clone(), - &geometry.of_surface(surface_b), + geometry.of_surface(surface_b), geometry, ) .all(|d| d < config.distinct_min_distance)