Merge pull request #1644 from hannobraun/edge

Remove redundant references to `Vertex` from `GlobalEdge`

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

@@ -111,7 +111,7 @@ impl Sweep for (Handle<HalfEdge>, &Handle<Vertex>, &Surface, Color) {
         // even if the original edge was a circle, it's still going to be a line
         // when projected into the new surface. For the side edges, because
         // we're sweeping along a straight path.
-        for ((mut half_edge, start), (next_half_edge, end)) in [
+        for ((mut half_edge, start), (_, end)) in [
             edge_bottom.clone(),
             edge_up.clone(),
             edge_top.clone(),
@@ -122,9 +122,6 @@ impl Sweep for (Handle<HalfEdge>, &Handle<Vertex>, &Surface, Color) {
         .circular_tuple_windows()
         {
             half_edge.write().update_as_line_segment(start, end);
-            half_edge
-                .write()
-                .infer_global_form(next_half_edge.read().start_vertex.clone());
         }
 
         // Finally, we can make sure that all edges refer to the correct global

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

@@ -28,7 +28,7 @@ impl Sweep for Handle<Vertex> {
             .clone();
 
         let vertices = [a, b];
-        let global_edge = GlobalEdge::new(vertices.clone()).insert(objects);
+        let global_edge = GlobalEdge::new().insert(objects);
 
         // The vertices of the returned `GlobalEdge` are in normalized order,
         // which means the order can't be relied upon by the caller. Return the

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

@@ -34,15 +34,13 @@ impl TransformObject for HalfEdge {
 impl TransformObject for GlobalEdge {
     fn transform_with_cache(
         self,
-        transform: &Transform,
-        objects: &mut Service<Objects>,
-        cache: &mut TransformCache,
+        _: &Transform,
+        _: &mut Service<Objects>,
+        _: &mut TransformCache,
     ) -> Self {
-        let vertices =
-            self.vertices().access_in_normalized_order().map(|vertex| {
-                vertex.transform_with_cache(transform, objects, cache)
-            });
-
-        Self::new(vertices)
+        // There's nothing to actually transform here, as `GlobalEdge` holds no
+        // data. We still need this implementation though, as a new `GlobalEdge`
+        // must be created to represent the new and transformed edge.
+        Self::new()
     }
 }

crates/fj-kernel/src/builder/cycle.rs

@@ -54,41 +54,9 @@ pub trait CycleBuilder {
 
 impl CycleBuilder for PartialCycle {
     fn add_half_edge(&mut self) -> Partial<HalfEdge> {
-        let mut new_half_edge = Partial::<HalfEdge>::new();
-
-        let (first_half_edge, mut last_half_edge) =
-            match self.half_edges.first() {
-                Some(first_half_edge) => {
-                    let first_half_edge = first_half_edge.clone();
-                    let last_half_edge = self
-                        .half_edges
-                        .last()
-                        .cloned()
-                        .unwrap_or_else(|| first_half_edge.clone());
-
-                    (first_half_edge, last_half_edge)
-                }
-                None => (new_half_edge.clone(), new_half_edge.clone()),
-            };
-
-        {
-            let shared_surface_vertex =
-                new_half_edge.read().start_vertex.clone();
-            last_half_edge
-                .write()
-                .infer_global_form(shared_surface_vertex);
-        }
-
-        {
-            let shared_surface_vertex =
-                first_half_edge.read().start_vertex.clone();
-            new_half_edge
-                .write()
-                .infer_global_form(shared_surface_vertex);
-        }
-
-        self.half_edges.push(new_half_edge.clone());
-        new_half_edge
+        let half_edge = Partial::new();
+        self.half_edges.push(half_edge.clone());
+        half_edge
     }
 
     fn update_as_polygon_from_points<O, P>(

crates/fj-kernel/src/builder/edge.rs

@@ -6,7 +6,7 @@ use crate::{
         curve::{Curve, GlobalPath},
         surface::SurfaceGeometry,
     },
-    objects::{GlobalEdge, HalfEdge, Vertex},
+    objects::{HalfEdge, Vertex},
     partial::{MaybeCurve, Partial, PartialGlobalEdge, PartialHalfEdge},
 };
 
@@ -38,15 +38,6 @@ pub trait HalfEdgeBuilder {
         end: Point<2>,
     ) -> Curve;
 
-    /// Infer the global form of the half-edge
-    ///
-    /// Updates the global form referenced by this half-edge, and also returns
-    /// it.
-    fn infer_global_form(
-        &mut self,
-        next_vertex: Partial<Vertex>,
-    ) -> Partial<GlobalEdge>;
-
     /// Infer the vertex positions (surface and global), if not already set
     fn infer_vertex_positions_if_necessary(
         &mut self,
@@ -88,9 +79,6 @@ impl HalfEdgeBuilder for PartialHalfEdge {
             *point_boundary = Some(point_curve);
         }
 
-        let next_vertex = self.start_vertex.clone();
-        self.infer_global_form(next_vertex);
-
         path
     }
 
@@ -151,16 +139,6 @@ impl HalfEdgeBuilder for PartialHalfEdge {
         path
     }
 
-    fn infer_global_form(
-        &mut self,
-        next_vertex: Partial<Vertex>,
-    ) -> Partial<GlobalEdge> {
-        self.global_form.write().vertices =
-            [&self.start_vertex, &next_vertex].map(|vertex| vertex.clone());
-
-        self.global_form.clone()
-    }
-
     fn infer_vertex_positions_if_necessary(
         &mut self,
         surface: &SurfaceGeometry,

crates/fj-kernel/src/objects/full/edge.rs

@@ -1,6 +1,10 @@
 use fj_math::Point;
 
-use crate::{geometry::curve::Curve, objects::Vertex, storage::Handle};
+use crate::{
+    geometry::curve::Curve,
+    objects::Vertex,
+    storage::{Handle, HandleWrapper},
+};
 
 /// A directed edge, defined in a surface's 2D space
 ///
@@ -43,7 +47,7 @@ pub struct HalfEdge {
     curve: Curve,
     boundary: [Point<1>; 2],
     start_vertex: Handle<Vertex>,
-    global_form: Handle<GlobalEdge>,
+    global_form: HandleWrapper<GlobalEdge>,
 }
 
 impl HalfEdge {
@@ -58,7 +62,7 @@ impl HalfEdge {
             curve,
             boundary,
             start_vertex,
-            global_form,
+            global_form: global_form.into(),
         }
     }
 
@@ -96,115 +100,21 @@ impl HalfEdge {
 /// An undirected edge, defined in global (3D) coordinates
 ///
 /// In contrast to [`HalfEdge`], `GlobalEdge` is undirected, meaning it has no
-/// defined direction, and its vertices have no defined order. This means it can
-/// be used to determine whether two [`HalfEdge`]s map to the same `GlobalEdge`,
-/// regardless of their direction.
+/// 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`.
-#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub struct GlobalEdge {
-    vertices: VerticesInNormalizedOrder,
-}
+#[derive(Clone, Debug, Default, Hash)]
+pub struct GlobalEdge {}
 
 impl GlobalEdge {
     /// Create a new instance
     ///
     /// The order of `vertices` is irrelevant. Two `GlobalEdge`s with the same
     /// `curve` and `vertices` will end up being equal, regardless of the order
     /// of `vertices` here.
-    pub fn new(vertices: [Handle<Vertex>; 2]) -> Self {
-        let (vertices, _) = VerticesInNormalizedOrder::new(vertices);
-
-        Self { vertices }
-    }
-
-    /// Access the vertices that bound the edge on the curve
-    ///
-    /// As the name indicates, the order of the returned vertices is normalized
-    /// and might not match the order of the vertices that were passed to
-    /// [`GlobalEdge::new`]. You must not rely on the vertices being in any
-    /// specific order.
-    pub fn vertices(&self) -> &VerticesInNormalizedOrder {
-        &self.vertices
-    }
-}
-
-/// The vertices of a [`GlobalEdge`]
-///
-/// Since [`GlobalEdge`] is the single global representation of an edge in
-/// global space, it must normalize the order of its vertices. Otherwise, it is
-/// possible to construct two [`GlobalEdge`] instances that are meant to
-/// represent the same edge, but aren't equal.
-#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub struct VerticesInNormalizedOrder([Handle<Vertex>; 2]);
-
-impl VerticesInNormalizedOrder {
-    /// Construct a new instance of `VerticesInNormalizedOrder`
-    ///
-    /// The provided vertices can be in any order. The returned `bool` value
-    /// indicates whether the normalization changed the order of the vertices.
-    pub fn new([a, b]: [Handle<Vertex>; 2]) -> (Self, bool) {
-        if a < b {
-            (Self([a, b]), false)
-        } else {
-            (Self([b, a]), true)
-        }
-    }
-
-    /// Access the vertices
-    ///
-    /// The vertices in the returned array will be in normalized order.
-    pub fn access_in_normalized_order(&self) -> [Handle<Vertex>; 2] {
-        self.0.clone()
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use pretty_assertions::assert_eq;
-
-    use crate::{
-        builder::{CycleBuilder, HalfEdgeBuilder},
-        partial::PartialCycle,
-        services::Services,
-    };
-
-    #[test]
-    fn global_edge_equality() {
-        let mut services = Services::new();
-
-        let surface = services.objects.surfaces.xy_plane();
-
-        let a = [0., 0.];
-        let b = [1., 0.];
-        let c = [0., 1.];
-
-        let a_to_b = {
-            let mut cycle = PartialCycle::default();
-
-            let [mut half_edge, next_half_edge, _] =
-                cycle.update_as_polygon_from_points([a, b, c]);
-            half_edge.write().infer_vertex_positions_if_necessary(
-                &surface.geometry(),
-                next_half_edge.read().start_vertex.clone(),
-            );
-
-            half_edge.build(&mut services.objects)
-        };
-        let b_to_a = {
-            let mut cycle = PartialCycle::default();
-
-            let [mut half_edge, next_half_edge, _] =
-                cycle.update_as_polygon_from_points([b, a, c]);
-            half_edge.write().infer_vertex_positions_if_necessary(
-                &surface.geometry(),
-                next_half_edge.read().start_vertex.clone(),
-            );
-
-            half_edge.build(&mut services.objects)
-        };
-
-        assert_eq!(a_to_b.global_form(), b_to_a.global_form());
+    pub fn new() -> Self {
+        Self::default()
     }
 }

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

@@ -80,7 +80,7 @@ mod stores;
 pub use self::{
     full::{
         cycle::{Cycle, HalfEdgesOfCycle},
-        edge::{GlobalEdge, HalfEdge, VerticesInNormalizedOrder},
+        edge::{GlobalEdge, HalfEdge},
         face::{Face, FaceSet, Handedness},
         shell::Shell,
         sketch::Sketch,

crates/fj-kernel/src/partial/objects/edge.rs

@@ -86,11 +86,7 @@ impl Default for PartialHalfEdge {
     fn default() -> Self {
         let curve = None;
         let start_vertex = Partial::default();
-        let end_vertex = Partial::default();
-
-        let global_form = Partial::from_partial(PartialGlobalEdge {
-            vertices: [start_vertex.clone(), end_vertex],
-        });
+        let global_form = Partial::default();
 
         Self {
             curve,
@@ -103,28 +99,16 @@ impl Default for PartialHalfEdge {
 
 /// A partial [`GlobalEdge`]
 #[derive(Clone, Debug, Default)]
-pub struct PartialGlobalEdge {
-    /// The vertices that bound the edge on the curve
-    pub vertices: [Partial<Vertex>; 2],
-}
+pub struct PartialGlobalEdge {}
 
 impl PartialObject for PartialGlobalEdge {
     type Full = GlobalEdge;
 
-    fn from_full(
-        global_edge: &Self::Full,
-        cache: &mut FullToPartialCache,
-    ) -> Self {
-        Self {
-            vertices: global_edge
-                .vertices()
-                .access_in_normalized_order()
-                .map(|vertex| Partial::from_full(vertex, cache)),
-        }
+    fn from_full(_: &Self::Full, _: &mut FullToPartialCache) -> Self {
+        Self {}
     }
 
-    fn build(self, objects: &mut Service<Objects>) -> Self::Full {
-        let vertices = self.vertices.map(|vertex| vertex.build(objects));
-        GlobalEdge::new(vertices)
+    fn build(self, _: &mut Service<Objects>) -> Self::Full {
+        GlobalEdge::new()
     }
 }