Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CairoMakie: polygon with hole example not working #2456

Open
fatteneder opened this issue Nov 28, 2022 · 11 comments · May be fixed by #2458
Open

CairoMakie: polygon with hole example not working #2456

fatteneder opened this issue Nov 28, 2022 · 11 comments · May be fixed by #2458
Labels
CairoMakie This relates to CairoMakie, the vector backend for Makie based on Cairo. documentation GeometryBasics rendering typically backend specific

Comments

@fatteneder
Copy link
Contributor

See the 2nd example at the docs page: https://docs.makie.org/v0.18.3/examples/plotting_functions/poly/index.html

Expected output:
image

@jaakkor2
Copy link
Contributor

jaakkor2 commented Nov 30, 2022

This polygon works in v0.18.3, i.e., with reversed winding of the hole

p = Polygon(
    Point2f[(0, 0), (2, 0), (3, 1), (1, 1)],
    [Point2f[(0.75, 0.25), (1.25, 0.75), (2.25, 0.75), (1.75, 0.25)]]
)

@jkrumbiegel
Copy link
Member

Yes exactly, it's just that I didn't notice before that the winding order was incorrect because earcut doesn't care, on the mesh conversion route.

We could check winding order and error or flip the polygons in case it's wrong, not sure what the better option is.

@jaakkor2
Copy link
Contributor

Checking the winding order is not trivial

p = Polygon(
    Point2f[(0, 0), (5, 0), (5, 5), (0, 5)],
    [Point2f[(1, 1), (1, 4), (4, 4), (4, 1)],
     Point2f[(2, 2), (3, 2), (3, 3), (2, 3)]]
)

should produce (as it does on CairoMakie + Makie 0.18.3)
image

@fatteneder
Copy link
Contributor Author

fatteneder commented Nov 30, 2022

Checking the winding order is not trivial
p = Polygon(
Point2f[(0, 0), (5, 0), (5, 5), (0, 5)],
[Point2f[(1, 1), (1, 4), (4, 4), (4, 1)],
Point2f[(2, 2), (3, 2), (3, 3), (2, 3)]]
)

To me this doesn't look like a correct usage of poly!.
Also if you try to add another rectangular cutout inside the small rectangle you would have to type (not tested):

p = Polygon(
    Point2f[(0, 0), (5, 0), (5, 5), (0, 5)],
    [Point2f[(1, 1), (1, 4), (4, 4), (4, 1)],
     Point2f[(2, 2), (3, 2), (3, 3), (2, 3)],
     Point2f[(2.2,2.2), (2.2,2.8), (2.8,2.8), (2.8,2.2)]]
)
poly!(ax, p)

Whereas there is a (logically) much simpler way of doing things:

p1 = Polygon(
    Point2f[(0, 0), (5, 0), (5, 5), (0, 5)],
    [Point2f[(1, 1), (1, 4), (4, 4), (4, 1)]]
)
poly!(ax, p1)
p2 = Polygon(
    Point2f[(2, 2), (3, 2), (3, 3), (2, 3)],
    [Point2f[(2.2,2.2), (2.2,2.8), (2.8,2.8), (2.8,2.2)]]
)
poly!(ax, p2)

@fatteneder
Copy link
Contributor Author

Yes exactly, it's just that I didn't notice before that the winding order was incorrect because earcut doesn't care, on the mesh conversion route.

We could check winding order and error or flip the polygons in case it's wrong, not sure what the better option is.

Is there anything mentioned about winding order in the docs?
But even if there were, the API still seems not intuitive to me.
E.g. there are only two fields of Polygon which are called exterior and interiors (singular and plural). As it is coded in CairoMakie, I think it is always assumed that cutouts are the ones in interiors. After all, you cannot even nest a Polygon inside the interiors, or can you?

To me the signature poly(exterior, interiors) suggests that this should draw a polygon that is build from the shape given by exterior and every shape given in interiors is then subtracted from that.

@fatteneder
Copy link
Contributor Author

To me the signature poly(exterior, interiors) suggests that this should draw a polygon that is build from the shape given by exterior and every shape given in interiors is then subtracted from that.

If we were to define its usage like that, then there would be no confusion about winding order.

@jkrumbiegel
Copy link
Member

jkrumbiegel commented Nov 30, 2022

The reason the nested polygon works in CairoMakie, is that the outlines are just drawn as a multi-segment path, and then filling is determined via the even-odd rule. But I would also argue that this is not canonical polygon behavior. I concur with Florian's opinion that we are talking about one exterior and multiple interior holes.

In order to match CairoMakie and GLMakie we should just clearly define somewhere what data we expect, so far it has been to loosely defined. We can probably use whatever definitions the leading GIS packages use, as they probably have the most experience with polygons.

@jaakkor2
Copy link
Contributor

Sorry for my confusion, I agree my example is malformed

p = Polygon(
    Point2f[(0, 0), (5, 0), (5, 5), (0, 5)],
    [Point2f[(1, 1), (1, 4), (4, 4), (4, 1)],
     Point2f[(2, 2), (3, 2), (3, 3), (2, 3)]]
)

@jaakkor2
Copy link
Contributor

The reason the nested polygon works in CairoMakie, is that the outlines are just drawn as a multi-segment path, and then filling is determined via the even-odd rule.

The fill rule seems to be nonzero

using CairoMakie
using Makie.GeometryBasics

f = Figure()
Axis(f[1, 1])

# polygon with hole
p = Polygon(
    Point2f[(0, 0), (2, 0), (3, 1), (1, 1)],
    [Point2f[(0.75, 0.25), (1.75, 0.25), (2.25, 0.75), (1.25, 0.75)]]
)

poly!(p, color = :blue)

fn = "cm.svg"
save(fn, f)
collect(eachmatch(r"fill-rule:(.*?);", read(fn, String)))

gives

2-element Vector{RegexMatch}:
 RegexMatch("fill-rule:nonzero;", 1="nonzero")
 RegexMatch("fill-rule:nonzero;", 1="nonzero")

With evenodd fill rule, holes seem to work regardless of winding.

@ffreyer
Copy link
Collaborator

ffreyer commented Aug 23, 2024

The docs have been fixed and I think the Polygon discussion is something for GeometryBasics instead... Can this be closed?

Ref JuliaGeometry/GeometryBasics.jl#173

@asinghvi17
Copy link
Member

asinghvi17 commented Aug 23, 2024

IMO this is still something we have to figure out in CairoMakie - whether it should respect this kind of "malformed polygon" or somehow try and correct it before plotting (either assert winding order for the rings, or change the fill rule).

The correct GIS thing in this case would be a MultiPolygon that can contain the inner filled rectangle as the second polygon.

@ffreyer ffreyer added CairoMakie This relates to CairoMakie, the vector backend for Makie based on Cairo. rendering typically backend specific labels Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CairoMakie This relates to CairoMakie, the vector backend for Makie based on Cairo. documentation GeometryBasics rendering typically backend specific
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

5 participants