From 83ae3846e344a3d56e4058740d482a50d968d28a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:10:16 +0100 Subject: [PATCH 01/21] Move `Faces` to `shape::faces` --- src/kernel/algorithms/sweep.rs | 4 ++-- src/kernel/shape/faces.rs | 22 ++++++++++++++++++++++ src/kernel/shape/mod.rs | 8 ++++++-- src/kernel/shapes/circle.rs | 4 ++-- src/kernel/shapes/difference_2d.rs | 7 ++----- src/kernel/shapes/sketch.rs | 4 ++-- src/kernel/shapes/union.rs | 2 +- src/kernel/topology/faces.rs | 17 ----------------- 8 files changed, 37 insertions(+), 31 deletions(-) create mode 100644 src/kernel/shape/faces.rs diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index b0c155758..46dbfcb0f 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -1,7 +1,7 @@ use crate::{ kernel::{ - shape::Shape, - topology::faces::{Face, Faces}, + shape::{faces::Faces, Shape}, + topology::faces::Face, }, math::{Scalar, Transform, Vector}, }; diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs new file mode 100644 index 000000000..5a07deede --- /dev/null +++ b/src/kernel/shape/faces.rs @@ -0,0 +1,22 @@ +use crate::{ + debug::DebugInfo, + kernel::topology::faces::Face, + math::{Scalar, Triangle}, +}; + +/// The faces of a shape +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct Faces(pub Vec); + +impl Faces { + pub fn triangles( + &self, + tolerance: Scalar, + out: &mut Vec>, + debug_info: &mut DebugInfo, + ) { + for face in &self.0 { + face.triangles(tolerance, out, debug_info); + } + } +} diff --git a/src/kernel/shape/mod.rs b/src/kernel/shape/mod.rs index dc8719697..becb1928d 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -1,13 +1,17 @@ pub mod cycles; pub mod edges; +pub mod faces; pub mod handle; pub mod vertices; use crate::math::Scalar; -use super::topology::{edges::Cycle, faces::Faces, vertices::Vertex}; +use super::topology::{edges::Cycle, vertices::Vertex}; -use self::{cycles::Cycles, edges::Edges, handle::Storage, vertices::Vertices}; +use self::{ + cycles::Cycles, edges::Edges, faces::Faces, handle::Storage, + vertices::Vertices, +}; /// The boundary representation of a shape /// diff --git a/src/kernel/shapes/circle.rs b/src/kernel/shapes/circle.rs index f6304730f..a304a3d59 100644 --- a/src/kernel/shapes/circle.rs +++ b/src/kernel/shapes/circle.rs @@ -2,10 +2,10 @@ use crate::{ debug::DebugInfo, kernel::{ geometry::Surface, - shape::Shape, + shape::{faces::Faces, Shape}, topology::{ edges::{Cycle, Edge}, - faces::{Face, Faces}, + faces::Face, }, }, math::{Aabb, Point, Scalar}, diff --git a/src/kernel/shapes/difference_2d.rs b/src/kernel/shapes/difference_2d.rs index 31394126e..a6874e85c 100644 --- a/src/kernel/shapes/difference_2d.rs +++ b/src/kernel/shapes/difference_2d.rs @@ -1,11 +1,8 @@ use crate::{ debug::DebugInfo, kernel::{ - shape::Shape, - topology::{ - edges::Cycle, - faces::{Face, Faces}, - }, + shape::{faces::Faces, Shape}, + topology::{edges::Cycle, faces::Face}, }, math::{Aabb, Scalar}, }; diff --git a/src/kernel/shapes/sketch.rs b/src/kernel/shapes/sketch.rs index 601a131de..26cd33789 100644 --- a/src/kernel/shapes/sketch.rs +++ b/src/kernel/shapes/sketch.rs @@ -2,10 +2,10 @@ use crate::{ debug::DebugInfo, kernel::{ geometry::Surface, - shape::Shape, + shape::{faces::Faces, Shape}, topology::{ edges::{Cycle, Edge}, - faces::{Face, Faces}, + faces::Face, }, }, math::{Aabb, Point, Scalar}, diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index 9c35d85b5..f47a8b82e 100644 --- a/src/kernel/shapes/union.rs +++ b/src/kernel/shapes/union.rs @@ -1,6 +1,6 @@ use crate::{ debug::DebugInfo, - kernel::{shape::Shape, topology::faces::Faces}, + kernel::shape::{faces::Faces, Shape}, math::{Aabb, Scalar}, }; diff --git a/src/kernel/topology/faces.rs b/src/kernel/topology/faces.rs index c11461391..3f51e1ac9 100644 --- a/src/kernel/topology/faces.rs +++ b/src/kernel/topology/faces.rs @@ -16,23 +16,6 @@ use crate::{ use super::edges::Cycle; -/// The faces of a shape -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Faces(pub Vec); - -impl Faces { - pub fn triangles( - &self, - tolerance: Scalar, - out: &mut Vec>, - debug_info: &mut DebugInfo, - ) { - for face in &self.0 { - face.triangles(tolerance, out, debug_info); - } - } -} - /// A face of a shape #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub enum Face { From 4c276addd5b7b5d769d2a150b1082c788c9516e1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:14:11 +0100 Subject: [PATCH 02/21] Take `Shape` by value in `transform_shape` I'm certain this is not what this function signature will look like long-term, but for now, it will help with the next steps. And it doesn't cause problems for the single caller. --- src/kernel/algorithms/transform.rs | 2 +- src/kernel/shapes/transform.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernel/algorithms/transform.rs b/src/kernel/algorithms/transform.rs index 2dc1cc7d6..5031f2b40 100644 --- a/src/kernel/algorithms/transform.rs +++ b/src/kernel/algorithms/transform.rs @@ -19,7 +19,7 @@ use crate::{ /// /// Addressing the shortcomings in this method probably doesn't make sense, /// except as a side effect of addressing the shortcomings of `Shape`. -pub fn transform_shape(original: &Shape, transform: &Transform) -> Shape { +pub fn transform_shape(original: Shape, transform: &Transform) -> Shape { let mut transformed = Shape::new(); for face in &original.faces.0 { diff --git a/src/kernel/shapes/transform.rs b/src/kernel/shapes/transform.rs index 79f22d30e..3c3513f49 100644 --- a/src/kernel/shapes/transform.rs +++ b/src/kernel/shapes/transform.rs @@ -12,7 +12,7 @@ impl ToShape for fj::Transform { fn to_shape(&self, tolerance: Scalar, debug_info: &mut DebugInfo) -> Shape { let shape = self.shape.to_shape(tolerance, debug_info); let transform = transform(self); - transform_shape(&shape, &transform) + transform_shape(shape, &transform) } fn bounding_volume(&self) -> Aabb<3> { From 43f7778927ae37ba29bf733b3a6281afc2c62778 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:16:00 +0100 Subject: [PATCH 03/21] Refactor --- src/kernel/shapes/union.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index f47a8b82e..8f3ecf7e4 100644 --- a/src/kernel/shapes/union.rs +++ b/src/kernel/shapes/union.rs @@ -10,8 +10,8 @@ impl ToShape for fj::Union { fn to_shape(&self, tolerance: Scalar, debug_info: &mut DebugInfo) -> Shape { let mut shape = Shape::new(); - let a = self.a.to_shape(tolerance, debug_info).faces; - let b = self.b.to_shape(tolerance, debug_info).faces; + let a = self.a.to_shape(tolerance, debug_info).faces.0; + let b = self.b.to_shape(tolerance, debug_info).faces.0; // This doesn't create a true union, as it doesn't eliminate, merge, or // split faces. @@ -19,8 +19,8 @@ impl ToShape for fj::Union { // See issue: // https://github.com/hannobraun/Fornjot/issues/42 let mut faces = Vec::new(); - faces.extend(a.0); - faces.extend(b.0); + faces.extend(a); + faces.extend(b); shape.faces = Faces(faces); From 353dfc80b1b15572286cd1e422b6cefe7f730ec4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:17:53 +0100 Subject: [PATCH 04/21] Refactor --- src/main.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index 43230e146..89dd0b8af 100644 --- a/src/main.rs +++ b/src/main.rs @@ -100,10 +100,12 @@ fn main() -> anyhow::Result<()> { }; let mut debug_info = DebugInfo::new(); - let faces = shape.to_shape(tolerance, &mut debug_info).faces; - let mut triangles = Vec::new(); - faces.triangles(tolerance, &mut triangles, &mut debug_info); + shape.to_shape(tolerance, &mut debug_info).faces.triangles( + tolerance, + &mut triangles, + &mut debug_info, + ); if let Some(path) = args.export { let mut mesh_maker = MeshMaker::new(); From cc39e911219aaffbdf17202d63f3ceb925778748 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:19:14 +0100 Subject: [PATCH 05/21] Refactor --- src/main.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index 89dd0b8af..0f37241f0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -228,10 +228,12 @@ fn main() -> anyhow::Result<()> { debug_info.clear(); triangles.clear(); - let faces = shape.to_shape(tolerance, &mut debug_info).faces; - aabb = shape.bounding_volume(); - faces.triangles(tolerance, &mut triangles, &mut debug_info); + shape.to_shape(tolerance, &mut debug_info).faces.triangles( + tolerance, + &mut triangles, + &mut debug_info, + ); renderer.update_geometry( (&triangles).into(), From e0bb22d6e5353c05cfb32519d18040ce302c4509 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:20:18 +0100 Subject: [PATCH 06/21] Add `Shape::faces` --- src/kernel/algorithms/sweep.rs | 12 ++++++------ src/kernel/algorithms/transform.rs | 6 +++--- src/kernel/shape/mod.rs | 5 +++++ src/kernel/shapes/circle.rs | 2 +- src/kernel/shapes/difference_2d.rs | 6 +++--- src/kernel/shapes/sketch.rs | 2 +- src/kernel/shapes/union.rs | 6 +++--- src/main.rs | 18 ++++++++---------- 8 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index 46dbfcb0f..6474c27c8 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -22,7 +22,7 @@ pub fn sweep_shape( let mut top_faces = Vec::new(); let mut side_faces = Vec::new(); - for face in &original.faces.0 { + for face in &original.faces().0 { bottom_faces.push(face.clone()); top_faces.push(transform_face(face, &translation, &mut shape)); } @@ -60,7 +60,7 @@ pub fn sweep_shape( faces.extend(top_faces); faces.extend(side_faces); - shape.faces = Faces(faces); + *shape.faces() = Faces(faces); shape } @@ -85,7 +85,7 @@ mod tests { fn sweep() { let sketch = Triangle::new([[0., 0., 0.], [1., 0., 0.], [0., 1., 0.]]); - let swept = sweep_shape( + let mut swept = sweep_shape( sketch.shape, Vector::from([0., 0., 1.]), Scalar::from_f64(0.), @@ -95,8 +95,8 @@ mod tests { let top_face = Triangle::new([[0., 0., 1.], [1., 0., 1.], [0., 1., 1.]]).face; - assert!(swept.faces.0.contains(&bottom_face)); - assert!(swept.faces.0.contains(&top_face)); + assert!(swept.faces().0.contains(&bottom_face)); + assert!(swept.faces().0.contains(&top_face)); // Side faces are not tested, as those use triangle representation. The // plan is to start testing them, as they are transitioned to b-rep. @@ -134,7 +134,7 @@ mod tests { }], }; - shape.faces.0.push(abc.clone()); + shape.faces().0.push(abc.clone()); Self { shape, face: abc } } diff --git a/src/kernel/algorithms/transform.rs b/src/kernel/algorithms/transform.rs index 5031f2b40..0d937316f 100644 --- a/src/kernel/algorithms/transform.rs +++ b/src/kernel/algorithms/transform.rs @@ -19,12 +19,12 @@ use crate::{ /// /// Addressing the shortcomings in this method probably doesn't make sense, /// except as a side effect of addressing the shortcomings of `Shape`. -pub fn transform_shape(original: Shape, transform: &Transform) -> Shape { +pub fn transform_shape(mut original: Shape, transform: &Transform) -> Shape { let mut transformed = Shape::new(); - for face in &original.faces.0 { + for face in &original.faces().0 { let face = transform_face(face, transform, &mut transformed); - transformed.faces.0.push(face); + transformed.faces().0.push(face); } transformed diff --git a/src/kernel/shape/mod.rs b/src/kernel/shape/mod.rs index becb1928d..a4bb16168 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -82,6 +82,11 @@ impl Shape { cycles: &mut self.cycles, } } + + /// Access the shape's faces + pub fn faces(&mut self) -> &mut Faces { + &mut self.faces + } } type VerticesInner = Vec>; diff --git a/src/kernel/shapes/circle.rs b/src/kernel/shapes/circle.rs index a304a3d59..c6e3ce36d 100644 --- a/src/kernel/shapes/circle.rs +++ b/src/kernel/shapes/circle.rs @@ -25,7 +25,7 @@ impl ToShape for fj::Circle { .add(Edge::circle(Scalar::from_f64(self.radius))); shape.cycles().add(Cycle { edges: vec![edge] }); - shape.faces = Faces(vec![Face::Face { + *shape.faces() = Faces(vec![Face::Face { cycles: shape .cycles() .all() diff --git a/src/kernel/shapes/difference_2d.rs b/src/kernel/shapes/difference_2d.rs index a6874e85c..6325c5d86 100644 --- a/src/kernel/shapes/difference_2d.rs +++ b/src/kernel/shapes/difference_2d.rs @@ -38,10 +38,10 @@ impl ToShape for fj::Difference2d { ); } - shape.faces = { - let (a, b) = if a.faces.0.len() == 1 && b.faces.0.len() == 1 { + *shape.faces() = { + let (a, b) = if a.faces().0.len() == 1 && b.faces().0.len() == 1 { // Can't panic. We just checked that length of `a` and `b` is 1. - (a.faces.0.pop().unwrap(), b.faces.0.pop().unwrap()) + (a.faces().0.pop().unwrap(), b.faces().0.pop().unwrap()) } else { // See issue: // https://github.com/hannobraun/Fornjot/issues/95 diff --git a/src/kernel/shapes/sketch.rs b/src/kernel/shapes/sketch.rs index 26cd33789..9ca26f789 100644 --- a/src/kernel/shapes/sketch.rs +++ b/src/kernel/shapes/sketch.rs @@ -55,7 +55,7 @@ impl ToShape for fj::Sketch { .collect(), surface: Surface::x_y_plane(), }; - shape.faces = Faces(vec![face]); + *shape.faces() = Faces(vec![face]); shape } diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index 8f3ecf7e4..fc98b0d80 100644 --- a/src/kernel/shapes/union.rs +++ b/src/kernel/shapes/union.rs @@ -10,8 +10,8 @@ impl ToShape for fj::Union { fn to_shape(&self, tolerance: Scalar, debug_info: &mut DebugInfo) -> Shape { let mut shape = Shape::new(); - let a = self.a.to_shape(tolerance, debug_info).faces.0; - let b = self.b.to_shape(tolerance, debug_info).faces.0; + let a = self.a.to_shape(tolerance, debug_info).faces().0.clone(); + let b = self.b.to_shape(tolerance, debug_info).faces().0.clone(); // This doesn't create a true union, as it doesn't eliminate, merge, or // split faces. @@ -22,7 +22,7 @@ impl ToShape for fj::Union { faces.extend(a); faces.extend(b); - shape.faces = Faces(faces); + *shape.faces() = Faces(faces); shape } diff --git a/src/main.rs b/src/main.rs index 0f37241f0..43e5e634a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -101,11 +101,10 @@ fn main() -> anyhow::Result<()> { let mut debug_info = DebugInfo::new(); let mut triangles = Vec::new(); - shape.to_shape(tolerance, &mut debug_info).faces.triangles( - tolerance, - &mut triangles, - &mut debug_info, - ); + shape + .to_shape(tolerance, &mut debug_info) + .faces() + .triangles(tolerance, &mut triangles, &mut debug_info); if let Some(path) = args.export { let mut mesh_maker = MeshMaker::new(); @@ -229,11 +228,10 @@ fn main() -> anyhow::Result<()> { triangles.clear(); aabb = shape.bounding_volume(); - shape.to_shape(tolerance, &mut debug_info).faces.triangles( - tolerance, - &mut triangles, - &mut debug_info, - ); + shape + .to_shape(tolerance, &mut debug_info) + .faces() + .triangles(tolerance, &mut triangles, &mut debug_info); renderer.update_geometry( (&triangles).into(), From cf5efbc1a4ff5abd0e88fed73acda73e4500e546 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:20:52 +0100 Subject: [PATCH 07/21] Remove unnecessary `pub` --- src/kernel/shape/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/kernel/shape/mod.rs b/src/kernel/shape/mod.rs index a4bb16168..ad0c9f88f 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -29,8 +29,7 @@ pub struct Shape { vertices: VerticesInner, cycles: CyclesInner, - - pub faces: Faces, + faces: Faces, } impl Shape { From e18868611b80d95149ab6f328a381872997f84d3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:27:42 +0100 Subject: [PATCH 08/21] Refactor --- src/kernel/shapes/union.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index fc98b0d80..524e009e1 100644 --- a/src/kernel/shapes/union.rs +++ b/src/kernel/shapes/union.rs @@ -10,8 +10,8 @@ impl ToShape for fj::Union { fn to_shape(&self, tolerance: Scalar, debug_info: &mut DebugInfo) -> Shape { let mut shape = Shape::new(); - let a = self.a.to_shape(tolerance, debug_info).faces().0.clone(); - let b = self.b.to_shape(tolerance, debug_info).faces().0.clone(); + let mut a = self.a.to_shape(tolerance, debug_info); + let mut b = self.b.to_shape(tolerance, debug_info); // This doesn't create a true union, as it doesn't eliminate, merge, or // split faces. @@ -19,8 +19,8 @@ impl ToShape for fj::Union { // See issue: // https://github.com/hannobraun/Fornjot/issues/42 let mut faces = Vec::new(); - faces.extend(a); - faces.extend(b); + faces.extend(a.faces().0.clone()); + faces.extend(b.faces().0.clone()); *shape.faces() = Faces(faces); From ae3bbb3d67d5a2aff642a25b739690e0b35484d7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:29:46 +0100 Subject: [PATCH 09/21] Add `Faces::all` --- src/kernel/algorithms/sweep.rs | 4 ++-- src/kernel/algorithms/transform.rs | 4 ++-- src/kernel/shape/faces.rs | 5 +++++ src/kernel/shapes/difference_2d.rs | 9 +++++++-- src/kernel/shapes/union.rs | 4 ++-- 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index 6474c27c8..0218c09ea 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -22,9 +22,9 @@ pub fn sweep_shape( let mut top_faces = Vec::new(); let mut side_faces = Vec::new(); - for face in &original.faces().0 { + for face in original.faces().all() { bottom_faces.push(face.clone()); - top_faces.push(transform_face(face, &translation, &mut shape)); + top_faces.push(transform_face(&face, &translation, &mut shape)); } for cycle in original.cycles().all() { diff --git a/src/kernel/algorithms/transform.rs b/src/kernel/algorithms/transform.rs index 0d937316f..1c2f63c4e 100644 --- a/src/kernel/algorithms/transform.rs +++ b/src/kernel/algorithms/transform.rs @@ -22,8 +22,8 @@ use crate::{ pub fn transform_shape(mut original: Shape, transform: &Transform) -> Shape { let mut transformed = Shape::new(); - for face in &original.faces().0 { - let face = transform_face(face, transform, &mut transformed); + for face in original.faces().all() { + let face = transform_face(&face, transform, &mut transformed); transformed.faces().0.push(face); } diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs index 5a07deede..18d281e7d 100644 --- a/src/kernel/shape/faces.rs +++ b/src/kernel/shape/faces.rs @@ -9,6 +9,11 @@ use crate::{ pub struct Faces(pub Vec); impl Faces { + /// Access an iterator over all faces + pub fn all(&self) -> impl Iterator + '_ { + self.0.iter().cloned() + } + pub fn triangles( &self, tolerance: Scalar, diff --git a/src/kernel/shapes/difference_2d.rs b/src/kernel/shapes/difference_2d.rs index 6325c5d86..a2f9be3f2 100644 --- a/src/kernel/shapes/difference_2d.rs +++ b/src/kernel/shapes/difference_2d.rs @@ -39,9 +39,14 @@ impl ToShape for fj::Difference2d { } *shape.faces() = { - let (a, b) = if a.faces().0.len() == 1 && b.faces().0.len() == 1 { + let (a, b) = if a.faces().all().count() == 1 + && b.faces().all().count() == 1 + { // Can't panic. We just checked that length of `a` and `b` is 1. - (a.faces().0.pop().unwrap(), b.faces().0.pop().unwrap()) + ( + a.faces().all().next().unwrap(), + b.faces().all().next().unwrap(), + ) } else { // See issue: // https://github.com/hannobraun/Fornjot/issues/95 diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index 524e009e1..52167bf28 100644 --- a/src/kernel/shapes/union.rs +++ b/src/kernel/shapes/union.rs @@ -19,8 +19,8 @@ impl ToShape for fj::Union { // See issue: // https://github.com/hannobraun/Fornjot/issues/42 let mut faces = Vec::new(); - faces.extend(a.faces().0.clone()); - faces.extend(b.faces().0.clone()); + faces.extend(a.faces().all()); + faces.extend(b.faces().all()); *shape.faces() = Faces(faces); From 3d48f7c2b65c99dc0cf0679431f5502cf1f98147 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:47:45 +0100 Subject: [PATCH 10/21] Refactor --- src/kernel/shapes/circle.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/kernel/shapes/circle.rs b/src/kernel/shapes/circle.rs index c6e3ce36d..0366b4816 100644 --- a/src/kernel/shapes/circle.rs +++ b/src/kernel/shapes/circle.rs @@ -25,12 +25,13 @@ impl ToShape for fj::Circle { .add(Edge::circle(Scalar::from_f64(self.radius))); shape.cycles().add(Cycle { edges: vec![edge] }); + let cycles = shape + .cycles() + .all() + .map(|handle| (*handle).clone()) + .collect(); *shape.faces() = Faces(vec![Face::Face { - cycles: shape - .cycles() - .all() - .map(|handle| (*handle).clone()) - .collect(), + cycles, surface: Surface::x_y_plane(), }]); From a90d132a3314381c48cb34172939515c790747bf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:50:06 +0100 Subject: [PATCH 11/21] Add `Faces::add` --- src/kernel/algorithms/sweep.rs | 22 +++++++++++----------- src/kernel/algorithms/transform.rs | 2 +- src/kernel/shape/faces.rs | 8 ++++++++ src/kernel/shapes/circle.rs | 6 +++--- src/kernel/shapes/difference_2d.rs | 6 +++--- src/kernel/shapes/sketch.rs | 4 ++-- src/kernel/shapes/union.rs | 13 +++++++------ 7 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index 0218c09ea..104e2ec4f 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -1,8 +1,5 @@ use crate::{ - kernel::{ - shape::{faces::Faces, Shape}, - topology::faces::Face, - }, + kernel::{shape::Shape, topology::faces::Face}, math::{Scalar, Transform, Vector}, }; @@ -55,12 +52,15 @@ pub fn sweep_shape( side_faces.push(Face::Triangles(side_face)); } - let mut faces = Vec::new(); - faces.extend(bottom_faces); - faces.extend(top_faces); - faces.extend(side_faces); - - *shape.faces() = Faces(faces); + for face in bottom_faces { + shape.faces().add(face); + } + for face in top_faces { + shape.faces().add(face); + } + for face in side_faces { + shape.faces().add(face); + } shape } @@ -134,7 +134,7 @@ mod tests { }], }; - shape.faces().0.push(abc.clone()); + shape.faces().add(abc.clone()); Self { shape, face: abc } } diff --git a/src/kernel/algorithms/transform.rs b/src/kernel/algorithms/transform.rs index 1c2f63c4e..f737dce1e 100644 --- a/src/kernel/algorithms/transform.rs +++ b/src/kernel/algorithms/transform.rs @@ -24,7 +24,7 @@ pub fn transform_shape(mut original: Shape, transform: &Transform) -> Shape { for face in original.faces().all() { let face = transform_face(&face, transform, &mut transformed); - transformed.faces().0.push(face); + transformed.faces().add(face); } transformed diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs index 18d281e7d..71ed8255b 100644 --- a/src/kernel/shape/faces.rs +++ b/src/kernel/shape/faces.rs @@ -4,11 +4,19 @@ use crate::{ math::{Scalar, Triangle}, }; +use super::handle::{Handle, Storage}; + /// The faces of a shape #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Faces(pub Vec); impl Faces { + /// Add a face to the shape + pub fn add(&mut self, face: Face) -> Handle { + self.0.push(face.clone()); + Storage::new(face).handle() + } + /// Access an iterator over all faces pub fn all(&self) -> impl Iterator + '_ { self.0.iter().cloned() diff --git a/src/kernel/shapes/circle.rs b/src/kernel/shapes/circle.rs index 0366b4816..3048af1c8 100644 --- a/src/kernel/shapes/circle.rs +++ b/src/kernel/shapes/circle.rs @@ -2,7 +2,7 @@ use crate::{ debug::DebugInfo, kernel::{ geometry::Surface, - shape::{faces::Faces, Shape}, + shape::Shape, topology::{ edges::{Cycle, Edge}, faces::Face, @@ -30,10 +30,10 @@ impl ToShape for fj::Circle { .all() .map(|handle| (*handle).clone()) .collect(); - *shape.faces() = Faces(vec![Face::Face { + shape.faces().add(Face::Face { cycles, surface: Surface::x_y_plane(), - }]); + }); shape } diff --git a/src/kernel/shapes/difference_2d.rs b/src/kernel/shapes/difference_2d.rs index a2f9be3f2..80ff5dfc2 100644 --- a/src/kernel/shapes/difference_2d.rs +++ b/src/kernel/shapes/difference_2d.rs @@ -1,7 +1,7 @@ use crate::{ debug::DebugInfo, kernel::{ - shape::{faces::Faces, Shape}, + shape::Shape, topology::{edges::Cycle, faces::Face}, }, math::{Aabb, Scalar}, @@ -38,7 +38,7 @@ impl ToShape for fj::Difference2d { ); } - *shape.faces() = { + { let (a, b) = if a.faces().all().count() == 1 && b.faces().all().count() == 1 { @@ -84,7 +84,7 @@ impl ToShape for fj::Difference2d { let mut cycles = a; cycles.extend(b); - Faces(vec![Face::Face { cycles, surface }]) + shape.faces().add(Face::Face { cycles, surface }); }; shape diff --git a/src/kernel/shapes/sketch.rs b/src/kernel/shapes/sketch.rs index 9ca26f789..296604b4f 100644 --- a/src/kernel/shapes/sketch.rs +++ b/src/kernel/shapes/sketch.rs @@ -2,7 +2,7 @@ use crate::{ debug::DebugInfo, kernel::{ geometry::Surface, - shape::{faces::Faces, Shape}, + shape::Shape, topology::{ edges::{Cycle, Edge}, faces::Face, @@ -55,7 +55,7 @@ impl ToShape for fj::Sketch { .collect(), surface: Surface::x_y_plane(), }; - *shape.faces() = Faces(vec![face]); + shape.faces().add(face); shape } diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index 52167bf28..5b6a5cee3 100644 --- a/src/kernel/shapes/union.rs +++ b/src/kernel/shapes/union.rs @@ -1,6 +1,6 @@ use crate::{ debug::DebugInfo, - kernel::shape::{faces::Faces, Shape}, + kernel::shape::Shape, math::{Aabb, Scalar}, }; @@ -18,11 +18,12 @@ impl ToShape for fj::Union { // // See issue: // https://github.com/hannobraun/Fornjot/issues/42 - let mut faces = Vec::new(); - faces.extend(a.faces().all()); - faces.extend(b.faces().all()); - - *shape.faces() = Faces(faces); + for face in a.faces().all() { + shape.faces().add(face); + } + for face in b.faces().all() { + shape.faces().add(face); + } shape } From 07d6989e921317c36bdd31ac5564e9e56022e515 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:52:02 +0100 Subject: [PATCH 12/21] Add `Faces::contains` --- src/kernel/algorithms/sweep.rs | 4 ++-- src/kernel/shape/faces.rs | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index 104e2ec4f..bcd3ed1b1 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -95,8 +95,8 @@ mod tests { let top_face = Triangle::new([[0., 0., 1.], [1., 0., 1.], [0., 1., 1.]]).face; - assert!(swept.faces().0.contains(&bottom_face)); - assert!(swept.faces().0.contains(&top_face)); + assert!(swept.faces().contains(&bottom_face)); + assert!(swept.faces().contains(&top_face)); // Side faces are not tested, as those use triangle representation. The // plan is to start testing them, as they are transitioned to b-rep. diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs index 71ed8255b..e5c648d02 100644 --- a/src/kernel/shape/faces.rs +++ b/src/kernel/shape/faces.rs @@ -17,6 +17,12 @@ impl Faces { Storage::new(face).handle() } + /// Check whether the shape contains a specific face + #[cfg(test)] + pub fn contains(&self, face: &Face) -> bool { + self.0.contains(face) + } + /// Access an iterator over all faces pub fn all(&self) -> impl Iterator + '_ { self.0.iter().cloned() From e53c9655f17987a492cfd41b364db46cd3cb6e1f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:52:19 +0100 Subject: [PATCH 13/21] Make field of `Faces` private --- src/kernel/shape/faces.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs index e5c648d02..bb5a54b77 100644 --- a/src/kernel/shape/faces.rs +++ b/src/kernel/shape/faces.rs @@ -8,7 +8,7 @@ use super::handle::{Handle, Storage}; /// The faces of a shape #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Faces(pub Vec); +pub struct Faces(pub(super) Vec); impl Faces { /// Add a face to the shape From 98c28517cc5a754309c8d6b4817071828bb8a41d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:53:32 +0100 Subject: [PATCH 14/21] Refactor --- src/kernel/algorithms/sweep.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index bcd3ed1b1..8b006fe7b 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -70,7 +70,7 @@ mod tests { use crate::{ kernel::{ geometry::{surfaces::Swept, Surface}, - shape::Shape, + shape::{handle::Handle, Shape}, topology::{ edges::{Cycle, Edge}, faces::Face, @@ -104,7 +104,7 @@ mod tests { pub struct Triangle { shape: Shape, - face: Face, + face: Handle, } impl Triangle { @@ -134,9 +134,9 @@ mod tests { }], }; - shape.faces().add(abc.clone()); + let face = shape.faces().add(abc); - Self { shape, face: abc } + Self { shape, face } } } } From d7a8a2935b0ff379d9e73edfb8829a68d9bd126d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:55:22 +0100 Subject: [PATCH 15/21] Convert tuple struct into regular struct --- src/kernel/shape/faces.rs | 12 +++++++----- src/kernel/shape/mod.rs | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs index bb5a54b77..daf3101ca 100644 --- a/src/kernel/shape/faces.rs +++ b/src/kernel/shape/faces.rs @@ -8,24 +8,26 @@ use super::handle::{Handle, Storage}; /// The faces of a shape #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Faces(pub(super) Vec); +pub struct Faces { + pub(super) faces: Vec, +} impl Faces { /// Add a face to the shape pub fn add(&mut self, face: Face) -> Handle { - self.0.push(face.clone()); + self.faces.push(face.clone()); Storage::new(face).handle() } /// Check whether the shape contains a specific face #[cfg(test)] pub fn contains(&self, face: &Face) -> bool { - self.0.contains(face) + self.faces.contains(face) } /// Access an iterator over all faces pub fn all(&self) -> impl Iterator + '_ { - self.0.iter().cloned() + self.faces.iter().cloned() } pub fn triangles( @@ -34,7 +36,7 @@ impl Faces { out: &mut Vec>, debug_info: &mut DebugInfo, ) { - for face in &self.0 { + for face in &self.faces { face.triangles(tolerance, out, debug_info); } } diff --git a/src/kernel/shape/mod.rs b/src/kernel/shape/mod.rs index ad0c9f88f..ef9161c9b 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -43,7 +43,7 @@ impl Shape { vertices: VerticesInner::new(), cycles: CyclesInner::new(), - faces: Faces(Vec::new()), + faces: Faces { faces: Vec::new() }, } } From a932ce5fe8ce9b0c94800aca32e422e0bf156736 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 13:58:24 +0100 Subject: [PATCH 16/21] Add type definition --- src/kernel/shape/faces.rs | 7 +++++-- src/kernel/shape/mod.rs | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs index daf3101ca..6d83bf02e 100644 --- a/src/kernel/shape/faces.rs +++ b/src/kernel/shape/faces.rs @@ -4,12 +4,15 @@ use crate::{ math::{Scalar, Triangle}, }; -use super::handle::{Handle, Storage}; +use super::{ + handle::{Handle, Storage}, + FacesInner, +}; /// The faces of a shape #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Faces { - pub(super) faces: Vec, + pub(super) faces: FacesInner, } impl Faces { diff --git a/src/kernel/shape/mod.rs b/src/kernel/shape/mod.rs index ef9161c9b..13f308a1e 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -6,7 +6,7 @@ pub mod vertices; use crate::math::Scalar; -use super::topology::{edges::Cycle, vertices::Vertex}; +use super::topology::{edges::Cycle, faces::Face, vertices::Vertex}; use self::{ cycles::Cycles, edges::Edges, faces::Faces, handle::Storage, @@ -90,3 +90,4 @@ impl Shape { type VerticesInner = Vec>; type CyclesInner = Vec>; +type FacesInner = Vec; From 4d5a279f7152d7917b5aa624253d42d4619c1ab5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 14:00:11 +0100 Subject: [PATCH 17/21] Prevent write access to faces of `Shape` --- src/kernel/shape/faces.rs | 9 ++++----- src/kernel/shape/mod.rs | 10 ++++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs index 6d83bf02e..0c177317b 100644 --- a/src/kernel/shape/faces.rs +++ b/src/kernel/shape/faces.rs @@ -10,12 +10,11 @@ use super::{ }; /// The faces of a shape -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Faces { - pub(super) faces: FacesInner, +pub struct Faces<'r> { + pub(super) faces: &'r mut FacesInner, } -impl Faces { +impl Faces<'_> { /// Add a face to the shape pub fn add(&mut self, face: Face) -> Handle { self.faces.push(face.clone()); @@ -39,7 +38,7 @@ impl Faces { out: &mut Vec>, debug_info: &mut DebugInfo, ) { - for face in &self.faces { + for face in &*self.faces { face.triangles(tolerance, out, debug_info); } } diff --git a/src/kernel/shape/mod.rs b/src/kernel/shape/mod.rs index 13f308a1e..95e1786bd 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -29,7 +29,7 @@ pub struct Shape { vertices: VerticesInner, cycles: CyclesInner, - faces: Faces, + faces: FacesInner, } impl Shape { @@ -43,7 +43,7 @@ impl Shape { vertices: VerticesInner::new(), cycles: CyclesInner::new(), - faces: Faces { faces: Vec::new() }, + faces: FacesInner::new(), } } @@ -83,8 +83,10 @@ impl Shape { } /// Access the shape's faces - pub fn faces(&mut self) -> &mut Faces { - &mut self.faces + pub fn faces(&mut self) -> Faces { + Faces { + faces: &mut self.faces, + } } } From 2aeb7249924bae57f638bae0deaeafa71340de42 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 14:13:06 +0100 Subject: [PATCH 18/21] Store faces in `Storage` --- src/kernel/algorithms/sweep.rs | 2 +- src/kernel/shape/faces.rs | 14 ++++++++----- src/kernel/shape/mod.rs | 2 +- src/kernel/shapes/difference_2d.rs | 33 +++++++++++++++--------------- src/kernel/shapes/union.rs | 4 ++-- 5 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index 8b006fe7b..cad604db5 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -53,7 +53,7 @@ pub fn sweep_shape( } for face in bottom_faces { - shape.faces().add(face); + shape.faces().add((*face).clone()); } for face in top_faces { shape.faces().add(face); diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs index 0c177317b..e1c8b3787 100644 --- a/src/kernel/shape/faces.rs +++ b/src/kernel/shape/faces.rs @@ -17,19 +17,23 @@ pub struct Faces<'r> { impl Faces<'_> { /// Add a face to the shape pub fn add(&mut self, face: Face) -> Handle { - self.faces.push(face.clone()); - Storage::new(face).handle() + let storage = Storage::new(face); + let handle = storage.handle(); + + self.faces.push(storage); + + handle } /// Check whether the shape contains a specific face #[cfg(test)] pub fn contains(&self, face: &Face) -> bool { - self.faces.contains(face) + self.faces.contains(&Storage::new(face.clone())) } /// Access an iterator over all faces - pub fn all(&self) -> impl Iterator + '_ { - self.faces.iter().cloned() + pub fn all(&self) -> impl Iterator> + '_ { + self.faces.iter().map(|storage| storage.handle()) } pub fn triangles( diff --git a/src/kernel/shape/mod.rs b/src/kernel/shape/mod.rs index 95e1786bd..1c37ddcfb 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -92,4 +92,4 @@ impl Shape { type VerticesInner = Vec>; type CyclesInner = Vec>; -type FacesInner = Vec; +type FacesInner = Vec>; diff --git a/src/kernel/shapes/difference_2d.rs b/src/kernel/shapes/difference_2d.rs index 80ff5dfc2..c1eac9f4c 100644 --- a/src/kernel/shapes/difference_2d.rs +++ b/src/kernel/shapes/difference_2d.rs @@ -56,22 +56,23 @@ impl ToShape for fj::Difference2d { ); }; - let (a, b, surface_a, surface_b) = match (a, b) { - ( - Face::Face { - cycles: a, - surface: surface_a, - }, - Face::Face { - cycles: b, - surface: surface_b, - }, - ) => (a, b, surface_a, surface_b), - _ => { - // None of the 2D types still use the triangles representation. - unreachable!() - } - }; + let (a, b, surface_a, surface_b) = + match ((*a).clone(), (*b).clone()) { + ( + Face::Face { + cycles: a, + surface: surface_a, + }, + Face::Face { + cycles: b, + surface: surface_b, + }, + ) => (a, b, surface_a, surface_b), + _ => { + // None of the 2D types still use the triangles representation. + unreachable!() + } + }; assert!( surface_a == surface_b, diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index 5b6a5cee3..8147510fc 100644 --- a/src/kernel/shapes/union.rs +++ b/src/kernel/shapes/union.rs @@ -19,10 +19,10 @@ impl ToShape for fj::Union { // See issue: // https://github.com/hannobraun/Fornjot/issues/42 for face in a.faces().all() { - shape.faces().add(face); + shape.faces().add((*face).clone()); } for face in b.faces().all() { - shape.faces().add(face); + shape.faces().add((*face).clone()); } shape From 14e304346de0065e54849bd72d51b542fad9f457 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 14:13:24 +0100 Subject: [PATCH 19/21] Fix return type of `Cycles::all` --- src/kernel/shape/cycles.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernel/shape/cycles.rs b/src/kernel/shape/cycles.rs index 83c9315b5..6a06295cf 100644 --- a/src/kernel/shape/cycles.rs +++ b/src/kernel/shape/cycles.rs @@ -29,7 +29,7 @@ impl Cycles<'_> { } /// Access an iterator over all cycles - pub fn all(&self) -> impl Iterator> + '_ { - self.cycles.iter().cloned() + pub fn all(&self) -> impl Iterator> + '_ { + self.cycles.iter().map(|storage| storage.handle()) } } From dddcc8c3774d8ede486c0894403765b0f9314721 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 14:13:48 +0100 Subject: [PATCH 20/21] Update comment --- src/kernel/shapes/difference_2d.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/kernel/shapes/difference_2d.rs b/src/kernel/shapes/difference_2d.rs index c1eac9f4c..fc1113902 100644 --- a/src/kernel/shapes/difference_2d.rs +++ b/src/kernel/shapes/difference_2d.rs @@ -69,7 +69,8 @@ impl ToShape for fj::Difference2d { }, ) => (a, b, surface_a, surface_b), _ => { - // None of the 2D types still use the triangles representation. + // None of the 2D types still use triangle + // representation. unreachable!() } }; From 9a88774fffbf6996ee1f01d6ba2ed06f2303f4ef Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 14:14:54 +0100 Subject: [PATCH 21/21] Remove outdated implementation note --- src/kernel/shape/mod.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/kernel/shape/mod.rs b/src/kernel/shape/mod.rs index 1c37ddcfb..265fab1a4 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -14,12 +14,6 @@ use self::{ }; /// The boundary representation of a shape -/// -/// # Implementation note -/// -/// The goal for `Shape` is to enforce full self-consistency, through the API it -/// provides. Steps have been made in that direction, but right now, the API is -/// still full of holes, forcing callers to just be careful for the time being. #[derive(Clone, Debug)] pub struct Shape { /// The minimum distance between two vertices