Invalid convex_hull when points are repeated

[asinghvi17]

Consider the following MWE:

using CairoMakie, DelaunayTriangulation
data = rand(Point2f, 10)
Makie.convert_arguments(::Makie.PointBased, ch::DelaunayTriangulation.ConvexHull) = (Makie.Point.(view(ch.points, ch.vertices)),) # just to make plotting easier

f, a1, p1 = lines(convex_hull(data); axis = (; title = "points")) # this is fine
a2, p2 = lines(f[1, 2], convex_hull(vcat(data, data)); axis = (; title = "vcat(points, points)")) # this is NOT fine

[asinghvi17]

For context, I was looking at depending on DelaunayTriangulation.jl in GeometryOps for some triangulation tasks. All geographic polygons must have polygon[begin] == polygon[end], so this comes up quite often when taking the convex hull of a set of polygons.

[DanielVandH]

Hm. I didn't really intend to support duplicate points anywhere - in fact, I thought this function also would throw a DuplicatePointsError like triangulate does. I'm open to supporting it though. Can you check if it works if you add

unique!(i -> get_point(points, i), insertion_order)

in convex_hull! after insertion_order = lexicographic_order(points)?

[asinghvi17]

Yep, that works! I can perform that check locally, but it would make sense to place that here as well. It could also be controlled by a keyword argument if that's desired for performance.

[DanielVandH]

Probably don't need to bother with a keyword, I don't think it'll slow things down too much. If you're able to, would you mind making a PR with that change in the convex_hull! function? A simple test that convex_hull(points) == convex_hull(vcat(points, points)) (or something like that, not sure if == works on ConvexHull or if I never implemented it yet) would probably suffice.

[asinghvi17]

Will get a PR up by tomorrow!

[DanielVandH]

Is this something you still need @asinghvi17?

[asinghvi17]

Yes! Sorry, I lost track of this and forgot to push the PR.

[DanielVandH]

No worries, there's no rush. I just happened to remember about it today so thought I'd check in. Thanks.

[DanielVandH]

Should now be fixed once v1.0.4 is released