Fully flatten plots and sort by atomic zorder in all backends

CairoMakie/src/infrastructure.jl

@@ -11,7 +11,7 @@ function cairo_draw(screen::Screen, scene::Scene)
     Cairo.save(screen.context)
     draw_background(screen, scene)
 
-    allplots = get_all_plots(scene)
+    allplots = Makie.collect_atomic_plots(scene; is_atomic_plot = is_cairomakie_atomic_plot)
     zvals = Makie.zvalue2d.(allplots)
     permute!(allplots, sortperm(zvals))
 
@@ -23,10 +23,12 @@ function cairo_draw(screen::Screen, scene::Scene)
 
     Cairo.save(screen.context)
     for p in allplots
-        to_value(get(p, :visible, true)) || continue
+        check_parent_plots(p) do plot
+            to_value(get(plot, :visible, true))
+        end || continue
         # only prepare for scene when it changes
         # this should reduce the number of unnecessary clipping masks etc.
-        pparent = p.parent::Scene
+        pparent = Makie.parent_scene(p)
         pparent.visible[] || continue
         if pparent != last_scene
             Cairo.restore(screen.context)
@@ -36,8 +38,8 @@ function cairo_draw(screen::Screen, scene::Scene)
         end
         Cairo.save(screen.context)
 
-        # When a plot is too large to save with a reasonable file size on a vector backend, 
-        # the user can choose to rasterize it when plotting to vector backends, by using the 
+        # When a plot is too large to save with a reasonable file size on a vector backend,
+        # the user can choose to rasterize it when plotting to vector backends, by using the
         # `rasterize` keyword argument.  This can be set to a Bool or an Int which describes
         # the density of rasterization (in terms of a direct scaling factor.)
         # TODO: In future, this can also be set to a Tuple{Module, Int} which describes
@@ -54,12 +56,33 @@ function cairo_draw(screen::Screen, scene::Scene)
     return
 end
 
-function get_all_plots(scene, plots = AbstractPlot[])
-    append!(plots, scene.plots)
-    for c in scene.children
-        get_all_plots(c, plots)
+"""
+    is_cairomakie_atomic_plot(plot::Combined)::Bool
+
+Returns whether the plot is considered atomic for the CairoMakie backend.
+This is overridden for `Poly`, `Band`, and `Tricontourf` so we can apply
+CairoMakie can treat them as atomic plots and render them directly.
+
+Plots with children are by default recursed into.  This can be overridden
+by defining specific dispatches for `is_cairomakie_atomic_plot` for a given plot type.
+"""
+is_cairomakie_atomic_plot(plot::Combined) = isempty(plot.plots) || to_value(get(plot, :rasterize, false)) != false
+
+"""
+    check_parent_plots(f, plot::Combined)::Bool
+Returns whether the plot's parent tree satisfies the predicate `f`.
+`f` must return a `Bool` and take a plot as its only argument.
+"""
+function check_parent_plots(f, plot::Combined)
+    if f(plot)
+        check_parent_plots(f, parent(plot))
+    else
+        return false
     end
-    plots
+end
+
+function check_parent_plots(f, scene::Scene)
+    return true
 end
 
 function prepare_for_scene(screen::Screen, scene::Scene)

CairoMakie/src/overrides.jl

