Fix regressions (#3104)

* fix #3096

* fix #3060

* improve performance

* try to fix case mentioned in #3100, continuation of #2817

* fix #3109

* fix scene tutorial

* fix unit tests

CairoMakie/src/overrides.jl

@@ -65,7 +65,7 @@ function draw_poly(scene::Scene, screen::Screen, poly, points_list::Vector{<:Vec
     color = to_cairo_color(poly.color[], poly)
     strokecolor = to_cairo_color(poly.strokecolor[], poly)
     strokestyle = Makie.convert_attribute(poly.linestyle[], key"linestyle"())
-    
+
     broadcast_foreach(points_list, color,
         strokecolor, strokestyle, poly.strokewidth[], Ref(poly.model[])) do points, color, strokecolor, strokestyle, strokewidth, model
             draw_poly(scene, screen, poly, points, color, model, strokecolor, strokestyle, strokewidth)
@@ -81,16 +81,21 @@ function draw_poly(scene::Scene, screen::Screen, poly, rects::Vector{<:Rect2})
 
     color = to_cairo_color(poly.color[], poly)
 
-    strokestyle = Makie.convert_attribute(poly.linestyle[], key"linestyle"())
-
+    linestyle = Makie.convert_attribute(poly.linestyle[], key"linestyle"())
+    if isnothing(linestyle)
+        linestyle_diffed = nothing
+    elseif linestyle isa AbstractVector{Float64}
+        linestyle_diffed = diff(Float64.(linestyle))
+    else
+        error("Wrong type for linestyle: $(poly.linestyle[]).")
+    end
     strokecolor = to_cairo_color(poly.strokecolor[], poly)
-
-    broadcast_foreach(projected_rects, color, strokecolor, strokestyle, poly.strokewidth[]) do r, c, sc, ss, sw
+    broadcast_foreach(projected_rects, color, strokecolor, poly.strokewidth[]) do r, c, sc, sw
         Cairo.rectangle(screen.context, origin(r)..., widths(r)...)
         set_source(screen.context, c)
         Cairo.fill_preserve(screen.context)
+        isnothing(linestyle_diffed) || Cairo.set_dash(screen.context, linestyle_diffed .* sw)
         set_source(screen.context, sc)
-        isnothing(ss) || Cairo.set_dash(screen.context, diff(Float64.(ss)) .* sw)
         Cairo.set_line_width(screen.context, sw)
         Cairo.stroke(screen.context)
     end

docs/tutorials/scenes.md

@@ -90,7 +90,7 @@ We can fix this by translating the scene further back:
 \begin{examplefigure}{}
 ```julia
 GLMakie.activate!() # hide
-translate!(scene.plots[1], 0, 0, -1000)
+translate!(scene.plots[1], 0, 0, -10000)
 scene
 ```
 \end{examplefigure}

src/basic_recipes/contours.jl

@@ -183,6 +183,24 @@ function color_per_level(::Nothing, colormap, colorscale, colorrange, a, levels)
     end
 end
 
+
+function contourlines(x, y, z::AbstractMatrix{ET}, levels, level_colors, labels, T) where {ET}
+    # Compute contours
+    xv, yv = to_vector(x, size(z, 1), ET), to_vector(y, size(z, 2), ET)
+    contours = Contours.contours(xv, yv, z, convert(Vector{ET}, levels))
+    return contourlines(T, contours, level_colors, labels)
+end
+
+function has_changed(old_args, new_args)
+    length(old_args) === length(new_args) || return true
+    for (old, new) in zip(old_args, new_args)
+        if old != new
+            return true
+        end
+    end
+    return false
+end
+
 function plot!(plot::T) where T <: Union{Contour, Contour3d}
     x, y, z = plot[1:3]
     zrange = lift(nan_extrema, plot, z)
@@ -201,12 +219,19 @@ function plot!(plot::T) where T <: Union{Contour, Contour3d}
     @extract plot (labels, labelsize, labelfont, labelcolor, labelformatter)
     args = @extract plot (color, colormap, colorscale, colorrange, alpha)
     level_colors = lift(color_per_level, plot, args..., levels)
-    cont_lines = lift(plot, x, y, z, levels, level_colors, labels) do x, y, z, levels, level_colors, labels
-        t = eltype(z)
-        # Compute contours
-        xv, yv = to_vector(x, size(z, 1), t), to_vector(y, size(z, 2), t)
-        contours = Contours.contours(xv, yv, z,  convert(Vector{t}, levels))
-        contourlines(T, contours, level_colors, labels)
+    args = (x, y, z, levels, level_colors, labels)
+    arg_values = map(to_value, args)
+    old_values = map(copy, arg_values)
+    points, colors, lev_pos_col = Observable.(contourlines(arg_values..., T); ignore_equal_values=true)
+    onany(plot, args...) do args...
+        # contourlines is expensive enough, that it's worth to copy & check against old values
+        # We need to copy, since the values may get mutated in place
+        if has_changed(old_values, args)
+            old_values = map(copy, args)
+            _points, _colors, _lev_pos_col = contourlines(args..., T)
+            points[] = _points; colors[] = _colors; lev_pos_col[] = _lev_pos_col
+            return
+        end
     end
 
     P = T <: Contour ? Point2f : Point3f
@@ -224,8 +249,8 @@ function plot!(plot::T) where T <: Union{Contour, Contour3d}
         font = labelfont,
     )
 
-    lift(scene.camera.projectionview, scene.px_area, labels, labelcolor, labelformatter, cont_lines) do _, _,
-            labels, labelcolor, labelformatter, (_, _, lev_pos_col)
+    lift(scene.camera.projectionview, scene.px_area, labels, labelcolor, labelformatter,
+         lev_pos_col) do _, _, labels, labelcolor, labelformatter, lev_pos_col
         labels || return
         pos = texts.positions.val; empty!(pos)
         rot = texts.rotation.val; empty!(rot)
@@ -246,19 +271,18 @@ function plot!(plot::T) where T <: Union{Contour, Contour3d}
             push!(pos, p1)
         end
         notify(texts.text)
-        nothing
+        return
     end
 
-    bboxes = lift(labels, texts.text) do labels, _
+    bboxes = lift(labels, texts.text; ignore_equal_values=true) do labels, _
         labels || return
-        broadcast(texts.plots[1][1].val, texts.positions.val, texts.rotation.val) do gc, pt, rot
+        return broadcast(texts.plots[1][1].val, texts.positions.val, texts.rotation.val) do gc, pt, rot
             # drop the depth component of the bounding box for 3D
             Rect2f(boundingbox(gc, project(scene.camera, space, :pixel, pt), to_rotation(rot)))
         end
     end
 
-    masked_lines = lift(labels, bboxes) do labels, bboxes
-        segments = cont_lines.val[1]
+    masked_lines = lift(labels, bboxes, points) do labels, bboxes, segments
         labels || return segments
         n = 1
         bb = bboxes[n]
@@ -287,7 +311,7 @@ function plot!(plot::T) where T <: Union{Contour, Contour3d}
 
     lines!(
         plot, masked_lines;
-        color = lift(x -> x[2], plot, cont_lines),
+        color = colors,
         linewidth = plot.linewidth,
         inspectable = plot.inspectable,
         transparency = plot.transparency,

src/colorsampler.jl

@@ -236,7 +236,7 @@ function assemble_colors(::T, @nospecialize(color), @nospecialize(plot)) where {
     colorrange_scaled = lift(colorrange, colorscale; ignore_equal_values=true) do range, scale
         return Vec2f(apply_scale(scale, range))
     end
-    color_scaled = lift(color_tight, colorscale; ignore_equal_values=true) do color, scale
+    color_scaled = lift(color_tight, colorscale) do color, scale
         return el32convert(apply_scale(scale, color))
     end
     return ColorMap{N, T, typeof(color_scaled[])}(

src/figures.jl

@@ -100,11 +100,11 @@ function Figure(; kwargs...)
     padding = pop!(kwargs_dict, :figure_padding, theme(:figure_padding))
     scene = Scene(; camera=campixel!, kwargs_dict...)
     padding = convert(Observable{Any}, padding)
-    alignmode = lift(Outside ∘ to_rectsides, scene, padding)
+    alignmode = lift(Outside ∘ to_rectsides, padding)
 
     layout = GridLayout(scene)
 
-    on(scene, alignmode) do al
+    on(alignmode) do al
         layout.alignmode[] = al
         GridLayoutBase.update!(layout)
     end

src/makielayout/helpers.jl

@@ -137,7 +137,9 @@ function tightlimits!(la::Axis, ::Top)
     autolimits!(la)
 end
 
-GridLayoutBase.GridLayout(scene::Scene, args...; kwargs...) = GridLayout(args...; bbox = lift(x -> Rect2f(x), scene, pixelarea(scene)), kwargs...)
+function GridLayoutBase.GridLayout(scene::Scene, args...; kwargs...)
+    return GridLayout(args...; bbox=lift(Rect2f, pixelarea(scene)), kwargs...)
+end
 
 function axislines!(scene, rect, spinewidth, topspinevisible, rightspinevisible,
     leftspinevisible, bottomspinevisible, topspinecolor, leftspinecolor,

src/scenes.jl

@@ -150,7 +150,7 @@ mutable struct Scene <: AbstractScene
             lights,
             Observables.ObserverFunction[]
         )
-        finalizer(empty!, scene)
+        finalizer(free, scene)
         return scene
     end
 end
@@ -261,7 +261,7 @@ function Scene(;
     camera isa Function && camera(scene)
 
     if wasnothing
-        on(scene, events.window_area, priority = typemax(Int)) do w_area
+        on(events.window_area, priority = typemax(Int)) do w_area
             if !any(x -> x ≈ 0.0, widths(w_area)) && px_area[] != w_area
                 px_area[] = w_area
             end
@@ -332,11 +332,11 @@ function Scene(
     )
     if isnothing(px_area)
         map!(identity, child, child_px_area, parent.px_area)
-    elseif !(px_area isa Observable) # observables are assumed to be already corrected against the parent to avoid double updates
-        a = Rect2i(px_area)
-        on(child, pixelarea(parent)) do p
-            # make coordinates relative to parent
-            return Rect2i(minimum(p) .+ minimum(a), widths(a))
+    elseif px_area isa Rect2
+        child_px_area[] = Rect2i(px_area)
+    else
+        if !(px_area isa Observable)
+            error("px_area must be an Observable{Rect2} or a Rect2")
         end
     end
     push!(parent.children, child)
@@ -390,7 +390,7 @@ struct OldAxis end
 zero_origin(area) = Recti(0, 0, widths(area))
 
 function child(scene::Scene; camera, attributes...)
-    return Scene(scene, lift(zero_origin, pixelarea(scene)); camera=camera, attributes...)
+    return Scene(scene; camera=camera, attributes...)
 end
 
 """
@@ -421,32 +421,40 @@ function delete_scene!(scene::Scene)
     return nothing
 end
 
-function Base.empty!(scene::Scene)
+function free(scene::Scene)
+    empty!(scene; free=true)
+    for field in [:backgroundcolor, :px_area, :visible]
+        Observables.clear(getfield(scene, field))
+    end
+    for screen in copy(scene.current_screens)
+        delete!(screen, scene)
+    end
+    empty!(scene.current_screens)
+    scene.parent = nothing
+    return
+end
+
+function Base.empty!(scene::Scene; free=false)
     foreach(empty!, copy(scene.children))
     # clear plots of this scene
     for plot in copy(scene.plots)
         delete!(scene, plot)
     end
-    for screen in copy(scene.current_screens)
-        delete!(screen, scene)
-    end
+
     # clear all child scenes
     if !isnothing(scene.parent)
         filter!(x-> x !== scene, scene.parent.children)
     end
-    scene.parent = nothing
 
-    empty!(scene.current_screens)
     empty!(scene.children)
     empty!(scene.plots)
     empty!(scene.theme)
-    merge_without_obs!(scene.theme, CURRENT_DEFAULT_THEME)
+    # conditional, since in free we dont want this!
+    free || merge_without_obs!(scene.theme, CURRENT_DEFAULT_THEME)
+
     disconnect!(scene.camera)
     scene.camera_controls = EmptyCamera()
 
-    for field in [:backgroundcolor, :px_area, :visible]
-        Observables.clear(getfield(scene, field))
-    end
     for fieldname in (:rotation, :translation, :scale, :transform_func, :model)
         Observables.clear(getfield(scene.transformation, fieldname))
     end