Merge pull request #625 from hannobraun/edge

Simplify local representation of edge vertices

crates/fj-kernel/src/algorithms/approx/edges.rs

@@ -1,7 +1,9 @@
+use fj_math::Point;
+
 use crate::{geometry, shape::LocalForm, topology::Vertex};
 
 pub fn approximate_edge(
-    vertices: Option<[LocalForm<Vertex<1>, Vertex<3>>; 2]>,
+    vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>,
     points: &mut Vec<geometry::Point<1, 3>>,
 ) {
     // Insert the exact vertices of this edge into the approximation. This means
@@ -15,7 +17,7 @@ pub fn approximate_edge(
     let vertices = vertices.map(|vertices| {
         vertices.map(|vertex| {
             geometry::Point::new(
-                *vertex.local().point.local(),
+                *vertex.local(),
                 vertex.canonical().get().point(),
             )
         })
@@ -58,8 +60,8 @@ mod test {
         let v2 = Vertex::builder(&mut shape).build_from_point(d)?;
 
         let vertices = [
-            LocalForm::new(v1.get().with_local_form([0.]), v1),
-            LocalForm::new(v2.get().with_local_form([1.]), v2),
+            LocalForm::new(Point::from([0.]), v1),
+            LocalForm::new(Point::from([1.]), v2),
         ];
 
         let a = geometry::Point::new([0.0], a);

crates/fj-kernel/src/shape/api.rs

@@ -226,7 +226,7 @@ impl Shape {
     /// Access iterator over all vertices
     ///
     /// The caller must not make any assumptions about the order of vertices.
-    pub fn vertices(&self) -> Iter<Vertex<3>> {
+    pub fn vertices(&self) -> Iter<Vertex> {
         self.stores.vertices.iter()
     }
 
@@ -293,7 +293,7 @@ mod tests {
         assert!(shape.get_handle(&curve.get()).as_ref() == Some(&curve));
         assert!(shape.get_handle(&surface.get()).as_ref() == Some(&surface));
 
-        let vertex = Vertex::new(point);
+        let vertex = Vertex { point };
         let edge = Edge::new(curve, None);
 
         assert!(shape.get_handle(&vertex).is_none());
@@ -326,23 +326,23 @@ mod tests {
         let mut other = Shape::new();
 
         let point = shape.insert(Point::from([0., 0., 0.]))?;
-        shape.insert(Vertex::new(point))?;
+        shape.insert(Vertex { point })?;
 
         // Should fail, as `point` is not part of the shape.
         let point = other.insert(Point::from([1., 0., 0.]))?;
-        let result = shape.insert(Vertex::new(point));
+        let result = shape.insert(Vertex { point });
         assert!(matches!(result, Err(ValidationError::Structural(_))));
 
         // `point` is too close to the original point. `assert!` is commented,
         // because that only causes a warning to be logged right now.
         let point = shape.insert(Point::from([5e-8, 0., 0.]))?;
-        let result = shape.insert(Vertex::new(point));
+        let result = shape.insert(Vertex { point });
         assert!(matches!(result, Err(ValidationError::Uniqueness(_))));
 
         // `point` is farther than `MIN_DISTANCE` away from original point.
         // Should work.
         let point = shape.insert(Point::from([5e-6, 0., 0.]))?;
-        shape.insert(Vertex::new(point))?;
+        shape.insert(Vertex { point })?;
 
         Ok(())
     }

crates/fj-kernel/src/shape/mapping.rs

@@ -14,7 +14,7 @@ pub struct Mapping {
     pub(super) points: OneMapping<Point<3>>,
     pub(super) curves: OneMapping<Curve<3>>,
     pub(super) surfaces: OneMapping<Surface>,
-    pub(super) vertices: OneMapping<Vertex<3>>,
+    pub(super) vertices: OneMapping<Vertex>,
     pub(super) edges: OneMapping<Edge<3>>,
     pub(super) cycles: OneMapping<Cycle<3>>,
     pub(super) faces: OneMapping<Face>,
@@ -49,7 +49,7 @@ impl Mapping {
     }
 
     /// Access iterator over the mapped vertices
-    pub fn vertices(&self) -> &OneMapping<Vertex<3>> {
+    pub fn vertices(&self) -> &OneMapping<Vertex> {
         &self.vertices
     }
 

crates/fj-kernel/src/shape/object.rs

@@ -26,7 +26,7 @@ impl private::Sealed for Point<3> {}
 impl private::Sealed for Curve<3> {}
 impl private::Sealed for Surface {}
 
-impl private::Sealed for Vertex<3> {}
+impl private::Sealed for Vertex {}
 impl private::Sealed for Edge<3> {}
 impl private::Sealed for Cycle<3> {}
 impl private::Sealed for Face {}
@@ -82,19 +82,16 @@ impl Object for Surface {
     }
 }
 
-impl Object for Vertex<3> {
+impl Object for Vertex {
     fn merge_into(
         self,
         handle: Option<Handle<Self>>,
         shape: &mut Shape,
         mapping: &mut Mapping,
     ) -> ValidationResult<Self> {
-        let point = self.point().merge_into(
-            Some(self.point.canonical()),
-            shape,
-            mapping,
-        )?;
-        let merged = shape.get_handle_or_insert(Vertex::new(point))?;
+        let point =
+            self.point().merge_into(Some(self.point), shape, mapping)?;
+        let merged = shape.get_handle_or_insert(Vertex { point })?;
 
         if let Some(handle) = handle {
             mapping.vertices.insert(handle, merged.clone());

crates/fj-kernel/src/shape/stores.rs

@@ -22,7 +22,7 @@ pub struct Stores {
     pub curves: Store<Curve<3>>,
     pub surfaces: Store<Surface>,
 
-    pub vertices: Store<Vertex<3>>,
+    pub vertices: Store<Vertex>,
     pub edges: Store<Edge<3>>,
     pub cycles: Store<Cycle<3>>,
     pub faces: Store<Face>,

crates/fj-kernel/src/shape/validate/mod.rs

@@ -56,7 +56,7 @@ impl Validate for Surface {
     }
 }
 
-impl Validate for Vertex<3> {
+impl Validate for Vertex {
     /// Validate the vertex
     ///
     /// # Implementation note
@@ -163,7 +163,7 @@ impl ValidationError {
 
     /// Indicate whether validation found a missing vertex
     #[cfg(test)]
-    pub fn missing_vertex(&self, vertex: &Handle<Vertex<3>>) -> bool {
+    pub fn missing_vertex(&self, vertex: &Handle<Vertex>) -> bool {
         if let Self::Structural(StructuralIssues {
             missing_vertices, ..
         }) = self

crates/fj-kernel/src/shape/validate/structural.rs

@@ -9,14 +9,12 @@ use crate::{
 };
 
 pub fn validate_vertex(
-    vertex: &Vertex<3>,
+    vertex: &Vertex,
     stores: &Stores,
 ) -> Result<(), StructuralIssues> {
-    let point = vertex.point.canonical();
-
-    if !stores.points.contains(&point) {
+    if !stores.points.contains(&vertex.point) {
         return Err(StructuralIssues {
-            missing_point: Some(point),
+            missing_point: Some(vertex.point.clone()),
             ..StructuralIssues::default()
         });
     }
@@ -119,7 +117,7 @@ pub struct StructuralIssues {
     pub missing_curve: Option<Handle<Curve<3>>>,
 
     /// Missing vertices found in edge validation
-    pub missing_vertices: HashSet<Handle<Vertex<3>>>,
+    pub missing_vertices: HashSet<Handle<Vertex>>,
 
     /// Missing edges found in cycle validation
     pub missing_edges: HashSet<Handle<Edge<3>>>,

crates/fj-kernel/src/shape/validate/uniqueness.rs

@@ -8,8 +8,8 @@ use crate::{
 };
 
 pub fn validate_vertex(
-    vertex: &Vertex<3>,
-    handle: Option<&Handle<Vertex<3>>>,
+    vertex: &Vertex,
+    handle: Option<&Handle<Vertex>>,
     min_distance: Scalar,
     stores: &Stores,
 ) -> Result<(), UniquenessIssues> {

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

@@ -26,9 +26,9 @@ impl<'r> VertexBuilder<'r> {
     pub fn build_from_point(
         self,
         point: impl Into<Point<3>>,
-    ) -> ValidationResult<Vertex<3>> {
+    ) -> ValidationResult<Vertex> {
         let point = self.shape.get_handle_or_insert(point.into())?;
-        let vertex = self.shape.get_handle_or_insert(Vertex::new(point))?;
+        let vertex = self.shape.get_handle_or_insert(Vertex { point })?;
 
         Ok(vertex)
     }
@@ -80,7 +80,7 @@ impl<'r> EdgeBuilder<'r> {
     /// Build a line segment from two vertices
     pub fn build_line_segment_from_vertices(
         self,
-        vertices: [Handle<Vertex<3>>; 2],
+        vertices: [Handle<Vertex>; 2],
     ) -> ValidationResult<Edge<3>> {
         let curve = self.shape.insert(Curve::Line(Line::from_points(
             vertices.clone().map(|vertex| vertex.get().point()),

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

@@ -1,5 +1,7 @@
 use std::fmt;
 
+use fj_math::Point;
+
 use crate::{
     geometry::Curve,
     shape::{Handle, LocalForm, Shape},
@@ -32,7 +34,7 @@ pub struct Edge<const D: usize> {
     ///
     /// If there are no such vertices, that means that both the curve and the
     /// edge are continuous (i.e. connected to themselves).
-    pub vertices: Option<[LocalForm<Vertex<1>, Vertex<3>>; 2]>,
+    pub vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>,
 }
 
 impl Edge<3> {
@@ -58,7 +60,7 @@ impl Edge<3> {
     /// <https://github.com/hannobraun/Fornjot/issues/399>
     pub fn new(
         curve: Handle<Curve<3>>,
-        vertices: Option<[Handle<Vertex<3>>; 2]>,
+        vertices: Option<[Handle<Vertex>; 2]>,
     ) -> Self {
         let curve = LocalForm::canonical_only(curve);
 
@@ -69,7 +71,6 @@ impl Edge<3> {
                     .get()
                     .point_to_curve_coords(canonical.get().point())
                     .local();
-                let local = canonical.get().with_local_form(local);
                 LocalForm::new(local, canonical)
             })
         });
@@ -96,7 +97,7 @@ impl<const D: usize> Edge<D> {
     ///
     /// This is a convenience method that saves the caller from dealing with the
     /// [`Handle`]s.
-    pub fn vertices(&self) -> Option<[Vertex<3>; 2]> {
+    pub fn vertices(&self) -> Option<[Vertex; 2]> {
         self.vertices
             .as_ref()
             .map(|[a, b]| [a.canonical().get(), b.canonical().get()])

crates/fj-kernel/src/topology/vertex.rs

@@ -2,7 +2,7 @@ use std::hash::Hash;
 
 use fj_math::Point;
 
-use crate::shape::{Handle, LocalForm, Shape};
+use crate::shape::{Handle, Shape};
 
 use super::VertexBuilder;
 
@@ -31,19 +31,12 @@ use super::VertexBuilder;
 /// between distinct vertices can be configured using
 /// [`Shape::with_minimum_distance`].
 #[derive(Clone, Debug, Eq, Ord, PartialOrd)]
-pub struct Vertex<const D: usize> {
+pub struct Vertex {
     /// The point that defines the location of the vertex
-    pub point: LocalForm<Point<D>, Point<3>>,
+    pub point: Handle<Point<3>>,
 }
 
-impl Vertex<3> {
-    /// Construct a new instance of `Vertex`
-    pub fn new(point: Handle<Point<3>>) -> Self {
-        Self {
-            point: LocalForm::new(point.get(), point),
-        }
-    }
-
+impl Vertex {
     /// Build a vertex using the [`VertexBuilder`] API
     pub fn builder(shape: &mut Shape) -> VertexBuilder {
         VertexBuilder::new(shape)
@@ -54,28 +47,18 @@ impl Vertex<3> {
     /// This is a convenience method that saves the caller from dealing with the
     /// [`Handle`].
     pub fn point(&self) -> Point<3> {
-        self.point.canonical().get()
-    }
-
-    /// Add a local form of the point to the vertex
-    pub fn with_local_form<const D: usize>(
-        self,
-        local: impl Into<Point<D>>,
-    ) -> Vertex<D> {
-        Vertex {
-            point: LocalForm::new(local.into(), self.point.canonical()),
-        }
+        self.point.get()
     }
 }
 
-impl<const D: usize> PartialEq for Vertex<D> {
+impl PartialEq for Vertex {
     fn eq(&self, other: &Self) -> bool {
-        self.point == other.point
+        self.point() == other.point()
     }
 }
 
-impl<const D: usize> Hash for Vertex<D> {
+impl Hash for Vertex {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
-        self.point.hash(state);
+        self.point().hash(state);
     }
 }