diff --git a/fj-kernel/src/algorithms/triangulation/mod.rs b/fj-kernel/src/algorithms/triangulation/mod.rs index 3666c0c19..a18312d0b 100644 --- a/fj-kernel/src/algorithms/triangulation/mod.rs +++ b/fj-kernel/src/algorithms/triangulation/mod.rs @@ -57,7 +57,7 @@ pub fn triangulate( let mut triangles = delaunay(points); triangles.retain(|triangle| { face_as_polygon.contains_triangle( - &triangle.map(|point| point.native()), + triangle.map(|point| point.native()), debug_info, ) }); diff --git a/fj-kernel/src/algorithms/triangulation/polygon.rs b/fj-kernel/src/algorithms/triangulation/polygon.rs index 057e8b481..4b5e01bff 100644 --- a/fj-kernel/src/algorithms/triangulation/polygon.rs +++ b/fj-kernel/src/algorithms/triangulation/polygon.rs @@ -12,6 +12,21 @@ pub struct Polygon { } impl Polygon { + /// Construct an instance of `Polygon` + /// + /// # Implementation note + /// + /// This method takes a `Surface`, but `Polygon` only uses that for + /// generating debug info. It might be better, if `Polygon` had a field + /// where it stored debug info specific to its algorithm. Then code using + /// `Polygon` could access that `Polygon`-specific debug info and translate + /// that into `DebugInfo`, as necessary. + /// + /// This would have the advantage of removing this dependency on `Surface`. + /// It would also make the test code a bit cleaner, as it wouldn't have to + /// bother with the `DebugInfo` anymore. Also, the `Polygon`-specific debug + /// info could potentially be more useful in test code, as a debugging tool + /// there. pub fn new(surface: Surface) -> Self { Self { surface, @@ -46,22 +61,41 @@ impl Polygon { pub fn contains_triangle( &self, - &[a, b, c]: &[Point<2>; 3], + triangle: [impl Into>; 3], debug_info: &mut DebugInfo, ) -> bool { + let [a, b, c] = triangle.map(Into::into); + + let mut might_be_hole = true; + for edge in [a, b, c, a].windows(2) { // This can't panic, as we passed `2` to `windows`. It can be // cleaned up a bit, once `array_windows` is stable. let edge = Segment::from([edge[0], edge[1]]); - // If the segment is an edge of the face, we don't need to take a - // closer look. - if self.contains_edge(edge) { + let is_exterior_edge = self.contains_exterior_edge(edge); + let is_interior_edge = self.contains_interior_edge(edge); + + // If the triangle edge is not an interior edge of the polygon, we + // can rule out that the triangle is identical with a hole in the + // polygon. + if !is_interior_edge { + might_be_hole = false; + } + + // If the triangle edge is an edge of the face, we don't need to + // take a closer look. + if is_exterior_edge || is_interior_edge { continue; } // To determine if the edge is within the polygon, we determine if // its center point is in the polygon. + // + // Since we already checked above, whether the triangle edge is a + // polygon edge (and if we reached this point, it isn't), we don't + // need to care about the distinction between "inside the polygon" + // and "on the polygon boundary". if !self.contains_point(edge.center(), debug_info) { // The segment is outside of the face. This means we can throw // away the whole triangle. @@ -69,15 +103,26 @@ impl Polygon { } } + // We haven't rules out that the triangle is a polygon hole. Since we + // checked all its edges, this means we now know for certain that is is. + if might_be_hole { + return false; + } + // If we didn't throw away the triangle up till now, this means all its // edges are within the face. true } - pub fn contains_edge(&self, edge: Segment<2>) -> bool { + pub fn contains_exterior_edge(&self, edge: Segment<2>) -> bool { + self.exterior.segments().contains(&edge) + || self.exterior.segments().contains(&edge.reverse()) + } + + pub fn contains_interior_edge(&self, edge: Segment<2>) -> bool { let mut contains = false; - for chain in Some(&self.exterior).into_iter().chain(&self.interiors) { + for chain in &self.interiors { contains |= chain.segments().contains(&edge); contains |= chain.segments().contains(&edge.reverse()); } @@ -123,26 +168,26 @@ impl Polygon { } (Some(Hit::UpperVertex), Some(Hit::LowerVertex)) | (Some(Hit::LowerVertex), Some(Hit::UpperVertex)) => { - // If we're hitting a vertex, only count it if we've hit the - // other kind of vertex right before. + // If we're hitting a vertex, only count it if we've hit + // the other kind of vertex right before. // - // That means, we're passing through the polygon boundary - // at where two edges touch. Depending on the order in which - // edges are checked, we're seeing this as a hit to one - // edge's lower/upper vertex, then the other edge's opposite - // vertex. + // That means, we're passing through the polygon + // boundary at where two edges touch. Depending on the + // order in which edges are checked, we're seeing this + // as a hit to one edge's lower/upper vertex, then the + // other edge's opposite vertex. // - // If we're seeing two of the same vertices in a row, we're - // not actually passing through the polygon boundary. Then - // we're just touching a vertex without passing through - // anything. + // If we're seeing two of the same vertices in a row, + // we're not actually passing through the polygon + // boundary. Then we're just touching a vertex without + // passing through anything. true } (Some(Hit::Parallel), _) => { - // A parallel edge must be completely ignored. Its presence - // won't change anything, so we can treat it as if it - // wasn't there, and its neighbors were connected to each - // other. + // A parallel edge must be completely ignored. Its + // presence won't change anything, so we can treat it as + // if it wasn't there, and its neighbors were connected + // to each other. continue; } _ => { @@ -180,6 +225,23 @@ mod tests { use super::Polygon; + #[test] + fn contains_triangle_with_triangular_hole() { + let a = [0., 0.]; + let b = [3., 0.]; + let c = [0., 3.]; + + let d = [1., 1.]; + let e = [2., 1.]; + let f = [1., 2.]; + + let polygon = Polygon::new(Surface::x_y_plane()) + .with_exterior(PolyChain::from([a, b, c]).close()) + .with_interiors([PolyChain::from([d, e, f]).close()]); + + assert!(!polygon.contains_triangle([d, e, f], &mut DebugInfo::new())); + } + #[test] fn contains_point_ray_hits_vertex_while_passing_outside() { let a = [0., 0.];