Merge pull request #1999 from hannobraun/edge

Remove `GlobalEdge`

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

@@ -27,21 +27,15 @@ impl Sweep for (&HalfEdge, &Handle<Vertex>, &Surface, Option<Color>) {
 
         // Next, we need to define the boundaries of the face. Let's start with
         // the global vertices and edges.
-        let (vertices, global_edges, curves) = {
+        let (vertices, curves) = {
             let [a, b] = [edge.start_vertex(), next_vertex].map(Clone::clone);
-            let (curve_up, edge_up, [_, c]) =
+            let (curve_up, [_, c]) =
                 b.clone().sweep_with_cache(path, cache, services);
-            let (curve_down, edge_down, [_, d]) =
+            let (curve_down, [_, d]) =
                 a.clone().sweep_with_cache(path, cache, services);
 
             (
                 [a, b, c, d],
-                [
-                    Some(edge.global_form().clone()),
-                    Some(edge_up),
-                    None,
-                    Some(edge_down),
-                ],
                 [
                     Some(edge.curve().clone()),
                     Some(curve_up),
@@ -85,45 +79,33 @@ impl Sweep for (&HalfEdge, &Handle<Vertex>, &Surface, Option<Color>) {
             .zip_ext(surface_points_next)
             .zip_ext(vertices)
             .zip_ext(curves)
-            .zip_ext(global_edges)
+            .map(|((((boundary, start), end), start_vertex), curve)| {
-            .map(
+                let half_edge = {
-                |(
+                    let half_edge = HalfEdge::line_segment(
-                    ((((boundary, start), end), start_vertex), curve),
+                        [start, end],
-                    global_edge,
+                        Some(boundary),
-                )| {
+                        services,
-                    let half_edge = {
+                    )
-                        let half_edge = HalfEdge::line_segment(
+                    .replace_start_vertex(start_vertex);
-                            [start, end],
-                            Some(boundary),
-                            services,
-                        )
-                        .replace_start_vertex(start_vertex);
 
-                        let half_edge = if let Some(curve) = curve {
+                    let half_edge = if let Some(curve) = curve {
-                            half_edge.replace_curve(curve)
+                        half_edge.replace_curve(curve)
-                        } else {
+                    } else {
-                            half_edge
+                        half_edge
-                        };
-
-                        let half_edge = if let Some(global_edge) = global_edge {
-                            half_edge.replace_global_form(global_edge)
-                        } else {
-                            half_edge
-                        };
-
-                        half_edge.insert(services)
                     };
 
-                    exterior = Some(
+                    half_edge.insert(services)
-                        exterior
+                };
-                            .take()
-                            .unwrap()
-                            .add_half_edges([half_edge.clone()]),
-                    );
 
-                    half_edge
+                exterior = Some(
-                },
+                    exterior
-            );
+                        .take()
+                        .unwrap()
+                        .add_half_edges([half_edge.clone()]),
+                );
+
+                half_edge
+            });
 
         let region = Region::new(exterior.unwrap().insert(services), [], color)
             .insert(services);

crates/fj-core/src/algorithms/sweep/mod.rs

@@ -11,7 +11,7 @@ use std::collections::BTreeMap;
 use fj_math::Vector;
 
 use crate::{
-    objects::{Curve, GlobalEdge, Vertex},
+    objects::{Curve, Vertex},
     services::Services,
     storage::{Handle, ObjectId},
 };
@@ -50,7 +50,4 @@ pub struct SweepCache {
 
     /// Cache for vertices
     pub vertices: BTreeMap<ObjectId, Handle<Vertex>>,
-
-    /// Cache for global edges
-    pub global_edges: BTreeMap<ObjectId, Handle<GlobalEdge>>,
 }

crates/fj-core/src/algorithms/sweep/vertex.rs

@@ -1,7 +1,7 @@
 use fj_math::Vector;
 
 use crate::{
-    objects::{Curve, GlobalEdge, Vertex},
+    objects::{Curve, Vertex},
     operations::Insert,
     services::Services,
     storage::Handle,
@@ -10,7 +10,7 @@ use crate::{
 use super::{Sweep, SweepCache};
 
 impl Sweep for Handle<Vertex> {
-    type Swept = (Handle<Curve>, Handle<GlobalEdge>, [Self; 2]);
+    type Swept = (Handle<Curve>, [Self; 2]);
 
     fn sweep_with_cache(
         self,
@@ -32,12 +32,6 @@ impl Sweep for Handle<Vertex> {
             .clone();
         let vertices = [a, b];
 
-        let global_edge = cache
+        (curve, vertices)
-            .global_edges
-            .entry(self.id())
-            .or_insert_with(|| GlobalEdge::new().insert(services))
-            .clone();
-
-        (curve, global_edge, vertices)
     }
 }

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

@@ -1,9 +1,6 @@
 use fj_math::Transform;
 
-use crate::{
+use crate::{objects::HalfEdge, services::Services};
-    objects::{GlobalEdge, HalfEdge},
-    services::Services,
-};
 
 use super::{TransformCache, TransformObject};
 
@@ -26,25 +23,7 @@ impl TransformObject for HalfEdge {
             .start_vertex()
             .clone()
             .transform_with_cache(transform, services, cache);
-        let global_form = self
-            .global_form()
-            .clone()
-            .transform_with_cache(transform, services, cache);
 
-        Self::new(path, boundary, curve, start_vertex, global_form)
+        Self::new(path, boundary, curve, start_vertex)
-    }
-}
-
-impl TransformObject for GlobalEdge {
-    fn transform_with_cache(
-        self,
-        _: &Transform,
-        _: &mut Services,
-        _: &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`
-        // object must be created to represent the new and transformed edge.
-        Self::new()
     }
 }

crates/fj-core/src/objects/kinds/edge.rs

@@ -8,43 +8,33 @@ use crate::{
 
 /// A directed edge, defined in a surface's 2D space
 ///
-/// The concept of an "edge" in Fornjot is represented by two structs,
-/// `HalfEdge` and [`GlobalEdge`]. `HalfEdge` has two attributes that make it
-/// distinct from [`GlobalEdge`]:
-///
-/// - `HalfEdge` is directed, meaning it has a defined start and end vertex.
-/// - `HalfEdge` is defined in the 2-dimensional space of a surface.
-///
 /// When multiple faces, which are bound by edges, are combined to form a solid,
 /// the `HalfEdge`s that bound the face on the surface are then coincident with
 /// the `HalfEdge`s of other faces, where those faces touch. Those coincident
-/// `HalfEdge`s are different representations of the same edge. This edge is
+/// `HalfEdge`s are different representations of the same edge, and this fact
-/// represented by an instance of [`GlobalEdge`].
+/// must be represented in the following way:
 ///
-/// There are some requirements that a `HalfEdge` needs to uphold to be valid:
+/// - The coincident `HalfEdge`s must refer to the same `Curve`.
+/// - The coincident `HalfEdge`s must have the same boundary.
 ///
-/// 1. Coincident `HalfEdge`s *must* refer to the same [`GlobalEdge`].
+/// There is another, implicit requirement hidden here:
-/// 2. `HalfEdge`s that are coincident, i.e. located in the same space, must
-///    always be congruent. This means they must coincide *exactly*. The overlap
-///    must be complete. None of the coincident `HalfEdge`s must overlap with
-///    just a section of another.
 ///
-/// That second requirement means that a `HalfEdge` might need to be split into
+/// `HalfEdge`s that are coincident, i.e. located in the same space, must always
-/// multiple smaller `HalfEdge`s that are each coincident with a `HalfEdge` in
+/// be congruent. This means they must coincide *exactly*. The overlap must be
-/// another face.
+/// complete. None of the coincident `HalfEdge`s must overlap with just a
+/// section of another.
 ///
 /// # Implementation Note
 ///
-/// There is currently no validation code to verify that coincident `HalfEdge`s
+/// The limitation that coincident `HalfEdge`s must be congruent is currently
-/// are congruent:
+/// being lifted:
-/// <https://github.com/hannobraun/Fornjot/issues/1608>
+/// <https://github.com/hannobraun/fornjot/issues/1937>
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct HalfEdge {
     path: SurfacePath,
     boundary: CurveBoundary<Point<1>>,
     curve: HandleWrapper<Curve>,
     start_vertex: HandleWrapper<Vertex>,
-    global_form: HandleWrapper<GlobalEdge>,
 }
 
 impl HalfEdge {
@@ -54,14 +44,12 @@ impl HalfEdge {
         boundary: impl Into<CurveBoundary<Point<1>>>,
         curve: Handle<Curve>,
         start_vertex: Handle<Vertex>,
-        global_form: Handle<GlobalEdge>,
     ) -> Self {
         Self {
             path,
             boundary: boundary.into(),
             curve: curve.into(),
             start_vertex: start_vertex.into(),
-            global_form: global_form.into(),
         }
     }
 
@@ -94,41 +82,4 @@ impl HalfEdge {
     pub fn start_vertex(&self) -> &Handle<Vertex> {
         &self.start_vertex
     }
-
-    /// Access the global form of the half-edge
-    pub fn global_form(&self) -> &Handle<GlobalEdge> {
-        &self.global_form
-    }
-}
-
-/// An undirected edge, defined in global (3D) coordinates
-///
-/// In contrast to [`HalfEdge`], `GlobalEdge` is undirected, meaning it has no
-/// 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`.
-///
-/// # Equality
-///
-/// `GlobalEdge` contains no data and exists purely to be used within a
-/// `Handle`, where `Handle::id` can be used to compare different instances of
-/// `GlobalEdge`.
-///
-/// If `GlobalEdge` had `Eq`/`PartialEq` implementations, it containing no data
-/// would mean that all instances of `GlobalEdge` would be considered equal.
-/// This would be very error-prone.
-///
-/// If you need to reference a `GlobalEdge` from a struct that needs to derive
-/// `Eq`/`Ord`/..., you can use `HandleWrapper<GlobalEdge>` to do that. It will
-/// use `Handle::id` to provide those `Eq`/`Ord`/... implementations.
-#[derive(Clone, Debug, Default, Hash)]
-pub struct GlobalEdge {}
-
-impl GlobalEdge {
-    /// Create a new instance
-    pub fn new() -> Self {
-        Self::default()
-    }
 }

crates/fj-core/src/objects/mod.rs

@@ -48,7 +48,7 @@ pub use self::{
     kinds::{
         curve::Curve,
         cycle::{Cycle, HalfEdgesOfCycle},
-        edge::{GlobalEdge, HalfEdge},
+        edge::HalfEdge,
         face::{Face, FaceSet, Handedness},
         region::Region,
         shell::Shell,

crates/fj-core/src/objects/object.rs

@@ -1,7 +1,7 @@
 use crate::{
     objects::{
-        Curve, Cycle, Face, GlobalEdge, HalfEdge, Objects, Region, Shell,
+        Curve, Cycle, Face, HalfEdge, Objects, Region, Shell, Sketch, Solid,
-        Sketch, Solid, Surface, Vertex,
+        Surface, Vertex,
     },
     storage::{Handle, HandleWrapper, ObjectId},
     validate::{Validate, ValidationError},
@@ -94,7 +94,6 @@ object!(
     Curve, "curve", curves;
     Cycle, "cycle", cycles;
     Face, "face", faces;
-    GlobalEdge, "global edge", global_edges;
     HalfEdge, "half-edge", half_edges;
     Region, "region", regions;
     Shell, "shell", shells;

crates/fj-core/src/objects/set.rs

@@ -1,8 +1,7 @@
 use std::collections::{btree_set, BTreeSet};
 
 use super::{
-    BehindHandle, Curve, Cycle, Face, GlobalEdge, HalfEdge, Object, Surface,
+    BehindHandle, Curve, Cycle, Face, HalfEdge, Object, Surface, Vertex,
-    Vertex,
 };
 
 /// A graph of objects and their relationships
@@ -90,10 +89,6 @@ impl InsertIntoSet for Face {
     }
 }
 
-impl InsertIntoSet for GlobalEdge {
-    fn insert_into_set(&self, _: &mut ObjectSet) {}
-}
-
 impl InsertIntoSet for HalfEdge {
     fn insert_into_set(&self, objects: &mut ObjectSet) {
         objects.inner.insert(self.curve().clone().into());
@@ -101,9 +96,6 @@ impl InsertIntoSet for HalfEdge {
 
         objects.inner.insert(self.start_vertex().clone().into());
         self.start_vertex().insert_into_set(objects);
-
-        objects.inner.insert(self.global_form().clone().into());
-        self.global_form().insert_into_set(objects);
     }
 }
 

crates/fj-core/src/objects/stores.rs

@@ -6,8 +6,7 @@ use crate::{
 };
 
 use super::{
-    Curve, Cycle, Face, GlobalEdge, HalfEdge, Region, Shell, Sketch, Solid,
+    Curve, Cycle, Face, HalfEdge, Region, Shell, Sketch, Solid, Surface, Vertex,
-    Surface, Vertex,
 };
 
 /// The available object stores
@@ -22,9 +21,6 @@ pub struct Objects {
     /// Store for [`Face`]s
     pub faces: Store<Face>,
 
-    /// Store for [`GlobalEdge`]s
-    pub global_edges: Store<GlobalEdge>,
-
     /// Store for [`HalfEdge`]s
     pub half_edges: Store<HalfEdge>,
 

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

@@ -3,7 +3,7 @@ use fj_math::{Arc, Point, Scalar};
 
 use crate::{
     geometry::{CurveBoundary, SurfacePath},
-    objects::{Curve, GlobalEdge, HalfEdge, Vertex},
+    objects::{Curve, HalfEdge, Vertex},
     operations::Insert,
     services::Services,
 };
@@ -18,9 +18,8 @@ pub trait BuildHalfEdge {
     ) -> HalfEdge {
         let curve = Curve::new().insert(services);
         let start_vertex = Vertex::new().insert(services);
-        let global_form = GlobalEdge::new().insert(services);
 
-        HalfEdge::new(path, boundary, curve, start_vertex, global_form)
+        HalfEdge::new(path, boundary, curve, start_vertex)
     }
 
     /// Create an arc

crates/fj-core/src/operations/insert/insert_trait.rs

@@ -1,7 +1,7 @@
 use crate::{
     objects::{
-        Curve, Cycle, Face, GlobalEdge, HalfEdge, Region, Shell, Sketch, Solid,
+        Curve, Cycle, Face, HalfEdge, Region, Shell, Sketch, Solid, Surface,
-        Surface, Vertex,
+        Vertex,
     },
     operations::{Polygon, TetrahedronShell},
     services::Services,
@@ -47,7 +47,6 @@ impl_insert!(
     Curve, curves;
     Cycle, cycles;
     Face, faces;
-    GlobalEdge, global_edges;
     HalfEdge, half_edges;
     Region, regions;
     Shell, shells;

crates/fj-core/src/operations/join/cycle.rs

@@ -86,7 +86,6 @@ impl JoinCycle for Cycle {
                 HalfEdge::unjoined(curve, boundary, services)
                     .replace_curve(half_edge.curve().clone())
                     .replace_start_vertex(prev.start_vertex().clone())
-                    .replace_global_form(half_edge.global_form().clone())
                     .insert(services)
             },
         ))
@@ -132,7 +131,6 @@ impl JoinCycle for Cycle {
             let this_joined = half_edge
                 .replace_curve(half_edge_other.curve().clone())
                 .replace_start_vertex(vertex_a)
-                .replace_global_form(half_edge_other.global_form().clone())
                 .insert(services);
             let next_joined =
                 next_edge.replace_start_vertex(vertex_b).insert(services);

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

@@ -16,7 +16,6 @@ impl Reverse for Cycle {
                     current.boundary().reverse(),
                     current.curve().clone(),
                     next.start_vertex().clone(),
-                    current.global_form().clone(),
                 )
                 .insert(services)
             })

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

@@ -12,7 +12,6 @@ impl ReverseCurveCoordinateSystems for HalfEdge {
             boundary,
             self.curve().clone(),
             self.start_vertex().clone(),
-            self.global_form().clone(),
         )
     }
 }

crates/fj-core/src/operations/update/edge.rs

@@ -2,7 +2,7 @@ use fj_math::Point;
 
 use crate::{
     geometry::{CurveBoundary, SurfacePath},
-    objects::{Curve, GlobalEdge, HalfEdge, Vertex},
+    objects::{Curve, HalfEdge, Vertex},
     storage::Handle,
 };
 
@@ -23,10 +23,6 @@ pub trait UpdateHalfEdge {
     /// Replace the start vertex of the half-edge
     #[must_use]
     fn replace_start_vertex(&self, start_vertex: Handle<Vertex>) -> Self;
-
-    /// Replace the global form of the half-edge
-    #[must_use]
-    fn replace_global_form(&self, global_form: Handle<GlobalEdge>) -> Self;
 }
 
 impl UpdateHalfEdge for HalfEdge {
@@ -36,7 +32,6 @@ impl UpdateHalfEdge for HalfEdge {
             self.boundary(),
             self.curve().clone(),
             self.start_vertex().clone(),
-            self.global_form().clone(),
         )
     }
 
@@ -46,7 +41,6 @@ impl UpdateHalfEdge for HalfEdge {
             boundary,
             self.curve().clone(),
             self.start_vertex().clone(),
-            self.global_form().clone(),
         )
     }
 
@@ -56,7 +50,6 @@ impl UpdateHalfEdge for HalfEdge {
             self.boundary(),
             curve,
             self.start_vertex().clone(),
-            self.global_form().clone(),
         )
     }
 
@@ -66,17 +59,6 @@ impl UpdateHalfEdge for HalfEdge {
             self.boundary(),
             self.curve().clone(),
             start_vertex,
-            self.global_form().clone(),
-        )
-    }
-
-    fn replace_global_form(&self, global_form: Handle<GlobalEdge>) -> Self {
-        HalfEdge::new(
-            self.path(),
-            self.boundary(),
-            self.curve().clone(),
-            self.start_vertex().clone(),
-            global_form,
         )
     }
 }

crates/fj-core/src/validate/edge.rs

@@ -1,6 +1,6 @@
 use fj_math::{Point, Scalar};
 
-use crate::objects::{GlobalEdge, HalfEdge};
+use crate::objects::HalfEdge;
 
 use super::{Validate, ValidationConfig, ValidationError};
 
@@ -14,15 +14,6 @@ impl Validate for HalfEdge {
     }
 }
 
-impl Validate for GlobalEdge {
-    fn validate_with_config(
-        &self,
-        _: &ValidationConfig,
-        _: &mut Vec<ValidationError>,
-    ) {
-    }
-}
-
 /// [`HalfEdge`] validation failed
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum HalfEdgeValidationError {
@@ -97,7 +88,6 @@ mod tests {
                 boundary,
                 valid.curve().clone(),
                 valid.start_vertex().clone(),
-                valid.global_form().clone(),
             )
         };
 

crates/fj-core/src/validate/shell.rs

@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, HashMap};
+use std::collections::BTreeMap;
 
 use fj_math::{Point, Scalar};
 
@@ -6,7 +6,7 @@ use crate::{
     geometry::SurfaceGeometry,
     objects::{HalfEdge, Shell, Surface},
     queries::{AllEdgesWithSurface, BoundingVerticesOfEdge},
-    storage::{Handle, HandleWrapper, ObjectId},
+    storage::{Handle, HandleWrapper},
 };
 
 use super::{Validate, ValidationConfig, ValidationError};
@@ -222,10 +222,7 @@ impl ShellValidationError {
                     continue;
                 }
 
-                let identical_according_to_global_form =
+                let identical = {
-                    edge_a.global_form().id() == edge_b.global_form().id();
-
-                let identical_according_to_curve = {
                     let on_same_curve =
                         edge_a.curve().id() == edge_b.curve().id();
 
@@ -244,12 +241,7 @@ impl ShellValidationError {
                     on_same_curve && have_same_boundary
                 };
 
-                assert_eq!(
+                match identical {
-                    identical_according_to_curve,
-                    identical_according_to_global_form,
-                );
-
-                match identical_according_to_curve {
                     true => {
                         // All points on identical curves should be within
                         // identical_max_distance, so we shouldn't have any
@@ -328,61 +320,12 @@ impl ShellValidationError {
         if num_edges.into_values().any(|num| num != 2) {
             errors.push(Self::NotWatertight.into());
         }
-
-        let mut global_edge_to_faces: HashMap<ObjectId, usize> = HashMap::new();
-
-        for face in shell.faces() {
-            for cycle in face.region().all_cycles() {
-                for half_edge in cycle.half_edges() {
-                    let id = half_edge.global_form().id();
-                    let entry = global_edge_to_faces.entry(id);
-                    *entry.or_insert(0) += 1;
-                }
-            }
-        }
-
-        // Each global edge should have exactly two half edges that are part of
-        // the shell
-        if global_edge_to_faces.iter().any(|(_, c)| *c != 2) {
-            errors.push(Self::NotWatertight.into())
-        }
     }
 
     fn validate_same_orientation(
         shell: &Shell,
         errors: &mut Vec<ValidationError>,
     ) {
-        let mut global_to_half: HashMap<ObjectId, Vec<_>> = HashMap::new();
-
-        for face in shell.faces() {
-            for cycle in face.region().all_cycles() {
-                for half_edge in cycle.half_edges() {
-                    let id = half_edge.global_form().id();
-                    global_to_half
-                        .entry(id)
-                        .or_insert(Vec::new())
-                        .push(half_edge.clone());
-                }
-            }
-        }
-
-        // In order for the faces to all have the same outside winding global
-        // edge should have two half edges in opposite directions.
-        for (_, halfs) in global_to_half {
-            if let (Some(a), Some(b)) = (halfs.get(0), halfs.get(1)) {
-                // Check if a is reverse of b
-                if a.boundary().reverse() != b.boundary() {
-                    errors.push(Self::MixedOrientations.into());
-                    return;
-                }
-            }
-        }
-
-        // Here's the same check again a second time, except using `Curve`
-        // instead of `GlobalEdge`. This redundancy can be fixed once the
-        // transition from `Curve` to `GlobalEdge` is finished, and `GlobalEdge`
-        // can be removed.
-
         let mut edges_by_coincidence = BTreeMap::new();
 
         for face in shell.faces() {
@@ -434,7 +377,7 @@ pub struct CurveCoordinateSystemMismatch {
 mod tests {
     use crate::{
         assert_contains_err,
-        objects::{Curve, GlobalEdge, Shell},
+        objects::{Curve, Shell},
         operations::{
             BuildShell, Insert, Reverse, UpdateCycle, UpdateFace,
             UpdateHalfEdge, UpdateRegion, UpdateShell,
@@ -508,12 +451,9 @@ mod tests {
                                 .update_nth_half_edge(0, |half_edge| {
                                     let curve =
                                         Curve::new().insert(&mut services);
-                                    let global_form =
-                                        GlobalEdge::new().insert(&mut services);
 
                                     half_edge
                                         .replace_curve(curve)
-                                        .replace_global_form(global_form)
                                         .insert(&mut services)
                                 })
                                 .insert(&mut services)