From 428c7c8849225c607f2bb27f40ca84d2815412f3 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Wed, 20 Nov 2024 14:42:45 -0500 Subject: [PATCH 01/10] Move GeoAxis tests to their own file --- test/runtests.jl | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 250d7f5d..e32ab082 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -9,9 +9,9 @@ Makie.set_theme!(Theme( @testset "GeoMakie" begin - @testset "MeshImage" begin - include("meshimage.jl") - end + @testset "MeshImage" include("meshimage.jl") + + @testset "GeoAxis" include("geoaxis.jl") @testset "Basics" begin lons = -180:180 @@ -37,26 +37,4 @@ Makie.set_theme!(Theme( @test GeoMakie.coastlines(ga)[] isa AbstractVector end - @testset "Legend" begin - fig = Figure() - ga = GeoAxis(fig[1, 1]) - lines!(ga, 1:10, 1:10; label = "series 1") - scatter!(ga, 1:19, 2:20; label= "series 2") - @test_nowarn Legend(fig[1, 2], ga) - fig - end - - @testset "Plotlists get transformed" begin - fig = Figure() - ax = GeoAxis(fig[1,1]) - plotspecs = [S.Lines(Point2f.(1:10, 1:10)), S.Scatter(Point2f.(1:10, 1:10))] - - p1 = plotlist!(ax, plotspecs) - - @test p1.transformation.transform_func[] isa GeoMakie.Proj.Transformation - - for plot in p1.plots - @test plot.transformation.transform_func[] isa GeoMakie.Proj.Transformation - end - end end From 6144b80e85c4ea382833e2c32b1ca1f77f1fb78f Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Wed, 20 Nov 2024 16:30:47 -0500 Subject: [PATCH 02/10] Disambiguate `Transformation` in meshimage recipe --- src/mesh_image.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesh_image.jl b/src/mesh_image.jl index fc27d662..994f852a 100644 --- a/src/mesh_image.jl +++ b/src/mesh_image.jl @@ -123,7 +123,7 @@ function Makie.plot!(plot::MeshImage) color = plot.converted[3], # pass on the color directly MakieCore.colormap_attributes(plot)..., # pass on all colormap attributes shading = NoShading, # - transformation = Transformation( + transformation = Makie.Transformation( plot.transformation; # connect up the model matrix to the parent's model matrix transform_func = identity # do NOT connect the transform func, since we've already done that. identity provides a custom transform func, while `nothing` signals that you don't care. ), From 8dcc48f67df979f8910afae4f8c9f70522fe3979 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 13 Dec 2024 04:48:48 +0800 Subject: [PATCH 03/10] Fix minor typos/errors/ambiguities --- src/GeoMakie.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/GeoMakie.jl b/src/GeoMakie.jl index 7d82806f..8a520cdc 100644 --- a/src/GeoMakie.jl +++ b/src/GeoMakie.jl @@ -45,7 +45,6 @@ const Text = Makie.Text Base.convert(::Type{Rect{N, Float64}}, x::Rect{N}) where N = Rect{N, Float64}(x) include("makie_piracy.jl") -include("triangulation3d.jl") include("geojson.jl") # GeoJSON/GeoInterface support include("conversions.jl") include("data.jl") @@ -61,6 +60,8 @@ include("makie-axis.jl") include("mesh_image.jl") include("linesplitting.jl") +include("triangulation3d.jl") + @reexport using Colors, Makie export Proj From 65e14e5bd691840cfda0dd74627e90662f3de721 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 13 Dec 2024 04:24:07 +0800 Subject: [PATCH 04/10] Fix bug where protrusions were calculated for invisible components --- src/geoaxis.jl | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/src/geoaxis.jl b/src/geoaxis.jl index 2d0c5036..cccb9eac 100644 --- a/src/geoaxis.jl +++ b/src/geoaxis.jl @@ -701,22 +701,42 @@ function Makie.initialize_block!(axis::GeoAxis) fonts = theme(axis.blockscene, :fonts) # Finally calculate protrusions and report all bounding boxes # to the layout system. - approx_x_protrusion = map(axis.blockscene, axis.yticklabelfont, axis.yticklabelsize, lat_text) do font, size, lat_text - max_height = 0.0f0 - for str in lat_text - bb = Makie.text_bb(str, to_font(fonts, font), size) - max_height = max(max_height, widths(bb)[2]) + approx_x_protrusion = map( + axis.blockscene, + axis.yticklabelfont, axis.yticklabelsize, axis.yticklabelpad, lat_text, axis.yticklabelsvisible + ) do ticklabel_font, ticklabel_size, ticklabel_pad, text, ticklabelsvisible + ret = 0.0f0 + + if ticklabelsvisible + max_height = 0.0 + for str in text + bb = Makie.text_bb(str, Makie.to_font(fonts, ticklabel_font), ticklabel_size) + max_height = max(max_height, widths(bb)[2]) + end + ret += max_height# + ticklabel_pad end - return max_height + + return ret end - approx_y_protrusion = map(axis.blockscene, axis.yticklabelfont, axis.yticklabelsize, lon_text) do font, size, lon_text - max_width = 0.0f0 - for str in lon_text - bb = Makie.text_bb(str, to_font(fonts, font), size) - max_width = max(max_width, widths(bb)[1]) + approx_y_protrusion = map( + axis.blockscene, + axis.xticklabelfont, axis.xticklabelsize, axis.xticklabelpad, lon_text, axis.xticklabelsvisible, + ) do ticklabel_font, ticklabel_size, ticklabel_pad, text, ticklabelsvisible + + ret = 0.0f0 + + if ticklabelsvisible + max_width = 0.0 + for str in text + bb = Makie.text_bb(str, Makie.to_font(fonts, ticklabel_font), ticklabel_size) + max_width = max(max_width, widths(bb)[1]) + end + ret += max_width #+ ticklabel_pad end - return max_width + + return ret + end elements = Dict{Symbol,Any}() @@ -773,7 +793,8 @@ function Makie.initialize_block!(axis::GeoAxis) xaxis = (; protrusion=approx_y_protrusion) map!(compute_protrusions, axis.blockscene, axis.layoutobservables.protrusions, axis.title, axis.titlesize, axis.titlegap, axis.titlevisible, - xaxis.protrusion, yaxis.protrusion, + xaxis.protrusion, + yaxis.protrusion, axis.subtitle, axis.subtitlevisible, axis.subtitlesize, axis.subtitlegap, axis.titlelineheight, axis.subtitlelineheight, subtitlet, titlet) From 655f61c6ccff83e93a2bcabe2f1e41d4d1fd8a2b Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 13 Dec 2024 04:24:29 +0800 Subject: [PATCH 05/10] Add test that hidedecorations reduces protrusions --- test/geoaxis.jl | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/geoaxis.jl b/test/geoaxis.jl index 03ea7335..103146b3 100644 --- a/test/geoaxis.jl +++ b/test/geoaxis.jl @@ -46,4 +46,25 @@ end @test_nowarn contourf!(ax, lons, lats, field) end +end + +@testset "Protrusions are correctly updated when visible = false" begin + + f, a, p = meshimage(-180..180, -90..90, GeoMakie.earth() |> rotr90; figure = (; figure_padding = 0), axis = (; type = GeoAxis, dest = "+proj=longlat +type=crs")) + + w = widths(a.finallimits[]) + colsize!(f.layout, 1, Aspect(1, w[1] / w[2])) + resize_to_layout!(f) + + Makie.update_state_before_display!(f) + original_prots = a.layoutobservables.protrusions[] + Makie.hidedecorations!(a) + Makie.update_state_before_display!(f) + new_prots = a.layoutobservables.protrusions[] + + @test new_prots.left == 0 + @test new_prots.right == 0 + @test new_prots.top == 0 + @test new_prots.bottom == 0 + end \ No newline at end of file From f4242f9c4070db37d99163a43dde936e714aa115 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 13 Dec 2024 04:33:06 +0800 Subject: [PATCH 06/10] Hook `meshimage` up to the tightlimits infrastructure --- src/mesh_image.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mesh_image.jl b/src/mesh_image.jl index 994f852a..d581bd2e 100644 --- a/src/mesh_image.jl +++ b/src/mesh_image.jl @@ -51,6 +51,9 @@ Makie.conversion_trait(::Type{<: MeshImage}) = Makie.ImageLike() # There really is no difference between this and Image, # except the implementation under the hood. +# We also define that `meshimage` needs tight limits when plotted... +Makie.needs_tight_limits(::MeshImage) = true + # This is the recipe implementation. function Makie.plot!(plot::MeshImage) From 703039fe2dc5ae6effbb46291a9e2584b6ed2e42 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 13 Dec 2024 04:33:41 +0800 Subject: [PATCH 07/10] Add test to make sure that bounding boxes are correct for axes --- src/geoaxis.jl | 1 + src/makie-axis.jl | 47 ++++++++++++++++++++++++++++++++++++++++++++++- test/geoaxis.jl | 19 +++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/geoaxis.jl b/src/geoaxis.jl index cccb9eac..67b7804d 100644 --- a/src/geoaxis.jl +++ b/src/geoaxis.jl @@ -861,6 +861,7 @@ function Makie.plot!(axis::GeoAxis, plot::Makie.AbstractPlot) # some area-like plots basically always look better if they cover the whole plot area. # adjust the limit margins in those cases automatically. Makie.needs_tight_limits(plot) && reset_limits && Makie.tightlimits!(axis) + if Makie.is_open_or_any_parent(axis.scene) && reset_limits Makie.reset_limits!(axis) end diff --git a/src/makie-axis.jl b/src/makie-axis.jl index a8a9f1ea..49e5f5fb 100644 --- a/src/makie-axis.jl +++ b/src/makie-axis.jl @@ -16,10 +16,14 @@ function axis_setup!(axis::GeoAxis) setfield!(axis, :targetlimits, targetlimits) setfield!(axis, :finallimits, finallimits) topscene = axis.blockscene + scenearea = Makie.sceneareanode!(axis.layoutobservables.computedbbox, finallimits, axis.aspect) + scene = Scene(topscene, viewport=scenearea) - axis.scene = scene setfield!(scene, :float32convert, Makie.Float32Convert()) + + axis.scene = scene + onany(scene, scene.transformation.transform_func, finallimits, axis.xreversed, axis.yreversed) do transform_func, finallimits, xreversed, yreversed Makie.update_axis_camera(scene, transform_func, finallimits, xreversed, yreversed) end @@ -565,3 +569,44 @@ function Makie.Legend(fig_or_scene, axis::GeoAxis, title = nothing; merge = fals isempty(plots) && error("There are no plots with labels in the given axis that can be put in the legend. Supply labels to plotting functions like `plot(args...; label = \"My label\")`") Makie.Legend(fig_or_scene, plots, labels, title; kwargs...) end + + + +# Taken from makielayout/helpers.jl + +""" + tightlimits!(la::Axis) + +Sets the autolimit margins to zero on all sides. +""" +function Makie.tightlimits!(la::GeoAxis) + la.xautolimitmargin = (0, 0) + la.yautolimitmargin = (0, 0) + reset_limits!(la) +end + +function Makie.tightlimits!(la::GeoAxis, sides::Union{Left, Right, Bottom, Top}...) + for s in sides + Makie.tightlimits!(la, s) + end +end + +function Makie.tightlimits!(la::GeoAxis, ::Left) + la.xautolimitmargin = Base.setindex(la.xautolimitmargin[], 0.0, 1) + autolimits!(la) +end + +function Makie.tightlimits!(la::GeoAxis, ::Right) + la.xautolimitmargin = Base.setindex(la.xautolimitmargin[], 0.0, 2) + autolimits!(la) +end + +function Makie.tightlimits!(la::GeoAxis, ::Bottom) + la.yautolimitmargin = Base.setindex(la.yautolimitmargin[], 0.0, 1) + autolimits!(la) +end + +function Makie.tightlimits!(la::GeoAxis, ::Top) + la.yautolimitmargin = Base.setindex(la.yautolimitmargin[], 0.0, 2) + autolimits!(la) +end diff --git a/test/geoaxis.jl b/test/geoaxis.jl index 103146b3..5b283f36 100644 --- a/test/geoaxis.jl +++ b/test/geoaxis.jl @@ -67,4 +67,23 @@ end @test new_prots.top == 0 @test new_prots.bottom == 0 +end + +@testset "Aspect ratio is equal to Axis with DataAspect" begin + # Create two figures, one with regular axis and one with geoaxis + # the transformation in both cases is the identity + f1, a1, p1 = meshimage(-180..180, -90..90, GeoMakie.earth() |> rotr90; figure = (; figure_padding = 0), axis = (; aspect = DataAspect())); + f2, a2, p2 = meshimage(-180..180, -90..90, GeoMakie.earth() |> rotr90; figure = (; figure_padding = 0), axis = (; type = GeoAxis, dest = "+proj=longlat +type=crs")); + + Makie.tightlimits!(a1) + hidedecorations!(a1) + hidedecorations!(a2) + + Makie.update_state_before_display!(f1) + Makie.update_state_before_display!(f2) + + Makie.resize_to_layout!(f1) + Makie.resize_to_layout!(f2) + + @test a1.scene.viewport[] == a2.scene.viewport[] end \ No newline at end of file From d0993517e7f983750e9b4d3cf801a5baac8191c4 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 13 Dec 2024 04:37:45 +0800 Subject: [PATCH 08/10] Add back ticklabelpadding + clean up reset_limits logic --- src/geoaxis.jl | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/geoaxis.jl b/src/geoaxis.jl index 67b7804d..e67e1818 100644 --- a/src/geoaxis.jl +++ b/src/geoaxis.jl @@ -713,7 +713,7 @@ function Makie.initialize_block!(axis::GeoAxis) bb = Makie.text_bb(str, Makie.to_font(fonts, ticklabel_font), ticklabel_size) max_height = max(max_height, widths(bb)[2]) end - ret += max_height# + ticklabel_pad + ret += max_height + ticklabel_pad end return ret @@ -732,7 +732,7 @@ function Makie.initialize_block!(axis::GeoAxis) bb = Makie.text_bb(str, Makie.to_font(fonts, ticklabel_font), ticklabel_size) max_width = max(max_width, widths(bb)[1]) end - ret += max_width #+ ticklabel_pad + ret += max_width + ticklabel_pad end return ret @@ -858,13 +858,17 @@ function Makie.plot!(axis::GeoAxis, plot::Makie.AbstractPlot) # actually plot Makie.plot!(axis.scene, plot) - # some area-like plots basically always look better if they cover the whole plot area. - # adjust the limit margins in those cases automatically. - Makie.needs_tight_limits(plot) && reset_limits && Makie.tightlimits!(axis) - - if Makie.is_open_or_any_parent(axis.scene) && reset_limits - Makie.reset_limits!(axis) + # reset limits ONLY IF the user has not said otherwise + if reset_limits + # some area-like plots basically always look better if they cover the whole plot area. + # adjust the limit margins in those cases automatically. + Makie.needs_tight_limits(plot) && Makie.tightlimits!(axis) + + if Makie.is_open_or_any_parent(axis.scene) + Makie.reset_limits!(axis) + end end + return plot end From e43ae31514afe08ce1bdc97364808fdf43a3e753 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 13 Dec 2024 04:53:39 +0800 Subject: [PATCH 09/10] add a per-axis-type needs_tight_limits --- src/geoaxis.jl | 3 +++ src/makie_piracy.jl | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/src/geoaxis.jl b/src/geoaxis.jl index e67e1818..5d29c365 100644 --- a/src/geoaxis.jl +++ b/src/geoaxis.jl @@ -885,4 +885,7 @@ end # TODO implement Makie.tightlimits!(axis::GeoAxis) = nothing +# this is generally false, but I want to deviate from that here. +Makie.needs_tight_limits(axis::GeoAxis, ::Surface) = true + Makie.get_scene(ga::GeoAxis) = ga.scene diff --git a/src/makie_piracy.jl b/src/makie_piracy.jl index 272b9056..9075ba11 100644 --- a/src/makie_piracy.jl +++ b/src/makie_piracy.jl @@ -50,3 +50,10 @@ function Makie.transform_bbox(scenelike, lims::Rect{N, T}) where {N, T} end =# + +# `needs_tight_limits`` should dispatch on axes too +# because what looks bad on one axis (regular Axis) +# will look good on a GeoAxis +# and vice versa + +Makie.needs_tight_limits(axis::Makie.AbstractAxis, plot::Makie.Plot) = Makie.needs_tight_limits(plot) From 51c5a5111a8f7278916bee6fdbba7d86225069ce Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 13 Dec 2024 04:54:27 +0800 Subject: [PATCH 10/10] remove no-op tightlimits definition --- src/geoaxis.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/geoaxis.jl b/src/geoaxis.jl index 5d29c365..c6707317 100644 --- a/src/geoaxis.jl +++ b/src/geoaxis.jl @@ -882,8 +882,8 @@ function Makie.MakieCore._create_plot!(F, attributes::Dict, ax::GeoAxis, args... return plot end -# TODO implement -Makie.tightlimits!(axis::GeoAxis) = nothing + +# ## Makie generic axis/block API # this is generally false, but I want to deviate from that here. Makie.needs_tight_limits(axis::GeoAxis, ::Surface) = true