@@ -12,6 +12,10 @@ function draw_plot(scene::Scene, screen::Screen, poly::Poly)
     draw_poly(scene, screen, poly, to_value.(poly.input_args)...)
 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.
 """
@@ -182,6 +186,12 @@ function draw_plot(scene::Scene, screen::Screen,
     nothing
 end
 
+# Override `is_cairomakie_atomic_plot` to allow this dispatch of `band` to remain a unit,
+# instead of auto-decomposing in lines and mesh.
+function is_cairomakie_atomic_plot(plot::Band{<:Tuple{<:AbstractVector{<:Point2},<:AbstractVector{<:Point2}}})
+    return true
+end
+
 #################################################################################
 #                                  Tricontourf                                  #
 # Tricontourf creates many disjoint polygons that are adjacent and form contour #
@@ -214,3 +224,9 @@ function draw_plot(scene::Scene, screen::Screen, tric::Tricontourf)
 
     return
 end
+
+# Override `is_cairomakie_atomic_plot` to allow `Tricontourf` to remain a unit,
+# instead of auto-decomposing in lines and mesh.
+function is_cairomakie_atomic_plot(plot::Tricontourf)
+    return true
+end

GLMakie/src/screen.jl

@@ -518,7 +518,7 @@ end
 function Base.delete!(screen::Screen, scene::Scene, plot::AbstractPlot)
     if !isempty(plot.plots)
         # this plot consists of children, so we flatten it and delete the children instead
-        for cplot in Makie.flatten_plots(plot)
+        for cplot in Makie.collect_atomic_plots(plot)
             delete!(screen, scene, cplot)
         end
     else
@@ -919,7 +919,7 @@ function renderloop(screen)
 end
 
 function plot2robjs(screen::Screen, plot)
-    plots = Makie.flatten_plots(plot)
+    plots = Makie.collect_atomic_plots(plot)
     return map(x-> screen.cache[objectid(x)], plots)
 end
 

NEWS.md

@@ -2,6 +2,10 @@
 
 ## master
 
+- Deprecated `flatten_plots` in favor of `collect_atomic_plots`. Using the new `collect_atomic_plots` fixed a bug in CairoMakie where the z-level of plots within recipes was not respected. [#2793](https://github.com/MakieOrg/Makie.jl/pull/2793)
+- Fixed incorrect line depth in GLMakie [#2843](https://github.com/MakieOrg/Makie.jl/pull/2843)
+- Fixed incorrect line alpha in dense lines in GLMakie [#2843](https://github.com/MakieOrg/Makie.jl/pull/2843)
+- Fixed DataInspector interaction with transformations [#3002](https://github.com/MakieOrg/Makie.jl/pull/3002)
 - Added option `WGLMakie.activate!(resize_to_body=true)`, to make plots resize to the VSCode plotpane. Resizes to the HTML body element, so may work outside VSCode [#3044](https://github.com/MakieOrg/Makie.jl/pull/3044), [#3042](https://github.com/MakieOrg/Makie.jl/pull/3042).
 - Fixed DataInspector interaction with transformations [#3002](https://github.com/MakieOrg/Makie.jl/pull/3002).
 - Fix incomplete stroke with some Bezier markers in CairoMakie and blurry strokes in GLMakie [#2961](https://github.com/MakieOrg/Makie.jl/pull/2961)
@@ -11,7 +15,6 @@
 - Updated `bracket` when the screen is resized or transformations change [#3012](https://github.com/MakieOrg/Makie.jl/pull/3012).
 - Added auto-resizing functionality to WGLMakie plot figures [#3042](https://github.com/MakieOrg/Makie.jl/pull/3042)
 
-
 ## v0.19.6
 
 - Fixed broken AA for lines with strongly varying linewidth [#2953](https://github.com/MakieOrg/Makie.jl/pull/2953).

ReferenceTests/src/tests/examples2d.jl

@@ -1019,3 +1019,28 @@ end
     end
     f
 end
+
+@reference_test "Z-translation within a recipe" begin
+    # This is testing whether backends respect the 
+    # z-level of plots within recipes in 2d.
+    # Ideally, the output of this test
+    # would be a blue line with red scatter markers.
+    # However, if a backend does not correctly pick up on translations,
+    # then this will be drawn in the drawing order, and blue
+    # will completely obscure red.
+
+    # It seems like we can't define recipes in `@reference_test` yet,
+    # so we'll have to fake a recipe's structure.
+
+    fig = Figure(resolution = (600, 600))
+    # Create a recipe plot
+    ax, plot_top = heatmap(fig[1, 1], randn(10, 10))
+    # Plot some recipes at the level below the contour
+    scatterlineplot_1 = scatterlines!(plot_top, 1:10, 1:10; linewidth = 20, markersize = 20, color = :red)
+    scatterlineplot_2 = scatterlines!(plot_top, 1:10, 1:10; linewidth = 20, markersize = 30, color = :blue)
+    # Translate the lowest level plots (scatters)
+    translate!(scatterlineplot_1.plots[2], 0, 0, 1)
+    translate!(scatterlineplot_2.plots[2], 0, 0, -1)
+    # Display
+    fig
+end

WGLMakie/src/display.jl

@@ -343,7 +343,7 @@ const DISABLE_JS_FINALZING = Base.RefValue(false)
 const DELETE_QUEUE = LockfreeQueue{Tuple{Screen,String,Vector{String}}}(delete_plot!)
 
 function Base.delete!(screen::Screen, scene::Scene, plot::Combined)
-    atomics = Makie.flatten_plots(plot) # delete all atomics
+    atomics = Makie.collect_atomic_plots(plot) # delete all atomics
     # only queue atomics to actually delete on js
     if !DISABLE_JS_FINALZING[]
         push!(DELETE_QUEUE, (screen, js_uuid(scene), js_uuid.(atomics)))

WGLMakie/src/picking.jl

@@ -17,7 +17,7 @@ function pick_native(screen::Screen, rect::Rect2i)
     if isempty(matrix)
         return empty
     else
-        all_children = Makie.flatten_plots(scene)
+        all_children = Makie.collect_atomic_plots(scene)
         lookup = Dict(Pair.(js_uuid.(all_children), all_children))
         return map(matrix) do (uuid, index)
             !haskey(lookup, uuid) && return (nothing, 0)
@@ -27,7 +27,7 @@ function pick_native(screen::Screen, rect::Rect2i)
 end
 
 function plot_lookup(scene::Scene)
-    all_plots = Makie.flatten_plots(scene)
+    all_plots = Makie.collect_atomic_plots(scene)
     return Dict(Pair.(js_uuid.(all_plots), all_plots))
 end
 
@@ -107,7 +107,7 @@ struct ToolTip
         if isnothing(plots)
             plots = scene.plots
         end
-        all_plots = js_uuid.(filter!(x-> x.inspectable[], Makie.flatten_plots(plots)))
+        all_plots = js_uuid.(filter!(x-> x.inspectable[], Makie.collect_atomic_plots(plots)))
         new(scene, callback, all_plots)
     end
 end

WGLMakie/src/three_plot.jl

@@ -6,7 +6,7 @@ js_uuid(object) = string(objectid(object))
 function all_plots_scenes(scene::Scene; scene_uuids=String[], plot_uuids=String[])
     push!(scene_uuids, js_uuid(scene))
     for plot in scene.plots
-        append!(plot_uuids, (js_uuid(p) for p in Makie.flatten_plots(plot)))
+        append!(plot_uuids, (js_uuid(p) for p in Makie.collect_atomic_plots(plot)))
     end
     for child in scene.children
         all_plots_scenes(child, plot_uuids=plot_uuids, scene_uuids=scene_uuids)
@@ -15,7 +15,7 @@ function all_plots_scenes(scene::Scene; scene_uuids=String[], plot_uuids=String[
 end
 
 function JSServe.print_js_code(io::IO, plot::AbstractPlot, context::JSServe.JSSourceContext)
-    uuids = js_uuid.(Makie.flatten_plots(plot))
+    uuids = js_uuid.(Makie.collect_atomic_plots(plot))
     # This is a bit more complicated then it has to be, since evaljs / on_document_load
     # isn't guaranteed to run after plot initialization in an App... So, if we don't find any plots,
     # we have to check again after inserting new plots

src/interaction/interactive_api.jl

@@ -10,7 +10,7 @@ Returns true if the mouse currently hovers any of `plots`.
 mouseover(x, plots::AbstractPlot...) = mouseover(get_scene(x), plots...)
 function mouseover(scene::Scene, plots::AbstractPlot...)
     p, idx = pick(scene)
-    return p in flatten_plots(plots)
+    return p in collect_atomic_plots(plots)
 end
 
 """
