Fix some errors caused by updating Axis xscale or yscale (#3084)

* bump priority of transfunc inheritance * add priority keyword to map, map! and lift * cleanup dead code * avoid inverse transform * undo transformation priority changes * update barplot before limits * fix error on transform_func update ok we do need this... * avoid Vector to ReinterpretedArray conversion error * add NEWS entry * add simple test * remove extra attributes from passthrough * Update hvspan.jl * Update hvlines.jl --------- Co-authored-by: Simon <sdanisch@protonmail.com>
MakieOrg · Aug 29, 2023 · 5d42257 · 5d42257
1 parent fc95b21
commit 5d42257
Show file tree

Hide file tree

Showing 10 changed files with 84 additions and 44 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -2,6 +2,7 @@
 
 ## master
 
+- Fixed some errors around dynamic changes of `ax.xscale` or `ax.yscale` [#3084](https://github.com/MakieOrg/Makie.jl/pull/3084)
 - Improved Barplot Label Alignment [#3160](https://github.com/MakieOrg/Makie.jl/issues/3160).
 - Fixed regression in determining axis limits [#3179](https://github.com/MakieOrg/Makie.jl/pull/3179)
 - Added a theme `theme_latexfonts` that uses the latex font family as default fonts [#3147](https://github.com/MakieOrg/Makie.jl/pull/3147).

diff --git a/src/basic_recipes/ablines.jl b/src/basic_recipes/ablines.jl
@@ -26,7 +26,6 @@ function Makie.plot!(p::ABLines)
     points = Observable(Point2f[])
 
     onany(p, limits, p[1], p[2]) do lims, intercept, slope
-        inv = inverse_transform(transf)
         empty!(points[])
         f(x) = x * b + a
         broadcast_foreach(intercept, slope) do intercept, slope

diff --git a/src/basic_recipes/barplot.jl b/src/basic_recipes/barplot.jl
@@ -5,15 +5,15 @@ end
 """
     bar_default_fillto(tf, ys, offset)::(ys, offset)
 
-Returns the default y-positions and offset positions for the given transform `tf`.  
+Returns the default y-positions and offset positions for the given transform `tf`.
 
-In order to customize this for your own transformation type, you can dispatch on 
+In order to customize this for your own transformation type, you can dispatch on
 `tf`.
 
 Returns a Tuple of new y positions and offset arrays.
 
 ## Arguments
-- `tf`: `plot.transformation.transform_func[]`. 
+- `tf`: `plot.transformation.transform_func[]`.
 - `ys`: The y-values passed to `barplot`.
 - `offset`: The `offset` parameter passed to `barplot`.
 """
@@ -313,14 +313,14 @@ function Makie.plot!(p::BarPlot)
 
     bars = lift(calculate_bars, p, p[1], p.fillto, p.offset, p.transformation.transform_func, p.width, p.dodge, p.n_dodge, p.gap,
                 p.dodge_gap, p.stack, p.direction, p.bar_labels, p.flip_labels_at,
-                p.label_color, p.color_over_background, p.color_over_bar, p.label_formatter, p.label_offset, p.label_rotation, p.label_align)
-
+                p.label_color, p.color_over_background, p.color_over_bar, p.label_formatter, p.label_offset, p.label_rotation, p.label_align; priority = 1)
     poly!(
         p, bars, color = p.color, colormap = p.colormap, colorscale = p.colorscale, colorrange = p.colorrange,
         strokewidth = p.strokewidth, strokecolor = p.strokecolor, visible = p.visible,
         inspectable = p.inspectable, transparency = p.transparency,
-        highclip = p.highclip, lowclip = p.lowclip, nan_color = p.nan_color, alpha = p.alpha
+        highclip = p.highclip, lowclip = p.lowclip, nan_color = p.nan_color, alpha = p.alpha,
     )
+
     if !isnothing(p.bar_labels[])
         text!(p, labels; align=label_aligns, offset=label_offsets, color=label_colors, font=p.label_font, fontsize=p.label_size, rotation=p.label_rotation)
     end

diff --git a/src/basic_recipes/error_and_rangebars.jl b/src/basic_recipes/error_and_rangebars.jl
@@ -143,11 +143,15 @@ function Makie.plot!(plot::Errorbars{T}) where T <: Tuple{AbstractVector{<:VecTy
     end
 
     linesegpairs = lift(plot, x_y_low_high, is_in_y_direction) do x_y_low_high, in_y
-        return map(x_y_low_high) do (x, y, l, h)
-            in_y ?
-                (Point2f(x, y - l), Point2f(x, y + h)) :
-                (Point2f(x - l, y), Point2f(x + h, y))
+        output = sizehint!(Point2f[], 2length(x_y_low_high))
+        for (x, y, l, h) in x_y_low_high
+            if in_y
+                push!(output, Point2f(x, y - l), Point2f(x, y + h))
+            else
+                push!(output, Point2f(x - l, y), Point2f(x + h, y))
+            end
         end
+        return output
     end
 
     _plot_bars!(plot, linesegpairs, is_in_y_direction)
@@ -169,11 +173,15 @@ function Makie.plot!(plot::Rangebars{T}) where T <: Tuple{AbstractVector{<:VecTy
     end
 
     linesegpairs = lift(plot, val_low_high, is_in_y_direction) do vlh, in_y
-        return map(vlh) do (v, l, h)
-            in_y ?
-                (Point2f(v, l), Point2f(v, h)) :
-                (Point2f(l, v), Point2f(h, v))
+        output = sizehint!(Point2f[], 2length(vlh))
+        for (v, l, h) in vlh
+            if in_y
+                push!(output, Point2f(v, l), Point2f(v, h))
+            else
+                push!(output, Point2f(l, v), Point2f(h, v))
+            end
         end
+        return output
     end
 
     _plot_bars!(plot, linesegpairs, is_in_y_direction)
@@ -190,14 +198,13 @@ function _plot_bars!(plot, linesegpairs, is_in_y_direction)
     scene = parent_scene(plot)
 
     whiskers = lift(plot, linesegpairs, scene.camera.projectionview, plot.model,
-        scene.px_area, transform_func(plot), whiskerwidth) do pairs, _, _, _, _, whiskerwidth
+        scene.px_area, transform_func(plot), whiskerwidth) do endpoints, _, _, _, _, whiskerwidth
 
-        endpoints = [p for pair in pairs for p in pair]
         screenendpoints = plot_to_screen(plot, endpoints)
 
         screenendpoints_shifted_pairs = map(screenendpoints) do sep
             (sep .+ f_if(is_in_y_direction[], reverse, Point(0, -whiskerwidth/2)),
-             sep .+ f_if(is_in_y_direction[], reverse, Point(0,  whiskerwidth/2)))
+            sep .+ f_if(is_in_y_direction[], reverse, Point(0,  whiskerwidth/2)))
         end
 
         return [p for pair in screenendpoints_shifted_pairs for p in pair]

diff --git a/src/basic_recipes/hvlines.jl b/src/basic_recipes/hvlines.jl
@@ -57,23 +57,20 @@ function Makie.plot!(p::Union{HLines, VLines})
     ma = p isa HLines ? p.xmax : p.ymax
 
     onany(p, limits, p[1], mi, ma, transf) do lims, vals, mi, ma, transf
-        inv = inverse_transform(transf)
         empty!(points[])
         min_x, min_y = minimum(lims)
         max_x, max_y = maximum(lims)
         broadcast_foreach(vals, mi, ma) do val, mi, ma
             if p isa HLines
                 x_mi = min_x + (max_x - min_x) * mi
                 x_ma = min_x + (max_x - min_x) * ma
-                x_mi = _apply_x_transform(inv, x_mi)
-                x_ma = _apply_x_transform(inv, x_ma)
+                val = _apply_y_transform(transf, val)
                 push!(points[], Point2f(x_mi, val))
                 push!(points[], Point2f(x_ma, val))
             elseif p isa VLines
                 y_mi = min_y + (max_y - min_y) * mi
                 y_ma = min_y + (max_y - min_y) * ma
-                y_mi = _apply_y_transform(inv, y_mi)
-                y_ma = _apply_y_transform(inv, y_ma)
+                val = _apply_x_transform(transf, val)
                 push!(points[], Point2f(val, y_mi))
                 push!(points[], Point2f(val, y_ma))
             end
@@ -84,7 +81,9 @@ function Makie.plot!(p::Union{HLines, VLines})
     notify(p[1])
 
     line_attributes = copy(p.attributes)
-    delete!.(line_attributes, (:ymin, :ymax, :yautolimits))
-    linesegments!(p, line_attributes, points)
+    foreach(key-> delete!(line_attributes, key), [:ymin, :ymax, :xmin, :xmax, :xautolimits, :yautolimits])
+    # Drop transform_func because we handle it manually
+    T = Transformation(p, transform_func = identity)
+    linesegments!(p, line_attributes, points, transformation = T)
     p
 end
diff --git a/src/basic_recipes/hvspan.jl b/src/basic_recipes/hvspan.jl
@@ -48,24 +48,23 @@ function Makie.plot!(p::Union{HSpan, VSpan})
 
     mi = p isa HSpan ? p.xmin : p.ymin
     ma = p isa HSpan ? p.xmax : p.ymax
-    
+
     onany(limits, p[1], p[2], mi, ma, transf) do lims, lows, highs, mi, ma, transf
-        inv = inverse_transform(transf)
         empty!(rects[])
         min_x, min_y = minimum(lims)
         max_x, max_y = maximum(lims)
         broadcast_foreach(lows, highs, mi, ma) do low, high, mi, ma
             if p isa HSpan
                 x_mi = min_x + (max_x - min_x) * mi
                 x_ma = min_x + (max_x - min_x) * ma
-                x_mi = _apply_x_transform(inv, x_mi)
-                x_ma = _apply_x_transform(inv, x_ma)
+                low  = _apply_y_transform(transf, low)
+                high = _apply_y_transform(transf, high)
                 push!(rects[], Rect2f(Point2f(x_mi, low), Vec2f(x_ma - x_mi, high - low)))
             elseif p isa VSpan
                 y_mi = min_y + (max_y - min_y) * mi
                 y_ma = min_y + (max_y - min_y) * ma
-                y_mi = _apply_y_transform(inv, y_mi)
-                y_ma = _apply_y_transform(inv, y_ma)
+                low  = _apply_x_transform(transf, low)
+                high = _apply_x_transform(transf, high)
                 push!(rects[], Rect2f(Point2f(low, y_mi), Vec2f(high - low, y_ma - y_mi)))
             end
         end
@@ -74,7 +73,12 @@ function Makie.plot!(p::Union{HSpan, VSpan})
 
     notify(p[1])
 
-    poly!(p, rects; p.attributes...)
+    poly_attributes = copy(p.attributes)
+    foreach(x-> delete!(poly_attributes, x), [:ymin, :ymax, :xmin, :xmax, :xautolimits, :yautolimits])
+
+    # we handle transform_func manually
+    T = Transformation(p, transform_func = identity)
+    poly!(p, poly_attributes, rects; transformation = T)
     p
 end
 

diff --git a/src/layouting/transformation.jl b/src/layouting/transformation.jl
@@ -39,7 +39,7 @@ function Transformation(transformable::Transformable;
         scale_o,
         rotation_o,
         model,
-        transform_func
+        convert(Observable{Any}, transform_func)
     )
 
     trans.parent[] = parent_transform

diff --git a/src/makielayout/blocks/axis.jl b/src/makielayout/blocks/axis.jl
@@ -178,12 +178,6 @@ function initialize_block!(ax::Axis; palette = nothing)
 
     ax.cycler = Cycler()
 
-    # the first thing to do when setting a new scale is
-    # resetting the limits because simply through expanding they might be invalid for log
-    onany(blockscene, ax.xscale, ax.yscale) do _, _
-        reset_limits!(ax)
-    end
-
     on(blockscene, targetlimits) do lims
         # this should validate the targetlimits before anything else happens with them
         # so there should be nothing before this lifting `targetlimits`
@@ -247,14 +241,23 @@ function initialize_block!(ax::Axis; palette = nothing)
     translate!(yminorgridlines, 0, 0, -10)
     elements[:yminorgridlines] = yminorgridlines
 
+    # When the transform function (xscale, yscale) of a plot changes we
+    # 1. communicate this change to plots (barplot needs this to make bars
+    #    compatible with the new transform function/scale)
     onany(blockscene, ax.xscale, ax.yscale) do xsc, ysc
         scene.transformation.transform_func[] = (xsc, ysc)
         return
     end
 
+    # 2. Update the limits of the plot
+    onany(blockscene, scene.transformation.transform_func, priority = -1) do _
+        reset_limits!(ax)
+    end
+
     notify(ax.xscale)
 
-    onany(update_axis_camera, camera(scene), scene.transformation.transform_func, finallimits, ax.xreversed, ax.yreversed)
+    # 3. Update the view onto the plot (camera matrices)
+    onany(update_axis_camera, camera(scene), scene.transformation.transform_func, finallimits, ax.xreversed, ax.yreversed, priority = -2)
 
     xaxis_endpoints = lift(blockscene, ax.xaxisposition, scene.px_area;
                            ignore_equal_values=true) do xaxisposition, area

diff --git a/src/scenes.jl b/src/scenes.jl
@@ -169,21 +169,21 @@ function Observables.onany(f, scene::Union{Combined,Scene}, observables...; prio
 end
 
 @inline function Base.map!(@nospecialize(f), scene::Union{Combined,Scene}, result::AbstractObservable, os...;
-                           update::Bool=true)
+                           update::Bool=true, priority = 0)
     # note: the @inline prevents de-specialization due to the splatting
     callback = Observables.MapCallback(f, result, os)
     for o in os
-        o isa AbstractObservable && on(callback, scene, o)
+        o isa AbstractObservable && on(callback, scene, o, priority = priority)
     end
     update && callback(nothing)
     return result
 end
 
 @inline function Base.map(f::F, scene::Union{Combined,Scene}, arg1::AbstractObservable, args...;
-                          ignore_equal_values=false) where {F}
+                          ignore_equal_values=false, priority = 0) where {F}
     # note: the @inline prevents de-specialization due to the splatting
     obs = Observable(f(arg1[], map(Observables.to_value, args)...); ignore_equal_values=ignore_equal_values)
-    map!(f, scene, obs, arg1, args...; update=false)
+    map!(f, scene, obs, arg1, args...; update=false, priority = priority)
     return obs
 end
 

diff --git a/test/makielayout.jl b/test/makielayout.jl
@@ -376,3 +376,30 @@ end
     @test_nowarn axislegend()
     f
 end
+
+
+@testset "Axis scale" begin
+    # This just shouldn't error
+    try
+        fig, ax, li = lines(1:10, 1:10)
+        vlines!(ax, 3)
+        hlines!(ax, 3)
+        bp = barplot!(ax, 1 .+ 5 .* rand(10))
+        vspan!(ax, 3, 4)
+        hspan!(ax, 3, 4)
+        bracket!(ax, 1, 1, 2, 2)
+        eb = errorbars!(ax, 1:10, 1:10, [0.3 for _ in 1:10], whiskerwidth = 5)
+        text!(ax, Point2f(2), text = "abba")
+        tooltip!(ax, Point2f(8), "baab")
+        tricontourf!(ax, 1 .+ 4 .* rand(5), 1 .+ 4 .* rand(5), rand(5))
+        qqplot!(ax, 5:10, 1:5)
+        ax.yscale = log10
+        ax.yscale = identity
+        ax.yscale = log10
+        ax.yscale = identity
+        @test true
+    catch e
+        @test false
+        rethrow(e)
+    end
+end