PolarAxis improvements (full rlimits, thetalimits, improved controls, argument order) #3154
Conversation
Compile Times benchmarkNote, that these numbers may fluctuate on the CI servers, so take them with a grain of salt. All benchmark results are based on the mean time and negative percent mean faster than the base branch. Note, that GLMakie + WGLMakie run on an emulated GPU, so the runtime benchmark is much slower. Results are from running: using_time = @ctime using Backend
# Compile time
create_time = @ctime fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @ctime Makie.colorbuffer(display(fig))
# Runtime
create_time = @benchmark fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @benchmark Makie.colorbuffer(display(fig))
|
|
For testing: begin
f = Figure()
theta_0 = Slider(f[1, 1], range = range(0, 2pi, length=101)).value
thetalims = IntervalSlider(f[2, 1], range = range(0, 2pi, length=101)).interval
rlims = IntervalSlider(f[3, 1], range = range(0, 10, length=101)).interval
po = PolarAxis(f[4, 1], thetalimits = thetalims, rlimits=rlims, theta_0 = theta_0)
bb = map(po.thetalimits, po.target_radius, po.theta_0, po.direction) do _, _, _, _
Makie.transformed_bbox(po)
end
p = scatter!(po, 8 .* rand(1000) .+ 1, 2pi .* rand(1000))
l = lines!(po, bb, color = :yellow, transformation = Transformation())
translate!(l, 0, 0, 9000)
f
endI'm currently handling r and theta limits here by adjusting the clip polygon. The Polar transform still works the same as before, i.e. no extra distortion and fully invertable. This however means that if you're interested in a large rmin, rmax with a small difference, like (8, 10), you will only get a slim ring like this: I'm wondering if instead I should add a radial shift to the Polar transform, so I can keep the clip polygon at set radii (e.g. 0.1 and 1.0). With that the shift would make sure that whatever range needs to be visible would fit into the clip poylgon and the ticks would adjust to communicate that. |
|
If it's not too bold to comment, I personally have no problem with a 'slim ring' when I have asked for a small range of r, so I would be happy with that. (I don't have any opinion on the implementation as I don't understand that.) |
|
I think it would be better to not go the "slim ring" route and instead do it via shift of the transform (a slimmer ring might still be desirable to keep the amount of radial stretching or "aspect ratio" of the data at some desired value. That would make the transform not fully invertible, but I guess if we throw a clear error if any data clips inwards that's ok? |
Yea I added an
Since we don't use the inverse transform in PolarAxis at all we could, I guess, but I'm not sure if it's good to do that. It's quite easy for stuff to end up with r < 0 through zooming (and now translating) the axis. We probably shouldn't throw an error every time that happens if someone is using the inverse transform in some way. Probably better to just hide/leave stuff at r = 0. |
Missing reference imagesFound 1 new images without existing references. |
|
Some Notes on how I currently have things set up:
Updated test plot: f = Figure()
theta_0 = Slider(f[1, 1], range = range(0, 2pi, length=101)).value
thetalims = IntervalSlider(f[2, 1], range = range(0, 2pi, length=101)).interval
rlims = IntervalSlider(f[3, 1], range = range(0, 11, length=111)).interval
po = PolarAxis(f[4, 1:2], thetalimits = thetalims, rlimits=rlims, theta_0 = theta_0)
po.theta_translation_button[] = Mouse.right
bb = map(Makie.transformed_bbox,
po.target_radius, po.thetalimits,
map(x -> x.r0, po.scene.transformation.transform_func),
po.direction,
po.theta_0,
)
# these are the real values, not the values set by the slider (axis interaction causes desync)
Label(f[1, 2], map(x -> @sprintf("θ₀ = %0.1f°", x * 180/pi), po.theta_0))
Label(f[2, 2], map(lims -> @sprintf("θ = %0.1f° .. %0.1f°", (180/pi .* lims)...), po.thetalimits))
Label(f[3, 2], map(lims -> @sprintf("r = %0.2f .. %0.2f", lims...), po.target_radius))
p = scatter!(po, Point2f.(2, 2pi .* (0:7) ./ 8), color = 1:8, colormap = :RdBu)
lines!(po, range(1, 10, 101), range(0, 2pi, length=101))
p = scatter!(po, Point2f.(10, 2pi .* (0:7) ./ 8), color = 1:8, colormap = :RdBu)
l = lines!(po, bb, color = :yellow, transformation = Transformation())
translate!(l, 0, 0, 9000)
f |
Missing reference imagesFound 1 new images without existing references. |
Missing reference imagesFound 1 new images without existing references. |
|
Ok, the things you noted should be taken care of now. Since I switched back to full-circle PolarAxis by default I also adjusted the default interactions:
Other changes:
|
Missing reference imagesFound 1 new images without existing references. |
src/makielayout/blocks/polaraxis.jl
Outdated
| function Makie.plot!( | ||
| po::PolarAxis, P::Makie.PlotFunc, | ||
| attributes::Makie.Attributes, args...; | ||
| needs_tight_limits(::Surface) = true |
There was a problem hiding this comment.
One set of test failures (basically everything surface related) comes from this, because this also affects Axis. Personally I think surface plots on an Axis defaulting to tight limits is something we want but I can also just revert this.
|
The other test failures around PolarAxis have a few sources... PolarAxis Decorations before:
After:
PolarAxis scatterlines spine before:
After:Pre fixes
This has a different default rtick angle, which is also coming from rtick alignment in polar sectors and is easy to revert if we want. It also moved away from tight limits, which I think we want? Voronoiplot with a nonlinear transform, Triplot with nonlinear transformationSame rtick angle change as above tl;dr question @jkrumbiegel, @SimonDanisch
|
I'd say we shouldn't!
I like them on a straight line but don't have a strong opinion ;) |
|
The straight line would be problematic with slightly longer ticks, I'd argue for keeping them horizontal because of that. |
|
Looking back at this I think there are two specific question about rticks:
I think we should, because in a 2d Axis surface is more or less the same as image which has tight limits. But if we don't I'd just surface untightened here too |
|
I meant, we shouldn't do it in this PR, since its unrelated and strictly speaking breaking (by the definition that we need to update reference images). |
|
Removed/Commented out the surface tightlimits change, so now a surface plot in PolarAxis produces
(from test w/o rlims!() call) Default rtickangle is unchanged, so the default rtick placement is still on a straight horizontal line:
With explicitly set rtickangle, the individual label rotation is now horizontal if thetalimits are > 1.9pi
and ":aligned" otherwise. That means they rotate according to their position, but stay close to upright and horizontal
|
|
@ffreyer and I decided to merge this to get it in the next release, since there are only styling changes pending and no other breaking change in sight. |
Changelog
Breaking changes
(r, theta)to(theta, r). This can be changed back withpolaraxis.theta_as_x[] = falseax.radiusrepresenting rmax toax.rlimitsrepresenting (rmin, rmax)`Visual/Interactivity changes
ax.rzoomkey = Keyboard.randax.thetazoomkey = Keyboard.thetaand more permanently withax.rzoomlock = false,ax.thetazoomlock = trueandax.fixrmin = true(defaults)polaraxis.griddepth[] = 8999.New Additions/Features
delete!(::PolarAxis, ::Plot)rlims!(ax, rmin, rmax)thetalims!(ax, thetamin, thetamax)andax.thetalimits[]autolimits!(ax[, unlock = true])to allow the PolarAxis to derive its limits freely and unlock interactions.rticklabelrotation = automaticto orient rtick labels according to the corresponding grid lines.zoomspeed = 0.1to control the speed of zoomingrzoomkey = Keyboard.rto restrict zooming to the r directionthetazoomkey = Keyboard.tto restrict zooming to the theta directionfixrmin = trueto control whether zooming and translating affects rminrzoomlock = falseandthetazoomlock = trueto block zooming in the respective directionsr_translation_button = Mouse.rightfor translating the plot in theta directiontheta_translation_button = Mouse.rightfor translating the axis in theta directionaxis_rotation_button = Keyboard.left_control & Mouse.rightto rotate the axis as a wholereset_axis_orientation = falseto control whether the reset interaction reorients the axisnormalize_theta_ticks = trueto control whether theta ticks are normalized to a -2pi..2pi rangeBackground Changes
sample_densitynow applies to clip polygonstarget_thetalimsAngularTicksto derive more pleasant theta ticksDescription
PolarAxis: Allow limits on radius and angle #3152, adding rlimits and thetalimits to generate circle sectorsax.griddepthand adjusting the default behaviourPolarAxis(and make the swapped version default) #3178 by addingtheta_as_xand changing the default argument orderTODO
improveaddMultipleTicksfor anglesAngularTicksrlims!, addthetalims!detach clip orientation from theta_0 and theta limits. You should be able to adjustReview
maximum_clip_radius(is it better?)direction = -1rtick_rotation = true/false/automatic/angleattributereconsiderdirection * (theta + theta_0)vsdirection * theta + theta_0isfinite(vmax)error when translating a lotType of change
Checklist