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),
-                    global_edge,
-                )| {
-                    let half_edge = {
-                        let half_edge = HalfEdge::line_segment(
-                            [start, end],
-                            Some(boundary),
-                            services,
-                        )
-                        .replace_start_vertex(start_vertex);
-
-                        let half_edge = if let Some(curve) = curve {
-                            half_edge.replace_curve(curve)
-                        } else {
-                            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)
+            .map(|((((boundary, start), end), start_vertex), curve)| {
+                let half_edge = {
+                    let half_edge = HalfEdge::line_segment(
+                        [start, end],
+                        Some(boundary),
+                        services,
+                    )
+                    .replace_start_vertex(start_vertex);
+
+                    let half_edge = if let Some(curve) = curve {
+                        half_edge.replace_curve(curve)
+                    } else {
+                        half_edge
                     };
 
-                    exterior = Some(
-                        exterior
-                            .take()
-                            .unwrap()
-                            .add_half_edges([half_edge.clone()]),
-                    );
+                    half_edge.insert(services)
+                };
+
+                exterior = Some(
+                    exterior
+                        .take()
+                        .unwrap()
+                        .add_half_edges([half_edge.clone()]),
+                );
 
-                    half_edge
-                },
-            );
+                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
-            .global_edges
-            .entry(self.id())
-            .or_insert_with(|| GlobalEdge::new().insert(services))
-            .clone();
-
-        (curve, global_edge, vertices)
+        (curve, vertices)
     }
 }

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

@@ -1,9 +1,6 @@
 use fj_math::Transform;
 
-use crate::{
-    objects::{GlobalEdge, HalfEdge},
-    services::Services,
-};
+use crate::{objects::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)
-    }
-}
 
-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()
+        Self::new(path, boundary, curve, start_vertex)
     }
 }

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
-/// represented by an instance of [`GlobalEdge`].
+/// `HalfEdge`s are different representations of the same edge, and this fact
+/// 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`].
-/// 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.
+/// There is another, implicit requirement hidden here:
 ///
-/// That second requirement means that a `HalfEdge` might need to be split into
-/// multiple smaller `HalfEdge`s that are each coincident with a `HalfEdge` in
-/// another face.
+/// `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.
 ///
 /// # Implementation Note
 ///
-/// There is currently no validation code to verify that coincident `HalfEdge`s
-/// are congruent:
-/// <https://github.com/hannobraun/Fornjot/issues/1608>
+/// The limitation that coincident `HalfEdge`s must be congruent is currently
+/// being lifted:
+/// <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,
-        Sketch, Solid, Surface, Vertex,
+        Curve, Cycle, Face, HalfEdge, Objects, Region, Shell, Sketch, Solid,
+        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,
-    Vertex,
+    BehindHandle, Curve, Cycle, Face, HalfEdge, Object, Surface, Vertex,
 };
 
 /// A graph of objects and their relationships
@@ -90,20 +89,13 @@ 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());
         self.curve().insert_into_set(objects);
 
         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,
-    Surface, Vertex,
+    Curve, Cycle, Face, HalfEdge, Region, Shell, Sketch, Solid, 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