Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 19 commits into from Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
Expand Up @@ -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).
Expand Down
1 change: 0 additions & 1 deletion src/basic_recipes/ablines.jl
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions src/basic_recipes/barplot.jl
Expand Up @@ -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`.
"""
Expand Down Expand Up @@ -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
Expand Down
29 changes: 18 additions & 11 deletions src/basic_recipes/error_and_rangebars.jl
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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]
Expand Down
13 changes: 6 additions & 7 deletions src/basic_recipes/hvlines.jl
Expand Up @@ -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
Expand All @@ -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
18 changes: 11 additions & 7 deletions src/basic_recipes/hvspan.jl
Expand Up @@ -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
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/layouting/transformation.jl
Expand Up @@ -39,7 +39,7 @@ function Transformation(transformable::Transformable;
scale_o,
rotation_o,
model,
transform_func
convert(Observable{Any}, transform_func)
)

trans.parent[] = parent_transform
Expand Down
17 changes: 10 additions & 7 deletions src/makielayout/blocks/axis.jl
Expand Up @@ -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`
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/scenes.jl
Expand Up @@ -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

Expand Down
27 changes: 27 additions & 0 deletions test/makielayout.jl
Expand Up @@ -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