From 3f69cd059237f39a2b30c7ac6fc1a6c207e10078 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:34:32 +0100 Subject: [PATCH 01/21] Add `Surface` reference to `Cycle` This is preparation for removing the `Surface` reference from `HalfEdge` later on. --- crates/fj-kernel/src/algorithms/reverse/cycle.rs | 2 +- crates/fj-kernel/src/algorithms/sweep/edge.rs | 7 +++++-- crates/fj-kernel/src/algorithms/transform/cycle.rs | 6 +++++- crates/fj-kernel/src/objects/full/cycle.rs | 11 +++++++++-- crates/fj-kernel/src/partial/objects/cycle.rs | 3 ++- crates/fj-kernel/src/validate/cycle.rs | 4 ++-- crates/fj-operations/src/sketch.rs | 11 +++++++---- 7 files changed, 31 insertions(+), 13 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse/cycle.rs b/crates/fj-kernel/src/algorithms/reverse/cycle.rs index 23f517430..fd08da75a 100644 --- a/crates/fj-kernel/src/algorithms/reverse/cycle.rs +++ b/crates/fj-kernel/src/algorithms/reverse/cycle.rs @@ -17,6 +17,6 @@ impl Reverse for Handle { edges.reverse(); - Cycle::new(edges).insert(objects) + Cycle::new(self.surface().clone(), edges).insert(objects) } } diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 488aeb75e..488a76ef4 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -248,7 +248,7 @@ mod tests { }; let side_down = { let mut side_down = PartialHalfEdge { - surface, + surface: surface.clone(), ..Default::default() }; @@ -273,7 +273,10 @@ mod tests { .clone() }; - let mut cycle = PartialCycle::default(); + let mut cycle = PartialCycle { + surface, + ..Default::default() + }; cycle.half_edges.extend( [bottom, side_up, top, side_down].map(Partial::from_partial), ); diff --git a/crates/fj-kernel/src/algorithms/transform/cycle.rs b/crates/fj-kernel/src/algorithms/transform/cycle.rs index 5fbc6eb32..e56793752 100644 --- a/crates/fj-kernel/src/algorithms/transform/cycle.rs +++ b/crates/fj-kernel/src/algorithms/transform/cycle.rs @@ -14,12 +14,16 @@ impl TransformObject for Cycle { objects: &mut Service, cache: &mut TransformCache, ) -> Self { + let surface = self + .surface() + .clone() + .transform_with_cache(transform, objects, cache); let half_edges = self.half_edges().map(|half_edge| { half_edge .clone() .transform_with_cache(transform, objects, cache) }); - Self::new(half_edges) + Self::new(surface, half_edges) } } diff --git a/crates/fj-kernel/src/objects/full/cycle.rs b/crates/fj-kernel/src/objects/full/cycle.rs index 66e56a200..9d7192719 100644 --- a/crates/fj-kernel/src/objects/full/cycle.rs +++ b/crates/fj-kernel/src/objects/full/cycle.rs @@ -12,6 +12,7 @@ use crate::{ /// A cycle of connected half-edges #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Cycle { + surface: Handle, half_edges: Vec>, } @@ -21,7 +22,10 @@ impl Cycle { /// # Panics /// /// Panics, if `half_edges` does not yield at least one half-edge. - pub fn new(half_edges: impl IntoIterator>) -> Self { + pub fn new( + surface: Handle, + half_edges: impl IntoIterator>, + ) -> Self { let half_edges = half_edges.into_iter().collect::>(); // This is not a validation check, and thus not part of the validation @@ -33,7 +37,10 @@ impl Cycle { "Cycle must contain at least one half-edge" ); - Self { half_edges } + Self { + surface, + half_edges, + } } /// Access the surface that the cycle is in diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index e1d109555..b7ee51cf6 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -29,11 +29,12 @@ impl PartialObject for PartialCycle { } fn build(self, objects: &mut Service) -> Self::Full { + let surface = self.surface.build(objects); let half_edges = self .half_edges .into_iter() .map(|half_edge| half_edge.build(objects)); - Cycle::new(half_edges) + Cycle::new(surface, half_edges) } } diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index c903e2ed0..32b573e0a 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -176,7 +176,7 @@ mod tests { .into_iter() .map(|half_edge| half_edge.build(&mut services.objects)); - Cycle::new(half_edges) + Cycle::new(valid.surface().clone(), half_edges) }; valid.validate_and_return_first_error()?; @@ -212,7 +212,7 @@ mod tests { .into_iter() .map(|half_edge| half_edge.build(&mut services.objects)); - Cycle::new(half_edges) + Cycle::new(valid.surface().clone(), half_edges) }; valid.validate_and_return_first_error()?; diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index a790d391c..97a666c5c 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -27,11 +27,11 @@ impl Shape for fj::Sketch { let face = match self.chain() { fj::Chain::Circle(circle) => { - let half_edge = { - let surface = Partial::from(surface); + let surface = Partial::from(surface); + let half_edge = { let mut half_edge = PartialHalfEdge { - surface, + surface: surface.clone(), ..Default::default() }; half_edge.update_as_circle_from_radius(circle.radius()); @@ -39,7 +39,10 @@ impl Shape for fj::Sketch { Partial::from_partial(half_edge) }; let exterior = { - let mut cycle = PartialCycle::default(); + let mut cycle = PartialCycle { + surface, + ..Default::default() + }; cycle.half_edges.push(half_edge); Partial::from_partial(cycle) }; From cd972c70cb7579373d4ac41d1fe978be339adc04 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:36:07 +0100 Subject: [PATCH 02/21] Simplify `Cycle::surface` --- crates/fj-kernel/src/objects/full/cycle.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/objects/full/cycle.rs b/crates/fj-kernel/src/objects/full/cycle.rs index 9d7192719..5fdfb4a4a 100644 --- a/crates/fj-kernel/src/objects/full/cycle.rs +++ b/crates/fj-kernel/src/objects/full/cycle.rs @@ -45,13 +45,7 @@ impl Cycle { /// Access the surface that the cycle is in pub fn surface(&self) -> &Handle { - if let Some(half_edge) = self.half_edges.first() { - return half_edge.surface(); - } - - unreachable!( - "Cycle has no half-edges, which the constructor should prevent." - ) + &self.surface } /// Access the half-edges that make up the cycle From ee622ddd5952449c76d882c88e93ae782dab2340 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:37:46 +0100 Subject: [PATCH 03/21] Prepare code for follow-on change --- crates/fj-kernel/src/algorithms/approx/edge.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 2d74e66d5..1355b99b4 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -24,16 +24,19 @@ impl Approx for &Handle { tolerance: impl Into, cache: &mut Self::Cache, ) -> Self::Approximation { - let boundary = self.boundary(); + let half_edge = self; + + let boundary = half_edge.boundary(); let range = RangeOnPath { boundary }; let first = ApproxPoint::new( - self.start_vertex().position(), - self.start_vertex().global_form().position(), + half_edge.start_vertex().position(), + half_edge.start_vertex().global_form().position(), ) - .with_source((self.clone(), self.boundary()[0])); - let curve_approx = (self.curve(), self.surface().deref(), range) - .approx_with_cache(tolerance, cache); + .with_source((half_edge.clone(), half_edge.boundary()[0])); + let curve_approx = + (half_edge.curve(), half_edge.surface().deref(), range) + .approx_with_cache(tolerance, cache); HalfEdgeApprox { first, From c2bcd2fed01d6869a0a1b7ab1cd15bc3626b396e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:39:46 +0100 Subject: [PATCH 04/21] Avoid use of `HalfEdge::surface` --- crates/fj-kernel/src/algorithms/approx/cycle.rs | 7 ++++++- crates/fj-kernel/src/algorithms/approx/edge.rs | 16 ++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/cycle.rs b/crates/fj-kernel/src/algorithms/approx/cycle.rs index 9d398f2bc..b819de3e3 100644 --- a/crates/fj-kernel/src/algorithms/approx/cycle.rs +++ b/crates/fj-kernel/src/algorithms/approx/cycle.rs @@ -2,6 +2,8 @@ //! //! See [`CycleApprox`]. +use std::ops::Deref; + use fj_math::Segment; use crate::objects::Cycle; @@ -23,7 +25,10 @@ impl Approx for &Cycle { let half_edges = self .half_edges() - .map(|half_edge| half_edge.approx_with_cache(tolerance, cache)) + .map(|half_edge| { + (half_edge, self.surface().deref()) + .approx_with_cache(tolerance, cache) + }) .collect(); CycleApprox { half_edges } diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 1355b99b4..cefa0cee0 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -5,9 +5,10 @@ //! approximations are usually used to build cycle approximations, and this way, //! the caller doesn't have to call with duplicate vertices. -use std::ops::Deref; - -use crate::{objects::HalfEdge, storage::Handle}; +use crate::{ + objects::{HalfEdge, Surface}, + storage::Handle, +}; use super::{ curve::{CurveApprox, CurveCache}, @@ -15,7 +16,7 @@ use super::{ Approx, ApproxPoint, Tolerance, }; -impl Approx for &Handle { +impl Approx for (&Handle, &Surface) { type Approximation = HalfEdgeApprox; type Cache = CurveCache; @@ -24,7 +25,7 @@ impl Approx for &Handle { tolerance: impl Into, cache: &mut Self::Cache, ) -> Self::Approximation { - let half_edge = self; + let (half_edge, surface) = self; let boundary = half_edge.boundary(); let range = RangeOnPath { boundary }; @@ -34,9 +35,8 @@ impl Approx for &Handle { half_edge.start_vertex().global_form().position(), ) .with_source((half_edge.clone(), half_edge.boundary()[0])); - let curve_approx = - (half_edge.curve(), half_edge.surface().deref(), range) - .approx_with_cache(tolerance, cache); + let curve_approx = (half_edge.curve(), surface, range) + .approx_with_cache(tolerance, cache); HalfEdgeApprox { first, From 56bcbf03036fe3c9812c281f6a86f66f9b7295ba Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:41:29 +0100 Subject: [PATCH 05/21] Prepare code for follow-on change --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 488a76ef4..ca6657661 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -168,10 +168,12 @@ mod tests { fn sweep() { let mut services = Services::new(); + let surface = services.objects.surfaces.xy_plane(); + let half_edge = { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), + surface, [[0., 0.], [1., 0.]], ); From 40decb5a49ed71121bbb6a1408e9674105dc6f7c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:44:54 +0100 Subject: [PATCH 06/21] Avoid use of `HalfEdge::surface` --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 16 ++++++++-------- crates/fj-kernel/src/algorithms/sweep/face.rs | 19 ++++++++++++------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index ca6657661..c8a5d2917 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -1,12 +1,10 @@ -use std::ops::Deref; - use fj_interop::{ext::ArrayExt, mesh::Color}; use fj_math::{Point, Scalar, Vector}; use crate::{ builder::{CycleBuilder, HalfEdgeBuilder}, insert::Insert, - objects::{Face, HalfEdge, Objects}, + objects::{Face, HalfEdge, Objects, Surface}, partial::{Partial, PartialFace, PartialObject}, services::Service, storage::Handle, @@ -14,7 +12,7 @@ use crate::{ use super::{Sweep, SweepCache}; -impl Sweep for (Handle, Color) { +impl Sweep for (Handle, &Surface, Color) { type Swept = (Handle, Handle); fn sweep_with_cache( @@ -23,7 +21,7 @@ impl Sweep for (Handle, Color) { cache: &mut SweepCache, objects: &mut Service, ) -> Self::Swept { - let (edge, color) = self; + let (edge, surface, color) = self; let path = path.into(); // The result of sweeping an edge is a face. Let's create that. @@ -36,7 +34,7 @@ impl Sweep for (Handle, Color) { // be created by sweeping a curve, so let's sweep the curve of the edge // we're sweeping. face.exterior.write().surface = Partial::from( - (edge.curve().clone(), edge.surface().deref()) + (edge.curve().clone(), surface) .sweep_with_cache(path, cache, objects), ); @@ -150,6 +148,8 @@ impl Sweep for (Handle, Color) { #[cfg(test)] mod tests { + use std::ops::Deref; + use fj_interop::{ext::ArrayExt, mesh::Color}; use fj_math::Point; use pretty_assertions::assert_eq; @@ -173,7 +173,7 @@ mod tests { let half_edge = { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - surface, + surface.clone(), [[0., 0.], [1., 0.]], ); @@ -182,7 +182,7 @@ mod tests { .insert(&mut services.objects) }; - let (face, _) = (half_edge, Color::default()) + let (face, _) = (half_edge, surface.deref(), Color::default()) .sweep([0., 0., 1.], &mut services.objects); let expected_face = { diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index db79e37f2..586a56fa5 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -1,3 +1,5 @@ +use std::ops::Deref; + use fj_interop::ext::ArrayExt; use fj_math::{Scalar, Vector}; @@ -70,8 +72,9 @@ impl Sweep for Handle { let mut original_edges = Vec::new(); let mut top_edges = Vec::new(); for half_edge in cycle.half_edges().cloned() { - let (face, top_edge) = (half_edge.clone(), self.color()) - .sweep_with_cache(path, cache, objects); + let (face, top_edge) = + (half_edge.clone(), self.surface().deref(), self.color()) + .sweep_with_cache(path, cache, objects); faces.push(face); @@ -128,6 +131,8 @@ impl Sweep for Handle { #[cfg(test)] mod tests { + use std::ops::Deref; + use fj_interop::{ext::SliceExt, mesh::Color}; use crate::{ @@ -182,7 +187,7 @@ mod tests { .reverse(&mut services.objects); let mut top = PartialFace::default(); top.exterior.write().surface = - Partial::from(surface.translate(UP, &mut services.objects)); + Partial::from(surface.clone().translate(UP, &mut services.objects)); top.exterior.write().update_as_polygon_from_points(TRIANGLE); let top = top .build(&mut services.objects) @@ -204,8 +209,8 @@ mod tests { .build(&mut services.objects) .insert(&mut services.objects) }; - let (face, _) = - (half_edge, Color::default()).sweep(UP, &mut services.objects); + let (face, _) = (half_edge, surface.deref(), Color::default()) + .sweep(UP, &mut services.objects); face }); @@ -250,7 +255,7 @@ mod tests { .insert(&mut services.objects) .reverse(&mut services.objects); let mut top = PartialFace::default(); - top.exterior.write().surface = Partial::from(surface); + top.exterior.write().surface = Partial::from(surface.clone()); top.exterior.write().update_as_polygon_from_points(TRIANGLE); let top = top .build(&mut services.objects) @@ -273,7 +278,7 @@ mod tests { .insert(&mut services.objects) .reverse(&mut services.objects) }; - let (face, _) = (half_edge, Color::default()) + let (face, _) = (half_edge, surface.deref(), Color::default()) .sweep(DOWN, &mut services.objects); face }); From 5913db46604e61fc0e8ceaeca9ee76689441ae7f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:49:59 +0100 Subject: [PATCH 07/21] Avoid use of `PartialHalfEdge`'s `surface` field --- crates/fj-kernel/src/builder/cycle.rs | 6 ++++-- crates/fj-kernel/src/builder/edge.rs | 19 +++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 3e7202905..5d08f7a11 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -247,7 +247,8 @@ impl CycleBuilder for PartialCycle { let mut this = half_edges.pop_front().expect( "Pushed correct number of half-edges; should be able to pop", ); - this.write().update_from_other_edge(&other); + this.write() + .update_from_other_edge(&other, &self.surface.read().geometry); this }) } @@ -261,7 +262,8 @@ impl CycleBuilder for PartialCycle { { edges.map(|other| { let mut this = self.add_half_edge(); - this.write().update_from_other_edge(&other); + this.write() + .update_from_other_edge(&other, &self.surface.read().geometry); this }) } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 78f85e921..ea0c69254 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -2,7 +2,10 @@ use fj_interop::ext::ArrayExt; use fj_math::{Point, Scalar}; use crate::{ - geometry::path::{GlobalPath, SurfacePath}, + geometry::{ + path::{GlobalPath, SurfacePath}, + surface::SurfaceGeometry, + }, objects::{GlobalEdge, HalfEdge, Surface}, partial::{MaybeSurfacePath, Partial, PartialGlobalEdge, PartialHalfEdge}, }; @@ -47,7 +50,11 @@ pub trait HalfEdgeBuilder { /// under various circumstances. As long as you're only dealing with lines /// and planes, you should be fine. Otherwise, please read the code of this /// method carefully, to make sure you don't run into trouble. - fn update_from_other_edge(&mut self, other: &Partial); + fn update_from_other_edge( + &mut self, + other: &Partial, + surface: &Option, + ); } impl HalfEdgeBuilder for PartialHalfEdge { @@ -166,14 +173,18 @@ impl HalfEdgeBuilder for PartialHalfEdge { self.global_form.clone() } - fn update_from_other_edge(&mut self, other: &Partial) { + fn update_from_other_edge( + &mut self, + other: &Partial, + surface: &Option, + ) { let global_curve = other.read().curve.read().global_form.clone(); self.curve.write().global_form = global_curve.clone(); self.global_form.write().curve = global_curve; self.curve.write().path = other.read().curve.read().path.as_ref().and_then(|path| { - match other.read().surface.read().geometry { + match surface { Some(surface) => { // We have information about the other edge's surface // available. We need to use that to interpret what the From e024bc472f9bc18244e52b4665d33b73889f7ef9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:59:26 +0100 Subject: [PATCH 08/21] Make variable name more explicit --- crates/fj-kernel/src/partial/objects/edge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index a0057c85a..a302264ca 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -86,11 +86,11 @@ impl PartialObject for PartialHalfEdge { Some(position_global); } - let position = + let position_curve = vertex.0.expect("Can't build `Vertex` without position"); let surface_form = vertex.1.build(objects); - (position, surface_form) + (position_curve, surface_form) }); let global_form = self.global_form.build(objects); From 3f6ce64d7612522ab9ab977d7c697250222b4539 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 14:07:44 +0100 Subject: [PATCH 09/21] Consolidate redundant code --- crates/fj-kernel/src/partial/objects/edge.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index a302264ca..60fc86a6c 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -58,15 +58,16 @@ impl PartialObject for PartialHalfEdge { let surface = self.surface.build(objects); let curve = self.curve.build(objects); let vertices = self.vertices.map(|mut vertex| { + let position_curve = vertex + .0 + .expect("Can't infer surface position without curve position"); + let position_surface = vertex.1.read().position; // Infer surface position, if not available. let position_surface = match position_surface { Some(position_surface) => position_surface, None => { - let position_curve = vertex.0.expect( - "Can't infer surface position without curve position", - ); let position_surface = curve.path().point_from_path_coords(position_curve); @@ -86,8 +87,6 @@ impl PartialObject for PartialHalfEdge { Some(position_global); } - let position_curve = - vertex.0.expect("Can't build `Vertex` without position"); let surface_form = vertex.1.build(objects); (position_curve, surface_form) From d6957d7cd74013e410fcfd1198590aeae18bbc62 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 14:16:22 +0100 Subject: [PATCH 10/21] Derive `Copy` for `MaybeSurfacePath` --- crates/fj-kernel/src/partial/objects/curve.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/partial/objects/curve.rs b/crates/fj-kernel/src/partial/objects/curve.rs index 3826d4cb2..4377536ae 100644 --- a/crates/fj-kernel/src/partial/objects/curve.rs +++ b/crates/fj-kernel/src/partial/objects/curve.rs @@ -46,7 +46,7 @@ impl PartialObject for PartialCurve { /// /// Can be a fully defined [`SurfacePath`], or just the type of path might be /// known. -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] pub enum MaybeSurfacePath { /// The surface path is fully defined Defined(SurfacePath), From bec9db47327cce744e6ad162c624b85d47fbdc1f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 14:17:42 +0100 Subject: [PATCH 11/21] Add new `HalfEdgeBuilder` method --- crates/fj-kernel/src/builder/edge.rs | 49 ++++++++++++++++++++ crates/fj-kernel/src/partial/objects/edge.rs | 35 +++----------- 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index ea0c69254..d1b78eed1 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -41,6 +41,9 @@ pub trait HalfEdgeBuilder { /// it. fn infer_global_form(&mut self) -> Partial; + /// Infer the vertex positions (surface and global), if not already set + fn infer_vertex_positions_if_necessary(&mut self); + /// Update this edge from another /// /// Infers as much information about this edge from the other, under the @@ -173,6 +176,52 @@ impl HalfEdgeBuilder for PartialHalfEdge { self.global_form.clone() } + fn infer_vertex_positions_if_necessary(&mut self) { + let path = self + .curve + .read() + .path + .expect("Can't infer vertex positions without curve"); + let MaybeSurfacePath::Defined(path) = path else { + panic!("Can't infer vertex positions with undefined path"); + }; + + let surface = + self.surface.read().geometry.expect( + "Can't infer surface positions without surface geometry", + ); + + for vertex in &mut self.vertices { + let position_curve = vertex + .0 + .expect("Can't infer surface position without curve position"); + + let position_surface = vertex.1.read().position; + + // Infer surface position, if not available. + let position_surface = match position_surface { + Some(position_surface) => position_surface, + None => { + let position_surface = + path.point_from_path_coords(position_curve); + + vertex.1.write().position = Some(position_surface); + + position_surface + } + }; + + // Infer global position, if not available. + let position_global = vertex.1.read().global_form.read().position; + if position_global.is_none() { + let position_global = + surface.point_from_surface_coords(position_surface); + vertex.1.write().global_form.write().position = + Some(position_global); + } + } + } + fn update_from_other_edge( &mut self, other: &Partial, diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 60fc86a6c..69121d05f 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -4,6 +4,7 @@ use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ + builder::HalfEdgeBuilder, objects::{ Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, Surface, SurfaceVertex, @@ -54,39 +55,15 @@ impl PartialObject for PartialHalfEdge { } } - fn build(self, objects: &mut Service) -> Self::Full { + fn build(mut self, objects: &mut Service) -> Self::Full { + self.infer_vertex_positions_if_necessary(); + let surface = self.surface.build(objects); let curve = self.curve.build(objects); - let vertices = self.vertices.map(|mut vertex| { + let vertices = self.vertices.map(|vertex| { let position_curve = vertex .0 - .expect("Can't infer surface position without curve position"); - - let position_surface = vertex.1.read().position; - - // Infer surface position, if not available. - let position_surface = match position_surface { - Some(position_surface) => position_surface, - None => { - let position_surface = - curve.path().point_from_path_coords(position_curve); - - vertex.1.write().position = Some(position_surface); - - position_surface - } - }; - - // Infer global position, if not available. - let position_global = vertex.1.read().global_form.read().position; - if position_global.is_none() { - let position_global = surface - .geometry() - .point_from_surface_coords(position_surface); - vertex.1.write().global_form.write().position = - Some(position_global); - } - + .expect("Can't build `HalfEdge` without boundary positions"); let surface_form = vertex.1.build(objects); (position_curve, surface_form) From 913689b350d362e75e544b396ea0a63a02db3e9e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 14:28:36 +0100 Subject: [PATCH 12/21] Make vertex position inference explicit --- crates/fj-kernel/src/algorithms/intersect/curve_edge.rs | 4 ++++ crates/fj-kernel/src/algorithms/sweep/edge.rs | 3 +++ crates/fj-kernel/src/algorithms/sweep/face.rs | 2 ++ crates/fj-kernel/src/objects/full/edge.rs | 2 ++ crates/fj-kernel/src/partial/objects/cycle.rs | 9 +++++---- crates/fj-kernel/src/partial/objects/edge.rs | 5 +---- crates/fj-kernel/src/validate/edge.rs | 4 ++++ 7 files changed, 21 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs index d7ca55ac7..a6d23f467 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs @@ -96,6 +96,7 @@ mod tests { surface, [[1., -1.], [1., 1.]], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; @@ -124,6 +125,7 @@ mod tests { surface, [[-1., -1.], [-1., 1.]], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; @@ -152,6 +154,7 @@ mod tests { surface, [[-1., -1.], [1., -1.]], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; @@ -175,6 +178,7 @@ mod tests { surface, [[-1., 0.], [1., 0.]], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index c8a5d2917..df3d81355 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -176,6 +176,7 @@ mod tests { surface.clone(), [[0., 0.], [1., 0.]], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge .build(&mut services.objects) @@ -240,6 +241,7 @@ mod tests { top.infer_global_form(); top.update_as_line_segment(); + top.infer_vertex_positions_if_necessary(); Partial::from( top.build(&mut services.objects) @@ -265,6 +267,7 @@ mod tests { side_down.infer_global_form(); side_down.update_as_line_segment(); + side_down.infer_vertex_positions_if_necessary(); Partial::from( side_down diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 586a56fa5..f887af75f 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -204,6 +204,7 @@ mod tests { services.objects.surfaces.xy_plane(), [a, b], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge .build(&mut services.objects) @@ -272,6 +273,7 @@ mod tests { services.objects.surfaces.xy_plane(), [a, b], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge .build(&mut services.objects) diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index f7a77b378..85c43e0ff 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -175,12 +175,14 @@ mod tests { let mut half_edge = PartialHalfEdge::default(); half_edge .update_as_line_segment_from_points(surface.clone(), [a, b]); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; let b_to_a = { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points(surface, [b, a]); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index b7ee51cf6..795695dce 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,4 +1,5 @@ use crate::{ + builder::HalfEdgeBuilder, objects::{Cycle, HalfEdge, Objects, Surface}, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, @@ -30,10 +31,10 @@ impl PartialObject for PartialCycle { fn build(self, objects: &mut Service) -> Self::Full { let surface = self.surface.build(objects); - let half_edges = self - .half_edges - .into_iter() - .map(|half_edge| half_edge.build(objects)); + let half_edges = self.half_edges.into_iter().map(|mut half_edge| { + half_edge.write().infer_vertex_positions_if_necessary(); + half_edge.build(objects) + }); Cycle::new(surface, half_edges) } diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 69121d05f..899ffe2b3 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -4,7 +4,6 @@ use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ - builder::HalfEdgeBuilder, objects::{ Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, Surface, SurfaceVertex, @@ -55,9 +54,7 @@ impl PartialObject for PartialHalfEdge { } } - fn build(mut self, objects: &mut Service) -> Self::Full { - self.infer_vertex_positions_if_necessary(); - + fn build(self, objects: &mut Service) -> Self::Full { let surface = self.surface.build(objects); let curve = self.curve.build(objects); let vertices = self.vertices.map(|vertex| { diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index d5658975f..6f50c2066 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -262,6 +262,7 @@ mod tests { services.objects.surfaces.xy_plane(), [[0., 0.], [1., 0.]], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; @@ -301,6 +302,7 @@ mod tests { services.objects.surfaces.xy_plane(), [[0., 0.], [1., 0.]], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; @@ -348,6 +350,7 @@ mod tests { services.objects.surfaces.xy_plane(), [[0., 0.], [1., 0.]], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; @@ -380,6 +383,7 @@ mod tests { services.objects.surfaces.xy_plane(), [[0., 0.], [1., 0.]], ); + half_edge.infer_vertex_positions_if_necessary(); half_edge.build(&mut services.objects) }; From 773e9eb050f8dbd473cc3bc7ba59c8abb2e5cda9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 14:30:25 +0100 Subject: [PATCH 13/21] Remove redundant code --- .../fj-kernel/src/algorithms/intersect/curve_edge.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs index a6d23f467..a764a74dd 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs @@ -76,7 +76,7 @@ mod tests { use crate::{ builder::{CurveBuilder, HalfEdgeBuilder}, - partial::{Partial, PartialCurve, PartialHalfEdge, PartialObject}, + partial::{PartialCurve, PartialHalfEdge, PartialObject}, services::Services, }; @@ -86,7 +86,7 @@ mod tests { fn compute_edge_in_front_of_curve_origin() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); + let surface = services.objects.surfaces.xy_plane(); let mut curve = PartialCurve::default(); curve.update_as_u_axis(); let curve = curve.build(&mut services.objects); @@ -115,7 +115,7 @@ mod tests { fn compute_edge_behind_curve_origin() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); + let surface = services.objects.surfaces.xy_plane(); let mut curve = PartialCurve::default(); curve.update_as_u_axis(); let curve = curve.build(&mut services.objects); @@ -144,7 +144,7 @@ mod tests { fn compute_edge_parallel_to_curve() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); + let surface = services.objects.surfaces.xy_plane(); let mut curve = PartialCurve::default(); curve.update_as_u_axis(); let curve = curve.build(&mut services.objects); @@ -168,7 +168,7 @@ mod tests { fn compute_edge_on_curve() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); + let surface = services.objects.surfaces.xy_plane(); let mut curve = PartialCurve::default(); curve.update_as_u_axis(); let curve = curve.build(&mut services.objects); From d95c30a3ea35762876834e409e42d0b2ef4c1f12 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 14:40:26 +0100 Subject: [PATCH 14/21] Keep around original `Surface` This creates some redundancy when setting the `HalfEdge` fields, but this is something that I'm working on removing anyway. --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index df3d81355..7a3169749 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -187,7 +187,7 @@ mod tests { .sweep([0., 0., 1.], &mut services.objects); let expected_face = { - let surface = Partial::from(services.objects.surfaces.xz_plane()); + let surface = services.objects.surfaces.xz_plane(); let bottom = { let mut half_edge = PartialHalfEdge::default(); @@ -200,7 +200,7 @@ mod tests { }; let side_up = { let mut side_up = PartialHalfEdge { - surface: surface.clone(), + surface: Partial::from(surface.clone()), ..Default::default() }; @@ -223,7 +223,7 @@ mod tests { }; let top = { let mut top = PartialHalfEdge { - surface: surface.clone(), + surface: Partial::from(surface.clone()), ..Default::default() }; @@ -252,7 +252,7 @@ mod tests { }; let side_down = { let mut side_down = PartialHalfEdge { - surface: surface.clone(), + surface: Partial::from(surface.clone()), ..Default::default() }; @@ -279,7 +279,7 @@ mod tests { }; let mut cycle = PartialCycle { - surface, + surface: Partial::from(surface), ..Default::default() }; cycle.half_edges.extend( From 89c134c3817d695d7019958a6c0508cb0af58b70 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 14:43:25 +0100 Subject: [PATCH 15/21] Extract values into variables --- crates/fj-kernel/src/validate/edge.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 6f50c2066..b402f77df 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -257,9 +257,11 @@ mod tests { let mut services = Services::new(); let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), + surface, [[0., 0.], [1., 0.]], ); half_edge.infer_vertex_positions_if_necessary(); @@ -297,9 +299,11 @@ mod tests { let mut services = Services::new(); let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), + surface, [[0., 0.], [1., 0.]], ); half_edge.infer_vertex_positions_if_necessary(); @@ -345,9 +349,11 @@ mod tests { let mut services = Services::new(); let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), + surface, [[0., 0.], [1., 0.]], ); half_edge.infer_vertex_positions_if_necessary(); @@ -378,9 +384,11 @@ mod tests { let mut services = Services::new(); let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), + surface, [[0., 0.], [1., 0.]], ); half_edge.infer_vertex_positions_if_necessary(); From 46c831ddc650e175c33feac844134b34bf1b2af4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 14:46:47 +0100 Subject: [PATCH 16/21] Avoid use of `PartialHalfEdge`'s `surface` field --- .../src/algorithms/intersect/curve_edge.rs | 16 ++++++++-------- crates/fj-kernel/src/algorithms/sweep/edge.rs | 7 ++++--- crates/fj-kernel/src/algorithms/sweep/face.rs | 6 ++++-- crates/fj-kernel/src/builder/edge.rs | 15 ++++++++------- crates/fj-kernel/src/objects/full/edge.rs | 7 ++++--- crates/fj-kernel/src/partial/objects/cycle.rs | 5 ++++- crates/fj-kernel/src/validate/edge.rs | 16 ++++++++-------- 7 files changed, 40 insertions(+), 32 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs index a764a74dd..74f0ffb93 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs @@ -93,10 +93,10 @@ mod tests { let half_edge = { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - surface, + surface.clone(), [[1., -1.], [1., 1.]], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -122,10 +122,10 @@ mod tests { let half_edge = { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - surface, + surface.clone(), [[-1., -1.], [-1., 1.]], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -151,10 +151,10 @@ mod tests { let half_edge = { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - surface, + surface.clone(), [[-1., -1.], [1., -1.]], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -175,10 +175,10 @@ mod tests { let half_edge = { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - surface, + surface.clone(), [[-1., 0.], [1., 0.]], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 7a3169749..01450aa6d 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -176,7 +176,7 @@ mod tests { surface.clone(), [[0., 0.], [1., 0.]], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge .build(&mut services.objects) @@ -241,7 +241,7 @@ mod tests { top.infer_global_form(); top.update_as_line_segment(); - top.infer_vertex_positions_if_necessary(); + top.infer_vertex_positions_if_necessary(&surface.geometry()); Partial::from( top.build(&mut services.objects) @@ -267,7 +267,8 @@ mod tests { side_down.infer_global_form(); side_down.update_as_line_segment(); - side_down.infer_vertex_positions_if_necessary(); + side_down + .infer_vertex_positions_if_necessary(&surface.geometry()); Partial::from( side_down diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index f887af75f..618923d5a 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -204,7 +204,8 @@ mod tests { services.objects.surfaces.xy_plane(), [a, b], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge + .infer_vertex_positions_if_necessary(&surface.geometry()); half_edge .build(&mut services.objects) @@ -273,7 +274,8 @@ mod tests { services.objects.surfaces.xy_plane(), [a, b], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge + .infer_vertex_positions_if_necessary(&surface.geometry()); half_edge .build(&mut services.objects) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index d1b78eed1..d2e4d0924 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -42,7 +42,10 @@ pub trait HalfEdgeBuilder { fn infer_global_form(&mut self) -> Partial; /// Infer the vertex positions (surface and global), if not already set - fn infer_vertex_positions_if_necessary(&mut self); + fn infer_vertex_positions_if_necessary( + &mut self, + surface: &SurfaceGeometry, + ); /// Update this edge from another /// @@ -176,7 +179,10 @@ impl HalfEdgeBuilder for PartialHalfEdge { self.global_form.clone() } - fn infer_vertex_positions_if_necessary(&mut self) { + fn infer_vertex_positions_if_necessary( + &mut self, + surface: &SurfaceGeometry, + ) { let path = self .curve .read() @@ -186,11 +192,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { panic!("Can't infer vertex positions with undefined path"); }; - let surface = - self.surface.read().geometry.expect( - "Can't infer surface positions without surface geometry", - ); - for vertex in &mut self.vertices { let position_curve = vertex .0 diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 85c43e0ff..d216538c2 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -175,14 +175,15 @@ mod tests { let mut half_edge = PartialHalfEdge::default(); half_edge .update_as_line_segment_from_points(surface.clone(), [a, b]); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; let b_to_a = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points(surface, [b, a]); - half_edge.infer_vertex_positions_if_necessary(); + half_edge + .update_as_line_segment_from_points(surface.clone(), [b, a]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index 795695dce..386fce9e1 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -31,8 +31,11 @@ impl PartialObject for PartialCycle { fn build(self, objects: &mut Service) -> Self::Full { let surface = self.surface.build(objects); + let surface_geometry = surface.geometry(); let half_edges = self.half_edges.into_iter().map(|mut half_edge| { - half_edge.write().infer_vertex_positions_if_necessary(); + half_edge + .write() + .infer_vertex_positions_if_necessary(&surface_geometry); half_edge.build(objects) }); diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index b402f77df..c23e5bce0 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -261,10 +261,10 @@ mod tests { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - surface, + surface.clone(), [[0., 0.], [1., 0.]], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -303,10 +303,10 @@ mod tests { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - surface, + surface.clone(), [[0., 0.], [1., 0.]], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -353,10 +353,10 @@ mod tests { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - surface, + surface.clone(), [[0., 0.], [1., 0.]], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -388,10 +388,10 @@ mod tests { let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_line_segment_from_points( - surface, + surface.clone(), [[0., 0.], [1., 0.]], ); - half_edge.infer_vertex_positions_if_necessary(); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; From b4d05d88b15624bd0661d42ff920fa731cc29441 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 15:17:51 +0100 Subject: [PATCH 17/21] Move validation check from `HalfEdge` to `Cycle` I'm working on removing the reference to `Surface` from `HalfEdge`. Once that is done, `HalfEdge` won't be able to support this validation check anymore. --- crates/fj-kernel/src/validate/cycle.rs | 131 ++++++++++++++++++++++++- crates/fj-kernel/src/validate/edge.rs | 107 +------------------- 2 files changed, 130 insertions(+), 108 deletions(-) diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 32b573e0a..cd3a30d81 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -17,6 +17,7 @@ impl Validate for Cycle { ) { CycleValidationError::check_half_edge_connections(self, errors); CycleValidationError::check_half_edge_boundaries(self, config, errors); + CycleValidationError::check_vertex_positions(self, config, errors); // We don't need to check that all half-edges are defined in the same // surface. We already check that they are connected by identical @@ -70,6 +71,33 @@ pub enum CycleValidationError { /// The half-edge half_edge: Handle, }, + + /// Mismatch between [`SurfaceVertex`] and `GlobalVertex` positions + #[error( + "`SurfaceVertex` position doesn't match position of its global form\n\ + - Surface position: {surface_position:?}\n\ + - Surface position converted to global position: \ + {surface_position_as_global:?}\n\ + - Global position: {global_position:?}\n\ + - Distance between the positions: {distance}\n\ + - `SurfaceVertex`: {surface_vertex:#?}" + )] + VertexSurfacePositionMismatch { + /// The position of the surface vertex + surface_position: Point<2>, + + /// The surface position converted into a global position + surface_position_as_global: Point<3>, + + /// The position of the global vertex + global_position: Point<3>, + + /// The distance between the positions + distance: Scalar, + + /// The surface vertex + surface_vertex: Handle, + }, } impl CycleValidationError { @@ -130,15 +158,49 @@ impl CycleValidationError { } } } + + fn check_vertex_positions( + cycle: &Cycle, + config: &ValidationConfig, + errors: &mut Vec, + ) { + for half_edge in cycle.half_edges() { + for surface_vertex in half_edge.surface_vertices() { + let surface_position_as_global = half_edge + .surface() + .geometry() + .point_from_surface_coords(surface_vertex.position()); + let global_position = surface_vertex.global_form().position(); + + let distance = + surface_position_as_global.distance_to(&global_position); + + if distance > config.identical_max_distance { + errors.push( + Self::VertexSurfacePositionMismatch { + surface_position: surface_vertex.position(), + surface_position_as_global, + global_position, + distance, + surface_vertex: surface_vertex.clone(), + } + .into(), + ); + } + } + } + } } #[cfg(test)] mod tests { - use fj_math::Point; + use fj_interop::ext::ArrayExt; + use fj_math::{Point, Scalar, Vector}; use crate::{ - builder::CycleBuilder, - objects::Cycle, + builder::{CycleBuilder, HalfEdgeBuilder}, + insert::Insert, + objects::{Cycle, HalfEdge, SurfaceVertex}, partial::{Partial, PartialCycle, PartialObject}, services::Services, validate::Validate, @@ -220,4 +282,67 @@ mod tests { Ok(()) } + + #[test] + fn surface_vertex_position_mismatch() -> anyhow::Result<()> { + let mut services = Services::new(); + + let valid = { + let surface = services.objects.surfaces.xy_plane(); + + let mut cycle = PartialCycle { + surface: Partial::from(surface.clone()), + ..Default::default() + }; + + let mut half_edge = cycle.add_half_edge(); + half_edge.write().update_as_circle_from_radius(1.); + half_edge + .write() + .infer_vertex_positions_if_necessary(&surface.geometry()); + + cycle.build(&mut services.objects) + }; + let invalid = { + let half_edge = { + let half_edge = valid.half_edges().next().unwrap(); + + let boundary = half_edge + .boundary() + .map(|point| point + Vector::from([Scalar::PI / 2.])); + + let mut surface_vertices = + half_edge.surface_vertices().map(Clone::clone); + + let mut invalid = None; + for surface_vertex in surface_vertices.each_mut_ext() { + let invalid = invalid.get_or_insert_with(|| { + SurfaceVertex::new( + [0., 1.], + surface_vertex.global_form().clone(), + ) + .insert(&mut services.objects) + }); + *surface_vertex = invalid.clone(); + } + + let boundary = boundary.zip_ext(surface_vertices); + + HalfEdge::new( + half_edge.surface().clone(), + half_edge.curve().clone(), + boundary, + half_edge.global_form().clone(), + ) + .insert(&mut services.objects) + }; + + Cycle::new(valid.surface().clone(), [half_edge]) + }; + + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); + + Ok(()) + } } diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index c23e5bce0..a801f2ca9 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -1,9 +1,7 @@ use fj_math::{Point, Scalar}; use crate::{ - objects::{ - GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface, SurfaceVertex, - }, + objects::{GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface}, storage::Handle, }; @@ -18,7 +16,6 @@ impl Validate for HalfEdge { HalfEdgeValidationError::check_global_curve_identity(self, errors); HalfEdgeValidationError::check_global_vertex_identity(self, errors); HalfEdgeValidationError::check_vertex_coincidence(self, config, errors); - HalfEdgeValidationError::check_vertex_positions(self, config, errors); } } @@ -107,33 +104,6 @@ pub enum HalfEdgeValidationError { /// The half-edge half_edge: HalfEdge, }, - - /// Mismatch between [`SurfaceVertex`] and [`GlobalVertex`] positions - #[error( - "`SurfaceVertex` position doesn't match position of its global form\n\ - - Surface position: {surface_position:?}\n\ - - Surface position converted to global position: \ - {surface_position_as_global:?}\n\ - - Global position: {global_position:?}\n\ - - Distance between the positions: {distance}\n\ - - `SurfaceVertex`: {surface_vertex:#?}" - )] - VertexSurfacePositionMismatch { - /// The position of the surface vertex - surface_position: Point<2>, - - /// The surface position converted into a global position - surface_position_as_global: Point<3>, - - /// The position of the global vertex - global_position: Point<3>, - - /// The distance between the positions - distance: Scalar, - - /// The surface vertex - surface_vertex: Handle, - }, } impl HalfEdgeValidationError { @@ -206,36 +176,6 @@ impl HalfEdgeValidationError { ); } } - - fn check_vertex_positions( - half_edge: &HalfEdge, - config: &ValidationConfig, - errors: &mut Vec, - ) { - for surface_vertex in half_edge.surface_vertices() { - let surface_position_as_global = half_edge - .surface() - .geometry() - .point_from_surface_coords(surface_vertex.position()); - let global_position = surface_vertex.global_form().position(); - - let distance = - surface_position_as_global.distance_to(&global_position); - - if distance > config.identical_max_distance { - errors.push( - Box::new(Self::VertexSurfacePositionMismatch { - surface_position: surface_vertex.position(), - surface_position_as_global, - global_position, - distance, - surface_vertex: surface_vertex.clone(), - }) - .into(), - ); - } - } - } } #[cfg(test)] @@ -246,7 +186,7 @@ mod tests { use crate::{ builder::HalfEdgeBuilder, insert::Insert, - objects::{GlobalCurve, HalfEdge, SurfaceVertex}, + objects::{GlobalCurve, HalfEdge}, partial::{Partial, PartialHalfEdge, PartialObject}, services::Services, validate::Validate, @@ -378,47 +318,4 @@ mod tests { Ok(()) } - - #[test] - fn surface_vertex_position_mismatch() -> anyhow::Result<()> { - let mut services = Services::new(); - - let valid = { - let surface = services.objects.surfaces.xy_plane(); - - let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[0., 0.], [1., 0.]], - ); - half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); - - half_edge.build(&mut services.objects) - }; - let invalid = { - let mut surface_vertices = - valid.surface_vertices().map(Clone::clone); - - let [_, surface_vertex] = surface_vertices.each_mut_ext(); - *surface_vertex = SurfaceVertex::new( - [2., 0.], - surface_vertex.global_form().clone(), - ) - .insert(&mut services.objects); - - let boundary = valid.boundary().zip_ext(surface_vertices); - - HalfEdge::new( - valid.surface().clone(), - valid.curve().clone(), - boundary, - valid.global_form().clone(), - ) - }; - - valid.validate_and_return_first_error()?; - assert!(invalid.validate_and_return_first_error().is_err()); - - Ok(()) - } } From b0ccd19fbe449b879ea15398f7e6d30a00355a86 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 15:19:43 +0100 Subject: [PATCH 18/21] Avoid use of `HalfEdge::surface` --- crates/fj-kernel/src/validate/cycle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index cd3a30d81..07c6f5dfb 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -166,7 +166,7 @@ impl CycleValidationError { ) { for half_edge in cycle.half_edges() { for surface_vertex in half_edge.surface_vertices() { - let surface_position_as_global = half_edge + let surface_position_as_global = cycle .surface() .geometry() .point_from_surface_coords(surface_vertex.position()); From e33eba5251602a123ee3eaccdb7174a42a5eeb31 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 15:22:12 +0100 Subject: [PATCH 19/21] Remove surface from `HalfEdge`/`PartialHalfEdge` --- crates/fj-kernel/src/algorithms/reverse/edge.rs | 1 - crates/fj-kernel/src/algorithms/sweep/edge.rs | 3 --- crates/fj-kernel/src/algorithms/transform/edge.rs | 6 +----- crates/fj-kernel/src/builder/cycle.rs | 1 - crates/fj-kernel/src/builder/edge.rs | 4 +--- crates/fj-kernel/src/objects/full/edge.rs | 10 +--------- crates/fj-kernel/src/partial/objects/edge.rs | 12 ++---------- crates/fj-kernel/src/validate/cycle.rs | 1 - crates/fj-kernel/src/validate/edge.rs | 15 ++------------- crates/fj-operations/src/sketch.rs | 1 - 10 files changed, 7 insertions(+), 47 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse/edge.rs b/crates/fj-kernel/src/algorithms/reverse/edge.rs index 721f77dc8..1a9023356 100644 --- a/crates/fj-kernel/src/algorithms/reverse/edge.rs +++ b/crates/fj-kernel/src/algorithms/reverse/edge.rs @@ -19,7 +19,6 @@ impl Reverse for Handle { }; HalfEdge::new( - self.surface().clone(), self.curve().clone(), vertices, self.global_form().clone(), diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 01450aa6d..46db47daf 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -200,7 +200,6 @@ mod tests { }; let side_up = { let mut side_up = PartialHalfEdge { - surface: Partial::from(surface.clone()), ..Default::default() }; @@ -223,7 +222,6 @@ mod tests { }; let top = { let mut top = PartialHalfEdge { - surface: Partial::from(surface.clone()), ..Default::default() }; @@ -252,7 +250,6 @@ mod tests { }; let side_down = { let mut side_down = PartialHalfEdge { - surface: Partial::from(surface.clone()), ..Default::default() }; diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 796769f5e..4c5b7b242 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -15,10 +15,6 @@ impl TransformObject for HalfEdge { objects: &mut Service, cache: &mut TransformCache, ) -> Self { - let surface = self - .surface() - .clone() - .transform_with_cache(transform, objects, cache); let curve = self .curve() .clone() @@ -36,7 +32,7 @@ impl TransformObject for HalfEdge { .clone() .transform_with_cache(transform, objects, cache); - Self::new(surface, curve, boundary, global_form) + Self::new(curve, boundary, global_form) } } diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 5d08f7a11..3673b0ee8 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -149,7 +149,6 @@ impl CycleBuilder for PartialCycle { let [_, vertex] = &mut new_half_edge.vertices; vertex.1 = shared_surface_vertex; - new_half_edge.surface = self.surface.clone(); new_half_edge.infer_global_form(); } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index d2e4d0924..70a7dbaaf 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -124,11 +124,9 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn update_as_line_segment_from_points( &mut self, - surface: impl Into>, + _: impl Into>, points: [impl Into>; 2], ) { - self.surface = surface.into(); - for (vertex, point) in self.vertices.each_mut_ext().zip_ext(points) { let mut surface_form = vertex.1.write(); surface_form.position = Some(point.into()); diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index d216538c2..eed4f9f9a 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -4,14 +4,13 @@ use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ - objects::{Curve, GlobalCurve, GlobalVertex, Surface, SurfaceVertex}, + objects::{Curve, GlobalCurve, GlobalVertex, SurfaceVertex}, storage::{Handle, HandleWrapper}, }; /// A half-edge #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { - surface: Handle, curve: Handle, boundary: [(Point<1>, Handle); 2], global_form: Handle, @@ -20,24 +19,17 @@ pub struct HalfEdge { impl HalfEdge { /// Create an instance of `HalfEdge` pub fn new( - surface: Handle, curve: Handle, boundary: [(Point<1>, Handle); 2], global_form: Handle, ) -> Self { Self { - surface, curve, boundary, global_form, } } - /// Access the surface that the half-edge is defined in - pub fn surface(&self) -> &Handle { - &self.surface - } - /// Access the curve that defines the half-edge's geometry pub fn curve(&self) -> &Handle { &self.curve diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 899ffe2b3..db7ea206b 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -6,7 +6,7 @@ use fj_math::Point; use crate::{ objects::{ Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, - Surface, SurfaceVertex, + SurfaceVertex, }, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, @@ -15,9 +15,6 @@ use crate::{ /// A partial [`HalfEdge`] #[derive(Clone, Debug)] pub struct PartialHalfEdge { - /// The surface that the half-edge is defined in - pub surface: Partial, - /// The curve that the half-edge is defined in pub curve: Partial, @@ -36,7 +33,6 @@ impl PartialObject for PartialHalfEdge { cache: &mut FullToPartialCache, ) -> Self { Self { - surface: Partial::from_full(half_edge.surface().clone(), cache), curve: Partial::from_full(half_edge.curve().clone(), cache), vertices: half_edge .boundary() @@ -55,7 +51,6 @@ impl PartialObject for PartialHalfEdge { } fn build(self, objects: &mut Service) -> Self::Full { - let surface = self.surface.build(objects); let curve = self.curve.build(objects); let vertices = self.vertices.map(|vertex| { let position_curve = vertex @@ -67,14 +62,12 @@ impl PartialObject for PartialHalfEdge { }); let global_form = self.global_form.build(objects); - HalfEdge::new(surface, curve, vertices, global_form) + HalfEdge::new(curve, vertices, global_form) } } impl Default for PartialHalfEdge { fn default() -> Self { - let surface = Partial::new(); - let curve = Partial::::new(); let vertices = array::from_fn(|_| { let surface_form = Partial::default(); @@ -96,7 +89,6 @@ impl Default for PartialHalfEdge { }); Self { - surface, curve, vertices, global_form, diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 07c6f5dfb..264c4acf6 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -329,7 +329,6 @@ mod tests { let boundary = boundary.zip_ext(surface_vertices); HalfEdge::new( - half_edge.surface().clone(), half_edge.curve().clone(), boundary, half_edge.global_form().clone(), diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index a801f2ca9..89310fe5f 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -220,12 +220,7 @@ mod tests { .boundary() .zip_ext(valid.surface_vertices().map(Clone::clone)); - HalfEdge::new( - valid.surface().clone(), - valid.curve().clone(), - vertices, - global_form, - ) + HalfEdge::new(valid.curve().clone(), vertices, global_form) }; valid.validate_and_return_first_error()?; @@ -270,12 +265,7 @@ mod tests { .boundary() .zip_ext(valid.surface_vertices().map(Clone::clone)); - HalfEdge::new( - valid.surface().clone(), - valid.curve().clone(), - vertices, - global_form, - ) + HalfEdge::new(valid.curve().clone(), vertices, global_form) }; valid.validate_and_return_first_error()?; @@ -306,7 +296,6 @@ mod tests { }); HalfEdge::new( - valid.surface().clone(), valid.curve().clone(), vertices, valid.global_form().clone(), diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 97a666c5c..a7d89d7ff 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -31,7 +31,6 @@ impl Shape for fj::Sketch { let half_edge = { let mut half_edge = PartialHalfEdge { - surface: surface.clone(), ..Default::default() }; half_edge.update_as_circle_from_radius(circle.radius()); From 338450cb49a97b5ae64dfdb282310ff2b83dfd09 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 15:24:06 +0100 Subject: [PATCH 20/21] Simplify `PartialHalfEdge` construction --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 12 +++--------- crates/fj-operations/src/sketch.rs | 4 +--- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 46db47daf..cd36fc943 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -199,9 +199,7 @@ mod tests { half_edge }; let side_up = { - let mut side_up = PartialHalfEdge { - ..Default::default() - }; + let mut side_up = PartialHalfEdge::default(); { let [back, front] = side_up @@ -221,9 +219,7 @@ mod tests { side_up }; let top = { - let mut top = PartialHalfEdge { - ..Default::default() - }; + let mut top = PartialHalfEdge::default(); { let [(back, back_surface), (front, front_surface)] = @@ -249,9 +245,7 @@ mod tests { .clone() }; let side_down = { - let mut side_down = PartialHalfEdge { - ..Default::default() - }; + let mut side_down = PartialHalfEdge::default(); let [(back, back_surface), (front, front_surface)] = side_down.vertices.each_mut_ext(); diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index a7d89d7ff..f9e402aeb 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -30,9 +30,7 @@ impl Shape for fj::Sketch { let surface = Partial::from(surface); let half_edge = { - let mut half_edge = PartialHalfEdge { - ..Default::default() - }; + let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_circle_from_radius(circle.radius()); Partial::from_partial(half_edge) From d67ed1ac08f73e3c8120f7bc002c83482b2616bc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 15:26:03 +0100 Subject: [PATCH 21/21] Remove unused argument --- .../src/algorithms/intersect/curve_edge.rs | 22 +++++-------------- crates/fj-kernel/src/algorithms/sweep/edge.rs | 11 +++------- crates/fj-kernel/src/algorithms/sweep/face.rs | 10 ++------- crates/fj-kernel/src/builder/edge.rs | 4 +--- crates/fj-kernel/src/objects/full/edge.rs | 6 ++--- crates/fj-kernel/src/validate/edge.rs | 15 +++---------- 6 files changed, 17 insertions(+), 51 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs index 74f0ffb93..bec4fa99f 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs @@ -92,10 +92,7 @@ mod tests { let curve = curve.build(&mut services.objects); let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[1., -1.], [1., 1.]], - ); + half_edge.update_as_line_segment_from_points([[1., -1.], [1., 1.]]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) @@ -121,10 +118,8 @@ mod tests { let curve = curve.build(&mut services.objects); let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[-1., -1.], [-1., 1.]], - ); + half_edge + .update_as_line_segment_from_points([[-1., -1.], [-1., 1.]]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) @@ -150,10 +145,8 @@ mod tests { let curve = curve.build(&mut services.objects); let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[-1., -1.], [1., -1.]], - ); + half_edge + .update_as_line_segment_from_points([[-1., -1.], [1., -1.]]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) @@ -174,10 +167,7 @@ mod tests { let curve = curve.build(&mut services.objects); let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[-1., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[-1., 0.], [1., 0.]]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index cd36fc943..70a4c1cf1 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -172,10 +172,7 @@ mod tests { let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[0., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[0., 0.], [1., 0.]]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge @@ -191,10 +188,8 @@ mod tests { let bottom = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[0., 0.], [1., 0.]], - ); + half_edge + .update_as_line_segment_from_points([[0., 0.], [1., 0.]]); half_edge }; diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 618923d5a..34a7cfb07 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -200,10 +200,7 @@ mod tests { let side_faces = triangle.array_windows_ext().map(|&[a, b]| { let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [a, b], - ); + half_edge.update_as_line_segment_from_points([a, b]); half_edge .infer_vertex_positions_if_necessary(&surface.geometry()); @@ -270,10 +267,7 @@ mod tests { let side_faces = triangle.array_windows_ext().map(|&[a, b]| { let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [a, b], - ); + half_edge.update_as_line_segment_from_points([a, b]); half_edge .infer_vertex_positions_if_necessary(&surface.geometry()); diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 70a7dbaaf..c4f5b7402 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -6,7 +6,7 @@ use crate::{ path::{GlobalPath, SurfacePath}, surface::SurfaceGeometry, }, - objects::{GlobalEdge, HalfEdge, Surface}, + objects::{GlobalEdge, HalfEdge}, partial::{MaybeSurfacePath, Partial, PartialGlobalEdge, PartialHalfEdge}, }; @@ -28,7 +28,6 @@ pub trait HalfEdgeBuilder { /// Update partial half-edge to be a line segment, from the given points fn update_as_line_segment_from_points( &mut self, - surface: impl Into>, points: [impl Into>; 2], ); @@ -124,7 +123,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn update_as_line_segment_from_points( &mut self, - _: impl Into>, points: [impl Into>; 2], ) { for (vertex, point) in self.vertices.each_mut_ext().zip_ext(points) { diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index eed4f9f9a..5288eec25 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -165,16 +165,14 @@ mod tests { let a_to_b = { let mut half_edge = PartialHalfEdge::default(); - half_edge - .update_as_line_segment_from_points(surface.clone(), [a, b]); + half_edge.update_as_line_segment_from_points([a, b]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; let b_to_a = { let mut half_edge = PartialHalfEdge::default(); - half_edge - .update_as_line_segment_from_points(surface.clone(), [b, a]); + half_edge.update_as_line_segment_from_points([b, a]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 89310fe5f..e0f7e933c 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -200,10 +200,7 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[0., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[0., 0.], [1., 0.]]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) @@ -237,10 +234,7 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[0., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[0., 0.], [1., 0.]]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) @@ -282,10 +276,7 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[0., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[0., 0.], [1., 0.]]); half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects)