From 14f9a3ac7e63187e797e6ee138c97b74f76e60ef Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 6 Mar 2023 16:40:06 +0100 Subject: [PATCH 1/8] Fix constructor name --- crates/fj-kernel/src/builder/edge.rs | 2 +- crates/fj-kernel/src/geometry/curve.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index d86e1fe53..a5cc3e0f7 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -112,7 +112,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { let path = if let [Some(start), Some(end)] = boundary { let points = [start, end].zip_ext(points_surface); - let path = Curve::from_points_with_line_coords(points); + let path = Curve::line_from_points_with_coords(points); self.curve = Some(path.into()); path diff --git a/crates/fj-kernel/src/geometry/curve.rs b/crates/fj-kernel/src/geometry/curve.rs index fe4cb7d22..bb07226bd 100644 --- a/crates/fj-kernel/src/geometry/curve.rs +++ b/crates/fj-kernel/src/geometry/curve.rs @@ -57,7 +57,7 @@ impl Curve { } /// Create a line from two points that include line coordinates - pub fn from_points_with_line_coords( + pub fn line_from_points_with_coords( points: [(impl Into>, impl Into>); 2], ) -> Self { Self::Line(Line::from_points_with_line_coords(points)) From 5548436ecb1f06056e661051539253b822a1a41f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 6 Mar 2023 16:50:31 +0100 Subject: [PATCH 2/8] Inline redundant variable --- crates/fj-kernel/src/builder/edge.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index a5cc3e0f7..55b36ccea 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -106,10 +106,9 @@ impl HalfEdgeBuilder for PartialHalfEdge { start: Point<2>, end: Point<2>, ) -> Curve { - let boundary = self.boundary; let points_surface = [start, end]; - let path = if let [Some(start), Some(end)] = boundary { + let path = if let [Some(start), Some(end)] = self.boundary { let points = [start, end].zip_ext(points_surface); let path = Curve::line_from_points_with_coords(points); From 526a4ec10138a0434c1b334c671a5fd09ba02428 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 8 Mar 2023 13:00:33 +0100 Subject: [PATCH 3/8] Simplify undefined curve tracking --- crates/fj-kernel/src/builder/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 55b36ccea..775689d22 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -197,7 +197,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { | MaybeCurve::UndefinedLine => { // The other edge is a line segment on a plane. That // means our edge must be a line segment too. - Some(MaybeCurve::UndefinedLine) + None } _ => { // The other edge is a circle or arc on a plane. I'm From 453d6604181fdbac7abcb1d09a8648547937ffc2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 8 Mar 2023 13:01:26 +0100 Subject: [PATCH 4/8] Remove unused enum variant --- crates/fj-kernel/src/builder/edge.rs | 6 ++---- crates/fj-kernel/src/partial/objects/edge.rs | 3 --- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 775689d22..5434a0d46 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -148,8 +148,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { // represented using our current curve/surface // representation. match path { - MaybeCurve::Defined(Curve::Line(_)) - | MaybeCurve::UndefinedLine => { + MaybeCurve::Defined(Curve::Line(_)) => { // We're dealing with a line on a rounded surface. // // Based on the current uses of this method, we can @@ -193,8 +192,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { GlobalPath::Line(_) => { // The other edge is defined on a plane. match path { - MaybeCurve::Defined(Curve::Line(_)) - | MaybeCurve::UndefinedLine => { + MaybeCurve::Defined(Curve::Line(_)) => { // The other edge is a line segment on a plane. That // means our edge must be a line segment too. None diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 140c75dcd..e56b36a17 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -94,9 +94,6 @@ pub enum MaybeCurve { /// The radius of the undefined circle radius: Scalar, }, - - /// The curve is undefined, but we know it is a line - UndefinedLine, } impl From for MaybeCurve { From 8bd21b2ae1fb36b2d2fc122f658dc30bb303651a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 8 Mar 2023 13:00:33 +0100 Subject: [PATCH 5/8] Simplify undefined curve tracking --- crates/fj-kernel/src/builder/edge.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 5434a0d46..eec80ce15 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -142,7 +142,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { // need to use that to interpret what the other edge's curve path // means for our curve path. match surface.u { - GlobalPath::Circle(circle) => { + GlobalPath::Circle(_) => { // The other surface is curved. We're entering some dodgy // territory here, as only some edge cases can be // represented using our current curve/surface @@ -175,9 +175,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { // I hope that I'll come up with a better curve/ // surface representation before this becomes a // problem. - Some(MaybeCurve::UndefinedCircle { - radius: circle.radius(), - }) + None } _ => { // The other edge is a line segment in a curved From 731df3dd79f497f77f8dfde5be0dae2c8fbe2879 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 8 Mar 2023 13:03:59 +0100 Subject: [PATCH 6/8] Remove unused enum variant --- crates/fj-kernel/src/partial/objects/edge.rs | 25 ++++---------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index e56b36a17..f26fb2631 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -1,4 +1,4 @@ -use fj_math::{Point, Scalar}; +use fj_math::Point; use crate::{ geometry::curve::Curve, @@ -36,11 +36,8 @@ impl PartialHalfEdge { start.and_then(|start| { let curve = self.curve?; - if let MaybeCurve::Defined(curve) = curve { - return Some(curve.point_from_path_coords(start)); - } - - None + let MaybeCurve::Defined(curve) = curve; + Some(curve.point_from_path_coords(start)) }) } } @@ -67,14 +64,8 @@ impl PartialObject for PartialHalfEdge { } fn build(self, _: &mut Service) -> Self::Full { - let curve = match self.curve.expect("Need path to build curve") { - MaybeCurve::Defined(path) => path, - undefined => { - panic!( - "Trying to build curve with undefined path: {undefined:?}" - ) - } - }; + let MaybeCurve::Defined(curve) = + self.curve.expect("Need path to build curve"); let boundary = self.boundary.map(|point| { point.expect("Can't build `HalfEdge` without boundary positions") }); @@ -88,12 +79,6 @@ impl PartialObject for PartialHalfEdge { pub enum MaybeCurve { /// The curve is fully defined Defined(Curve), - - /// The curve is undefined, but we know it is a circle - UndefinedCircle { - /// The radius of the undefined circle - radius: Scalar, - }, } impl From for MaybeCurve { From 46ab64e2608e38e048f768b4adfab8ab175a5650 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 8 Mar 2023 13:05:46 +0100 Subject: [PATCH 7/8] Simplify `curve` field of `PartialHalfEdge` --- crates/fj-kernel/src/algorithms/sweep/face.rs | 2 +- crates/fj-kernel/src/builder/edge.rs | 14 +++++++------- crates/fj-kernel/src/partial/objects/edge.rs | 9 +++------ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 4e2e6fbb5..9ed217a3a 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -99,7 +99,7 @@ impl Sweep for Handle { .into_iter() .zip(top_cycle.write().half_edges.iter_mut()) { - top.write().curve = Some(bottom.curve().into()); + top.write().curve = Some(bottom.curve()); let boundary = bottom.boundary(); diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index eec80ce15..4b7febb57 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -7,7 +7,7 @@ use crate::{ surface::SurfaceGeometry, }, objects::HalfEdge, - partial::{MaybeCurve, Partial, PartialHalfEdge}, + partial::{Partial, PartialHalfEdge}, }; /// Builder API for [`PartialHalfEdge`] @@ -61,7 +61,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { radius: impl Into, ) -> Curve { let path = Curve::circle_from_radius(radius); - self.curve = Some(path.into()); + self.curve = Some(path); let [a_curve, b_curve] = [Scalar::ZERO, Scalar::TAU].map(|coord| Point::from([coord])); @@ -89,7 +89,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { let arc = fj_math::Arc::from_endpoints_and_angle(start, end, angle_rad); let path = Curve::circle_from_center_and_radius(arc.center, arc.radius); - self.curve = Some(path.into()); + self.curve = Some(path); let [a_curve, b_curve] = [arc.start_angle, arc.end_angle].map(|coord| Point::from([coord])); @@ -112,12 +112,12 @@ impl HalfEdgeBuilder for PartialHalfEdge { let points = [start, end].zip_ext(points_surface); let path = Curve::line_from_points_with_coords(points); - self.curve = Some(path.into()); + self.curve = Some(path); path } else { let (path, _) = Curve::line_from_points(points_surface); - self.curve = Some(path.into()); + self.curve = Some(path); for (vertex, position) in self.boundary.each_mut_ext().zip_ext([0., 1.]) @@ -148,7 +148,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { // represented using our current curve/surface // representation. match path { - MaybeCurve::Defined(Curve::Line(_)) => { + Curve::Line(_) => { // We're dealing with a line on a rounded surface. // // Based on the current uses of this method, we can @@ -190,7 +190,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { GlobalPath::Line(_) => { // The other edge is defined on a plane. match path { - MaybeCurve::Defined(Curve::Line(_)) => { + Curve::Line(_) => { // The other edge is a line segment on a plane. That // means our edge must be a line segment too. None diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index f26fb2631..916ad5790 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -13,7 +13,7 @@ use crate::{ #[derive(Clone, Debug)] pub struct PartialHalfEdge { /// The curve that the half-edge is defined in - pub curve: Option, + pub curve: Option, /// The boundary of the half-edge on the curve pub boundary: [Option>; 2], @@ -35,8 +35,6 @@ impl PartialHalfEdge { let [start, _] = self.boundary; start.and_then(|start| { let curve = self.curve?; - - let MaybeCurve::Defined(curve) = curve; Some(curve.point_from_path_coords(start)) }) } @@ -56,7 +54,7 @@ impl PartialObject for PartialHalfEdge { fn from_full(half_edge: &Self::Full, _: &mut FullToPartialCache) -> Self { Self { - curve: Some(half_edge.curve().into()), + curve: Some(half_edge.curve()), boundary: half_edge.boundary().map(Some), start_vertex: half_edge.start_vertex().clone(), global_form: half_edge.global_form().clone(), @@ -64,8 +62,7 @@ impl PartialObject for PartialHalfEdge { } fn build(self, _: &mut Service) -> Self::Full { - let MaybeCurve::Defined(curve) = - self.curve.expect("Need path to build curve"); + let curve = self.curve.expect("Need path to build curve"); let boundary = self.boundary.map(|point| { point.expect("Can't build `HalfEdge` without boundary positions") }); From fc4334bac66cce78887a70419b236d0c79f5328c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 8 Mar 2023 13:06:45 +0100 Subject: [PATCH 8/8] Remove unused `MaybeCurve` --- crates/fj-kernel/src/partial/mod.rs | 8 ++------ crates/fj-kernel/src/partial/objects/edge.rs | 13 ------------- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/crates/fj-kernel/src/partial/mod.rs b/crates/fj-kernel/src/partial/mod.rs index 17e9d8257..7f688571c 100644 --- a/crates/fj-kernel/src/partial/mod.rs +++ b/crates/fj-kernel/src/partial/mod.rs @@ -16,12 +16,8 @@ mod wrapper; pub use self::{ objects::{ - cycle::PartialCycle, - edge::{MaybeCurve, PartialHalfEdge}, - face::PartialFace, - shell::PartialShell, - sketch::PartialSketch, - solid::PartialSolid, + cycle::PartialCycle, edge::PartialHalfEdge, face::PartialFace, + shell::PartialShell, sketch::PartialSketch, solid::PartialSolid, }, traits::{HasPartial, PartialObject}, wrapper::{FullToPartialCache, Partial}, diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 916ad5790..ae6165fa4 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -70,16 +70,3 @@ impl PartialObject for PartialHalfEdge { HalfEdge::new(curve, boundary, self.start_vertex, self.global_form) } } - -/// A possibly undefined curve -#[derive(Clone, Copy, Debug)] -pub enum MaybeCurve { - /// The curve is fully defined - Defined(Curve), -} - -impl From for MaybeCurve { - fn from(path: Curve) -> Self { - Self::Defined(path) - } -}