Merge pull request #1660 from hannobraun/unify

Simplify curve handling in `PartialHalfEdge`

crates/fj-kernel/src/algorithms/sweep/face.rs

@@ -99,7 +99,7 @@ impl Sweep for Handle<Face> {
                 .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();
 

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<Scalar>,
     ) -> 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]));
@@ -106,19 +106,18 @@ 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::from_points_with_line_coords(points);
-            self.curve = Some(path.into());
+            let path = Curve::line_from_points_with_coords(points);
+            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.])
@@ -143,14 +142,13 @@ 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
                     // representation.
                     match path {
-                        MaybeCurve::Defined(Curve::Line(_))
-                        | MaybeCurve::UndefinedLine => {
+                        Curve::Line(_) => {
                             // We're dealing with a line on a rounded surface.
                             //
                             // Based on the current uses of this method, we can
@@ -177,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
@@ -194,11 +190,10 @@ impl HalfEdgeBuilder for PartialHalfEdge {
                 GlobalPath::Line(_) => {
                     // The other edge is defined on a plane.
                     match path {
-                        MaybeCurve::Defined(Curve::Line(_))
-                        | MaybeCurve::UndefinedLine => {
+                        Curve::Line(_) => {
                             // 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

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<Point<1>>, impl Into<Point<2>>); 2],
     ) -> Self {
         Self::Line(Line::from_points_with_line_coords(points))

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},

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,
@@ -13,7 +13,7 @@ use crate::{
 #[derive(Clone, Debug)]
 pub struct PartialHalfEdge {
     /// The curve that the half-edge is defined in
-    pub curve: Option<MaybeCurve>,
+    pub curve: Option<Curve>,
 
     /// The boundary of the half-edge on the curve
     pub boundary: [Option<Point<1>>; 2],
@@ -35,12 +35,7 @@ impl PartialHalfEdge {
         let [start, _] = self.boundary;
         start.and_then(|start| {
             let curve = self.curve?;
-
-            if let MaybeCurve::Defined(curve) = curve {
-                return Some(curve.point_from_path_coords(start));
-            }
-
-            None
+            Some(curve.point_from_path_coords(start))
         })
     }
 }
@@ -59,48 +54,19 @@ 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(),
         }
     }
 
     fn build(self, _: &mut Service<Objects>) -> 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 curve = self.curve.expect("Need path to build curve");
         let boundary = self.boundary.map(|point| {
             point.expect("Can't build `HalfEdge` without boundary positions")
         });
 
         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),
-
-    /// The curve is undefined, but we know it is a circle
-    UndefinedCircle {
-        /// The radius of the undefined circle
-        radius: Scalar,
-    },
-
-    /// The curve is undefined, but we know it is a line
-    UndefinedLine,
-}
-
-impl From<Curve> for MaybeCurve {
-    fn from(path: Curve) -> Self {
-        Self::Defined(path)
-    }
-}