From 5300217c35ca1898640184c8a23d4cafc382b5e4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 14:38:48 +0200 Subject: [PATCH 01/26] Consolidate redundant code --- crates/fj-kernel/src/objects/face.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 1076fa08c..5b565f8b1 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -78,7 +78,7 @@ impl Face { /// This is equivalent to chaining the iterators returned by /// [`Face::exteriors`] and [`Face::interiors`]. pub fn all_cycles(&self) -> impl Iterator + '_ { - self.exteriors().chain(self.interiors()) + self.brep().all_cycles() } /// Access the color of the face From 199c150e876836b5a50aec979645d11ff3f1088c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 14:39:49 +0200 Subject: [PATCH 02/26] Refactor --- crates/fj-kernel/src/objects/face.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 5b565f8b1..4f6bb34d0 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -48,14 +48,13 @@ impl Face { /// Access the boundary representation of the face pub fn brep(&self) -> &FaceBRep { - match self { - Self::Face(face) => face, - _ => { - // No code that still uses triangle representation is calling - // this method. - unreachable!() - } + if let Self::Face(face) = self { + return face; } + + // No code that still uses triangle representation is calling this + // method. + unreachable!() } /// Access this face's surface From 5e04bc31008ef227b146ab09f9a5bd3ddd7cca53 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 14:40:42 +0200 Subject: [PATCH 03/26] Return reference from `Face::surface` --- crates/fj-kernel/src/objects/face.rs | 6 +++--- crates/fj-operations/src/difference_2d.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 4f6bb34d0..acd62d01b 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -58,7 +58,7 @@ impl Face { } /// Access this face's surface - pub fn surface(&self) -> Surface { + pub fn surface(&self) -> &Surface { self.brep().surface() } @@ -123,8 +123,8 @@ pub struct FaceBRep { impl FaceBRep { /// Access this face's surface - pub fn surface(&self) -> Surface { - self.surface + pub fn surface(&self) -> &Surface { + &self.surface } /// Access this face's exterior cycles diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 35e6d6bc2..58c04a697 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -45,7 +45,7 @@ impl Shape for fj::Difference2d { assert_eq!( surface, - face.surface(), + *face.surface(), "Trying to subtract faces with different surfaces.", ); @@ -64,7 +64,7 @@ impl Shape for fj::Difference2d { assert_eq!( surface, - face.surface(), + *face.surface(), "Trying to subtract faces with different surfaces.", ); From 183deec7d0fb2315bd9622c340b7022210262ea8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 14:48:46 +0200 Subject: [PATCH 04/26] Return references from `Face`'s `Cycle` accessors --- crates/fj-kernel/src/algorithms/approx/faces.rs | 4 ++-- crates/fj-kernel/src/algorithms/sweep.rs | 4 ++-- crates/fj-kernel/src/algorithms/transform.rs | 4 +++- crates/fj-kernel/src/objects/face.rs | 16 ++++++++-------- crates/fj-operations/src/difference_2d.rs | 6 +++--- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/faces.rs b/crates/fj-kernel/src/algorithms/approx/faces.rs index 9bcefddd0..c296dd123 100644 --- a/crates/fj-kernel/src/algorithms/approx/faces.rs +++ b/crates/fj-kernel/src/algorithms/approx/faces.rs @@ -46,13 +46,13 @@ impl FaceApprox { let mut interiors = HashSet::new(); for cycle in face.exteriors() { - let cycle = CycleApprox::new(&cycle, tolerance); + let cycle = CycleApprox::new(cycle, tolerance); points.extend(cycle.points.iter().copied()); exteriors.push(cycle); } for cycle in face.interiors() { - let cycle = CycleApprox::new(&cycle, tolerance); + let cycle = CycleApprox::new(cycle, tolerance); points.extend(cycle.points.iter().copied()); interiors.insert(cycle); diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index 2e0d1c752..d0ef1f7ba 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -39,7 +39,7 @@ pub fn sweep( ); for cycle in face.all_cycles() { - for edge in cycle.edges { + for edge in &cycle.edges { if let Some(vertices) = edge.vertices().get() { create_non_continuous_side_face( path, @@ -52,7 +52,7 @@ pub fn sweep( } create_continuous_side_face( - edge, + *edge, path, tolerance, color, diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index 6a22e8268..7c2daa840 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/crates/fj-kernel/src/algorithms/transform.rs @@ -156,7 +156,9 @@ fn transform_cycles( cycles: &CyclesInFace, transform: &Transform, ) -> CyclesInFace { - let cycles = cycles.as_local().map(|cycle| cycle.transform(transform)); + let cycles = cycles + .as_local() + .map(|cycle| cycle.clone().transform(transform)); CyclesInFace::new(cycles) } diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index acd62d01b..0330aa4a6 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -63,12 +63,12 @@ impl Face { } /// Access this face's exterior cycles - pub fn exteriors(&self) -> impl Iterator + '_ { + pub fn exteriors(&self) -> impl Iterator + '_ { self.brep().exteriors() } /// Access this face's interior cycles - pub fn interiors(&self) -> impl Iterator + '_ { + pub fn interiors(&self) -> impl Iterator + '_ { self.brep().interiors() } @@ -76,7 +76,7 @@ impl Face { /// /// This is equivalent to chaining the iterators returned by /// [`Face::exteriors`] and [`Face::interiors`]. - pub fn all_cycles(&self) -> impl Iterator + '_ { + pub fn all_cycles(&self) -> impl Iterator + '_ { self.brep().all_cycles() } @@ -128,12 +128,12 @@ impl FaceBRep { } /// Access this face's exterior cycles - pub fn exteriors(&self) -> impl Iterator + '_ { + pub fn exteriors(&self) -> impl Iterator + '_ { self.exteriors.as_local() } /// Access this face's interior cycles - pub fn interiors(&self) -> impl Iterator + '_ { + pub fn interiors(&self) -> impl Iterator + '_ { self.interiors.as_local() } @@ -141,7 +141,7 @@ impl FaceBRep { /// /// This is equivalent to chaining the iterators returned by /// [`Face::exteriors`] and [`Face::interiors`]. - pub fn all_cycles(&self) -> impl Iterator + '_ { + pub fn all_cycles(&self) -> impl Iterator + '_ { self.exteriors().chain(self.interiors()) } } @@ -157,7 +157,7 @@ impl CyclesInFace { } /// Access an iterator over the canonical forms of the cycles - pub fn as_local(&self) -> impl Iterator + '_ { - self.0.iter().cloned() + pub fn as_local(&self) -> impl Iterator + '_ { + self.0.iter() } } diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 58c04a697..1938f6c7e 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -50,11 +50,11 @@ impl Shape for fj::Difference2d { ); for cycle in face.exteriors.as_local() { - let cycle = add_cycle(cycle, false); + let cycle = add_cycle(cycle.clone(), false); exteriors.push(cycle); } for cycle in face.interiors.as_local() { - let cycle = add_cycle(cycle, true); + let cycle = add_cycle(cycle.clone(), true); interiors.push(cycle); } } @@ -69,7 +69,7 @@ impl Shape for fj::Difference2d { ); for cycle in face.exteriors.as_local() { - let cycle = add_cycle(cycle, true); + let cycle = add_cycle(cycle.clone(), true); interiors.push(cycle); } } From f240d0e768436f71a52f76020c6c721c764c2c96 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 14:51:09 +0200 Subject: [PATCH 05/26] Refactor --- crates/fj-operations/src/difference_2d.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 1938f6c7e..8d0a9238f 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -38,43 +38,39 @@ impl Shape 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; + let surface = face.surface(); for face in a.face_iter() { - let face = face.brep(); - assert_eq!( surface, - *face.surface(), + face.surface(), "Trying to subtract faces with different surfaces.", ); - for cycle in face.exteriors.as_local() { + for cycle in face.exteriors() { let cycle = add_cycle(cycle.clone(), false); exteriors.push(cycle); } - for cycle in face.interiors.as_local() { + for cycle in face.interiors() { let cycle = add_cycle(cycle.clone(), true); interiors.push(cycle); } } for face in b.face_iter() { - let face = face.brep(); - assert_eq!( surface, - *face.surface(), + face.surface(), "Trying to subtract faces with different surfaces.", ); - for cycle in face.exteriors.as_local() { + for cycle in face.exteriors() { let cycle = add_cycle(cycle.clone(), true); interiors.push(cycle); } } - faces.push(Face::new(surface, exteriors, interiors, self.color())); + faces.push(Face::new(*surface, exteriors, interiors, self.color())); } let difference = Sketch::from_faces(faces); From e8b149a6fd6db554866feacf00e80459bb7ed060 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 14:51:49 +0200 Subject: [PATCH 06/26] Update order of methods `Face::brep` is no longer part of the public interface, and I prefer to to have it below the other methods that call it. --- crates/fj-kernel/src/objects/face.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 0330aa4a6..c1b9c0a9e 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -46,17 +46,6 @@ impl Face { FaceBuilder::new(surface) } - /// Access the boundary representation of the face - pub fn brep(&self) -> &FaceBRep { - if let Self::Face(face) = self { - return face; - } - - // No code that still uses triangle representation is calling this - // method. - unreachable!() - } - /// Access this face's surface pub fn surface(&self) -> &Surface { self.brep().surface() @@ -84,6 +73,17 @@ impl Face { pub fn color(&self) -> [u8; 4] { self.brep().color } + + /// Access the boundary representation of the face + fn brep(&self) -> &FaceBRep { + if let Self::Face(face) = self { + return face; + } + + // No code that still uses triangle representation is calling this + // method. + unreachable!() + } } /// The boundary representation of a face From 122150c8fbabdc242e142548f4d13c00c5acfdf8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 14:54:11 +0200 Subject: [PATCH 07/26] Remove outdated implementation notes --- crates/fj-kernel/src/objects/face.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index c1b9c0a9e..376ecd7ce 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -97,24 +97,11 @@ pub struct FaceBRep { pub surface: Surface, /// The cycles that bound the face on the outside - /// - /// # Implementation Note - /// - /// Since these cycles bound the face, the edges they consist of must - /// lie in the surface. The data we're using here is 3-dimensional - /// though, so no such limitation is enforced. - /// - /// It might be less error-prone to specify the cycles in surface - /// coordinates. pub exteriors: CyclesInFace, /// The cycles that bound the face on the inside /// /// Each of these cycles defines a hole in the face. - /// - /// # Implementation note - /// - /// See note on `exterior` field. pub interiors: CyclesInFace, /// The color of the face From 567a08e48a057db0b5539416155f1f6dfb6b5442 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 14:55:08 +0200 Subject: [PATCH 08/26] Update doc comment --- crates/fj-kernel/src/objects/face.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 376ecd7ce..ade0e85db 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -51,12 +51,14 @@ impl Face { self.brep().surface() } - /// Access this face's exterior cycles + /// Access the cycles that bound the face on the outside pub fn exteriors(&self) -> impl Iterator + '_ { self.brep().exteriors() } - /// Access this face's interior cycles + /// Access the cycles that bound the face on the inside + /// + /// Each of these cycles defines a hole in the face. pub fn interiors(&self) -> impl Iterator + '_ { self.brep().interiors() } From c79655c4d84a9539707b7c8416649ba87e624c22 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 14:59:08 +0200 Subject: [PATCH 09/26] Fix potential Clippy warning Clippy doesn't actually warn about that, but it's going to, after I'm making a change I'm currently working on. --- crates/fj-kernel/src/algorithms/reverse.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index 46ddd27d5..1ea8eb346 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -25,7 +25,7 @@ pub fn reverse_face(face: &Face) -> Face { fn reverse_local_coordinates_in_cycle( cycles: &CyclesInFace, ) -> impl Iterator + '_ { - let cycles = cycles.as_local().map(|cycle| { + cycles.as_local().map(|cycle| { let edges = cycle .edges .iter() @@ -57,9 +57,7 @@ fn reverse_local_coordinates_in_cycle( .collect(); Cycle { edges } - }); - - cycles + }) } #[cfg(test)] From a22f6ccf83a82691d785ddd86d7934a6397becfb Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:00:40 +0200 Subject: [PATCH 10/26] Remove unnecessary use of `CyclesInFace` --- crates/fj-kernel/src/algorithms/reverse.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index 1ea8eb346..ab2e77260 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -2,7 +2,7 @@ use fj_math::{Circle, Line, Point, Vector}; use crate::{ local::Local, - objects::{Curve, Cycle, CyclesInFace, Edge, Face}, + objects::{Curve, Cycle, Edge, Face}, }; /// Reverse the direction of a face @@ -16,16 +16,16 @@ pub fn reverse_face(face: &Face) -> Face { let surface = face.surface().reverse(); - let exteriors = reverse_local_coordinates_in_cycle(&face.exteriors); - let interiors = reverse_local_coordinates_in_cycle(&face.interiors); + let exteriors = reverse_local_coordinates_in_cycle(face.exteriors()); + let interiors = reverse_local_coordinates_in_cycle(face.interiors()); Face::new(surface, exteriors, interiors, face.color) } -fn reverse_local_coordinates_in_cycle( - cycles: &CyclesInFace, -) -> impl Iterator + '_ { - cycles.as_local().map(|cycle| { +fn reverse_local_coordinates_in_cycle<'r>( + cycles: impl IntoIterator + 'r, +) -> impl Iterator + 'r { + cycles.into_iter().map(|cycle| { let edges = cycle .edges .iter() From 6a9e714217ad14849413f8407a471000b7ba9b40 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:01:17 +0200 Subject: [PATCH 11/26] Refactor --- crates/fj-kernel/src/algorithms/reverse.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index ab2e77260..3a77606fb 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -7,19 +7,16 @@ use crate::{ /// Reverse the direction of a face pub fn reverse_face(face: &Face) -> Face { - let face = match face { - Face::Face(face) => face, - Face::Triangles(_) => { - panic!("Reversing tri-rep faces is not supported") - } - }; + if let Face::Triangles(_) = face { + panic!("Reversing tri-rep faces is not supported"); + } let surface = face.surface().reverse(); let exteriors = reverse_local_coordinates_in_cycle(face.exteriors()); let interiors = reverse_local_coordinates_in_cycle(face.interiors()); - Face::new(surface, exteriors, interiors, face.color) + Face::new(surface, exteriors, interiors, face.color()) } fn reverse_local_coordinates_in_cycle<'r>( From 276ced5f272ade6a25fa6f5518e9216b4fff0f66 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:05:58 +0200 Subject: [PATCH 12/26] Remove unnecessary use of `CyclesInFace` --- crates/fj-kernel/src/algorithms/transform.rs | 30 +++++++------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index 7c2daa840..162b0c442 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/crates/fj-kernel/src/algorithms/transform.rs @@ -3,8 +3,7 @@ use fj_math::{Transform, Vector}; use crate::{ local::Local, objects::{ - Curve, Cycle, CyclesInFace, Edge, Face, FaceBRep, GlobalVertex, Sketch, - Solid, Surface, Vertex, + Curve, Cycle, Edge, Face, GlobalVertex, Sketch, Solid, Surface, Vertex, }, }; @@ -76,17 +75,12 @@ impl TransformObject for Face { Self::Face(face) => { let surface = face.surface.transform(transform); - let exteriors = transform_cycles(&face.exteriors, transform); - let interiors = transform_cycles(&face.interiors, transform); + let exteriors = transform_cycles(face.exteriors(), transform); + let interiors = transform_cycles(face.interiors(), transform); let color = face.color; - Self::Face(FaceBRep { - surface, - exteriors, - interiors, - color, - }) + Face::new(surface, exteriors, interiors, color) } Self::Triangles(triangles) => { let mut target = Vec::new(); @@ -152,13 +146,11 @@ pub fn transform_faces(faces: &mut Vec, transform: &Transform) { } } -fn transform_cycles( - cycles: &CyclesInFace, - transform: &Transform, -) -> CyclesInFace { - let cycles = cycles - .as_local() - .map(|cycle| cycle.clone().transform(transform)); - - CyclesInFace::new(cycles) +fn transform_cycles<'a>( + cycles: impl IntoIterator + 'a, + transform: &'a Transform, +) -> impl Iterator + 'a { + cycles + .into_iter() + .map(|cycle| cycle.clone().transform(transform)) } From e8e419e1f800568f7d497ed76e98afbeaa36969f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:07:42 +0200 Subject: [PATCH 13/26] Refactor --- crates/fj-kernel/src/algorithms/transform.rs | 31 +++++++++----------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index 162b0c442..0bcb2f627 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/crates/fj-kernel/src/algorithms/transform.rs @@ -71,28 +71,25 @@ impl TransformObject for Edge { impl TransformObject for Face { fn transform(self, transform: &Transform) -> Self { - match self { - Self::Face(face) => { - let surface = face.surface.transform(transform); + if let Self::Triangles(triangles) = self { + let mut target = Vec::new(); - let exteriors = transform_cycles(face.exteriors(), transform); - let interiors = transform_cycles(face.interiors(), transform); + for (triangle, color) in triangles { + let triangle = transform.transform_triangle(&triangle); + target.push((triangle, color)); + } - let color = face.color; + return Self::Triangles(target); + } - Face::new(surface, exteriors, interiors, color) - } - Self::Triangles(triangles) => { - let mut target = Vec::new(); + let surface = self.surface().transform(transform); - for (triangle, color) in triangles { - let triangle = transform.transform_triangle(&triangle); - target.push((triangle, color)); - } + let exteriors = transform_cycles(self.exteriors(), transform); + let interiors = transform_cycles(self.interiors(), transform); - Self::Triangles(target) - } - } + let color = self.color(); + + Face::new(surface, exteriors, interiors, color) } } From dbbe368b8e93cc7196c0cc296e7b4b29be0eddb7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:10:16 +0200 Subject: [PATCH 14/26] Refactor --- .../src/algorithms/triangulate/mod.rs | 73 +++++++++---------- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/triangulate/mod.rs b/crates/fj-kernel/src/algorithms/triangulate/mod.rs index 6e17777df..32ba20fec 100644 --- a/crates/fj-kernel/src/algorithms/triangulate/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulate/mod.rs @@ -20,47 +20,40 @@ pub fn triangulate( let mut mesh = Mesh::new(); for face in faces { - match &face { - Face::Face(brep) => { - let surface = brep.surface; - let approx = FaceApprox::new(&face, tolerance); - - let points: Vec<_> = approx.points.into_iter().collect(); - let face_as_polygon = Polygon::new(surface) - .with_exterior( - approx - .exterior - .points - .into_iter() - .map(|point| point.local()), - ) - .with_interiors(approx.interiors.into_iter().map( - |interior| { - interior - .points - .into_iter() - .map(|point| point.local()) - }, - )); - - let mut triangles = delaunay::triangulate(points); - triangles.retain(|triangle| { - face_as_polygon.contains_triangle( - triangle.map(|point| point.local()), - debug_info, - ) - }); - - for triangle in triangles { - let points = triangle.map(|point| point.global()); - mesh.push_triangle(points, brep.color); - } - } - Face::Triangles(triangles) => { - for &(triangle, color) in triangles { - mesh.push_triangle(triangle.points(), color); - } + if let Face::Triangles(triangles) = &face { + for &(triangle, color) in triangles { + mesh.push_triangle(triangle.points(), color); } + continue; + } + + let surface = face.surface(); + let approx = FaceApprox::new(&face, tolerance); + + let points: Vec<_> = approx.points.into_iter().collect(); + let face_as_polygon = Polygon::new(*surface) + .with_exterior( + approx + .exterior + .points + .into_iter() + .map(|point| point.local()), + ) + .with_interiors(approx.interiors.into_iter().map(|interior| { + interior.points.into_iter().map(|point| point.local()) + })); + + let mut triangles = delaunay::triangulate(points); + triangles.retain(|triangle| { + face_as_polygon.contains_triangle( + triangle.map(|point| point.local()), + debug_info, + ) + }); + + for triangle in triangles { + let points = triangle.map(|point| point.global()); + mesh.push_triangle(points, face.color()); } } From cf371c13ae97efb391f58c799436dec6901c5773 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:11:02 +0200 Subject: [PATCH 15/26] Make fields of `FaceBRep` private --- crates/fj-kernel/src/objects/face.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index ade0e85db..93648226e 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -95,19 +95,10 @@ impl Face { /// `Face::Triangles` can finally be removed. #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct FaceBRep { - /// The surface that defines this face - pub surface: Surface, - - /// The cycles that bound the face on the outside - pub exteriors: CyclesInFace, - - /// The cycles that bound the face on the inside - /// - /// Each of these cycles defines a hole in the face. - pub interiors: CyclesInFace, - - /// The color of the face - pub color: [u8; 4], + surface: Surface, + exteriors: CyclesInFace, + interiors: CyclesInFace, + color: [u8; 4], } impl FaceBRep { From 8ef9cfc9726a8844940d3203ce190f50111c411b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:22:04 +0200 Subject: [PATCH 16/26] Refactor --- crates/fj-kernel/src/iter.rs | 113 +++++++++++++++++------------------ 1 file changed, 56 insertions(+), 57 deletions(-) diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 95f7507b6..5425a8988 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -252,45 +252,45 @@ impl ObjectIters for Edge { impl ObjectIters for Face { fn curve_iter(&self) -> Iter> { - if let Face::Face(face) = self { - let mut iter = Iter::empty().with(face.surface().curve_iter()); + if let Face::Triangles(_) = self { + return Iter::empty(); + } - for cycle in face.all_cycles() { - iter = iter.with(cycle.curve_iter()); - } + let mut iter = Iter::empty().with(self.surface().curve_iter()); - return iter; + for cycle in self.all_cycles() { + iter = iter.with(cycle.curve_iter()); } - Iter::empty() + iter } fn cycle_iter(&self) -> Iter { - if let Face::Face(face) = self { - let mut iter = Iter::empty().with(face.surface().cycle_iter()); + if let Face::Triangles(_) = self { + return Iter::empty(); + } - for cycle in face.all_cycles() { - iter = iter.with(cycle.cycle_iter()); - } + let mut iter = Iter::empty().with(self.surface().cycle_iter()); - return iter; + for cycle in self.all_cycles() { + iter = iter.with(cycle.cycle_iter()); } - Iter::empty() + iter } fn edge_iter(&self) -> Iter { - if let Face::Face(face) = self { - let mut iter = Iter::empty().with(face.surface().edge_iter()); + if let Face::Triangles(_) = self { + return Iter::empty(); + } - for cycle in face.all_cycles() { - iter = iter.with(cycle.edge_iter()); - } + let mut iter = Iter::empty().with(self.surface().edge_iter()); - return iter; + for cycle in self.all_cycles() { + iter = iter.with(cycle.edge_iter()); } - Iter::empty() + iter } fn face_iter(&self) -> Iter { @@ -298,74 +298,73 @@ impl ObjectIters for Face { } fn global_vertex_iter(&self) -> Iter { - if let Face::Face(face) = self { - let mut iter = - Iter::empty().with(face.surface().global_vertex_iter()); + if let Face::Triangles(_) = self { + return Iter::empty(); + } - for cycle in face.all_cycles() { - iter = iter.with(cycle.global_vertex_iter()); - } + let mut iter = Iter::empty().with(self.surface().global_vertex_iter()); - return iter; + for cycle in self.all_cycles() { + iter = iter.with(cycle.global_vertex_iter()); } - Iter::empty() + iter } fn sketch_iter(&self) -> Iter { - if let Face::Face(face) = self { - let mut iter = Iter::empty().with(face.surface().sketch_iter()); + if let Face::Triangles(_) = self { + return Iter::empty(); + } - for cycle in face.all_cycles() { - iter = iter.with(cycle.sketch_iter()); - } + let mut iter = Iter::empty().with(self.surface().sketch_iter()); - return iter; + for cycle in self.all_cycles() { + iter = iter.with(cycle.sketch_iter()); } - Iter::empty() + iter } fn solid_iter(&self) -> Iter { - if let Face::Face(face) = self { - let mut iter = Iter::empty().with(face.surface().solid_iter()); + if let Face::Triangles(_) = self { + return Iter::empty(); + } - for cycle in face.all_cycles() { - iter = iter.with(cycle.solid_iter()); - } + let mut iter = Iter::empty().with(self.surface().solid_iter()); - return iter; + for cycle in self.all_cycles() { + iter = iter.with(cycle.solid_iter()); } - Iter::empty() + iter } fn surface_iter(&self) -> Iter { - if let Face::Face(face) = self { - let mut iter = Iter::empty().with(face.surface().surface_iter()); + if let Face::Triangles(_) = self { + return Iter::empty(); + } - for cycle in face.all_cycles() { - iter = iter.with(cycle.surface_iter()); - } + let mut iter = Iter::empty().with(self.surface().surface_iter()); - return iter; + for cycle in self.all_cycles() { + iter = iter.with(cycle.surface_iter()); } - Iter::empty() + iter } fn vertex_iter(&self) -> Iter { - if let Face::Face(face) = self { - let mut iter = Iter::empty().with(face.surface().vertex_iter()); + if let Face::Triangles(_) = self { + return Iter::empty(); + } - for cycle in face.all_cycles() { - iter = iter.with(cycle.vertex_iter()); - } + let mut iter = Iter::empty().with(self.surface().vertex_iter()); - return iter; + for cycle in self.all_cycles() { + iter = iter.with(cycle.vertex_iter()); } - Iter::empty() + iter } } From 7241a3945d377f83d98f2e04dbecb2f2b6190425 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:22:34 +0200 Subject: [PATCH 17/26] Remove redundant accessors --- crates/fj-kernel/src/objects/face.rs | 33 ++++------------------------ 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 93648226e..b2f48f9f4 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -48,19 +48,19 @@ impl Face { /// Access this face's surface pub fn surface(&self) -> &Surface { - self.brep().surface() + &self.brep().surface } /// Access the cycles that bound the face on the outside pub fn exteriors(&self) -> impl Iterator + '_ { - self.brep().exteriors() + self.brep().exteriors.as_local() } /// Access the cycles that bound the face on the inside /// /// Each of these cycles defines a hole in the face. pub fn interiors(&self) -> impl Iterator + '_ { - self.brep().interiors() + self.brep().interiors.as_local() } /// Access all cycles of this face @@ -68,7 +68,7 @@ impl Face { /// This is equivalent to chaining the iterators returned by /// [`Face::exteriors`] and [`Face::interiors`]. pub fn all_cycles(&self) -> impl Iterator + '_ { - self.brep().all_cycles() + self.exteriors().chain(self.interiors()) } /// Access the color of the face @@ -101,31 +101,6 @@ pub struct FaceBRep { color: [u8; 4], } -impl FaceBRep { - /// Access this face's surface - pub fn surface(&self) -> &Surface { - &self.surface - } - - /// Access this face's exterior cycles - pub fn exteriors(&self) -> impl Iterator + '_ { - self.exteriors.as_local() - } - - /// Access this face's interior cycles - pub fn interiors(&self) -> impl Iterator + '_ { - self.interiors.as_local() - } - - /// Access all cycles of this face - /// - /// This is equivalent to chaining the iterators returned by - /// [`Face::exteriors`] and [`Face::interiors`]. - pub fn all_cycles(&self) -> impl Iterator + '_ { - self.exteriors().chain(self.interiors()) - } -} - /// A list of cycles, as they are stored in `Face` #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct CyclesInFace(Vec); From 965930c8eef73aba1721bd9e03631efc62f54acc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:24:32 +0200 Subject: [PATCH 18/26] Remove `CyclesInFace` It used to serve a purpose, but no longer carries its weight. --- crates/fj-kernel/src/objects/face.rs | 28 ++++++---------------------- crates/fj-kernel/src/objects/mod.rs | 2 +- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index b2f48f9f4..6d5b15b11 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -31,8 +31,8 @@ impl Face { interiors: impl IntoIterator, color: [u8; 4], ) -> Self { - let exteriors = CyclesInFace::new(exteriors); - let interiors = CyclesInFace::new(interiors); + let exteriors = exteriors.into_iter().collect(); + let interiors = interiors.into_iter().collect(); Self::Face(FaceBRep { surface, @@ -53,14 +53,14 @@ impl Face { /// Access the cycles that bound the face on the outside pub fn exteriors(&self) -> impl Iterator + '_ { - self.brep().exteriors.as_local() + self.brep().exteriors.iter() } /// Access the cycles that bound the face on the inside /// /// Each of these cycles defines a hole in the face. pub fn interiors(&self) -> impl Iterator + '_ { - self.brep().interiors.as_local() + self.brep().interiors.iter() } /// Access all cycles of this face @@ -96,23 +96,7 @@ impl Face { #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct FaceBRep { surface: Surface, - exteriors: CyclesInFace, - interiors: CyclesInFace, + exteriors: Vec, + interiors: Vec, color: [u8; 4], } - -/// A list of cycles, as they are stored in `Face` -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct CyclesInFace(Vec); - -impl CyclesInFace { - /// Create a new instance of `CyclesInFace` - pub fn new(cycles: impl IntoIterator) -> Self { - Self(cycles.into_iter().collect()) - } - - /// Access an iterator over the canonical forms of the cycles - pub fn as_local(&self) -> impl Iterator + '_ { - self.0.iter() - } -} diff --git a/crates/fj-kernel/src/objects/mod.rs b/crates/fj-kernel/src/objects/mod.rs index 72021c07b..180a18e36 100644 --- a/crates/fj-kernel/src/objects/mod.rs +++ b/crates/fj-kernel/src/objects/mod.rs @@ -18,7 +18,7 @@ pub use self::{ curve::Curve, cycle::Cycle, edge::{Edge, VerticesOfEdge}, - face::{CyclesInFace, Face, FaceBRep}, + face::{Face, FaceBRep}, global_vertex::GlobalVertex, sketch::Sketch, solid::Solid, From 24e8194a142ccc6eff9c721114dee75bed6e34d2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:27:14 +0200 Subject: [PATCH 19/26] Rename enum variant --- crates/fj-kernel/src/objects/face.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 6d5b15b11..852595aea 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -12,7 +12,7 @@ pub enum Face { /// /// A face is defined by a surface, and is bounded by edges that lie in that /// surface. - Face(FaceBRep), + BRep(FaceBRep), /// The triangles of the face /// @@ -34,7 +34,7 @@ impl Face { let exteriors = exteriors.into_iter().collect(); let interiors = interiors.into_iter().collect(); - Self::Face(FaceBRep { + Self::BRep(FaceBRep { surface, exteriors, interiors, @@ -78,7 +78,7 @@ impl Face { /// Access the boundary representation of the face fn brep(&self) -> &FaceBRep { - if let Self::Face(face) = self { + if let Self::BRep(face) = self { return face; } From d025d6ed8965558c88d9571e40d06bf238749aad Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:31:24 +0200 Subject: [PATCH 20/26] Add type definition for triangle representation --- crates/fj-kernel/src/objects/face.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 852595aea..dccb7deda 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -20,7 +20,7 @@ pub enum Face { /// The plan is to eventually represent faces as a geometric surface, /// bounded by edges. While the transition is being made, this variant is /// still required. - Triangles(Vec<(Triangle<3>, Color)>), + Triangles(TriRep), } impl Face { @@ -100,3 +100,5 @@ pub struct FaceBRep { interiors: Vec, color: [u8; 4], } + +type TriRep = Vec<(Triangle<3>, Color)>; From 4043d19db7ed4df5f8c22d623fc97b2288d36c2c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:33:13 +0200 Subject: [PATCH 21/26] Add `Face::triangles` --- crates/fj-kernel/src/algorithms/reverse.rs | 2 +- crates/fj-kernel/src/algorithms/transform.rs | 4 ++-- .../fj-kernel/src/algorithms/triangulate/mod.rs | 2 +- crates/fj-kernel/src/iter.rs | 16 ++++++++-------- crates/fj-kernel/src/objects/face.rs | 13 +++++++++++++ 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index 3a77606fb..d6dfa0ba7 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -7,7 +7,7 @@ use crate::{ /// Reverse the direction of a face pub fn reverse_face(face: &Face) -> Face { - if let Face::Triangles(_) = face { + if face.triangles().is_some() { panic!("Reversing tri-rep faces is not supported"); } diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index 0bcb2f627..f676b9bfd 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/crates/fj-kernel/src/algorithms/transform.rs @@ -71,10 +71,10 @@ impl TransformObject for Edge { impl TransformObject for Face { fn transform(self, transform: &Transform) -> Self { - if let Self::Triangles(triangles) = self { + if let Some(triangles) = self.triangles() { let mut target = Vec::new(); - for (triangle, color) in triangles { + for (triangle, color) in triangles.clone() { let triangle = transform.transform_triangle(&triangle); target.push((triangle, color)); } diff --git a/crates/fj-kernel/src/algorithms/triangulate/mod.rs b/crates/fj-kernel/src/algorithms/triangulate/mod.rs index 32ba20fec..bb38e73ce 100644 --- a/crates/fj-kernel/src/algorithms/triangulate/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulate/mod.rs @@ -20,7 +20,7 @@ pub fn triangulate( let mut mesh = Mesh::new(); for face in faces { - if let Face::Triangles(triangles) = &face { + if let Some(triangles) = face.triangles() { for &(triangle, color) in triangles { mesh.push_triangle(triangle.points(), color); } diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 5425a8988..ffb27f783 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -252,7 +252,7 @@ impl ObjectIters for Edge { impl ObjectIters for Face { fn curve_iter(&self) -> Iter> { - if let Face::Triangles(_) = self { + if self.triangles().is_some() { return Iter::empty(); } @@ -266,7 +266,7 @@ impl ObjectIters for Face { } fn cycle_iter(&self) -> Iter { - if let Face::Triangles(_) = self { + if self.triangles().is_some() { return Iter::empty(); } @@ -280,7 +280,7 @@ impl ObjectIters for Face { } fn edge_iter(&self) -> Iter { - if let Face::Triangles(_) = self { + if self.triangles().is_some() { return Iter::empty(); } @@ -298,7 +298,7 @@ impl ObjectIters for Face { } fn global_vertex_iter(&self) -> Iter { - if let Face::Triangles(_) = self { + if self.triangles().is_some() { return Iter::empty(); } @@ -312,7 +312,7 @@ impl ObjectIters for Face { } fn sketch_iter(&self) -> Iter { - if let Face::Triangles(_) = self { + if self.triangles().is_some() { return Iter::empty(); } @@ -326,7 +326,7 @@ impl ObjectIters for Face { } fn solid_iter(&self) -> Iter { - if let Face::Triangles(_) = self { + if self.triangles().is_some() { return Iter::empty(); } @@ -340,7 +340,7 @@ impl ObjectIters for Face { } fn surface_iter(&self) -> Iter { - if let Face::Triangles(_) = self { + if self.triangles().is_some() { return Iter::empty(); } @@ -354,7 +354,7 @@ impl ObjectIters for Face { } fn vertex_iter(&self) -> Iter { - if let Face::Triangles(_) = self { + if self.triangles().is_some() { return Iter::empty(); } diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index dccb7deda..4376b6b70 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -76,6 +76,19 @@ impl Face { self.brep().color } + /// Access triangles, if this face uses triangle representation + /// + /// Only some faces still use triangle representation. At some point, none + /// will. This method exists as a workaround, while the transition is still + /// in progress. + pub fn triangles(&self) -> Option<&TriRep> { + if let Self::Triangles(triangles) = self { + return Some(triangles); + } + + None + } + /// Access the boundary representation of the face fn brep(&self) -> &FaceBRep { if let Self::BRep(face) = self { From 4fe9c730a82180551d48e3dccbfcf8b48be8a789 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:51:11 +0200 Subject: [PATCH 22/26] Add `Face::from_triangles` --- crates/fj-kernel/src/algorithms/sweep.rs | 2 +- crates/fj-kernel/src/algorithms/transform.rs | 2 +- crates/fj-kernel/src/objects/face.rs | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index d0ef1f7ba..2563ef24e 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -193,7 +193,7 @@ fn create_continuous_side_face( side_face.push(([v0, v2, v3].into(), color)); } - target.push(Face::Triangles(side_face)); + target.push(Face::from_triangles(side_face)); } #[cfg(test)] diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index f676b9bfd..fa304c20f 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/crates/fj-kernel/src/algorithms/transform.rs @@ -79,7 +79,7 @@ impl TransformObject for Face { target.push((triangle, color)); } - return Self::Triangles(target); + return Self::from_triangles(target); } let surface = self.surface().transform(transform); diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 4376b6b70..473e9ee7f 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -41,6 +41,12 @@ impl Face { color, }) } + + /// Contact an instance that uses triangle representation + pub fn from_triangles(triangles: TriRep) -> Self { + Self::Triangles(triangles) + } + /// Build a face using the [`FaceBuilder`] API pub fn builder(surface: Surface) -> FaceBuilder { FaceBuilder::new(surface) From 0a504538903ac076cd8b9c4ab91f68b3f08fdfad Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:52:01 +0200 Subject: [PATCH 23/26] Rename enum variant --- crates/fj-kernel/src/objects/face.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index 473e9ee7f..a4a8a6f65 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -20,7 +20,7 @@ pub enum Face { /// The plan is to eventually represent faces as a geometric surface, /// bounded by edges. While the transition is being made, this variant is /// still required. - Triangles(TriRep), + TriRep(TriRep), } impl Face { @@ -44,7 +44,7 @@ impl Face { /// Contact an instance that uses triangle representation pub fn from_triangles(triangles: TriRep) -> Self { - Self::Triangles(triangles) + Self::TriRep(triangles) } /// Build a face using the [`FaceBuilder`] API @@ -88,7 +88,7 @@ impl Face { /// will. This method exists as a workaround, while the transition is still /// in progress. pub fn triangles(&self) -> Option<&TriRep> { - if let Self::Triangles(triangles) = self { + if let Self::TriRep(triangles) = self { return Some(triangles); } From 8a4d18ff1a25239d6cdf3d5183e5dd40b885f774 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:54:44 +0200 Subject: [PATCH 24/26] Convert `Face` into a struct --- crates/fj-kernel/src/objects/face.rs | 44 +++++++++++++--------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index a4a8a6f65..cb23dbca9 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -7,20 +7,8 @@ use super::{Cycle, Surface}; /// A face of a shape #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub enum Face { - /// A face of a shape - /// - /// A face is defined by a surface, and is bounded by edges that lie in that - /// surface. - BRep(FaceBRep), - - /// The triangles of the face - /// - /// Representing faces as a collection of triangles is a temporary state. - /// The plan is to eventually represent faces as a geometric surface, - /// bounded by edges. While the transition is being made, this variant is - /// still required. - TriRep(TriRep), +pub struct Face { + representation: Representation, } impl Face { @@ -34,17 +22,21 @@ impl Face { let exteriors = exteriors.into_iter().collect(); let interiors = interiors.into_iter().collect(); - Self::BRep(FaceBRep { - surface, - exteriors, - interiors, - color, - }) + Self { + representation: Representation::BRep(FaceBRep { + surface, + exteriors, + interiors, + color, + }), + } } /// Contact an instance that uses triangle representation pub fn from_triangles(triangles: TriRep) -> Self { - Self::TriRep(triangles) + Self { + representation: Representation::TriRep(triangles), + } } /// Build a face using the [`FaceBuilder`] API @@ -88,7 +80,7 @@ impl Face { /// will. This method exists as a workaround, while the transition is still /// in progress. pub fn triangles(&self) -> Option<&TriRep> { - if let Self::TriRep(triangles) = self { + if let Representation::TriRep(triangles) = &self.representation { return Some(triangles); } @@ -97,7 +89,7 @@ impl Face { /// Access the boundary representation of the face fn brep(&self) -> &FaceBRep { - if let Self::BRep(face) = self { + if let Representation::BRep(face) = &self.representation { return face; } @@ -107,6 +99,12 @@ impl Face { } } +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +enum Representation { + BRep(FaceBRep), + TriRep(TriRep), +} + /// The boundary representation of a face /// /// This type exists to ease the handling of faces that use boundary From 91df8086207e63e1f2aa72338001aee8094883db Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:55:26 +0200 Subject: [PATCH 25/26] Make `FaceBRep` private --- crates/fj-kernel/src/objects/face.rs | 7 +------ crates/fj-kernel/src/objects/mod.rs | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index cb23dbca9..c2d88b3ef 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -105,13 +105,8 @@ enum Representation { TriRep(TriRep), } -/// The boundary representation of a face -/// -/// This type exists to ease the handling of faces that use boundary -/// representation. It will eventually be merged into `Face`, once -/// `Face::Triangles` can finally be removed. #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct FaceBRep { +struct FaceBRep { surface: Surface, exteriors: Vec, interiors: Vec, diff --git a/crates/fj-kernel/src/objects/mod.rs b/crates/fj-kernel/src/objects/mod.rs index 180a18e36..3615cdf15 100644 --- a/crates/fj-kernel/src/objects/mod.rs +++ b/crates/fj-kernel/src/objects/mod.rs @@ -18,7 +18,7 @@ pub use self::{ curve::Curve, cycle::Cycle, edge::{Edge, VerticesOfEdge}, - face::{Face, FaceBRep}, + face::Face, global_vertex::GlobalVertex, sketch::Sketch, solid::Solid, From 0ba85d0f6cf79b570c90a3429ff4b148d968720e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 20 Jul 2022 15:55:51 +0200 Subject: [PATCH 26/26] Simplify struct name --- crates/fj-kernel/src/objects/face.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index c2d88b3ef..c9eaa6f80 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -23,7 +23,7 @@ impl Face { let interiors = interiors.into_iter().collect(); Self { - representation: Representation::BRep(FaceBRep { + representation: Representation::BRep(BRep { surface, exteriors, interiors, @@ -88,7 +88,7 @@ impl Face { } /// Access the boundary representation of the face - fn brep(&self) -> &FaceBRep { + fn brep(&self) -> &BRep { if let Representation::BRep(face) = &self.representation { return face; } @@ -101,12 +101,12 @@ impl Face { #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] enum Representation { - BRep(FaceBRep), + BRep(BRep), TriRep(TriRep), } #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -struct FaceBRep { +struct BRep { surface: Surface, exteriors: Vec, interiors: Vec,