Fix scroll lock in Menu

[ffreyer]

Description

Currently the scrolling interaction of menu scenes always triggers and always blocks lower priority/later scroll events (as long as it isn't blocked by something else). This means that creating a Menu before an Axis will make you unable to zoom in that axis.

This is fixed here by only running code and returning Consume(true) if is_mouseinside(menuscene) is true.

Type of change

• [x] Bug fix (non-breaking change which fixes an issue)

[jkrumbiegel]

I honestly don't understand how the priority system works in such cases so I will probably introduce similar bugs in the future. Is it explained somewhere which elements get which event at what time and how the priorities/consume settings affect that? Or are we currently doing it in an ad-hoc "this is blocked so let's increase its priority" way

[MakieBot]

Compile Times benchmark

Note, 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))

	using	create	display	create	display
GLMakie	18.89s (17.91, 20.02) 0.83+-	21.80s (20.90, 22.88) 0.80+-	18.13s (17.52, 18.61) 0.55+-	17.85ms (17.10, 18.54) 0.57+-	89.20ms (85.92, 93.39) 2.68+-
master	18.98s (17.92, 21.19) 1.25+-	21.81s (20.85, 23.23) 0.94+-	17.93s (17.14, 18.78) 0.65+-	18.35ms (17.78, 19.31) 0.58+-	89.02ms (85.55, 92.65) 2.77+-
evaluation	-0.51%, -0.1s invariant (-0.09d, 0.87p, 1.04std)	-0.05%, -0.01s invariant (-0.01d, 0.98p, 0.87std)	+1.10%, 0.2s invariant (0.33d, 0.55p, 0.60std)	-2.82%, -0.5ms invariant (-0.88d, 0.13p, 0.57std)	+0.20%, 0.18ms invariant (0.07d, 0.90p, 2.73std)
CairoMakie	18.73s (18.51, 19.00) 0.18+-	25.76s (25.58, 25.86) 0.11+-	3.19s (3.16, 3.25) 0.03+-	22.40ms (21.56, 22.87) 0.46+-	31.57ms (31.42, 31.84) 0.18+-
master	18.65s (18.55, 18.70) 0.05+-	25.62s (25.36, 26.22) 0.28+-	3.15s (3.11, 3.22) 0.04+-	21.95ms (21.67, 22.84) 0.41+-	31.54ms (31.19, 31.76) 0.23+-
evaluation	+0.42%, 0.08s invariant (0.61d, 0.29p, 0.11std)	+0.51%, 0.13s invariant (0.62d, 0.28p, 0.19std)	+1.49%, 0.05s slower X (1.34d, 0.03p, 0.04std)	+2.02%, 0.45ms invariant (1.04d, 0.08p, 0.43std)	+0.11%, 0.03ms invariant (0.17d, 0.76p, 0.20std)
WGLMakie	20.67s (20.39, 20.90) 0.17+-	32.91s (32.61, 33.09) 0.16+-	44.69s (44.37, 44.96) 0.19+-	21.77ms (20.07, 22.44) 0.84+-	1.69s (1.65, 1.72) 0.02+-
master	20.80s (20.64, 20.90) 0.12+-	32.89s (32.66, 33.06) 0.15+-	44.97s (44.69, 45.32) 0.23+-	21.79ms (21.24, 22.16) 0.30+-	1.70s (1.68, 1.72) 0.02+-
evaluation	-0.64%, -0.13s invariant (-0.93d, 0.11p, 0.14std)	+0.07%, 0.02s invariant (0.14d, 0.79p, 0.15std)	-0.63%, -0.28s faster ✓ (-1.32d, 0.03p, 0.21std)	-0.12%, -0.03ms invariant (-0.04d, 0.94p, 0.57std)	-0.33%, -0.01s invariant (-0.28d, 0.61p, 0.02std)

[ffreyer]

The rules are:

1. If two listeners have different priority the higher one executes first
2. If two listeners have the same priority the one added first (i.e. first in the list of listeners) executes first. (Like without priority)

And if a listener returns Consume(true) the callback loop exits. So it will deny any listener that has lower priority or is added later.

So basically priority is a way to set execution order, with listeners at equal priority executing more or less in a random order (because it often depends on outside factors, i.e. creation order) and Consumne is a way to shut out lower priority events.

In this case you probably want a high priority so the menu scrolling is considered first, and consume the event only if the menu is opened and hovered.

[jkrumbiegel]

Yes the basically random execution order otherwise is what's confusing me. Should it really be that random or should we have a clearer plan how events move through the scene graph?

[ffreyer]

I tried a set up something similar to axis interactions with priority and scene ordering in https://github.com/JuliaPlots/AbstractPlotting.jl/pull/586. It ended up too slow imo. (Maybe because the approach with recursive functions was bad. But I think triggering observables in each scene child-first would be awkward and slow too.)

Iirc I was also not sure on whether priority or scene order should come first. Scene order first makes more sense to me right now, but that might not always be what you want. For example you could set up some sort of pen widget that draws on top of everything on the root scene. With scene order first this would get blocked all the time. DataInspector would too, as it is now.

Going without priority entirely is problematic too. Any kind of optional interaction that supersedes an existing one needs it. Without, you run into the same ordering issue as now. And with priorities you still need to pick the right ones.

[jkrumbiegel]

Then maybe we should come up with an enum of priority levels or so? I face kind of the same problem with z values, for example the legend is shifted forward. But by how much is kind of arbitrary, I just picked some values that worked, there's no "system"

[ffreyer]

Then maybe we should come up with an enum of priority levels or so?

I don't think restricting it makes sense, but we could define const LOW_PRIORITY = 0 etc. Not sure how useful that is but I guess it's a bit more readable?

I face kind of the same problem with z values, for example the legend is shifted forward. But by how much is kind of arbitrary, I just picked some values that worked, there's no "system"

For that (at least in GLMakie) p = projection * view * model * point must be in a (-1, -1, 0) to (1, 1, 1) range to be drawn. You can go back from there.

Generally near and far describe the range in data space, after view * model: https://github.com/JuliaPlots/Makie.jl/blob/0252621cbd4e9479e5c0db8ea4b528d800bf3e22/src/camera/camera2d.jl#L85 https://github.com/JuliaPlots/Makie.jl/blob/0252621cbd4e9479e5c0db8ea4b528d800bf3e22/src/camera/camera2d.jl#L323 https://github.com/JuliaPlots/Makie.jl/blob/0252621cbd4e9479e5c0db8ea4b528d800bf3e22/src/camera/camera2d.jl#L343 https://github.com/JuliaPlots/Makie.jl/blob/0252621cbd4e9479e5c0db8ea4b528d800bf3e22/src/camera/camera3d.jl#L144-L145

For space = :pixel it's currently hardcoded to -10_000 .. 10_000 here: https://github.com/JuliaPlots/Makie.jl/blob/0252621cbd4e9479e5c0db8ea4b528d800bf3e22/src/camera/camera.jl#L71-L76

space = :clip and space = :relative are hardcoded to 0 .. 1 here: (The matrices constructed here usually end up replacing projection * view) https://github.com/JuliaPlots/Makie.jl/blob/0252621cbd4e9479e5c0db8ea4b528d800bf3e22/src/camera/projection_math.jl#L314-L326

[ffreyer]

Can we merge this?

[jkrumbiegel]

I think so!