Merge pull request #309 from hannobraun/shape

Fix various odds and ends around `Shape` API

src/kernel/algorithms/approximation.rs

@@ -262,9 +262,9 @@ mod tests {
         let cd = shape.edges().add(Edge::line_segment([v3, v4.clone()]));
         let da = shape.edges().add(Edge::line_segment([v4, v1]));
 
-        let abcd = Cycle {
+        let abcd = shape.cycles().add(Cycle {
             edges: vec![ab, bc, cd, da],
-        };
+        });
 
         let face = Face::Face {
             surface: Surface::x_y_plane(),

src/kernel/algorithms/sweep.rs

@@ -125,13 +125,15 @@ mod tests {
                 .edges()
                 .add(Edge::line_segment([c.clone(), a.clone()]));
 
+            let cycles = shape.cycles().add(Cycle {
+                edges: vec![ab, bc, ca],
+            });
+
             let abc = Face::Face {
                 surface: Surface::Swept(Swept::plane_from_points(
                     [a, b, c].map(|vertex| vertex.point()),
                 )),
-                cycles: vec![Cycle {
+                cycles: vec![cycles],
-                    edges: vec![ab, bc, ca],
-                }],
             };
 
             let face = shape.faces().add(abc);

src/kernel/algorithms/transform.rs

@@ -43,7 +43,7 @@ pub fn transform_face(
             for cycle in cycles {
                 let mut edges = Vec::new();
 
-                for edge in cycle.edges {
+                for edge in &cycle.edges {
                     let vertices = edge.vertices.clone().map(|vertices| {
                         vertices.map(|vertex| {
                             let point =
@@ -62,7 +62,7 @@ pub fn transform_face(
                     edges.push(edge);
                 }
 
-                cycles_trans.push(Cycle { edges });
+                cycles_trans.push(shape.cycles().add(Cycle { edges }));
             }
 
             Face::Face {

src/kernel/shapes/circle.rs

@@ -25,11 +25,7 @@ impl ToShape for fj::Circle {
             .add(Edge::circle(Scalar::from_f64(self.radius)));
         shape.cycles().add(Cycle { edges: vec![edge] });
 
-        let cycles = shape
+        let cycles = shape.cycles().all().collect();
-            .cycles()
-            .all()
-            .map(|handle| (*handle).clone())
-            .collect();
         shape.faces().add(Face::Face {
             cycles,
             surface: Surface::x_y_plane(),

src/kernel/shapes/sketch.rs

@@ -48,11 +48,7 @@ impl ToShape for fj::Sketch {
         };
 
         let face = Face::Face {
-            cycles: shape
+            cycles: shape.cycles().all().collect(),
-                .cycles()
-                .all()
-                .map(|handle| (*handle).clone())
-                .collect(),
             surface: Surface::x_y_plane(),
         };
         shape.faces().add(face);

src/kernel/topology/faces.rs

@@ -10,6 +10,7 @@ use crate::{
             approximation::Approximation, triangulation::triangulate,
         },
         geometry::Surface,
+        shape::handle::Handle,
     },
     math::{Aabb, Scalar, Segment, Triangle},
 };
@@ -37,7 +38,7 @@ pub enum Face {
         ///
         /// It might be less error-prone to specify the edges in surface
         /// coordinates.
-        cycles: Vec<Cycle>,
+        cycles: Vec<Handle<Cycle>>,
     },
 
     /// The triangles of the face

src/kernel/topology/vertices.rs

@@ -8,22 +8,6 @@ use crate::math::Point;
 ///
 /// Points, on the other hand, might be used to approximate a shape for various
 /// purposes, without presenting any deeper truth about the shape's structure.
-///
-/// # Uniqueness
-///
-/// You **MUST NOT** construct a new instance of `Vertex` that represents an
-/// already existing vertex. If there already exists a vertex and you need a
-/// `Vertex` instance to refer to it, acquire one by copying or converting the
-/// existing `Vertex` instance.
-///
-/// Every time you create a `Vertex` instance, you might do so using a point you
-/// have computed. When doing this for an existing vertex, you run the risk of
-/// computing a slightly different point, due to floating point accuracy issues.
-/// The resulting `Vertex` will then no longer be equal to the existing `Vertex`
-/// instance that refers to the same vertex, which will cause bugs.
-///
-/// This can be prevented outright by never creating a new `Vertex` instance
-/// for an existing vertex. Hence why this is strictly forbidden.
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Vertex(Point<3>);