From a5ea5877e81ea034f4e1a78626cd370670660dee Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 11 Apr 2022 16:07:55 +0200 Subject: [PATCH 1/6] Fix formatting Those comments went over the maximum line length. --- .../src/algorithms/triangulation/polygon.rs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/fj-kernel/src/algorithms/triangulation/polygon.rs b/fj-kernel/src/algorithms/triangulation/polygon.rs index 057e8b481..f19c822c4 100644 --- a/fj-kernel/src/algorithms/triangulation/polygon.rs +++ b/fj-kernel/src/algorithms/triangulation/polygon.rs @@ -123,26 +123,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; } _ => { From add9c4f0ef8cd870facff07e6734010aca454d94 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 11 Apr 2022 16:18:15 +0200 Subject: [PATCH 2/6] Add implementation note --- fj-kernel/src/algorithms/triangulation/polygon.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/fj-kernel/src/algorithms/triangulation/polygon.rs b/fj-kernel/src/algorithms/triangulation/polygon.rs index f19c822c4..9ee6baaf2 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, From d275c676bca64e07448e5dbdfaa563d906a531fc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 11 Apr 2022 16:20:51 +0200 Subject: [PATCH 3/6] Simplify function argument --- fj-kernel/src/algorithms/triangulation/mod.rs | 2 +- fj-kernel/src/algorithms/triangulation/polygon.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 9ee6baaf2..9a16def9c 100644 --- a/fj-kernel/src/algorithms/triangulation/polygon.rs +++ b/fj-kernel/src/algorithms/triangulation/polygon.rs @@ -61,7 +61,7 @@ impl Polygon { pub fn contains_triangle( &self, - &[a, b, c]: &[Point<2>; 3], + [a, b, c]: [Point<2>; 3], debug_info: &mut DebugInfo, ) -> bool { for edge in [a, b, c, a].windows(2) { From 7e3bd1c120f564ddf85eeda3c4727f460d4b3d65 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 11 Apr 2022 16:25:13 +0200 Subject: [PATCH 4/6] Split method --- fj-kernel/src/algorithms/triangulation/polygon.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/fj-kernel/src/algorithms/triangulation/polygon.rs b/fj-kernel/src/algorithms/triangulation/polygon.rs index 9a16def9c..3b3fece53 100644 --- a/fj-kernel/src/algorithms/triangulation/polygon.rs +++ b/fj-kernel/src/algorithms/triangulation/polygon.rs @@ -69,9 +69,12 @@ impl Polygon { // cleaned up a bit, once `array_windows` is stable. let edge = Segment::from([edge[0], edge[1]]); + let is_exterior_edge = self.contains_exterior_edge(edge); + let is_interior_edge = self.contains_interior_edge(edge); + // If the segment is an edge of the face, we don't need to take a // closer look. - if self.contains_edge(edge) { + if is_exterior_edge || is_interior_edge { continue; } @@ -89,10 +92,15 @@ impl Polygon { 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()); } From bb618195501ab98a6d51e014789f39afdec5664a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 11 Apr 2022 16:34:40 +0200 Subject: [PATCH 5/6] Update comments --- fj-kernel/src/algorithms/triangulation/polygon.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fj-kernel/src/algorithms/triangulation/polygon.rs b/fj-kernel/src/algorithms/triangulation/polygon.rs index 3b3fece53..37fa9f1aa 100644 --- a/fj-kernel/src/algorithms/triangulation/polygon.rs +++ b/fj-kernel/src/algorithms/triangulation/polygon.rs @@ -72,14 +72,19 @@ impl Polygon { let is_exterior_edge = self.contains_exterior_edge(edge); let is_interior_edge = self.contains_interior_edge(edge); - // If the segment is an edge of the face, we don't need to take a - // closer look. + // 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. From 0b2bc8c1f8efcdcb650ae565a00e0f450f2f63a3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 11 Apr 2022 16:41:46 +0200 Subject: [PATCH 6/6] Fix triangulation edge case Triangular holes in a face were treated as part of the face. --- .../src/algorithms/triangulation/polygon.rs | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/fj-kernel/src/algorithms/triangulation/polygon.rs b/fj-kernel/src/algorithms/triangulation/polygon.rs index 37fa9f1aa..4b5e01bff 100644 --- a/fj-kernel/src/algorithms/triangulation/polygon.rs +++ b/fj-kernel/src/algorithms/triangulation/polygon.rs @@ -61,9 +61,13 @@ 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. @@ -72,6 +76,13 @@ impl Polygon { 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 { @@ -92,6 +103,12 @@ 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 @@ -208,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.];