@@ -22,7 +22,7 @@ hovered element
 """
 onpick(f, x, plots::AbstractPlot...; range=1) = onpick(f, get_scene(x), plots..., range = range)
 function onpick(f, scene::Scene, plots::AbstractPlot...; range=1)
-    fplots = flatten_plots(plots)
+    fplots = collect_atomic_plots(plots)
     args = range == 1 ? (scene,) : (scene, range)
     on(events(scene).mouseposition) do mp
         p, idx = pick(args...)
@@ -33,31 +33,6 @@ end
 
 @deprecate mouse_selection pick
 
-function flatten_plots(x::Combined, plots = AbstractPlot[])
-    if isempty(x.plots)
-        # Atomic plot!
-        push!(plots, x)
-    else
-        for elem in x.plots
-            flatten_plots(elem, plots)
-        end
-    end
-    return plots
-end
-
-function flatten_plots(array, plots = AbstractPlot[])
-    for elem in array
-        flatten_plots(elem, plots)
-    end
-    plots
-end
-
-function flatten_plots(scene::Scene, plots = AbstractPlot[])
-    flatten_plots(scene.plots, plots)
-    flatten_plots(scene.children, plots)
-    plots
-end
-
 """
     mouse_in_scene(fig/ax/scene[, priority = 0])
 

