Merge pull request #736 from hannobraun/handle

Replace `Handle`s between objects with direct ownership

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

@@ -13,7 +13,7 @@ pub fn approx_edge(
     // the same vertex would be understood to refer to very close, but distinct
     // vertices.
     let vertices = vertices.convert(|vertex| {
-        geometry::Point::new(*vertex.local(), vertex.canonical().get().point)
+        geometry::Point::new(*vertex.local(), vertex.canonical().point)
     });
     if let Some([a, b]) = vertices {
         points.insert(0, a);
@@ -49,8 +49,8 @@ mod test {
         let c = Point::from([3., 5., 8.]);
         let d = Point::from([5., 8., 13.]);
 
-        let v1 = Vertex::builder(&mut shape).build_from_point(a);
-        let v2 = Vertex::builder(&mut shape).build_from_point(d);
+        let v1 = Vertex::builder(&mut shape).build_from_point(a).get();
+        let v2 = Vertex::builder(&mut shape).build_from_point(d).get();
 
         let vertices = VerticesOfEdge::from_vertices([
             LocalForm::new(Point::from([0.]), v1),

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

@@ -5,7 +5,7 @@ use crate::{
     objects::{
         Curve, Cycle, CyclesInFace, Edge, Face, Surface, Vertex, VerticesOfEdge,
     },
-    shape::{LocalForm, Shape},
+    shape::LocalForm,
 };
 
 use super::{transform::transform_cycles, CycleApprox, Tolerance};
@@ -61,8 +61,6 @@ fn create_bottom_faces(
     is_sweep_along_negative_direction: bool,
     target: &mut Vec<Face>,
 ) {
-    let mut tmp = Shape::new();
-
     let mut surface = face.surface();
 
     let mut exteriors = face.brep().exteriors.clone();
@@ -75,8 +73,6 @@ fn create_bottom_faces(
         interiors = reverse_local_coordinates_in_cycle(&interiors);
     };
 
-    let surface = tmp.insert(surface);
-
     let face = Face::new(
         surface,
         exteriors.as_local_form().cloned(),
@@ -111,9 +107,6 @@ fn create_top_face(
         interiors = reverse_local_coordinates_in_cycle(&interiors);
     };
 
-    let mut tmp = Shape::new();
-    let surface = tmp.insert(surface);
-
     let face = Face::new(
         surface,
         exteriors.as_local_form().cloned(),
@@ -156,8 +149,6 @@ fn create_non_continuous_side_face(
     color: [u8; 4],
     target: &mut Vec<Face>,
 ) {
-    let mut tmp = Shape::new();
-
     let vertices = {
         let vertices_top = vertices_bottom.map(|vertex| {
             let point = vertex.point + path;
@@ -166,20 +157,17 @@ fn create_non_continuous_side_face(
 
         let [[a, b], [c, d]] = [vertices_bottom, vertices_top];
 
-        let vertices = if is_sweep_along_negative_direction {
+        if is_sweep_along_negative_direction {
             [b, a, c, d]
         } else {
             [a, b, d, c]
-        };
-
-        vertices.map(|vertex| tmp.get_handle_or_insert(vertex))
+        }
     };
 
     let surface = {
-        let [a, b, _, c] = vertices.clone().map(|vertex| vertex.get().point);
+        let [a, b, _, c] = vertices.map(|vertex| vertex.point);
         Surface::plane_from_points([a, b, c])
     };
-    let surface = tmp.get_handle_or_insert(surface);
 
     let cycle = {
         let [a, b, c, d] = vertices;
@@ -201,16 +189,15 @@ fn create_non_continuous_side_face(
             let curve = {
                 let local = Curve::line_from_points([a.0, b.0]);
 
-                let global = [a, b].map(|vertex| vertex.1.get().point);
+                let global = [a, b].map(|vertex| vertex.1.point);
                 let global = Curve::line_from_points(global);
-                let global = tmp.get_handle_or_insert(global);
 
                 LocalForm::new(local, global)
             };
 
             let vertices = VerticesOfEdge::from_vertices([
-                LocalForm::new(Point::from([0.]), a.1.clone()),
-                LocalForm::new(Point::from([1.]), b.1.clone()),
+                LocalForm::new(Point::from([0.]), a.1),
+                LocalForm::new(Point::from([1.]), b.1),
             ]);
 
             let edge = {
@@ -223,7 +210,6 @@ fn create_non_continuous_side_face(
                     curve: LocalForm::canonical_only(curve.canonical()),
                     vertices,
                 };
-                let global = tmp.get_handle_or_insert(global);
 
                 LocalForm::new(local, global)
             };
@@ -236,7 +222,6 @@ fn create_non_continuous_side_face(
 
             let global =
                 Cycle::new(local.edges.iter().map(|edge| edge.canonical()));
-            let global = tmp.get_handle_or_insert(global);
 
             LocalForm::new(local, global)
         };
@@ -257,8 +242,6 @@ fn create_continuous_side_face(
 ) {
     let translation = Transform::translation(path);
 
-    let mut tmp = Shape::new();
-    let edge = tmp.merge(edge);
     let cycle = Cycle::new(vec![edge]);
     let approx = CycleApprox::new(&cycle, tolerance);
 

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

@@ -2,7 +2,7 @@ use fj_math::Transform;
 
 use crate::{
     objects::{Cycle, CyclesInFace, Edge, Face, FaceBRep, Vertex},
-    shape::{LocalForm, Shape},
+    shape::LocalForm,
 };
 
 /// Transform a shape
@@ -12,9 +12,7 @@ pub fn transform(faces: &[Face], transform: &Transform) -> Vec<Face> {
     for face in faces {
         let face = match face {
             Face::Face(face) => {
-                let mut tmp = Shape::new();
-                let surface = face.surface.get().transform(transform);
-                let surface = tmp.insert(surface);
+                let surface = face.surface.transform(transform);
 
                 let exteriors = transform_cycles(&face.exteriors, transform);
                 let interiors = transform_cycles(&face.interiors, transform);
@@ -49,74 +47,69 @@ pub fn transform_cycles(
     cycles: &CyclesInFace,
     transform: &Transform,
 ) -> CyclesInFace {
-    let mut tmp = Shape::new();
-
     let cycles = cycles.as_local_form().map(|cycle| {
         let edges_local = cycle
             .local()
             .edges
             .iter()
             .map(|edge| {
                 let curve_local = *edge.local().curve.local();
-                let curve_canonical = tmp
-                    .merge(edge.canonical().get().curve().transform(transform));
+                let curve_canonical =
+                    edge.canonical().curve().transform(transform);
 
-                let vertices = edge.canonical().get().vertices.map(|vertex| {
-                    let point = vertex.canonical().get().point;
+                let vertices = edge.canonical().vertices.map(|vertex| {
+                    let point = vertex.canonical().point;
                     let point = transform.transform_point(&point);
 
                     let local = *vertex.local();
-                    let canonical = tmp.merge(Vertex { point });
+                    let canonical = Vertex { point };
 
                     LocalForm::new(local, canonical)
                 });
 
                 let edge_local = Edge {
-                    curve: LocalForm::new(curve_local, curve_canonical.clone()),
+                    curve: LocalForm::new(curve_local, curve_canonical),
                     vertices: vertices.clone(),
                 };
-                let edge_canonical = tmp.merge(Edge {
+                let edge_canonical = Edge {
                     curve: LocalForm::canonical_only(curve_canonical),
                     vertices,
-                });
+                };
 
                 LocalForm::new(edge_local, edge_canonical)
             })
             .collect();
         let edges_canonical = cycle
             .canonical()
-            .get()
             .edges
             .iter()
             .map(|edge| {
-                let edge = edge.canonical().get();
+                let edge = edge.canonical();
 
                 let curve = {
                     let curve = edge.curve().transform(transform);
-
-                    let curve = tmp.merge(curve);
                     LocalForm::canonical_only(curve)
                 };
                 let vertices = edge.vertices.map(|vertex| {
-                    let point = vertex.canonical().get().point;
+                    let point = vertex.canonical().point;
                     let point = transform.transform_point(&point);
 
                     let local = *vertex.local();
-                    let canonical = tmp.merge(Vertex { point });
+                    let canonical = Vertex { point };
 
                     LocalForm::new(local, canonical)
                 });
 
-                let edge = tmp.merge(Edge { curve, vertices });
+                let edge = Edge { curve, vertices };
                 LocalForm::canonical_only(edge)
             })
             .collect();
 
         let cycle_local = Cycle { edges: edges_local };
 
-        let cycle_canonical = tmp.merge(Cycle {
+        let cycle_canonical = Cycle {
             edges: edges_canonical,
-        });
+        };
 
         LocalForm::new(cycle_local, cycle_canonical)
     });

crates/fj-kernel/src/algorithms/triangulation/mod.rs

@@ -22,7 +22,7 @@ pub fn triangulate(
     for face in faces {
         match &face {
             Face::Face(brep) => {
-                let surface = brep.surface.get();
+                let surface = brep.surface;
                 let approx = FaceApprox::new(&face, tolerance);
 
                 let points: Vec<_> = approx

crates/fj-kernel/src/builder.rs

@@ -51,21 +51,20 @@ impl<'r> EdgeBuilder<'r> {
             a: Vector::from([radius, Scalar::ZERO]),
             b: Vector::from([Scalar::ZERO, radius]),
         });
-        let curve_canonical =
-            self.shape.get_handle_or_insert(Curve::Circle(Circle {
-                center: Point::origin(),
-                a: Vector::from([radius, Scalar::ZERO, Scalar::ZERO]),
-                b: Vector::from([Scalar::ZERO, radius, Scalar::ZERO]),
-            }));
+        let curve_canonical = Curve::Circle(Circle {
+            center: Point::origin(),
+            a: Vector::from([radius, Scalar::ZERO, Scalar::ZERO]),
+            b: Vector::from([Scalar::ZERO, radius, Scalar::ZERO]),
+        });
 
         let edge_local = Edge {
-            curve: LocalForm::new(curve_local, curve_canonical.clone()),
+            curve: LocalForm::new(curve_local, curve_canonical),
             vertices: VerticesOfEdge::none(),
         };
-        let edge_canonical = self.shape.get_handle_or_insert(Edge {
+        let edge_canonical = Edge {
             curve: LocalForm::canonical_only(curve_canonical),
             vertices: VerticesOfEdge::none(),
-        });
+        };
 
         LocalForm::new(edge_local, edge_canonical)
     }
@@ -75,23 +74,22 @@ impl<'r> EdgeBuilder<'r> {
         self,
         vertices: [impl Into<Point<3>>; 2],
     ) -> Handle<Edge<3>> {
-        // Can be cleaned up with `try_map`, once that is stable:
-        // https://doc.rust-lang.org/std/primitive.array.html#method.try_map
-        let vertices = vertices
-            .map(|point| Vertex::builder(self.shape).build_from_point(point));
+        let vertices = vertices.map(|point| {
+            let point = point.into();
+            Vertex { point }
+        });
 
         self.build_line_segment_from_vertices(vertices)
     }
 
     /// Build a line segment from two vertices
     pub fn build_line_segment_from_vertices(
         self,
-        [a, b]: [Handle<Vertex>; 2],
+        [a, b]: [Vertex; 2],
     ) -> Handle<Edge<3>> {
         let curve = {
-            let points = [&a, &b].map(|vertex| vertex.get().point);
-            let curve = Curve::Line(Line::from_points(points));
-            self.shape.get_handle_or_insert(curve)
+            let points = [a, b].map(|vertex| vertex.point);
+            Curve::Line(Line::from_points(points))
         };
 
         let vertices = [
@@ -142,14 +140,15 @@ impl<'r> CycleBuilder<'r> {
             let points_canonical = points
                 .map(|point| self.surface.point_from_surface_coords(point));
             let edge_canonical = Edge::builder(self.shape)
-                .build_line_segment_from_points(points_canonical);
+                .build_line_segment_from_points(points_canonical)
+                .get();
 
             let edge_local = Edge {
                 curve: LocalForm::new(
                     Curve::Line(Line::from_points(points)),
-                    edge_canonical.get().curve.canonical(),
+                    edge_canonical.curve.canonical(),
                 ),
-                vertices: edge_canonical.get().vertices,
+                vertices: edge_canonical.vertices.clone(),
             };
 
             edges.push(LocalForm::new(edge_local, edge_canonical));
@@ -160,8 +159,7 @@ impl<'r> CycleBuilder<'r> {
         };
 
         let edges_canonical = edges.into_iter().map(|edge| edge.canonical());
-        let canonical =
-            self.shape.get_handle_or_insert(Cycle::new(edges_canonical));
+        let canonical = Cycle::new(edges_canonical);
 
         LocalForm::new(local, canonical)
     }
@@ -225,7 +223,7 @@ impl<'r> FaceBuilder<'r> {
 
     /// Build the face
     pub fn build(self) -> Handle<Face> {
-        let surface = self.shape.get_handle_or_insert(self.surface);
+        let surface = self.surface;
 
         let mut exteriors = Vec::new();
         if let Some(points) = self.exterior {

crates/fj-kernel/src/iter.rs

@@ -463,8 +463,7 @@ mod tests {
         let mut shape = Shape::new();
         let cycle = Cycle::builder(Surface::xy_plane(), &mut shape)
             .build_polygon([[0., 0.], [1., 0.], [0., 1.]])
-            .canonical()
-            .get();
+            .canonical();
 
         assert_eq!(3, cycle.curve_iter().count());
         assert_eq!(1, cycle.cycle_iter().count());

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

@@ -1,6 +1,6 @@
 use crate::{
     builder::CycleBuilder,
-    shape::{Handle, LocalForm, Shape},
+    shape::{LocalForm, Shape},
 };
 
 use super::{Edge, Surface};
@@ -11,11 +11,6 @@ use super::{Edge, Surface};
 /// edge. The end of the last edge must connect to the beginning of the first
 /// one.
 ///
-/// # Equality
-///
-/// Please refer to [`crate::kernel::topology`] for documentation on the
-/// equality of topological objects.
-///
 /// # Validation
 ///
 /// A cycle that is part of a [`Shape`] must be structurally sound. That means
@@ -28,7 +23,7 @@ pub struct Cycle<const D: usize> {
 
 impl Cycle<3> {
     /// Construct a `Cycle`
-    pub fn new(edges: impl IntoIterator<Item = Handle<Edge<3>>>) -> Self {
+    pub fn new(edges: impl IntoIterator<Item = Edge<3>>) -> Self {
         let edges = edges.into_iter().map(LocalForm::canonical_only).collect();
 
         Self { edges }
@@ -44,6 +39,6 @@ impl Cycle<3> {
     /// This is a convenience method that saves the caller from dealing with the
     /// [`Handle`]s.
     pub fn edges(&self) -> impl Iterator<Item = Edge<3>> + '_ {
-        self.edges.iter().map(|handle| handle.canonical().get())
+        self.edges.iter().map(|handle| handle.canonical())
     }
 }