reverse interior for Cairo hole drawing

[SimonDanisch]

GeometryBasics.Polygon doesn't care about winding order for holes and so doesn't GLMakie.
But Cairo needs winding directions for holes to be counter-clockwise, so this PR makes sure interiors are always drawn counter-clockwise.
Funnily enough, we have a test for this, which has been "failing" for cairomakie without going above test failure threshold -.-

Test before PR:

With PR:

[asinghvi17]

Should we make this change in GeometryBasics? We could also change the fill rule. I think, which might achieve the same result.

Geometries from other packages are reversed in order, so this might cause complications there. I'll PR some tests from ArchGDAL geometries to check this.

[jkrumbiegel]

the holes don't need to be counter-clockwise I think, they need to be opposite to the direction of the exterior. unless you change the fill rule, I don't remember the edge cases right now

[SimonDanisch]

I really don't like having this restriction in GeometryBasics.
It seems like a leaky abstraction, stemming from old drawing framework that needed this to actually draw the polygon correctly, while its not really required for most other algorithms.

Anyways, would be nice if someone with more CairoMakie knowledge could make the correct fix :)
If there's a drawing rule for holes, that'd be ideal?

[jkrumbiegel]

yeah no it shouldn't be in geometrybasics, just cairomakie should make sure to draw it correctly. is it reasonably fast to check that property so that drawing doesn't slow down so much?

[MakieBot]

Compile Times benchmark

Note, that these numbers may fluctuate on the CI servers, so take them with a grain of salt. All benchmark results are based on the mean time and negative percent mean faster than the base branch. Note, that GLMakie + WGLMakie run on an emulated GPU, so the runtime benchmark is much slower. Results are from running:

using_time = @ctime using Backend
# Compile time
create_time = @ctime fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @ctime Makie.colorbuffer(display(fig))
# Runtime
create_time = @benchmark fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @benchmark Makie.colorbuffer(display(fig))

	using	create	display	create	display
GLMakie	41.58s (39.10, 48.09) 3.80+-	18.76s (17.76, 21.27) 1.58+-	17.24s (16.30, 19.37) 1.42+-	15.27ms (14.65, 16.52) 0.78+-	167.20ms (160.33, 182.58) 9.85+-
master	41.03s (39.19, 46.05) 2.48+-	18.71s (17.69, 20.83) 1.42+-	17.24s (16.33, 19.52) 1.46+-	15.39ms (14.76, 16.83) 0.91+-	167.71ms (159.00, 186.60) 10.35+-
evaluation	+1.34%, 0.56s invariant (0.17d, 0.75p, 3.14std)	+0.26%, 0.05s invariant (0.03d, 0.95p, 1.50std)	-0.03%, -0.0s invariant (-0.00d, 1.00p, 1.44std)	-0.75%, -0.12ms invariant (-0.14d, 0.80p, 0.85std)	-0.31%, -0.51ms invariant (-0.05d, 0.93p, 10.10std)
CairoMakie	33.51s (33.28, 33.67) 0.16+-	17.74s (17.50, 17.97) 0.14+-	2.61s (2.56, 2.64) 0.03+-	10.07ms (9.96, 10.15) 0.07+-	4.86ms (4.68, 5.02) 0.13+-
master	33.55s (33.32, 33.76) 0.15+-	17.69s (17.57, 18.07) 0.17+-	2.60s (2.58, 2.62) 0.02+-	9.98ms (9.84, 10.07) 0.07+-	4.84ms (4.74, 4.93) 0.07+-
evaluation	-0.12%, -0.04s invariant (-0.26d, 0.63p, 0.15std)	+0.32%, 0.06s invariant (0.36d, 0.51p, 0.16std)	+0.40%, 0.01s invariant (0.46d, 0.41p, 0.02std)	+0.98%, 0.1ms slower X (1.45d, 0.02p, 0.07std)	+0.44%, 0.02ms invariant (0.20d, 0.72p, 0.10std)
WGLMakie	52.71s (51.59, 53.99) 0.79+-	26.34s (25.76, 26.87) 0.46+-	26.45s (25.62, 27.05) 0.50+-	18.16ms (17.70, 18.61) 0.31+-	950.63ms (917.08, 968.80) 17.27+-
master	53.49s (52.61, 54.60) 0.70+-	26.16s (25.36, 26.88) 0.54+-	26.74s (26.10, 27.19) 0.36+-	18.00ms (17.46, 20.31) 1.03+-	965.87ms (928.59, 999.55) 27.61+-
evaluation	-1.47%, -0.78s invariant (-1.04d, 0.08p, 0.74std)	+0.69%, 0.18s invariant (0.36d, 0.51p, 0.50std)	-1.10%, -0.29s invariant (-0.67d, 0.24p, 0.43std)	+0.89%, 0.16ms invariant (0.21d, 0.70p, 0.67std)	-1.60%, -15.24ms invariant (-0.66d, 0.24p, 22.44std)

[jkrumbiegel]

according to this it would need to be evenodd https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/fill-rule and is probably nonzero right now

[SimonDanisch]

Yeah, it's basically just a sum of neighboring cross products, so reasonably cheap ;)
So for a polygon with 10^7 points it takes ~0.008s
Also I removed the allocations from interior[2:end] which should almost make up for it^^

[jkrumbiegel]

yeah it's CAIRO_FILL_RULE_EVEN_ODD https://www.cairographics.org/manual/cairo-cairo-t.html#CAIRO-FILL-RULE-WINDING:CAPS

cairo_set_fill_rule (cairo_t *cr,
                     cairo_fill_rule_t fill_rule);

This would just need to be called before drawing a polygon, then the order doesn't matter anymore.

[jkrumbiegel]

Just to add, it wouldn't matter anymore for "normal" polygons. Where edges don't weirdly intersect, like in the svg example with the star. But I assume the polygons we care about are not weird, I think geo packages try not to generate these strange shapes but instead split into multiple polys if needed.