src/scenes.jl

@@ -623,3 +623,46 @@ struct FigureAxisPlot
 end
 
 const FigureLike = Union{Scene, Figure, FigureAxisPlot}
+
+"""
+    is_atomic_plot(plot::Combined)
+
+Defines what Makie considers an atomic plot, used in `collect_atomic_plots`.
+Backends may have a different definition of what is considered an atomic plot,
+but instead of overloading this function, they should create their own definition and pass it to `collect_atomic_plots`
+"""
+is_atomic_plot(plot::Combined) = isempty(plot.plots)
+
+"""
+    collect_atomic_plots(scene::Scene, plots = AbstractPlot[]; is_atomic_plot = is_atomic_plot)
+    collect_atomic_plots(x::Combined, plots = AbstractPlot[]; is_atomic_plot = is_atomic_plot)
+
+Collects all plots in the provided `<: ScenePlot` and returns a vector of all plots
+which satisfy `is_atomic_plot`, which defaults to Makie's definition of `Makie.is_atomic_plot`.
+"""
+function collect_atomic_plots(xplot::Combined, plots=AbstractPlot[]; is_atomic_plot=is_atomic_plot)
+    if is_atomic_plot(xplot)
+        # Atomic plot!
+        push!(plots, xplot)
+    else
+        for elem in xplot.plots
+            collect_atomic_plots(elem, plots; is_atomic_plot=is_atomic_plot)
+        end
+    end
+    return plots
+end
+
+function collect_atomic_plots(array, plots=AbstractPlot[]; is_atomic_plot=is_atomic_plot)
+    for elem in array
+        collect_atomic_plots(elem, plots; is_atomic_plot=is_atomic_plot)
+    end
+    plots
+end
+
+function collect_atomic_plots(scene::Scene, plots=AbstractPlot[]; is_atomic_plot=is_atomic_plot)
+    collect_atomic_plots(scene.plots, plots; is_atomic_plot=is_atomic_plot)
+    collect_atomic_plots(scene.children, plots; is_atomic_plot=is_atomic_plot)
+    plots
+end
+
+Base.@deprecate flatten_plots(scenelike) collect_atomic_plots(scenelike)