From 41e9a0060de97b876d729f5adcab1114328bc443 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 6 Oct 2022 17:08:22 +0200 Subject: [PATCH 1/6] Refactor --- crates/fj-kernel/src/objects/edge.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 938840fb4..92df35cb8 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -33,11 +33,11 @@ impl HalfEdge { /// That would just mean, that ends of the edge connect to each other.) pub fn new( curve: Curve, - vertices: [Vertex; 2], + [a, b]: [Vertex; 2], global_form: GlobalEdge, ) -> Self { // Make sure `curve` and `vertices` match. - for vertex in &vertices { + for vertex in [&a, &b] { assert_eq!( &curve, vertex.curve(), @@ -54,7 +54,7 @@ impl HalfEdge { ); assert_eq!( &normalize_vertex_order( - vertices.clone().map(|vertex| *vertex.global_form()) + [&a, &b].map(|vertex| *vertex.global_form()) ), global_form.vertices_in_normalized_order(), "The global forms of a half-edge's vertices must match the \ @@ -62,7 +62,6 @@ impl HalfEdge { ); // Make sure that the edge vertices are not coincident on the curve. - let [a, b] = &vertices; assert_ne!( a.position(), b.position(), @@ -72,7 +71,7 @@ impl HalfEdge { Self { surface: curve.surface().clone(), curve, - vertices, + vertices: [a, b], global_form, } } From e5cf303f9357ec4a181e835eade0fa60720c9a05 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 6 Oct 2022 17:10:38 +0200 Subject: [PATCH 2/6] Remove redundant argument --- .../fj-kernel/src/algorithms/reverse/edge.rs | 6 +---- crates/fj-kernel/src/algorithms/sweep/edge.rs | 4 ++-- .../fj-kernel/src/algorithms/sweep/vertex.rs | 2 +- .../fj-kernel/src/algorithms/validate/mod.rs | 2 +- crates/fj-kernel/src/objects/edge.rs | 22 ++++++++----------- crates/fj-kernel/src/partial/objects/edge.rs | 2 +- 6 files changed, 15 insertions(+), 23 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse/edge.rs b/crates/fj-kernel/src/algorithms/reverse/edge.rs index f8bcb5f27..8e69a474e 100644 --- a/crates/fj-kernel/src/algorithms/reverse/edge.rs +++ b/crates/fj-kernel/src/algorithms/reverse/edge.rs @@ -9,10 +9,6 @@ impl Reverse for HalfEdge { [b, a] }; - HalfEdge::new( - self.curve().clone(), - vertices, - self.global_form().clone(), - ) + HalfEdge::new(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 613c593b5..b2e19eee0 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -74,7 +74,7 @@ impl Sweep for (HalfEdge, Color) { }) }; - HalfEdge::new(curve, vertices, edge.global_form().clone()) + HalfEdge::new(vertices, edge.global_form().clone()) }; let side_edges = bottom_edge @@ -145,7 +145,7 @@ impl Sweep for (HalfEdge, Color) { }) }; - HalfEdge::new(curve, vertices, global) + HalfEdge::new(vertices, global) }; let cycle = { diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index 2a4f05ba1..6db7bc9eb 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -121,7 +121,7 @@ impl Sweep for (Vertex, Handle) { // And finally, creating the output `Edge` is just a matter of // assembling the pieces we've already created. - HalfEdge::new(curve, vertices, edge_global) + HalfEdge::new(vertices, edge_global) } } diff --git a/crates/fj-kernel/src/algorithms/validate/mod.rs b/crates/fj-kernel/src/algorithms/validate/mod.rs index d922851ec..eca7e9f9e 100644 --- a/crates/fj-kernel/src/algorithms/validate/mod.rs +++ b/crates/fj-kernel/src/algorithms/validate/mod.rs @@ -217,7 +217,7 @@ mod tests { let global_edge = GlobalEdge::partial() .from_curve_and_vertices(&curve, &vertices) .build(&stores); - let half_edge = HalfEdge::new(curve, vertices, global_edge); + let half_edge = HalfEdge::new(vertices, global_edge); let result = half_edge.clone().validate_with_config(&ValidationConfig { diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 92df35cb8..ad7afc2f5 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -31,19 +31,15 @@ impl HalfEdge { /// were, the edge would have no length, and thus not be valid. (It is /// perfectly fine for global forms of the the vertices to be coincident. /// That would just mean, that ends of the edge connect to each other.) - pub fn new( - curve: Curve, - [a, b]: [Vertex; 2], - global_form: GlobalEdge, - ) -> Self { + pub fn new([a, b]: [Vertex; 2], global_form: GlobalEdge) -> Self { // Make sure `curve` and `vertices` match. - for vertex in [&a, &b] { - assert_eq!( - &curve, - vertex.curve(), - "An edge and its vertices must be defined on the same curve" - ); - } + assert_eq!( + a.curve(), + b.curve(), + "An edge's vertices must be defined in the same curve", + ); + + let curve = a.curve(); // Make sure `curve` and `vertices` match `global_form`. assert_eq!( @@ -70,7 +66,7 @@ impl HalfEdge { Self { surface: curve.surface().clone(), - curve, + curve: curve.clone(), vertices: [a, b], global_form, } diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 47b9b5c57..b1663744a 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -208,7 +208,7 @@ impl PartialHalfEdge { }) .into_full(stores); - HalfEdge::new(curve, vertices, global_form) + HalfEdge::new(vertices, global_form) } } From 3ffd5f466678df22d32b28480f7471ff5acccc87 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 6 Oct 2022 17:11:17 +0200 Subject: [PATCH 3/6] Remove redundant struct field --- crates/fj-kernel/src/objects/edge.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index ad7afc2f5..2a4747eb6 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -9,7 +9,6 @@ use super::{Curve, GlobalCurve, GlobalVertex, Surface, Vertex}; /// A half-edge #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { - surface: Handle, curve: Curve, vertices: [Vertex; 2], global_form: GlobalEdge, @@ -65,7 +64,6 @@ impl HalfEdge { ); Self { - surface: curve.surface().clone(), curve: curve.clone(), vertices: [a, b], global_form, @@ -74,7 +72,7 @@ impl HalfEdge { /// Access the surface that the half-edge's [`Curve`] is defined on pub fn surface(&self) -> &Handle { - &self.surface + self.curve().surface() } /// Access the curve that defines the half-edge's geometry From 07e6002095f1e4f25d0c19674e1039f87c829846 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 6 Oct 2022 17:14:13 +0200 Subject: [PATCH 4/6] Remove redundant struct field --- crates/fj-kernel/src/objects/edge.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 2a4747eb6..43ebc5d26 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -9,7 +9,6 @@ use super::{Curve, GlobalCurve, GlobalVertex, Surface, Vertex}; /// A half-edge #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { - curve: Curve, vertices: [Vertex; 2], global_form: GlobalEdge, } @@ -64,7 +63,6 @@ impl HalfEdge { ); Self { - curve: curve.clone(), vertices: [a, b], global_form, } @@ -81,7 +79,8 @@ impl HalfEdge { /// or if the curve is continuous (i.e. connects to itself), the edge could /// be defined by the whole curve, and have no bounding vertices. pub fn curve(&self) -> &Curve { - &self.curve + let [vertex, _] = self.vertices(); + vertex.curve() } /// Access the vertices that bound the half-edge on the curve From 6666ec859b011c69241e67624f020c0fff2dac0d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 6 Oct 2022 17:15:27 +0200 Subject: [PATCH 5/6] Remove redundant method --- crates/fj-kernel/src/objects/cycle.rs | 2 +- crates/fj-kernel/src/objects/edge.rs | 7 +------ crates/fj-kernel/src/partial/objects/edge.rs | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/objects/cycle.rs b/crates/fj-kernel/src/objects/cycle.rs index d7d219b02..5c82a5459 100644 --- a/crates/fj-kernel/src/objects/cycle.rs +++ b/crates/fj-kernel/src/objects/cycle.rs @@ -30,7 +30,7 @@ impl Cycle { for edge in &half_edges { assert_eq!( surface.id(), - edge.surface().id(), + edge.curve().surface().id(), "Edges in cycle not defined in same surface" ); } diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 43ebc5d26..02b3c5ad9 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -4,7 +4,7 @@ use pretty_assertions::{assert_eq, assert_ne}; use crate::stores::{Handle, HandleWrapper}; -use super::{Curve, GlobalCurve, GlobalVertex, Surface, Vertex}; +use super::{Curve, GlobalCurve, GlobalVertex, Vertex}; /// A half-edge #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] @@ -68,11 +68,6 @@ impl HalfEdge { } } - /// Access the surface that the half-edge's [`Curve`] is defined on - pub fn surface(&self) -> &Handle { - self.curve().surface() - } - /// Access the curve that defines the half-edge's geometry /// /// The edge can be a segment of the curve that is bounded by two vertices, diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index b1663744a..e1401d4ab 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -215,7 +215,7 @@ impl PartialHalfEdge { impl From<&HalfEdge> for PartialHalfEdge { fn from(half_edge: &HalfEdge) -> Self { Self { - surface: Some(half_edge.surface().clone()), + surface: Some(half_edge.curve().surface().clone()), curve: Some(half_edge.curve().clone().into()), vertices: Some(half_edge.vertices().clone().map(Into::into)), global_form: Some(half_edge.global_form().clone().into()), From 0e19c59a7791d54fce34e7a1297ca7cb5de05313 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 6 Oct 2022 17:16:13 +0200 Subject: [PATCH 6/6] Update outdated doc comments --- crates/fj-kernel/src/objects/edge.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 02b3c5ad9..e99ccd925 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -69,20 +69,12 @@ impl HalfEdge { } /// Access the curve that defines the half-edge's geometry - /// - /// The edge can be a segment of the curve that is bounded by two vertices, - /// or if the curve is continuous (i.e. connects to itself), the edge could - /// be defined by the whole curve, and have no bounding vertices. pub fn curve(&self) -> &Curve { let [vertex, _] = self.vertices(); vertex.curve() } /// Access the vertices that bound the half-edge on the curve - /// - /// An edge has either two bounding vertices or none. The latter is possible - /// if the edge's curve is continuous (i.e. connects to itself), and defines - /// the whole edge. pub fn vertices(&self) -> &[Vertex; 2] { &self.vertices }