Check converted as well as input args in polygon drawing in CairoMakie

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- Fixed an issue where CairoMakie would unnecessarily rasterize polygons [#3605](https://github.com/MakieOrg/Makie.jl/pull/3605).
 - Added `PointBased` conversion trait to `scatterlines` recipe [#3603](https://github.com/MakieOrg/Makie.jl/pull/3603).
 
 ## [0.20.7] - 2024-02-04

CairoMakie/src/overrides.jl

@@ -8,29 +8,36 @@ complex and slower to draw than standard paths with single color.
 """
 function draw_plot(scene::Scene, screen::Screen, poly::Poly)
     # dispatch on input arguments to poly to use smarter drawing methods than
-    # meshes if possible
-    return draw_poly(scene, screen, poly, to_value.(poly.args)...)
+    # meshes if possible.
+    # however, since recipes exist, we can't explicitly handle all cases here
+    # so, we should also take a look at converted 
+    # First, we check whether a `draw_poly` method exists for the input arguments
+    # before conversion:
+    return if Base.hasmethod(draw_poly, Tuple{Scene, Screen, typeof(poly), typeof.(to_value.(poly.args))...})
+        draw_poly(scene, screen, poly, to_value.(poly.args)...)
+    # If not, we check whether a `draw_poly` method exists for the arguments after conversion
+    # (`plot.converted`).  This allows anything which decomposes to be checked for.
+    elseif Base.hasmethod(draw_poly, Tuple{Scene, Screen, typeof(poly), typeof.(to_value.(poly.converted))...})
+        draw_poly(scene, screen, poly, to_value.(poly.converted)...)
+    # In the worst case, we return to drawing the polygon as a mesh + lines.
+    else
+        draw_poly_as_mesh(scene, screen, poly)
+    end
 end
 
 # Override `is_cairomakie_atomic_plot` to allow `poly` to remain a unit,
 # instead of auto-decomposing in lines and mesh.
 is_cairomakie_atomic_plot(plot::Poly) = true
 
-"""
-Fallback method for args without special treatment.
-"""
-function draw_poly(scene::Scene, screen::Screen, poly, args...)
-    draw_poly_as_mesh(scene, screen, poly)
-end
 
 function draw_poly_as_mesh(scene, screen, poly)
     draw_plot(scene, screen, poly.plots[1])
     draw_plot(scene, screen, poly.plots[2])
 end
 
-
-# in the rare case of per-vertex colors redirect to mesh drawing
-function draw_poly(scene::Scene, screen::Screen, poly, points::Vector{<:Point2}, color::AbstractArray, model, strokecolor, strokewidth)
+# As a general fallback, draw all polys as meshes.
+# This also applies for e.g. per-vertex color.
+function draw_poly(scene::Scene, screen::Screen, poly, points, color, model, strokecolor, strokestyle, strokewidth)
     draw_poly_as_mesh(scene, screen, poly)
 end
 
@@ -150,6 +157,7 @@ function polypath(ctx, polygon)
 end
 
 draw_poly(scene::Scene, screen::Screen, poly, polygon::Polygon) = draw_poly(scene, screen, poly, [polygon])
+draw_poly(scene::Scene, screen::Screen, poly, multipolygon::MultiPolygon) = draw_poly(scene, screen, poly, multipolygon.polygons)
 draw_poly(scene::Scene, screen::Screen, poly, circle::Circle) = draw_poly(scene, screen, poly, decompose(Point2f, circle))
 
 function draw_poly(scene::Scene, screen::Screen, poly, polygons::AbstractArray{<:Polygon})

CairoMakie/test/Project.toml

@@ -1,4 +1,6 @@
 [deps]
+GeoInterface = "cf35fbd7-0cd7-5166-be24-54bfbe79505f"
+GeoInterfaceMakie = "0edc0954-3250-4c18-859d-ec71c1660c08"
 Makie = "ee78f7c6-11fb-53f2-987a-cfe4a2b5a57a"
 ReferenceTests = "d37af2e0-5618-4e00-9939-d430db56ee94"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

CairoMakie/test/rasterization_tests.jl

@@ -6,7 +6,7 @@ function svg_has_image(x)
         save(path, x)
         # this is rough but an easy way to catch rasterization,
         # if an image element is present in the svg
-        return !occursin("<image id=", read(path, String))
+        return occursin("<image id=", read(path, String))
     end
 end
 
@@ -18,13 +18,14 @@ end
     pl = poly!(ax, Makie.GeometryBasics.Polygon(pts))
 
     @testset "Unrasterized SVG" begin
-        @test svg_has_image(fig)
+        @test !svg_has_image(fig)
     end
 
     @testset "Rasterized SVG" begin
         lp.rasterize = true
-        @test !svg_has_image(fig)
+        @test svg_has_image(fig)
         lp.rasterize = 10
-        @test !svg_has_image(fig)
+        @test svg_has_image(fig)
     end
-end
+
+end

CairoMakie/test/svg_tests.jl

@@ -30,6 +30,21 @@ end
     @test svg_isnt_rasterized(poly(BezierPath([
         MoveTo(0.0, 0.0), LineTo(1.0, 0.0), LineTo(1.0, 1.0), CurveTo(1.0, 1.0, 0.5, 1.0, 0.5, 0.5), ClosePath()
     ])))
+    @test !svg_isnt_rasterized(poly(rand(Point2f, 10); color = rand(RGBAf, 10)))
+
+    poly1 = Makie.GeometryBasics.Polygon(rand(Point2f, 10))
+    @test svg_isnt_rasterized(poly(Makie.GeometryBasics.MultiPolygon([poly1, poly1])))
+    @test svg_isnt_rasterized(poly(Makie.GeometryBasics.MultiPolygon([poly1, poly1]), color = :red))
+    @test svg_isnt_rasterized(poly(Makie.GeometryBasics.MultiPolygon([poly1, poly1]), color = [:red, :blue]))
+
+    @testset "GeoInterface polygons" begin
+        using GeoInterface, GeoInterfaceMakie
+        poly2 = GeoInterface.convert(GeoInterface.Wrappers, poly1)
+        @test svg_isnt_rasterized(poly(poly2))
+        @test svg_isnt_rasterized(poly(poly2, color = :red))
+        @test svg_isnt_rasterized(poly(GeoInterface.Wrappers.MultiPolygon([poly2, poly2]), color = [:red, :blue]))
+    end
+
 end
 
 @testset "reproducable svg ids" begin