Remove path field from HalfEdgeGeom

crates/fj-core/src/algorithms/approx/curve.rs

@@ -31,7 +31,12 @@ impl Approx for (&Handle<Curve>, &HalfEdgeGeom, &Handle<Surface>) {
             Some(approx) => approx,
             None => {
                 let approx = approx_curve(
-                    &half_edge.path,
+                    &geometry
+                        .of_curve(curve)
+                        .unwrap()
+                        .local_on(surface)
+                        .unwrap()
+                        .path,
                     geometry.of_surface(surface),
                     half_edge.boundary,
                     tolerance,
@@ -198,7 +203,7 @@ mod tests {
         let curve =
             Curve::from_path_and_surface(path, surface.clone(), &mut core);
         let boundary = CurveBoundary::from(boundary);
-        let half_edge = HalfEdgeGeom { path, boundary };
+        let half_edge = HalfEdgeGeom { boundary };
 
         let tolerance = 1.;
         let approx = (&curve, &half_edge, &surface)
@@ -221,7 +226,7 @@ mod tests {
         let curve =
             Curve::from_path_and_surface(path, surface.clone(), &mut core);
         let boundary = CurveBoundary::from(boundary);
-        let half_edge = HalfEdgeGeom { path, boundary };
+        let half_edge = HalfEdgeGeom { boundary };
 
         let tolerance = 1.;
         let approx = (&curve, &half_edge, &surface)
@@ -243,7 +248,7 @@ mod tests {
         let curve =
             Curve::from_path_and_surface(path, surface.clone(), &mut core);
         let boundary = CurveBoundary::from([[0.], [TAU]]);
-        let half_edge = HalfEdgeGeom { path, boundary };
+        let half_edge = HalfEdgeGeom { boundary };
 
         let tolerance = 1.;
         let approx = (&curve, &half_edge, &surface)
@@ -274,7 +279,7 @@ mod tests {
         let curve =
             Curve::from_path_and_surface(path, surface.clone(), &mut core);
         let boundary = CurveBoundary::from([[0.], [TAU]]);
-        let half_edge = HalfEdgeGeom { path, boundary };
+        let half_edge = HalfEdgeGeom { boundary };
 
         let tolerance = 1.;
         let approx = (&curve, &half_edge, &surface)

crates/fj-core/src/algorithms/approx/edge.rs

@@ -61,7 +61,10 @@ impl Approx for (&Handle<HalfEdge>, &Handle<Surface>) {
 
             approx.points.into_iter().map(|point| {
                 let point_surface = geometry
-                    .of_half_edge(half_edge)
+                    .of_curve(half_edge.curve())
+                    .unwrap()
+                    .local_on(surface)
+                    .unwrap()
                     .path
                     .point_from_path_coords(point.local_form);
 

crates/fj-core/src/algorithms/bounding_volume/cycle.rs

@@ -1,15 +1,19 @@
 use fj_math::Aabb;
 
-use crate::{geometry::Geometry, topology::Cycle};
+use crate::{
+    geometry::Geometry,
+    storage::Handle,
+    topology::{Cycle, Surface},
+};
 
-impl super::BoundingVolume<2> for &Cycle {
+impl super::BoundingVolume<2> for (&Cycle, &Handle<Surface>) {
     fn aabb(self, geometry: &Geometry) -> Option<Aabb<2>> {
-        let cycle = self;
+        let (cycle, surface) = self;
 
         let mut aabb: Option<Aabb<2>> = None;
 
         for half_edge in cycle.half_edges() {
-            let new_aabb = half_edge
+            let new_aabb = (half_edge, surface)
                 .aabb(geometry)
                 .expect("`HalfEdge` can always compute AABB");
             aabb = Some(aabb.map_or(new_aabb, |aabb| aabb.merged(&new_aabb)));

crates/fj-core/src/algorithms/bounding_volume/face.rs

@@ -1,3 +1,5 @@
+use std::ops::Deref;
+
 use fj_math::Aabb;
 
 use crate::{
@@ -7,27 +9,30 @@ use crate::{
 
 impl super::BoundingVolume<3> for &Face {
     fn aabb(self, geometry: &Geometry) -> Option<Aabb<3>> {
-        self.region().exterior().aabb(geometry).map(|aabb2| {
-            let surface = geometry.of_surface(self.surface());
+        (self.region().exterior().deref(), self.surface())
+            .aabb(geometry)
+            .map(|aabb2| {
+                let surface = geometry.of_surface(self.surface());
 
-            match surface.u {
-                GlobalPath::Circle(circle) => {
-                    // This is not the most precise way to calculate the AABB,
-                    // doing it for the whole circle, but it should do.
+                match surface.u {
+                    GlobalPath::Circle(circle) => {
+                        // This is not the most precise way to calculate the
+                        // AABB, doing it for the whole circle, but it should
+                        // do.
 
-                    let aabb_bottom = circle.aabb();
-                    let aabb_top = Aabb {
-                        min: aabb_bottom.min + surface.v,
-                        max: aabb_bottom.max + surface.v,
-                    };
+                        let aabb_bottom = circle.aabb();
+                        let aabb_top = Aabb {
+                            min: aabb_bottom.min + surface.v,
+                            max: aabb_bottom.max + surface.v,
+                        };
 
-                    aabb_bottom.merged(&aabb_top)
+                        aabb_bottom.merged(&aabb_top)
+                    }
+                    GlobalPath::Line(_) => Aabb {
+                        min: surface.point_from_surface_coords(aabb2.min),
+                        max: surface.point_from_surface_coords(aabb2.max),
+                    },
                 }
-                GlobalPath::Line(_) => Aabb {
-                    min: surface.point_from_surface_coords(aabb2.min),
-                    max: surface.point_from_surface_coords(aabb2.max),
-                },
-            }
-        })
+            })
     }
 }

crates/fj-core/src/algorithms/bounding_volume/half_edge.rs

@@ -3,15 +3,20 @@ use fj_math::{Aabb, Vector};
 use crate::{
     geometry::{Geometry, SurfacePath},
     storage::Handle,
-    topology::HalfEdge,
+    topology::{HalfEdge, Surface},
 };
 
-impl super::BoundingVolume<2> for &Handle<HalfEdge> {
+impl super::BoundingVolume<2> for (&Handle<HalfEdge>, &Handle<Surface>) {
     fn aabb(self, geometry: &Geometry) -> Option<Aabb<2>> {
-        let half_edge = self;
+        let (half_edge, surface) = self;
 
         let half_edge_geom = geometry.of_half_edge(half_edge);
-        let path = half_edge_geom.path;
+        let path = geometry
+            .of_curve(half_edge.curve())
+            .unwrap()
+            .local_on(surface)
+            .unwrap()
+            .path;
 
         match path {
             SurfacePath::Circle(circle) => {

crates/fj-core/src/geometry/half_edge.rs

@@ -5,33 +5,6 @@ use super::{CurveBoundary, SurfacePath};
 /// The geometry of a half-edge
 #[derive(Copy, Clone, Debug)]
 pub struct HalfEdgeGeom {
-    /// # The path of the half-edge
-    ///
-    /// ## Implementation Note
-    ///
-    /// Currently, all curve-related geometry is defined locally, in terms of
-    /// the surface that the curve is on (or purely in 2D, if there is no
-    /// surface associated with this geometry). However, curves exist globally,
-    /// independently of surfaces. Half-edges in multiple surfaces can refer to
-    /// the same curve, and in fact, that is the whole reason for their
-    /// existence as a topological object.
-    ///
-    /// This contradiction, globally defined curves but locally defined curve
-    /// geometry, is the reason that this curve geometry is defined right here,
-    /// associated with a locally existing half-edge. (And, I might add,
-    /// redundantly so, as multiple half-edges within the same surface context
-    /// can refer to the same curve.)
-    ///
-    /// Instead, it should be possible to define curve geometry *either* locally
-    /// or globally. Then that respective definition can be associated with the
-    /// curve (and possibly, in addition, a surface). How exactly that is going
-    /// to work is up in the air.
-    ///
-    /// The point of all this exposition is to clarify that this field doesn't
-    /// really belong here. It exists here for practical reasons that are,
-    /// hopefully, temporary.
-    pub path: SurfacePath,
-
     /// # The boundary of the half-edge on its curve
     pub boundary: CurveBoundary<Point<1>>,
 }

crates/fj-core/src/operations/build/half_edge.rs

@@ -73,7 +73,6 @@ pub trait BuildHalfEdge {
         core.layers.geometry.define_half_edge(
             half_edge.clone(),
             HalfEdgeGeom {
-                path,
                 boundary: boundary.into(),
             },
         );
@@ -100,14 +99,6 @@ pub trait BuildHalfEdge {
         core.layers.geometry.define_half_edge(
             half_edge.clone(),
             HalfEdgeGeom {
-                path: core
-                    .layers
-                    .geometry
-                    .of_curve(half_edge.curve())
-                    .expect("Curve geometry was just defined in same function")
-                    .local_on(&surface)
-                    .expect("Curve geometry was just defined in same function")
-                    .path,
                 boundary: boundary.unwrap_or_default(),
             },
         );

crates/fj-core/src/operations/build/shell.rs

@@ -102,23 +102,7 @@ pub trait BuildShell {
                                 .update_curve(|_, _| curve.clone(), core)
                                 .insert(core)
                                 .set_geometry(
-                                    HalfEdgeGeom {
-                                        path: core
-                                            .layers
-                                            .geometry
-                                            .of_curve(&curve)
-                                            .expect(
-                                                "Curve geometry was just \
-                                                defined in same function",
-                                            )
-                                            .local_on(&surface)
-                                            .expect(
-                                                "Curve geometry was just \
-                                                defined in same function",
-                                            )
-                                            .path,
-                                        boundary,
-                                    },
+                                    HalfEdgeGeom { boundary },
                                     &mut core.layers.geometry,
                                 )
                         })

crates/fj-core/src/operations/reverse/half_edge.rs

@@ -17,7 +17,6 @@ impl ReverseCurveCoordinateSystems for (&Handle<HalfEdge>, &Handle<Surface>) {
         let (half_edge, surface) = self;
 
         let mut half_edge_geom = *core.layers.geometry.of_half_edge(half_edge);
-        half_edge_geom.path = half_edge_geom.path.reverse();
         half_edge_geom.boundary = half_edge_geom.boundary.reverse();
 
         let curve =

crates/fj-core/src/operations/sweep/half_edge.rs

@@ -143,23 +143,7 @@ impl SweepHalfEdge for Handle<HalfEdge> {
                     .update_curve(|_, _| curve.clone(), core)
                     .insert(core)
                     .set_geometry(
-                        HalfEdgeGeom {
-                            path: core
-                                .layers
-                                .geometry
-                                .of_curve(&curve)
-                                .expect(
-                                    "Curve geometry was just defined in same \
-                                    function",
-                                )
-                                .local_on(&surface)
-                                .expect(
-                                    "Curve geometry was just defined in same \
-                                    function",
-                                )
-                                .path,
-                            boundary,
-                        },
+                        HalfEdgeGeom { boundary },
                         &mut core.layers.geometry,
                     );
 

crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs

@@ -124,17 +124,21 @@ impl ValidationCheck<Shell> for CoincidentHalfEdgesAreNotSiblings {
                     continue;
                 }
 
-                // If all points on distinct curves are within
-                // `distinct_min_distance`, that's a problem.
-                if distances(
+                let Some(mut distances) = distances(
                     half_edge_a.clone(),
                     surface_a,
                     half_edge_b.clone(),
                     surface_b,
                     geometry,
-                )
-                .all(|d| d < config.distinct_min_distance)
-                {
+                ) else {
+                    // The geometry to compute the distances is not available,
+                    // hence these half-edges can't be coincident.
+                    continue;
+                };
+
+                // If all points on distinct curves are within
+                // `distinct_min_distance`, that's a problem.
+                if distances.all(|d| d < config.distinct_min_distance) {
                     let boundaries =
                         [half_edge_a, half_edge_b].map(|half_edge| {
                             geometry.of_half_edge(half_edge).boundary
@@ -174,20 +178,27 @@ fn distances(
     half_edge_b: Handle<HalfEdge>,
     surface_b: &Handle<Surface>,
     geometry: &Geometry,
-) -> impl Iterator<Item = Scalar> {
+) -> Option<impl Iterator<Item = Scalar>> {
     fn sample(
         percent: f64,
         half_edge: &Handle<HalfEdge>,
         surface: &Handle<Surface>,
         geometry: &Geometry,
-    ) -> Point<3> {
+    ) -> Option<Point<3>> {
         let [start, end] = geometry.of_half_edge(half_edge).boundary.inner;
         let path_coords = start + (end - start) * percent;
-        let path = geometry.of_half_edge(half_edge).path;
+        // let path = geometry.of_half_edge(half_edge).path;
+        let path = geometry
+            .of_curve(half_edge.curve())?
+            .local_on(surface)?
+            .path;
+        // assert_eq!(path, path_from_curve);
         let surface_coords = path.point_from_path_coords(path_coords);
-        geometry
-            .of_surface(surface)
-            .point_from_surface_coords(surface_coords)
+        Some(
+            geometry
+                .of_surface(surface)
+                .point_from_surface_coords(surface_coords),
+        )
     }
 
     // Three samples (start, middle, end), are enough to detect weather lines
@@ -199,11 +210,11 @@ fn distances(
     let mut distances = Vec::new();
     for i in 0..sample_count {
         let percent = i as f64 * step;
-        let sample1 = sample(percent, &half_edge_a, surface_a, geometry);
-        let sample2 = sample(1.0 - percent, &half_edge_b, surface_b, geometry);
+        let sample1 = sample(percent, &half_edge_a, surface_a, geometry)?;
+        let sample2 = sample(1.0 - percent, &half_edge_b, surface_b, geometry)?;
         distances.push(sample1.distance_to(&sample2))
     }
-    distances.into_iter()
+    Some(distances.into_iter())
 }
 
 #[cfg(test)]