From 8438f8a3c71b63bda09dab876cc5553969aded18 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 17:59:18 +0200 Subject: [PATCH] Give `Face` ownership of its surface --- crates/fj-kernel/src/algorithms/sweep.rs | 12 +----------- crates/fj-kernel/src/algorithms/transform.rs | 6 ++---- .../fj-kernel/src/algorithms/triangulation/mod.rs | 2 +- crates/fj-kernel/src/builder.rs | 2 +- crates/fj-kernel/src/objects/face.rs | 8 ++++---- crates/fj-kernel/src/shape/api.rs | 3 ++- crates/fj-kernel/src/shape/object.rs | 7 +++++-- crates/fj-kernel/src/validation/mod.rs | 14 ++++++-------- crates/fj-kernel/src/validation/structural.rs | 2 +- crates/fj-operations/src/circle.rs | 2 +- crates/fj-operations/src/difference_2d.rs | 6 +++--- 11 files changed, 27 insertions(+), 37 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index ddca4106b..3a56d057b 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/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, ) { - 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, ) { - let mut tmp = Shape::new(); - let vertices = { let vertices_top = vertices_bottom.map(|vertex| { let point = vertex.point + path; @@ -177,7 +168,6 @@ fn create_non_continuous_side_face( 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; diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index 4b9bf019f..11e88e4f3 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/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 { 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); diff --git a/crates/fj-kernel/src/algorithms/triangulation/mod.rs b/crates/fj-kernel/src/algorithms/triangulation/mod.rs index f921233fe..cdede849a 100644 --- a/crates/fj-kernel/src/algorithms/triangulation/mod.rs +++ b/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 diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index d1d5d24a2..e2dad7397 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -223,7 +223,7 @@ impl<'r> FaceBuilder<'r> { /// Build the face pub fn build(self) -> Handle { - 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 { diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index a04652bce..acd462e66 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -5,7 +5,7 @@ use fj_math::Triangle; use crate::{ builder::FaceBuilder, - shape::{Handle, LocalForm, Shape}, + shape::{LocalForm, Shape}, }; use super::{Cycle, Surface}; @@ -41,7 +41,7 @@ pub enum Face { impl Face { /// Construct a new instance of `Face` pub fn new( - surface: Handle, + surface: Surface, exteriors: impl IntoIterator, Cycle<3>>>, interiors: impl IntoIterator, Cycle<3>>>, color: [u8; 4], @@ -119,7 +119,7 @@ impl Face { #[derive(Clone, Debug, Eq, Ord, PartialOrd)] pub struct FaceBRep { /// The surface that defines this face - pub surface: Handle, + pub surface: Surface, /// The cycles that bound the face on the outside /// @@ -152,7 +152,7 @@ impl FaceBRep { /// This is a convenience method that saves the caller from dealing with the /// [`Handle`]. pub fn surface(&self) -> Surface { - self.surface.get() + self.surface } /// Access the exterior cycles that the face refers to diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index cd8ea89c7..6e239f12d 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -196,7 +196,8 @@ mod tests { let cycle = shape.insert(cycle); assert!(shape.get_handle(&cycle.get()).as_ref() == Some(&cycle)); - let face = Face::new(surface, Vec::new(), Vec::new(), [0, 0, 0, 0]); + let face = + Face::new(surface.get(), Vec::new(), Vec::new(), [0, 0, 0, 0]); assert!(shape.get_handle(&face).is_none()); let face = shape.insert(face); diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index 8d438418c..54f7d0369 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -72,7 +72,7 @@ impl Object for Face { fn merge_into(self, shape: &mut Shape) -> Handle { match self { Face::Face(face) => { - let surface = face.surface.get().merge_into(shape); + let surface = face.surface.merge_into(shape); let mut exts = Vec::new(); for cycle in face.exteriors.as_local_form() { @@ -93,7 +93,10 @@ impl Object for Face { } shape.get_handle_or_insert(Face::new( - surface, exts, ints, face.color, + surface.get(), + exts, + ints, + face.color, )) } Face::Triangles(_) => shape.get_handle_or_insert(self), diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index ad27aa345..744a6e7ea 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -343,25 +343,23 @@ mod tests { let triangle = [[0., 0.], [1., 0.], [0., 1.]]; - let surface = other.insert(Surface::xy_plane()); - let cycle = - Cycle::builder(surface.get(), &mut other).build_polygon(triangle); + let surface = Surface::xy_plane(); + let cycle = Cycle::builder(surface, &mut other).build_polygon(triangle); // Nothing has been added to `shape`. Should fail. shape.insert(Face::new( - surface.clone(), + surface, vec![cycle.clone()], Vec::new(), [255, 0, 0, 255], )); let err = validate(shape.clone(), &ValidationConfig::default()).unwrap_err(); - assert!(err.missing_surface(&surface.get())); + assert!(err.missing_surface(&surface)); assert!(err.missing_cycle(&cycle.canonical())); - let surface = shape.insert(Surface::xy_plane()); - let cycle = - Cycle::builder(surface.get(), &mut shape).build_polygon(triangle); + let surface = Surface::xy_plane(); + let cycle = Cycle::builder(surface, &mut shape).build_polygon(triangle); // Everything has been added to `shape` now. Should work! shape.insert(Face::new( diff --git a/crates/fj-kernel/src/validation/structural.rs b/crates/fj-kernel/src/validation/structural.rs index 484ea418d..403c1dee0 100644 --- a/crates/fj-kernel/src/validation/structural.rs +++ b/crates/fj-kernel/src/validation/structural.rs @@ -61,7 +61,7 @@ pub fn validate_face( let mut missing_cycles = HashSet::new(); if !surfaces.contains(&face.surface()) { - missing_surface = Some(face.surface.get()); + missing_surface = Some(face.surface); } for cycle in face.all_cycles() { if !cycles.contains(&cycle) { diff --git a/crates/fj-operations/src/circle.rs b/crates/fj-operations/src/circle.rs index e036eff27..daba5ea25 100644 --- a/crates/fj-operations/src/circle.rs +++ b/crates/fj-operations/src/circle.rs @@ -29,7 +29,7 @@ impl ToShape for fj::Circle { }; let cycle_canonical = Cycle::new(vec![edge.canonical()]); - let surface = tmp.insert(Surface::xy_plane()); + let surface = Surface::xy_plane(); let face = tmp .insert(Face::new( surface, diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index af23f4b42..ef6784042 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -36,13 +36,13 @@ impl ToShape for fj::Difference2d { if let Some(face) = a.face_iter().next() { // If there's at least one face to subtract from, we can proceed. - let surface = face.brep().surface.clone(); + let surface = face.brep().surface; for face in a.face_iter() { let face = face.brep(); assert_eq!( - surface.get(), + surface, face.surface(), "Trying to subtract faces with different surfaces.", ); @@ -61,7 +61,7 @@ impl ToShape for fj::Difference2d { let face = face.brep(); assert_eq!( - surface.get(), + surface, face.surface(), "Trying to subtract faces with different surfaces.", );