diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index b0c155758..cad604db5 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -1,8 +1,5 @@ use crate::{ - kernel::{ - shape::Shape, - topology::faces::{Face, Faces}, - }, + kernel::{shape::Shape, topology::faces::Face}, math::{Scalar, Transform, Vector}, }; @@ -22,9 +19,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() { @@ -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).clone()); + } + for face in top_faces { + shape.faces().add(face); + } + for face in side_faces { + shape.faces().add(face); + } shape } @@ -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, @@ -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().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. @@ -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.0.push(abc.clone()); + let face = shape.faces().add(abc); - Self { shape, face: abc } + Self { shape, face } } } } diff --git a/src/kernel/algorithms/transform.rs b/src/kernel/algorithms/transform.rs index 2dc1cc7d6..f737dce1e 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 { - let face = transform_face(face, transform, &mut transformed); - transformed.faces.0.push(face); + for face in original.faces().all() { + let face = transform_face(&face, transform, &mut transformed); + transformed.faces().add(face); } transformed 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()) } } diff --git a/src/kernel/shape/faces.rs b/src/kernel/shape/faces.rs new file mode 100644 index 000000000..e1c8b3787 --- /dev/null +++ b/src/kernel/shape/faces.rs @@ -0,0 +1,49 @@ +use crate::{ + debug::DebugInfo, + kernel::topology::faces::Face, + math::{Scalar, Triangle}, +}; + +use super::{ + handle::{Handle, Storage}, + FacesInner, +}; + +/// The faces of a shape +pub struct Faces<'r> { + pub(super) faces: &'r mut FacesInner, +} + +impl Faces<'_> { + /// Add a face to the shape + pub fn add(&mut self, face: 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(&Storage::new(face.clone())) + } + + /// Access an iterator over all faces + pub fn all(&self) -> impl Iterator> + '_ { + self.faces.iter().map(|storage| storage.handle()) + } + + pub fn triangles( + &self, + tolerance: Scalar, + out: &mut Vec>, + debug_info: &mut DebugInfo, + ) { + 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 dc8719697..265fab1a4 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -1,21 +1,19 @@ 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, faces::Face, 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 -/// -/// # 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 @@ -25,8 +23,7 @@ pub struct Shape { vertices: VerticesInner, cycles: CyclesInner, - - pub faces: Faces, + faces: FacesInner, } impl Shape { @@ -40,7 +37,7 @@ impl Shape { vertices: VerticesInner::new(), cycles: CyclesInner::new(), - faces: Faces(Vec::new()), + faces: FacesInner::new(), } } @@ -78,7 +75,15 @@ impl Shape { cycles: &mut self.cycles, } } + + /// Access the shape's faces + pub fn faces(&mut self) -> Faces { + Faces { + faces: &mut self.faces, + } + } } type VerticesInner = Vec>; type CyclesInner = Vec>; +type FacesInner = Vec>; diff --git a/src/kernel/shapes/circle.rs b/src/kernel/shapes/circle.rs index f6304730f..3048af1c8 100644 --- a/src/kernel/shapes/circle.rs +++ b/src/kernel/shapes/circle.rs @@ -5,7 +5,7 @@ use crate::{ shape::Shape, topology::{ edges::{Cycle, Edge}, - faces::{Face, Faces}, + faces::Face, }, }, math::{Aabb, Point, Scalar}, @@ -25,14 +25,15 @@ 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 { - cycles: shape - .cycles() - .all() - .map(|handle| (*handle).clone()) - .collect(), + let cycles = shape + .cycles() + .all() + .map(|handle| (*handle).clone()) + .collect(); + 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 31394126e..fc1113902 100644 --- a/src/kernel/shapes/difference_2d.rs +++ b/src/kernel/shapes/difference_2d.rs @@ -2,10 +2,7 @@ use crate::{ debug::DebugInfo, kernel::{ shape::Shape, - topology::{ - edges::Cycle, - faces::{Face, Faces}, - }, + topology::{edges::Cycle, faces::Face}, }, math::{Aabb, Scalar}, }; @@ -41,10 +38,15 @@ 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 @@ -54,22 +56,24 @@ 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 triangle + // representation. + unreachable!() + } + }; assert!( surface_a == surface_b, @@ -82,7 +86,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 601a131de..296604b4f 100644 --- a/src/kernel/shapes/sketch.rs +++ b/src/kernel/shapes/sketch.rs @@ -5,7 +5,7 @@ use crate::{ shape::Shape, topology::{ edges::{Cycle, Edge}, - faces::{Face, Faces}, + faces::Face, }, }, math::{Aabb, Point, Scalar}, @@ -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/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> { diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index 9c35d85b5..8147510fc 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::Shape, math::{Aabb, Scalar}, }; @@ -10,19 +10,20 @@ 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 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. // // See issue: // https://github.com/hannobraun/Fornjot/issues/42 - let mut faces = Vec::new(); - faces.extend(a.0); - faces.extend(b.0); - - shape.faces = Faces(faces); + for face in a.faces().all() { + shape.faces().add((*face).clone()); + } + for face in b.faces().all() { + shape.faces().add((*face).clone()); + } shape } 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 { diff --git a/src/main.rs b/src/main.rs index 43230e146..43e5e634a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -100,10 +100,11 @@ 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(); @@ -226,10 +227,11 @@ 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(),