From 47914a863ea2e2c7938d1ff1383e4ee030c5ac08 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 11:48:46 +0100 Subject: [PATCH 01/14] Remove validation check The next commit is going to make it redundant. --- crates/fj-kernel/src/validate/edge.rs | 107 +------------------------- 1 file changed, 2 insertions(+), 105 deletions(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 8aba4e6149..bacc0fd616 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -1,9 +1,6 @@ use fj_math::{Point, Scalar}; -use crate::{ - objects::{GlobalEdge, HalfEdge, Vertex}, - storage::Handle, -}; +use crate::objects::{GlobalEdge, HalfEdge}; use super::{Validate, ValidationConfig, ValidationError}; @@ -13,7 +10,6 @@ impl Validate for HalfEdge { config: &ValidationConfig, errors: &mut Vec, ) { - HalfEdgeValidationError::check_global_vertex_identity(self, errors); HalfEdgeValidationError::check_vertex_coincidence(self, config, errors); } } @@ -30,24 +26,6 @@ impl Validate for GlobalEdge { /// [`HalfEdge`] validation failed #[derive(Clone, Debug, thiserror::Error)] pub enum HalfEdgeValidationError { - /// [`HalfEdge`]'s [`Vertex`] objects do not match - #[error( - "`HalfEdge` vertices don't match vertices of `HalfEdge`'s global form\n\ - - Start vertex: {vertex_from_half_edge:#?}\n\ - - Vertices from `GlobalEdge`: {vertices_from_global_form:#?}\n\ - - `HalfEdge`: {half_edge:#?}" - )] - VertexMismatch { - /// The [`Vertex`] from the [`HalfEdge`]'s start vertex - vertex_from_half_edge: Handle, - - /// The [`Vertex`] instances from the [`HalfEdge`]'s global form - vertices_from_global_form: [Handle; 2], - - /// The half-edge - half_edge: HalfEdge, - }, - /// [`HalfEdge`]'s vertices are coincident #[error( "Vertices of `HalfEdge` on curve are coincident\n\ @@ -71,33 +49,6 @@ pub enum HalfEdgeValidationError { } impl HalfEdgeValidationError { - fn check_global_vertex_identity( - half_edge: &HalfEdge, - errors: &mut Vec, - ) { - let vertex_from_half_edge = half_edge.start_vertex().clone(); - let vertices_from_global_form = half_edge - .global_form() - .vertices() - .access_in_normalized_order(); - - let matching_vertex = - vertices_from_global_form.iter().find(|global_vertex| { - global_vertex.id() == vertex_from_half_edge.id() - }); - - if matching_vertex.is_none() { - errors.push( - Self::VertexMismatch { - vertex_from_half_edge, - vertices_from_global_form, - half_edge: half_edge.clone(), - } - .into(), - ); - } - } - fn check_vertex_coincidence( half_edge: &HalfEdge, config: &ValidationConfig, @@ -127,65 +78,11 @@ mod tests { use crate::{ builder::{CycleBuilder, HalfEdgeBuilder}, objects::HalfEdge, - partial::{Partial, PartialCycle}, + partial::PartialCycle, services::Services, validate::{HalfEdgeValidationError, Validate, ValidationError}, }; - #[test] - fn vertex_mismatch() -> anyhow::Result<()> { - let mut services = Services::new(); - - let valid = { - let surface = services.objects.surfaces.xy_plane(); - - let mut cycle = PartialCycle::default(); - - let [mut half_edge, next_half_edge, _] = cycle - .update_as_polygon_from_points([[0., 0.], [1., 0.], [1., 1.]]); - half_edge.write().infer_vertex_positions_if_necessary( - &surface.geometry(), - next_half_edge.read().start_vertex.clone(), - ); - - half_edge.build(&mut services.objects) - }; - let invalid = { - let global_form = { - let mut global_edge = - Partial::from(valid.global_form().clone()); - global_edge.write().vertices = valid - .global_form() - .vertices() - .access_in_normalized_order() - // Creating equal but not identical vertices here. - .map(|vertex| { - Partial::from_partial( - Partial::from(vertex).read().clone(), - ) - }); - global_edge.build(&mut services.objects) - }; - - HalfEdge::new( - valid.curve(), - valid.boundary(), - valid.start_vertex().clone(), - global_form, - ) - }; - - valid.validate_and_return_first_error()?; - assert!(matches!( - invalid.validate_and_return_first_error(), - Err(ValidationError::HalfEdge( - HalfEdgeValidationError::VertexMismatch { .. } - )) - )); - - Ok(()) - } - #[test] fn half_edge_vertices_are_coincident() -> anyhow::Result<()> { let mut services = Services::new(); From 0732ac192522f232dd1ceff30af7744332c46c96 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 11:56:09 +0100 Subject: [PATCH 02/14] Remove references to `Vertex` from `GlobalEdge` They were redundant, as they are already available through `HalfEdge`. --- .../fj-kernel/src/algorithms/sweep/vertex.rs | 2 +- .../src/algorithms/transform/edge.rs | 13 +++------ crates/fj-kernel/src/builder/edge.rs | 8 +----- crates/fj-kernel/src/objects/full/edge.rs | 22 +++------------ crates/fj-kernel/src/partial/objects/edge.rs | 28 ++++--------------- 5 files changed, 16 insertions(+), 57 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index e0af7cbf2b..9ba95787ae 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -28,7 +28,7 @@ impl Sweep for Handle { .clone(); let vertices = [a, b]; - let global_edge = GlobalEdge::new(vertices.clone()).insert(objects); + let global_edge = GlobalEdge::new().insert(objects); // The vertices of the returned `GlobalEdge` are in normalized order, // which means the order can't be relied upon by the caller. Return the diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index a94fc85019..169624ebc4 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -34,15 +34,10 @@ impl TransformObject for HalfEdge { impl TransformObject for GlobalEdge { fn transform_with_cache( self, - transform: &Transform, - objects: &mut Service, - cache: &mut TransformCache, + _: &Transform, + _: &mut Service, + _: &mut TransformCache, ) -> Self { - let vertices = - self.vertices().access_in_normalized_order().map(|vertex| { - vertex.transform_with_cache(transform, objects, cache) - }); - - Self::new(vertices) + Self::new() } } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 35f2f65512..ae6a588a38 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -151,13 +151,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { path } - fn infer_global_form( - &mut self, - next_vertex: Partial, - ) -> Partial { - self.global_form.write().vertices = - [&self.start_vertex, &next_vertex].map(|vertex| vertex.clone()); - + fn infer_global_form(&mut self, _: Partial) -> Partial { self.global_form.clone() } diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 610731cb5b..644c7864ad 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -102,10 +102,8 @@ impl HalfEdge { /// /// See [`HalfEdge`]'s documentation for more information on the relationship /// between [`HalfEdge`] and `GlobalEdge`. -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct GlobalEdge { - vertices: VerticesInNormalizedOrder, -} +#[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct GlobalEdge {} impl GlobalEdge { /// Create a new instance @@ -113,20 +111,8 @@ impl GlobalEdge { /// The order of `vertices` is irrelevant. Two `GlobalEdge`s with the same /// `curve` and `vertices` will end up being equal, regardless of the order /// of `vertices` here. - pub fn new(vertices: [Handle; 2]) -> Self { - let (vertices, _) = VerticesInNormalizedOrder::new(vertices); - - Self { vertices } - } - - /// Access the vertices that bound the edge on the curve - /// - /// As the name indicates, the order of the returned vertices is normalized - /// and might not match the order of the vertices that were passed to - /// [`GlobalEdge::new`]. You must not rely on the vertices being in any - /// specific order. - pub fn vertices(&self) -> &VerticesInNormalizedOrder { - &self.vertices + pub fn new() -> Self { + Self::default() } } diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 592f4fd9a2..cd5a18e4ca 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -86,11 +86,7 @@ impl Default for PartialHalfEdge { fn default() -> Self { let curve = None; let start_vertex = Partial::default(); - let end_vertex = Partial::default(); - - let global_form = Partial::from_partial(PartialGlobalEdge { - vertices: [start_vertex.clone(), end_vertex], - }); + let global_form = Partial::from_partial(PartialGlobalEdge {}); Self { curve, @@ -103,28 +99,16 @@ impl Default for PartialHalfEdge { /// A partial [`GlobalEdge`] #[derive(Clone, Debug, Default)] -pub struct PartialGlobalEdge { - /// The vertices that bound the edge on the curve - pub vertices: [Partial; 2], -} +pub struct PartialGlobalEdge {} impl PartialObject for PartialGlobalEdge { type Full = GlobalEdge; - fn from_full( - global_edge: &Self::Full, - cache: &mut FullToPartialCache, - ) -> Self { - Self { - vertices: global_edge - .vertices() - .access_in_normalized_order() - .map(|vertex| Partial::from_full(vertex, cache)), - } + fn from_full(_: &Self::Full, _: &mut FullToPartialCache) -> Self { + Self {} } - fn build(self, objects: &mut Service) -> Self::Full { - let vertices = self.vertices.map(|vertex| vertex.build(objects)); - GlobalEdge::new(vertices) + fn build(self, _: &mut Service) -> Self::Full { + GlobalEdge::new() } } From 7bd08836e7a4d734c6913d5519986f88053f1d2f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 11:58:22 +0100 Subject: [PATCH 03/14] Remove unused `VerticesInNormalizedOrder` --- crates/fj-kernel/src/objects/full/edge.rs | 30 ----------------------- crates/fj-kernel/src/objects/mod.rs | 2 +- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 644c7864ad..673cb303cc 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -116,36 +116,6 @@ impl GlobalEdge { } } -/// The vertices of a [`GlobalEdge`] -/// -/// Since [`GlobalEdge`] is the single global representation of an edge in -/// global space, it must normalize the order of its vertices. Otherwise, it is -/// possible to construct two [`GlobalEdge`] instances that are meant to -/// represent the same edge, but aren't equal. -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct VerticesInNormalizedOrder([Handle; 2]); - -impl VerticesInNormalizedOrder { - /// Construct a new instance of `VerticesInNormalizedOrder` - /// - /// The provided vertices can be in any order. The returned `bool` value - /// indicates whether the normalization changed the order of the vertices. - pub fn new([a, b]: [Handle; 2]) -> (Self, bool) { - if a < b { - (Self([a, b]), false) - } else { - (Self([b, a]), true) - } - } - - /// Access the vertices - /// - /// The vertices in the returned array will be in normalized order. - pub fn access_in_normalized_order(&self) -> [Handle; 2] { - self.0.clone() - } -} - #[cfg(test)] mod tests { use pretty_assertions::assert_eq; diff --git a/crates/fj-kernel/src/objects/mod.rs b/crates/fj-kernel/src/objects/mod.rs index 319425dbb2..6c19fa57f6 100644 --- a/crates/fj-kernel/src/objects/mod.rs +++ b/crates/fj-kernel/src/objects/mod.rs @@ -80,7 +80,7 @@ mod stores; pub use self::{ full::{ cycle::{Cycle, HalfEdgesOfCycle}, - edge::{GlobalEdge, HalfEdge, VerticesInNormalizedOrder}, + edge::{GlobalEdge, HalfEdge}, face::{Face, FaceSet, Handedness}, shell::Shell, sketch::Sketch, From f80bf317b4c1eef347c867fc27118f72c4925a70 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:00:39 +0100 Subject: [PATCH 04/14] Use `HandleWrapper` to refer to `GlobalEdge` With its references to `Vertex` removed, the `Eq`/`PartialEq`/`Ord`/ `PartialOrd` implementations of `GlobalEdge` are misleading and about to be removed. `HandleWrapper` is needed here, so `HalfEdge` can still derive those traits itself. --- crates/fj-kernel/src/objects/full/edge.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 673cb303cc..414f493d22 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -1,6 +1,10 @@ use fj_math::Point; -use crate::{geometry::curve::Curve, objects::Vertex, storage::Handle}; +use crate::{ + geometry::curve::Curve, + objects::Vertex, + storage::{Handle, HandleWrapper}, +}; /// A directed edge, defined in a surface's 2D space /// @@ -43,7 +47,7 @@ pub struct HalfEdge { curve: Curve, boundary: [Point<1>; 2], start_vertex: Handle, - global_form: Handle, + global_form: HandleWrapper, } impl HalfEdge { @@ -58,7 +62,7 @@ impl HalfEdge { curve, boundary, start_vertex, - global_form, + global_form: global_form.into(), } } From 39f000ac016a9bd6203f578ab173e3428939497a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:04:34 +0100 Subject: [PATCH 05/14] Remove redundant test It used to test the normalized ordering of `GlobalEdge`'s references to `Vertex`, so it is no longer relevant. --- crates/fj-kernel/src/objects/full/edge.rs | 49 ----------------------- 1 file changed, 49 deletions(-) diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 414f493d22..7a5a283fbe 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -119,52 +119,3 @@ impl GlobalEdge { Self::default() } } - -#[cfg(test)] -mod tests { - use pretty_assertions::assert_eq; - - use crate::{ - builder::{CycleBuilder, HalfEdgeBuilder}, - partial::PartialCycle, - services::Services, - }; - - #[test] - fn global_edge_equality() { - let mut services = Services::new(); - - let surface = services.objects.surfaces.xy_plane(); - - let a = [0., 0.]; - let b = [1., 0.]; - let c = [0., 1.]; - - let a_to_b = { - let mut cycle = PartialCycle::default(); - - let [mut half_edge, next_half_edge, _] = - cycle.update_as_polygon_from_points([a, b, c]); - half_edge.write().infer_vertex_positions_if_necessary( - &surface.geometry(), - next_half_edge.read().start_vertex.clone(), - ); - - half_edge.build(&mut services.objects) - }; - let b_to_a = { - let mut cycle = PartialCycle::default(); - - let [mut half_edge, next_half_edge, _] = - cycle.update_as_polygon_from_points([b, a, c]); - half_edge.write().infer_vertex_positions_if_necessary( - &surface.geometry(), - next_half_edge.read().start_vertex.clone(), - ); - - half_edge.build(&mut services.objects) - }; - - assert_eq!(a_to_b.global_form(), b_to_a.global_form()); - } -} From eac95c44706fc3d8b77ef326af7ade5a280bc66a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:05:56 +0100 Subject: [PATCH 06/14] Remove misleading trait implementations --- crates/fj-kernel/src/objects/full/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 7a5a283fbe..71a084927d 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -106,7 +106,7 @@ impl HalfEdge { /// /// See [`HalfEdge`]'s documentation for more information on the relationship /// between [`HalfEdge`] and `GlobalEdge`. -#[derive(Clone, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] +#[derive(Clone, Debug, Default, Hash)] pub struct GlobalEdge {} impl GlobalEdge { From f741cd7cb00c903289d97253238a8010ca98eb69 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:07:00 +0100 Subject: [PATCH 07/14] Remove reference to vertices from `GlobalEdge` doc --- crates/fj-kernel/src/objects/full/edge.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 71a084927d..aa41304c4c 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -100,9 +100,8 @@ impl HalfEdge { /// An undirected edge, defined in global (3D) coordinates /// /// In contrast to [`HalfEdge`], `GlobalEdge` is undirected, meaning it has no -/// defined direction, and its vertices have no defined order. This means it can -/// be used to determine whether two [`HalfEdge`]s map to the same `GlobalEdge`, -/// regardless of their direction. +/// defined direction. This means it can be used to determine whether two +/// [`HalfEdge`]s map to the same `GlobalEdge`, regardless of their direction. /// /// See [`HalfEdge`]'s documentation for more information on the relationship /// between [`HalfEdge`] and `GlobalEdge`. From dc70af941e30878ca53923fa7680d4b8a026e8b5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:10:04 +0100 Subject: [PATCH 08/14] Remove redundant calls to builder function --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 5 +---- crates/fj-kernel/src/builder/edge.rs | 3 --- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 9f8c5602a8..944e8544b4 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -111,7 +111,7 @@ impl Sweep for (Handle, &Handle, &Surface, Color) { // even if the original edge was a circle, it's still going to be a line // when projected into the new surface. For the side edges, because // we're sweeping along a straight path. - for ((mut half_edge, start), (next_half_edge, end)) in [ + for ((mut half_edge, start), (_, end)) in [ edge_bottom.clone(), edge_up.clone(), edge_top.clone(), @@ -122,9 +122,6 @@ impl Sweep for (Handle, &Handle, &Surface, Color) { .circular_tuple_windows() { half_edge.write().update_as_line_segment(start, end); - half_edge - .write() - .infer_global_form(next_half_edge.read().start_vertex.clone()); } // Finally, we can make sure that all edges refer to the correct global diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index ae6a588a38..5a8a5838a5 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -88,9 +88,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { *point_boundary = Some(point_curve); } - let next_vertex = self.start_vertex.clone(); - self.infer_global_form(next_vertex); - path } From 7eaad4503caf76d4291ce2afefeec22fe553755d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:12:26 +0100 Subject: [PATCH 09/14] Remove redundant code --- crates/fj-kernel/src/builder/cycle.rs | 34 +-------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index feed2ae9de..5b6c5a0874 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -54,39 +54,7 @@ pub trait CycleBuilder { impl CycleBuilder for PartialCycle { fn add_half_edge(&mut self) -> Partial { - let mut new_half_edge = Partial::::new(); - - let (first_half_edge, mut last_half_edge) = - match self.half_edges.first() { - Some(first_half_edge) => { - let first_half_edge = first_half_edge.clone(); - let last_half_edge = self - .half_edges - .last() - .cloned() - .unwrap_or_else(|| first_half_edge.clone()); - - (first_half_edge, last_half_edge) - } - None => (new_half_edge.clone(), new_half_edge.clone()), - }; - - { - let shared_surface_vertex = - new_half_edge.read().start_vertex.clone(); - last_half_edge - .write() - .infer_global_form(shared_surface_vertex); - } - - { - let shared_surface_vertex = - first_half_edge.read().start_vertex.clone(); - new_half_edge - .write() - .infer_global_form(shared_surface_vertex); - } - + let new_half_edge = Partial::::new(); self.half_edges.push(new_half_edge.clone()); new_half_edge } From 6b33d69153fe663b7f4343f098f1f1dcc1274016 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:13:08 +0100 Subject: [PATCH 10/14] Remove redundant type ascription --- crates/fj-kernel/src/builder/cycle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 5b6c5a0874..a5887b4d02 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -54,7 +54,7 @@ pub trait CycleBuilder { impl CycleBuilder for PartialCycle { fn add_half_edge(&mut self) -> Partial { - let new_half_edge = Partial::::new(); + let new_half_edge = Partial::new(); self.half_edges.push(new_half_edge.clone()); new_half_edge } From 56b337047a1f5082137bdd567c2920ac35742538 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:14:38 +0100 Subject: [PATCH 11/14] Simplify variable name --- crates/fj-kernel/src/builder/cycle.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index a5887b4d02..ddec16daa1 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -54,9 +54,9 @@ pub trait CycleBuilder { impl CycleBuilder for PartialCycle { fn add_half_edge(&mut self) -> Partial { - let new_half_edge = Partial::new(); - self.half_edges.push(new_half_edge.clone()); - new_half_edge + let half_edge = Partial::new(); + self.half_edges.push(half_edge.clone()); + half_edge } fn update_as_polygon_from_points( From 50f3a782203a11c1bc6cc73b5b9b769bd426a69c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:20:30 +0100 Subject: [PATCH 12/14] Remove unused and redundant builder method --- crates/fj-kernel/src/builder/edge.rs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 5a8a5838a5..571878f229 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -6,7 +6,7 @@ use crate::{ curve::{Curve, GlobalPath}, surface::SurfaceGeometry, }, - objects::{GlobalEdge, HalfEdge, Vertex}, + objects::{HalfEdge, Vertex}, partial::{MaybeCurve, Partial, PartialGlobalEdge, PartialHalfEdge}, }; @@ -38,15 +38,6 @@ pub trait HalfEdgeBuilder { end: Point<2>, ) -> Curve; - /// Infer the global form of the half-edge - /// - /// Updates the global form referenced by this half-edge, and also returns - /// it. - fn infer_global_form( - &mut self, - next_vertex: Partial, - ) -> Partial; - /// Infer the vertex positions (surface and global), if not already set fn infer_vertex_positions_if_necessary( &mut self, @@ -148,10 +139,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { path } - fn infer_global_form(&mut self, _: Partial) -> Partial { - self.global_form.clone() - } - fn infer_vertex_positions_if_necessary( &mut self, surface: &SurfaceGeometry, From 7b831de583ff3a27fbd30ddcd187d944b068a64f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:23:13 +0100 Subject: [PATCH 13/14] Add comment --- crates/fj-kernel/src/algorithms/transform/edge.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 169624ebc4..7ea2de3767 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -38,6 +38,9 @@ impl TransformObject for GlobalEdge { _: &mut Service, _: &mut TransformCache, ) -> Self { + // There's nothing to actually transform here, as `GlobalEdge` holds no + // data. We still need this implementation though, as a new `GlobalEdge` + // must be created to represent the new and transformed edge. Self::new() } } From 208623e14635c19bbc5f560daed0cfcfa9e41ad4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 3 Mar 2023 12:24:56 +0100 Subject: [PATCH 14/14] Simplify `Partial` construction --- crates/fj-kernel/src/partial/objects/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index cd5a18e4ca..4b49eb84d9 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -86,7 +86,7 @@ impl Default for PartialHalfEdge { fn default() -> Self { let curve = None; let start_vertex = Partial::default(); - let global_form = Partial::from_partial(PartialGlobalEdge {}); + let global_form = Partial::default(); Self { curve,