Use lazy iterator to ensure all rings in create_[a/b]_list are closed

src/methods/clipping/clipping_processor.jl

@@ -38,6 +38,30 @@ PolyNode(node::PolyNode{T};
 # Checks equality of two PolyNodes by backing point value, fractional value, and intersection status
 equals(pn1::PolyNode, pn2::PolyNode) = pn1.point == pn2.point && pn1.inter == pn2.inter && pn1.fracs == pn2.fracs
 
+#= 
+    _lazy_closed_ring_point_enumerator(ring) -> Iterator
+
+This function returns an enumerator over the points of a ring, and adds an extra point
+to the end, if the ring is not closed.
+=#
+function _lazy_closed_ring_point_enumerator(ring)
+    @assert GI.geomtrait(ring) isa GI.AbstractCurveTrait "`ring` must be a curve, got $(GI.trait(ring))"
+    # Check if poly_a is closed.  
+    # NOTE: `1` is defined as the starting index of a ring in GeoInterface.
+    if equals(GI.getpoint(ring, 1), GI.getpoint(ring, GI.npoint(ring)))
+        # If yes, pass the iterator as formed
+        Iterators.enumerate(GI.getpoint(ring))
+    else
+        # If no, pass the iterator as lazy vcat
+        Iterators.enumerate(
+            Iterators.flatten(
+                (GI.getpoint(ring),
+                (GI.getpoint(ring, 1),),)
+            )
+        )
+    end
+end
+
 #=
     _build_ab_list(::Type{T}, poly_a, poly_b, delay_cross_f, delay_bounce_f; exact) ->
         (a_list, b_list, a_idx_list)
@@ -89,7 +113,7 @@ function _build_a_list(::Type{T}, poly_a, poly_b; exact) where T
     n_b_intrs = 0
     # Loop through points of poly_a
     local a_pt1
-    for (i, a_p2) in enumerate(GI.getpoint(poly_a))
+    for (i, a_p2) in _lazy_closed_ring_point_enumerator(poly_a) # basically `enumerate(GI.getpoint(poly_a))` but makes sure `poly_a` is closed
         a_pt2 = (T(GI.x(a_p2)), T(GI.y(a_p2)))
         if i <= 1 || (a_pt1 == a_pt2)  # don't repeat points
             a_pt1 = a_pt2
@@ -102,7 +126,7 @@ function _build_a_list(::Type{T}, poly_a, poly_b; exact) where T
         # Find intersections with edges of poly_b
         local b_pt1
         prev_counter = a_count
-        for (j, b_p2) in enumerate(GI.getpoint(poly_b))
+        for (j, b_p2) in _lazy_closed_ring_point_enumerator(poly_b) # basically `enumerate(GI.getpoint(poly_b))` but makes sure `poly_b` is closed
             b_pt2 = _tuple_point(b_p2, T)
             if j <= 1 || (b_pt1 == b_pt2)  # don't repeat points
                 b_pt1 = b_pt2
@@ -193,7 +217,7 @@ function _build_b_list(::Type{T}, a_idx_list, a_list, n_b_intrs, poly_b) where T
     b_count = 0
     # Loop over points in poly_b and add each point and intersection point
     local b_pt1
-    for (i, b_p2) in enumerate(GI.getpoint(poly_b))
+    for (i, b_p2) in _lazy_closed_ring_point_enumerator(poly_b) # basically `enumerate(GI.getpoint(poly_b))` but makes sure `poly_b` is closed
         b_pt2 = _tuple_point(b_p2, T)
         if i ≤ 1 || (b_pt1 == b_pt2)  # don't repeat points
             b_pt1 = b_pt2

test/methods/clipping/polygon_clipping.jl

@@ -103,6 +103,8 @@ p54 = GI.Polygon([[(2.5, 2.5), (2.5, 7.5), (7.5, 7.5), (7.5, 2.5), (2.5, 2.5)],
 p55 = GI.Polygon([[(5.0, 0.25), (5.0, 5.0), (9.5, 5.0), (9.5, 0.25), (5.0, 0.25)],
     [(6.0, 3.0), (6.0, 4.0), (7.0, 4.0), (7.0, 3.0), (6.0, 3.0)],
     [(7.5, 0.5), (7.5, 2.5), (9.25, 2.5), (9.25, 0.5), (7.5, 0.5)]])
+p56 = GI.Polygon([[(0.0, 0.0), (0.0, 10.0), (10.0, 10.0), (10.0, 0.0)]]) # polygons with unclosed rings
+p57 = GI.Polygon([[(0.0, 0.0), (0.0, 11.0), (11.0, 11.0), (11.0, 0.0)]])
 
 mp1 = GI.MultiPolygon([p1])
 mp2 = GI.MultiPolygon([p2])
@@ -157,7 +159,8 @@ test_pairs = [
     (p34, mp3, "p34", "mp3", "Polygon overlaps with multipolygon, where on of the sub-polygons is equivalent"),
     (mp3, p35, "mp3", "p35", "Mulitpolygon where just one sub-polygon touches other polygon"),
     (p35, mp3, "p35", "mp3", "Polygon that touches just one of the sub-polygons of multipolygon"),
-    (mp4, mp3, "mp4", "mp3", "Two multipolygons, which with two sub-polygons, where some of the sub-polygons intersect and some don't")
+    (mp4, mp3, "mp4", "mp3", "Two multipolygons, which with two sub-polygons, where some of the sub-polygons intersect and some don't"),
+    # (p56, p57, "p56", "p57", "Polygons with unclosed rings (#191)"), # TODO: `difference` doesn't work yet on p56 and p57 in that order (#193)
 ]
 
 const ϵ = 1e-10
@@ -212,3 +215,33 @@ end
 @testset "Intersection" begin test_clipping(GO.intersection, LG.intersection, "intersection") end
 @testset "Union" begin test_clipping(GO.union, LG.union, "union") end
 @testset "Difference" begin test_clipping(GO.difference, LG.difference, "difference") end
+
+
+@testset "Lazy closed ring enumerator" begin
+    # first test an open ring
+    p = GI.Polygon([[(0.0, 0.0), (0.0, 10.0), (10.0, 10.0), (10.0, 0.0)]])
+    cl = collect(GO._lazy_closed_ring_point_enumerator(GI.getring(p, 1)))
+    @test length(cl) == 5
+    @test cl[end][2] == cl[1][2]
+    # then test a closed ring
+    p2 = GI.Polygon([[(0.0, 0.0), (0.0, 10.0), (10.0, 10.0), (10.0, 0.0), (0.0, 0.0)]])
+    cl2 = collect(GO._lazy_closed_ring_point_enumerator(GI.getring(p2, 1)))
+    @test length(cl2) == 5
+    @test cl2[end][2] == cl2[1][2]
+    @test all(cl .== cl2)
+    # TODO: `difference` doesn't work yet on p56 and p57 in that order,
+    # so we do some tests here
+    p_intersection = only(GO.intersection(p56, p57; target = GI.PolygonTrait()))
+    p_union = only(GO.union(p56, p57; target = GI.PolygonTrait()))
+    @test GI.extent(p_intersection) == GI.extent(p56)
+    @test GI.extent(p_union) == GI.extent(p57)
+    @test GO.equals(p_intersection, p56)
+    @test GO.equals(p_union, p57)
+    # Notice how the order of polygons is different here
+    p_diff = only(GO.difference(p57, p56; target = GI.PolygonTrait()))
+    @test GI.extent(p_diff) == GI.extent(p57)
+    p_lg_diff = LG.difference(GO.fix(p57), GO.fix(p56))
+    # The point orders differ so we have to run GO intersection again.
+    # We could test area of difference also like `compare_GO_LG_clipping` does.
+    @test GO.equals(p_diff, only(GO.intersection(p_diff, p_lg_diff; target = GI.PolygonTrait())))
+end