diff --git a/fj-kernel/src/algorithms/approximation.rs b/fj-kernel/src/algorithms/approximation.rs index 90b8e9a70..f9297e4bd 100644 --- a/fj-kernel/src/algorithms/approximation.rs +++ b/fj-kernel/src/algorithms/approximation.rs @@ -45,7 +45,7 @@ impl Approximation { let mut points = HashSet::new(); let mut segments = HashSet::new(); - for cycle in face.cycles() { + for cycle in face.all_cycles() { let cycle_points = approximate_cycle(&cycle, tolerance); let mut cycle_segments = Vec::new(); @@ -196,7 +196,8 @@ mod tests { let surface = shape.geometry().add_surface(Surface::x_y_plane()); let face = Face::Face { surface, - cycles: vec![abcd], + exteriors: vec![abcd], + interiors: Vec::new(), color: [255, 0, 0, 255], }; diff --git a/fj-kernel/src/algorithms/sweep.rs b/fj-kernel/src/algorithms/sweep.rs index 425351aa9..608026695 100644 --- a/fj-kernel/src/algorithms/sweep.rs +++ b/fj-kernel/src/algorithms/sweep.rs @@ -109,14 +109,19 @@ pub fn sweep_shape( .geometry() .add_surface(surface_bottom.get().transform(&translation)); - let cycles_bottom = source_to_bottom.cycles_for_face(&face_source); - let cycles_top = source_to_top.cycles_for_face(&face_source); + let exteriors_bottom = + source_to_bottom.exteriors_for_face(&face_source); + let interiors_bottom = + source_to_bottom.interiors_for_face(&face_source); + let exteriors_top = source_to_top.exteriors_for_face(&face_source); + let interiors_top = source_to_top.interiors_for_face(&face_source); target .topology() .add_face(Face::Face { surface: surface_bottom, - cycles: cycles_bottom, + exteriors: exteriors_bottom, + interiors: interiors_bottom, color, }) .unwrap(); @@ -124,7 +129,8 @@ pub fn sweep_shape( .topology() .add_face(Face::Face { surface: surface_top, - cycles: cycles_top, + exteriors: exteriors_top, + interiors: interiors_top, color, }) .unwrap(); @@ -256,7 +262,8 @@ pub fn sweep_shape( .topology() .add_face(Face::Face { surface, - cycles: vec![cycle], + exteriors: vec![cycle], + interiors: Vec::new(), color, }) .unwrap(); @@ -300,9 +307,9 @@ impl Relation { .collect() } - fn cycles_for_face(&self, face: &Face) -> Vec> { - let cycles = match face { - Face::Face { cycles, .. } => cycles, + fn exteriors_for_face(&self, face: &Face) -> Vec> { + let exteriors = match face { + Face::Face { exteriors, .. } => exteriors, _ => { // Sketches are created using boundary representation, so this // case can't happen. @@ -310,7 +317,23 @@ impl Relation { } }; - cycles + exteriors + .iter() + .map(|cycle| self.cycles.get(cycle).unwrap().clone()) + .collect() + } + + fn interiors_for_face(&self, face: &Face) -> Vec> { + let interiors = match face { + Face::Face { interiors, .. } => interiors, + _ => { + // Sketches are created using boundary representation, so this + // case can't happen. + unreachable!() + } + }; + + interiors .iter() .map(|cycle| self.cycles.get(cycle).unwrap().clone()) .collect() @@ -412,7 +435,8 @@ mod tests { )); let abc = Face::Face { surface, - cycles: vec![cycles], + exteriors: vec![cycles], + interiors: Vec::new(), color: [255, 0, 0, 255], }; diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index 250073f11..aa8779edb 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -179,7 +179,10 @@ impl Topology<'_> { /// not the case. pub fn add_face(&mut self, face: Face) -> ValidationResult { if let Face::Face { - surface, cycles, .. + surface, + exteriors, + interiors, + .. } = &face { let mut missing_surface = None; @@ -188,7 +191,7 @@ impl Topology<'_> { if !self.geometry.surfaces.contains(surface.storage()) { missing_surface = Some(surface.clone()); } - for cycle in cycles { + for cycle in exteriors.iter().chain(interiors) { if !self.cycles.contains(cycle.storage()) { missing_cycles.insert(cycle.clone()); } @@ -349,7 +352,8 @@ mod tests { .topology() .add_face(Face::Face { surface: surface.clone(), - cycles: vec![cycle.clone()], + exteriors: vec![cycle.clone()], + interiors: Vec::new(), color: [255, 0, 0, 255], }) .unwrap_err(); @@ -362,7 +366,8 @@ mod tests { // Everything has been added to `shape` now. Should work! shape.topology().add_face(Face::Face { surface, - cycles: vec![cycle], + exteriors: vec![cycle], + interiors: Vec::new(), color: [255, 0, 0, 255], })?; diff --git a/fj-kernel/src/topology/faces.rs b/fj-kernel/src/topology/faces.rs index d5120cbf2..b60d29efe 100644 --- a/fj-kernel/src/topology/faces.rs +++ b/fj-kernel/src/topology/faces.rs @@ -22,7 +22,7 @@ pub enum Face { /// The surface that defines this face surface: Handle, - /// The cycles that bound the face + /// The cycles that bound the face on the outside /// /// # Implementation Note /// @@ -32,7 +32,16 @@ pub enum Face { /// /// It might be less error-prone to specify the edges in surface /// coordinates. - cycles: Vec>, + exteriors: Vec>, + + /// 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. + interiors: Vec>, /// The color of the face color: [u8; 4], @@ -63,14 +72,31 @@ impl Face { } } - /// Access the cycles that the face refers to + /// Access the exterior cycles that the face refers to + /// + /// This is a convenience method that saves the caller from dealing with the + /// [`Handle`]s. + pub fn exteriors(&self) -> impl Iterator + '_ { + match self { + Self::Face { exteriors, .. } => { + exteriors.iter().map(|handle| handle.get().clone()) + } + _ => { + // No code that still uses triangle representation is calling + // this method. + unreachable!() + } + } + } + + /// Access the interior cycles that the face refers to /// /// This is a convenience method that saves the caller from dealing with the /// [`Handle`]s. - pub fn cycles(&self) -> impl Iterator + '_ { + pub fn interiors(&self) -> impl Iterator + '_ { match self { - Self::Face { cycles, .. } => { - cycles.iter().map(|handle| handle.get().clone()) + Self::Face { interiors, .. } => { + interiors.iter().map(|handle| handle.get().clone()) } _ => { // No code that still uses triangle representation is calling @@ -79,18 +105,28 @@ impl Face { } } } + + /// Access all cycles that the face refers to + /// + /// 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()) + } } impl PartialEq for Face { fn eq(&self, other: &Self) -> bool { - self.surface() == other.surface() && self.cycles().eq(other.cycles()) + self.surface() == other.surface() + && self.exteriors().eq(other.exteriors()) + && self.interiors().eq(other.interiors()) } } impl Hash for Face { fn hash(&self, state: &mut H) { self.surface().hash(state); - for cycle in self.cycles() { + for cycle in self.all_cycles() { cycle.hash(state); } } diff --git a/fj-operations/src/circle.rs b/fj-operations/src/circle.rs index 0bb8880c0..da6750e1e 100644 --- a/fj-operations/src/circle.rs +++ b/fj-operations/src/circle.rs @@ -29,7 +29,8 @@ impl ToShape for fj::Circle { shape .topology() .add_face(Face::Face { - cycles, + exteriors: cycles, + interiors: Vec::new(), surface, color: self.color(), }) diff --git a/fj-operations/src/difference_2d.rs b/fj-operations/src/difference_2d.rs index 9abe08efd..89178c598 100644 --- a/fj-operations/src/difference_2d.rs +++ b/fj-operations/src/difference_2d.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use fj_debug::DebugInfo; use fj_kernel::{ - shape::Shape, + shape::{Handle, Shape}, topology::{Cycle, Edge, Face, Vertex}, }; use fj_math::{Aabb, Scalar}; @@ -19,18 +19,18 @@ impl ToShape for fj::Difference2d { let [mut a, mut b] = [&self.a(), &self.b()] .map(|shape| shape.to_shape(tolerance, debug_info)); + // Check preconditions. + // + // See issue: + // https://github.com/hannobraun/Fornjot/issues/95 for shape in [&mut a, &mut b] { if shape.topology().cycles().count() != 1 { - // See issue: - // https://github.com/hannobraun/Fornjot/issues/95 todo!( "The 2-dimensional difference operation only supports one \ cycle in each operand." ); } if shape.topology().faces().count() != 1 { - // See issue: - // https://github.com/hannobraun/Fornjot/issues/95 todo!( "The 2-dimensional difference operation only supports one \ face in each operand." @@ -39,45 +39,15 @@ impl ToShape for fj::Difference2d { } // Can't panic, as we just verified that both shapes have one cycle. - let cycles_orig = [&mut a, &mut b] + let [cycle_a, cycle_b] = [&mut a, &mut b] .map(|shape| shape.topology().cycles().next().unwrap()); let mut vertices = HashMap::new(); - let mut cycles = Vec::new(); - - for cycle in cycles_orig { - let mut edges = Vec::new(); - for edge in &cycle.get().edges { - let edge = edge.get(); - - let curve = shape.geometry().add_curve(edge.curve()); - - let vertices = edge.vertices().clone().map(|vs| { - vs.map(|vertex| { - vertices - .entry(vertex.clone()) - .or_insert_with(|| { - let point = - shape.geometry().add_point(vertex.point()); - shape - .topology() - .add_vertex(Vertex { point }) - .unwrap() - }) - .clone() - }) - }); - - let edge = shape - .topology() - .add_edge(Edge { curve, vertices }) - .unwrap(); - edges.push(edge); - } + let mut exteriors = Vec::new(); + let mut interiors = Vec::new(); - let cycle = shape.topology().add_cycle(Cycle { edges }).unwrap(); - cycles.push(cycle); - } + exteriors.push(add_cycle(cycle_a, &mut vertices, &mut shape)); + interiors.push(add_cycle(cycle_b, &mut vertices, &mut shape)); // Can't panic, as we just verified that both shapes have one face. let [face_a, face_b] = [&mut a, &mut b] @@ -92,8 +62,9 @@ impl ToShape for fj::Difference2d { shape .topology() .add_face(Face::Face { - cycles, surface, + exteriors, + interiors, color: self.color(), }) .unwrap(); @@ -108,3 +79,31 @@ impl ToShape for fj::Difference2d { self.a().bounding_volume() } } + +fn add_cycle( + cycle: Handle, + vertices: &mut HashMap>, + shape: &mut Shape, +) -> Handle { + let mut edges = Vec::new(); + for edge in cycle.get().edges() { + let curve = shape.geometry().add_curve(edge.curve()); + + let vertices = edge.vertices().clone().map(|vs| { + vs.map(|vertex| { + vertices + .entry(vertex.clone()) + .or_insert_with(|| { + let point = shape.geometry().add_point(vertex.point()); + shape.topology().add_vertex(Vertex { point }).unwrap() + }) + .clone() + }) + }); + + let edge = shape.topology().add_edge(Edge { curve, vertices }).unwrap(); + edges.push(edge); + } + + shape.topology().add_cycle(Cycle { edges }).unwrap() +} diff --git a/fj-operations/src/group.rs b/fj-operations/src/group.rs index 94b4de971..5d3a43cd3 100644 --- a/fj-operations/src/group.rs +++ b/fj-operations/src/group.rs @@ -92,14 +92,19 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { match &*face_orig.get() { Face::Face { surface, - cycles: cs, + exteriors, + interiors, color, } => { target .topology() .add_face(Face::Face { surface: surfaces[surface].clone(), - cycles: cs + exteriors: exteriors + .iter() + .map(|cycle| cycles[cycle].clone()) + .collect(), + interiors: interiors .iter() .map(|cycle| cycles[cycle].clone()) .collect(), diff --git a/fj-operations/src/sketch.rs b/fj-operations/src/sketch.rs index cc48fc3c1..e1cca462c 100644 --- a/fj-operations/src/sketch.rs +++ b/fj-operations/src/sketch.rs @@ -45,7 +45,8 @@ impl ToShape for fj::Sketch { let surface = shape.geometry().add_surface(Surface::x_y_plane()); let face = Face::Face { - cycles: shape.topology().cycles().collect(), + exteriors: shape.topology().cycles().collect(), + interiors: Vec::new(), surface, color: self.color(), };