From ccf615202fc4e92698ecf7d13028d8dcade1298f Mon Sep 17 00:00:00 2001 From: Frederic Freyer Date: Mon, 27 Nov 2023 13:56:38 +0100 Subject: [PATCH 1/6] fix transformation passthrough (#3418) * fix transformation passthrough * update news [skip ci] --- NEWS.md | 2 ++ src/basic_recipes/hvlines.jl | 4 ++-- src/basic_recipes/hvspan.jl | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/NEWS.md b/NEWS.md index ca1f39dce8c..95da45b9d88 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## master +- Fixed a bug with h/vlines and h/vspan not correctly resolving transformations [#3418](https://github.com/MakieOrg/Makie.jl/pull/3418) + ## 0.20.1 - Fixed bad rendering of `poly` in GLMakie by triangulating points after transformations [#3402](https://github.com/MakieOrg/Makie.jl/pull/3402). diff --git a/src/basic_recipes/hvlines.jl b/src/basic_recipes/hvlines.jl index 23be173580a..b5b6d58605a 100644 --- a/src/basic_recipes/hvlines.jl +++ b/src/basic_recipes/hvlines.jl @@ -83,7 +83,7 @@ function Makie.plot!(p::Union{HLines, VLines}) line_attributes = copy(p.attributes) 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) + line_attributes[:transformation] = Transformation(p, transform_func = identity) + linesegments!(p, line_attributes, points) p end diff --git a/src/basic_recipes/hvspan.jl b/src/basic_recipes/hvspan.jl index 3ecd9589551..ebd391b1bc9 100644 --- a/src/basic_recipes/hvspan.jl +++ b/src/basic_recipes/hvspan.jl @@ -77,8 +77,8 @@ function Makie.plot!(p::Union{HSpan, VSpan}) 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) + poly_attributes[:transformation] = Transformation(p, transform_func = identity) + poly!(p, poly_attributes, rects) p end From a0a1bdc8a47d700d2302c6b4d7e033ab302eaea2 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 27 Nov 2023 15:11:53 +0100 Subject: [PATCH 2/6] use generic attributes (#3424) --- src/basic_recipes/contours.jl | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/basic_recipes/contours.jl b/src/basic_recipes/contours.jl index f4f21f70dfc..7ebd79febf1 100644 --- a/src/basic_recipes/contours.jl +++ b/src/basic_recipes/contours.jl @@ -22,26 +22,26 @@ To add contour labels, use `labels = true`, and pass additional label attributes $(ATTRIBUTES) """ @recipe(Contour) do scene - default = default_theme(scene) - # pop!(default, :color) - Attributes(; - default..., + attr = Attributes(; color = nothing, - colormap = theme(scene, :colormap), - colorscale = identity, - colorrange = Makie.automatic, levels = 5, linewidth = 1.0, linestyle = nothing, - alpha = 1.0, enable_depth = true, transparency = false, labels = false, + labelfont = theme(scene, :font), labelcolor = nothing, # matches color by default labelformatter = contour_label_formatter, labelsize = 10, # arbitrary ) + + + MakieCore.colormap_attributes!(attr, theme(scene, :colormap)) + MakieCore.generic_plot_attributes!(attr) + + return attr end """ @@ -146,11 +146,13 @@ function plot!(plot::Contour{<: Tuple{X, Y, Z, Vol}}) where {X, Y, Z, Vol} end attr = copy(Attributes(plot)) + attr[:colorrange] = cliprange attr[:colormap] = cmap attr[:algorithm] = 7 pop!(attr, :levels) pop!(attr, :alpha) # don't apply alpha 2 times + # unused attributes pop!(attr, :labels) pop!(attr, :labelfont) @@ -320,7 +322,8 @@ function plot!(plot::T) where T <: Union{Contour, Contour3d} linewidth = plot.linewidth, inspectable = plot.inspectable, transparency = plot.transparency, - linestyle = plot.linestyle + linestyle = plot.linestyle, + MakieCore.generic_plot_attributes(plot)... ) plot end From 5c35f22b666a80fb5afa5a632af8690a08e15af3 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 28 Nov 2023 11:21:20 +0100 Subject: [PATCH 3/6] fix contour attribute + plot!(fig, ..) (#3425) * use generic attributes * closes #3416, plot!(fig, ...) --- src/figureplotting.jl | 9 ++++++--- test/figures.jl | 5 +++-- test/pipeline.jl | 2 ++ 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/figureplotting.jl b/src/figureplotting.jl index c1ba49e8453..4abfc12e42c 100644 --- a/src/figureplotting.jl +++ b/src/figureplotting.jl @@ -269,9 +269,12 @@ end # This enables convert_arguments(::Type{<:AbstractPlot}, ::X) -> FigureSpec # Which skips axis creation # TODO, what to return for the dynamically created axes? -figurelike_return(f::GridPosition, p::AbstractPlot) = p -figurelike_return(f::Figure, p::AbstractPlot) = FigureAxisPlot(f, nothing, p) -MakieCore.create_axis_like!(::AbstractPlot, attributes::Dict, fig::Figure) = fig +const PlotSpecPlot = Plot{plot, Tuple{<: GridLayoutSpec}} + +figurelike_return(f::GridPosition, p::PlotSpecPlot) = p +figurelike_return(f::Figure, p::PlotSpecPlot) = FigureAxisPlot(f, nothing, p) +MakieCore.create_axis_like!(::PlotSpecPlot, attributes::Dict, fig::Figure) = fig +MakieCore.create_axis_like!(::AbstractPlot, attributes::Dict, fig::Figure) = nothing # Axis interface diff --git a/test/figures.jl b/test/figures.jl index de8c1e8a315..f9018b8b39f 100644 --- a/test/figures.jl +++ b/test/figures.jl @@ -62,6 +62,7 @@ end current_axis!(ax2) @test current_axis() === ax2 @test current_figure() === fig + end @testset "Deleting from figures" begin @@ -158,7 +159,7 @@ end @test_throws ArgumentError lines(1:10, axis = (aspect = DataAspect()), figure = (size = (100, 100))) @test_throws ArgumentError lines(1:10, figure = (size = (100, 100))) @test_throws ArgumentError lines(1:10, axis = (aspect = DataAspect())) - + # these just shouldn't error lines(1:10, axis = (aspect = DataAspect(),)) lines(1:10, axis = Attributes(aspect = DataAspect())) @@ -167,4 +168,4 @@ end f = Figure() @test_throws ArgumentError lines(f[1, 1], 1:10, axis = (aspect = DataAspect())) @test_throws ArgumentError lines(f[1, 1][2, 2], 1:10, axis = (aspect = DataAspect())) -end \ No newline at end of file +end diff --git a/test/pipeline.jl b/test/pipeline.jl index 54b9e7c58f3..6f18bc98729 100644 --- a/test/pipeline.jl +++ b/test/pipeline.jl @@ -106,6 +106,8 @@ end pl = meshscatter!(LScene(sub[1, 3]), rand(Point3f, 10)) @test pl isa AbstractPlot + f = Figure() + @test_throws ErrorException lines!(f, [1, 2]) end end From 5abc30381a5205658a906bcb3d91f54774b069c8 Mon Sep 17 00:00:00 2001 From: Frederic Freyer Date: Fri, 1 Dec 2023 09:49:20 +0100 Subject: [PATCH 4/6] Fix incorrect limits in h/vlines and h/vspan (#3427) * fix incorrect limits * update NEWS * fully go to data space * add tests --- NEWS.md | 1 + src/basic_recipes/hvlines.jl | 18 ++++++++++++++++++ src/basic_recipes/hvspan.jl | 21 +++++++++++++++++++++ test/boundingboxes.jl | 17 +++++++++++++++++ 4 files changed, 57 insertions(+) diff --git a/NEWS.md b/NEWS.md index 95da45b9d88..087cbf5309b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,7 @@ ## master - Fixed a bug with h/vlines and h/vspan not correctly resolving transformations [#3418](https://github.com/MakieOrg/Makie.jl/pull/3418) +- Fixed a bug with h/vlines and h/vspan returning the wrong limits, causing an error in Axis [#3427](https://github.com/MakieOrg/Makie.jl/pull/3427) ## 0.20.1 diff --git a/src/basic_recipes/hvlines.jl b/src/basic_recipes/hvlines.jl index b5b6d58605a..a919629d6b6 100644 --- a/src/basic_recipes/hvlines.jl +++ b/src/basic_recipes/hvlines.jl @@ -87,3 +87,21 @@ function Makie.plot!(p::Union{HLines, VLines}) linesegments!(p, line_attributes, points) p end + +function data_limits(p::HLines) + scene = parent_scene(p) + limits = projview_to_2d_limits(scene.camera.projectionview[]) + itf = inverse_transform(p.transformation.transform_func[]) + xmin, xmax = apply_transform.(itf[1], first.(extrema(limits))) + ymin, ymax = extrema(p[1][]) + return Rect3f(Point3f(xmin, ymin, 0), Vec3f(xmax - xmin, ymax - ymin, 0)) +end + +function data_limits(p::VLines) + scene = parent_scene(p) + limits = projview_to_2d_limits(scene.camera.projectionview[]) + itf = inverse_transform(p.transformation.transform_func[]) + xmin, xmax = extrema(p[1][]) + ymin, ymax = apply_transform.(itf[2], getindex.(extrema(limits), 2)) + return Rect3f(Point3f(xmin, ymin, 0), Vec3f(xmax - xmin, ymax - ymin, 0)) +end \ No newline at end of file diff --git a/src/basic_recipes/hvspan.jl b/src/basic_recipes/hvspan.jl index ebd391b1bc9..cb082347705 100644 --- a/src/basic_recipes/hvspan.jl +++ b/src/basic_recipes/hvspan.jl @@ -88,3 +88,24 @@ _apply_x_transform(other, v) = error("x transform not defined for transform func _apply_y_transform(t::Tuple, v) = apply_transform(t[2], v) _apply_y_transform(other, v) = error("y transform not defined for transform function $(typeof(other))") _apply_y_transform(::typeof(identity), v) = v + + +function data_limits(p::HSpan) + scene = parent_scene(p) + limits = projview_to_2d_limits(scene.camera.projectionview[]) + itf = inverse_transform(p.transformation.transform_func[]) + xmin, xmax = apply_transform.(itf[1], first.(extrema(limits))) + ymin = minimum(p[1][]) + ymax = maximum(p[2][]) + return Rect3f(Point3f(xmin, ymin, 0), Vec3f(xmax - xmin, ymax - ymin, 0)) +end + +function data_limits(p::VSpan) + scene = parent_scene(p) + limits = projview_to_2d_limits(scene.camera.projectionview[]) + itf = inverse_transform(p.transformation.transform_func[]) + xmin = minimum(p[1][]) + xmax = maximum(p[2][]) + ymin, ymax = apply_transform.(itf[2], getindex.(extrema(limits), 2)) + return Rect3f(Point3f(xmin, ymin, 0), Vec3f(xmax - xmin, ymax - ymin, 0)) +end \ No newline at end of file diff --git a/test/boundingboxes.jl b/test/boundingboxes.jl index 37aa126331e..e7a83fb34fd 100644 --- a/test/boundingboxes.jl +++ b/test/boundingboxes.jl @@ -15,6 +15,23 @@ end fig, ax, p = bracket(ps...) @test data_limits(p) ≈ Rect3f(Point3f(0), Vec3f(1, 1, 0)) + + fig = Figure() + ax = Axis(fig[1, 1], yscale=log, xscale=log) + scatter!(ax, [0.5, 1, 2], [0.5, 1, 2]) + p1 = vlines!(ax, [0.5]) + p2 = hlines!(ax, [0.5]) + p3 = vspan!(ax, [0.25], [0.75]) + p4 = hspan!(ax, [0.25], [0.75]) + Makie.reset_limits!(ax) + + lims = ax.finallimits[] + x, y = minimum(lims); w, h = widths(lims) + + @test data_limits(p1) ≈ Rect3f(Point3f(0.5, y, 0), Vec3f(0, h, 0)) + @test data_limits(p2) ≈ Rect3f(Point3f(x, 0.5, 0), Vec3f(w, 0, 0)) + @test data_limits(p3) ≈ Rect3f(Point3f(0.25, y, 0), Vec3f(0.5, h, 0)) + @test data_limits(p4) ≈ Rect3f(Point3f(x, 0.25, 0), Vec3f(w, 0.5, 0)) end @testset "boundingbox(plot)" begin From 83165c97fe9ad2bf519294ff497487b32435275f Mon Sep 17 00:00:00 2001 From: Frederic Freyer Date: Fri, 1 Dec 2023 09:50:16 +0100 Subject: [PATCH 5/6] Improve PolarAxis surface example (pass data as color) (#3426) use color attribute for 2D surface plot example --- docs/reference/blocks/polaraxis.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/reference/blocks/polaraxis.md b/docs/reference/blocks/polaraxis.md index 426db39665c..5c1e1877f2f 100644 --- a/docs/reference/blocks/polaraxis.md +++ b/docs/reference/blocks/polaraxis.md @@ -92,8 +92,7 @@ Not every plot type is compatible with the polar transform. For example `image` is not as it expects to be drawn on a rectangle. `heatmap` works to a degree in CairoMakie, but not GLMakie due to differences in the backend implementation. `surface` can be used as a replacement for `image` as it generates a triangle mesh. -However it also has a component in z-direction which will affect drawing order. -You can use `translate!(plot, 0, 0, z_shift)` to work around that. +To avoid having the `surface` plot extend in z-direction and thus messing with render order it is recommended to pass the color-data through the `color` attribute and use a matrix of zeros for the z-data. As a replacement for `heatmap` you can use `voronoiplot`, which generates cells of arbitrary shape around points given to it. Here you will generally need to set `rlims!(ax, rmax)` yourself. \begin{examplefigure}{svg = false} @@ -104,7 +103,7 @@ ax = PolarAxis(f[1, 1], title = "Surface") rs = 0:10 phis = range(0, 2pi, 37) cs = [r+cos(4phi) for phi in phis, r in rs] -p = surface!(ax, 0..2pi, 0..10, cs, shading = NoShading, colormap = :coolwarm) +p = surface!(ax, 0..2pi, 0..10, zeros(size(cs)), color = cs, shading = NoShading, colormap = :coolwarm) ax.gridz[] = 100 tightlimits!(ax) # surface plots include padding by default Colorbar(f[2, 1], p, vertical = false, flipaxis = false) From c91752f73b37739c33bb7b196b57ea4bcc328dde Mon Sep 17 00:00:00 2001 From: Sagnac <83491030+Sagnac@users.noreply.github.com> Date: Fri, 1 Dec 2023 09:03:06 +0000 Subject: [PATCH 6/6] Histogram: fix a plot updating bug for plot specific attributes (#3365) This fixes a bug where dynamic changes to histogram specific Observables would fail to cause the histogram plots to update. For `hist` this was because the plot updating is handled by watching `widths`, but that didn't receive updates whenever the `normalization`, `scale_to`, and `weights` attributes changed. To fix this we now explicitly observe these attributes and set the points whenever they update. Note that alternatively watching `points` doesn't work; if that's done instead the updating of the histogram breaks in some cases. Also, the Observables themselves are not passed to `barplot!` in order to avoid redundant updates as `points` and `widths` depend upon some of the same Observables. For the step histogram, the three aforementioned attributes along with `bins` were not being listened to so as a result the plot would fail to update whenever any of these Observables changed. This is fixed by simply passing the `points` Observable instead of the values to `stairs!`. `stephist` is different compared to `hist` as there's no `widths` so there are no efficiency gains by managing updates separately. Any updates to the four attributes will update `points` here so the plot updating is handled correctly. Fixes #3362 --- src/stats/hist.jl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/stats/hist.jl b/src/stats/hist.jl index 5c943a88ea2..165f361bc07 100644 --- a/src/stats/hist.jl +++ b/src/stats/hist.jl @@ -78,8 +78,8 @@ function Makie.plot!(plot::StepHist) pop!(attr, :normalization) pop!(attr, :scale_to) pop!(attr, :bins) - # plot the values, not the observables, to be in control of updating - stairs!(plot, points[]; attr..., color=color) + stairs!(plot, points; attr..., color=color) + plot end """ @@ -187,5 +187,8 @@ function Makie.plot!(plot::Hist) bp[1].val = points[] bp.width = w end + onany(plot, plot.normalization, plot.scale_to, plot.weights) do _, _, _ + bp[1][] = points[] + end plot end