Widget optimizations

CHANGELOG.md

@@ -25,6 +25,7 @@
 - Reverted change to `poly` which disallowed 3D geometries from being plotted [#4738](https://github.com/MakieOrg/Makie.jl/pull/4738)
 - Enabled autocompletion on Block types, e.g. `?Axis.xti...` [#4786](https://github.com/MakieOrg/Makie.jl/pull/4786)
 - Added `dpi` metadata to all rendered png files, where `px_per_unit = 1` means 96dpi, `px_per_unit = 2` means 192dpi, and so on. This gives frontends a chance to show plain Makie png images with the correct scaling [#4812](https://github.com/MakieOrg/Makie.jl/pull/4812).
+- Improved performance of some Blocks, mainly `Textbox` and `Menu` [#4821](https://github.com/MakieOrg/Makie.jl/pull/4821)
 
 ## [0.22.1] - 2025-01-17
 

ReferenceTests/src/tests/figures_and_makielayout.jl

@@ -50,12 +50,21 @@ end
         [
             Label(fig, "A", width = nothing) Label(fig, "C", width = nothing);
             menu1                            menu3;
+            Box(fig, visible = false) Box(fig, visible = false);
             Label(fig, "B", width = nothing) Label(fig, "D", width = nothing);
             menu2                            menu4;
         ]
     )
-    menu2.is_open = true
+
+    # simulated interaction
+    events(fig).mouseposition[] = (500, 380)
+    events(fig).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.press)
+    events(fig).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.release)
+    events(fig).mouseposition[] = (500, 320)
+
+    menu1.is_open = true
     menu4.is_open = true
+
     fig
 end
 

WGLMakie/test/message_counting.jl

@@ -0,0 +1,102 @@
+# These tests are relevant for general Makie performance but only tested here
+# because Bonito makes it easy to test. If the tests fail it is likely not
+# WGLMakies fault.
+# A lower number of messages maybe caused by optimizations or broken interactions.
+# A higher number could come from added functionality or performance regressions.
+
+
+@testset "TextBox with Menu" begin
+    f = Figure()
+    t1 = Textbox(f[1, 1])
+    m = Menu(f[2, 1], options = string.(1:1000))
+    display(edisplay, App(() -> f))
+
+    # trigger select
+    all_messages, summary_str = Bonito.collect_messages() do
+        events(f).mouseposition[] = (300, 250)
+        events(f).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.press)
+        events(f).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.release)
+    end
+    @test length(all_messages) == 44
+
+    # type text
+    for (char, expected) in zip(collect("test"), [18, 39, 39, 39])
+        _key = getproperty(Makie.Keyboard, Symbol(char))
+        all_messages, summary_str = Bonito.collect_messages() do
+            events(f).keyboardbutton[] = Makie.KeyEvent(_key, Keyboard.press)
+            events(f).unicode_input[] = char
+            events(f).keyboardbutton[] = Makie.KeyEvent(_key, Keyboard.release)
+        end
+        @test length(all_messages) == expected
+    end
+end
+
+@testset "Menu" begin
+    f = Figure()
+    m = Menu(f[1,1], options = string.(1:10))
+    display(edisplay, App(() -> f))
+
+    # open menu
+    all_messages, summary_str = Bonito.collect_messages() do
+        events(f).mouseposition[] = (300, 230)
+        events(f).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.press)
+        events(f).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.release)
+    end
+    @test length(all_messages) == 102
+
+    # scroll items
+    all_messages, summary_str = Bonito.collect_messages() do
+        events(f).mouseposition[] = (300, 200)
+        events(f).scroll[] = (0.0, -1.0)
+    end
+    @test length(all_messages) == 16
+
+    # select item
+    all_messages, summary_str = Bonito.collect_messages() do
+        events(f).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.press)
+        events(f).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.release)
+    end
+    @test length(all_messages) == 29
+end
+
+@testset "Textbox" begin
+    f = Figure()
+    t1 = Textbox(f[1, 1], tellwidth = false)
+    display(edisplay, App(() -> f))
+
+    all_messages, summary_str = Bonito.collect_messages() do
+        events(f).mouseposition[] = (300, 225)
+        events(f).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.press)
+        events(f).mousebutton[] = Makie.MouseButtonEvent(Mouse.left, Mouse.release)
+    end
+    @test length(all_messages) == 34
+
+    all_messages, summary_str = Bonito.collect_messages() do
+        events(f).keyboardbutton[] = Makie.KeyEvent(Keyboard.t, Keyboard.press)
+        events(f).unicode_input[] = 't'
+        events(f).keyboardbutton[] = Makie.KeyEvent(Keyboard.t, Keyboard.release)
+    end
+    @test length(all_messages) == 18
+
+    all_messages, summary_str = Bonito.collect_messages() do
+        events(f).keyboardbutton[] = Makie.KeyEvent(Keyboard.e, Keyboard.press)
+        events(f).unicode_input[] = 'e'
+        events(f).keyboardbutton[] = Makie.KeyEvent(Keyboard.e, Keyboard.release)
+    end
+    @test length(all_messages) == 30
+
+    all_messages, summary_str = Bonito.collect_messages() do
+        events(f).keyboardbutton[] = Makie.KeyEvent(Keyboard.s, Keyboard.press)
+        events(f).unicode_input[] = 's'
+        events(f).keyboardbutton[] = Makie.KeyEvent(Keyboard.s, Keyboard.release)
+    end
+    @test length(all_messages) == 30
+
+    all_messages, summary_str = Bonito.collect_messages() do
+        events(f).keyboardbutton[] = Makie.KeyEvent(Keyboard.t, Keyboard.press)
+        events(f).unicode_input[] = 't'
+        events(f).keyboardbutton[] = Makie.KeyEvent(Keyboard.t, Keyboard.release)
+    end
+    @test length(all_messages) == 30
+
+end

WGLMakie/test/runtests.jl

@@ -37,8 +37,8 @@ Makie.inline!(Makie.automatic)
 edisplay = Bonito.use_electron_display(devtools=true)
 
 @testset "reference tests" begin
+    WGLMakie.activate!()
     @testset "refimages" begin
-        WGLMakie.activate!()
         ReferenceTests.mark_broken_tests(excludes)
         recorded_files, recording_dir = @include_reference_tests WGLMakie "refimages.jl"
         missing_images, scores = ReferenceTests.record_comparison(recording_dir, "WGLMakie")

src/camera/camera.jl

@@ -118,7 +118,7 @@ Returns true if the current mouseposition is inside the given scene.
 """
 is_mouseinside(x) = is_mouseinside(get_scene(x))
 function is_mouseinside(scene::Scene)
-    return Vec(scene.events.mouseposition[]) in viewport(scene)[]
+    return scene.visible[] && in(Vec(scene.events.mouseposition[]), viewport(scene)[])
     # Check that mouse is not inside any other screen
     # for child in scene.children
     #     is_mouseinside(child) && return false

src/interaction/interactive_api.jl

@@ -1,6 +1,9 @@
 export mouseover, mouseposition, hovered_scene
 export select_rectangle, select_line, select_point
 
+# for debug/test tracking of pick
+const PICK_TRACKING = Ref(false)
+const _PICK_COUNTER = Ref(0)
 
 """
     mouseover(fig/ax/scene, plots::AbstractPlot...)
@@ -56,13 +59,13 @@ end
 Returns the plot and element index under the given pixel position `xy = Vec(x, y)`.
 If `range` is given, the nearest plot up to a distance of `range` is returned instead.
 
-The `plot` returned by this function is always a primitive plot, i.e. one that 
+The `plot` returned by this function is always a primitive plot, i.e. one that
 is not composed of other plot types.
 
 The index returned relates to the main input of the respective primitive plot.
 - For `scatter` and `meshscatter` it is an index into the positions given to the plot.
 - For `text` it is an index into the merged character array.
-- For `lines` and `linesegments` it is the end position of the selected line segment. 
+- For `lines` and `linesegments` it is the end position of the selected line segment.
 - For `image`, `heatmap` and `surface` it is the linear index into the matrix argument of the plot (i.e. the given image, value or z-value matrix) that is closest to the selected position.
 - For `voxels` it is the linear index into the given 3D Array.
 - For `mesh` it is the largest vertex index of the picked triangle face.
@@ -79,22 +82,33 @@ end
 pick(obj) = pick(get_scene(obj), events(obj).mouseposition[])
 pick(obj, xy::VecTypes{2}) = pick(get_scene(obj), xy)
 function pick(scene::Scene, xy::VecTypes{2})
+    PICK_TRACKING[] && (_PICK_COUNTER[] += 1)
     screen = getscreen(scene)
     screen === nothing && return (nothing, 0)
-    pick(scene, screen, Vec{2, Float64}(xy))
+    return pick(scene, screen, Vec{2, Float64}(xy))
 end
 
 pick(obj, range::Real) = pick(get_scene(obj), events(obj).mouseposition[], range)
 pick(obj, xy::VecTypes{2}, range::Real) = pick(get_scene(obj), xy, range)
 pick(obj, x::Real, y::Real, range::Real) = pick(get_scene(obj), Vec2(x, y), range)
 function pick(scene::Scene, xy::VecTypes{2}, range::Real)
+    PICK_TRACKING[] && (_PICK_COUNTER[] += 1)
     screen = getscreen(scene)
     screen === nothing && return (nothing, 0)
-    pick_closest(scene, screen, xy, range)
+    if PICK_TRACKING[]
+        # if the Makie implementation is used we'd double count if we just increment here
+        last = _PICK_COUNTER[]
+        result = pick_closest(scene, screen, xy, range)
+        _PICK_COUNTER[] = last
+        return result
+    else
+        return pick_closest(scene, screen, xy, range)
+    end
 end
 
 # The backend may handle this more optimally
 function pick_closest(scene::SceneLike, screen, xy, range)
+    PICK_TRACKING[] && (_PICK_COUNTER[] += 1)
     w, h = widths(screen)
     ((1.0 <= xy[1] <= w) && (1.0 <= xy[2] <= h)) || return (nothing, 0)
     x0, y0 = max.(1, floor.(Int, xy .- range))
@@ -126,12 +140,23 @@ Return all `(plot, index)` pairs in a `(xy .- range, xy .+ range)` region
 sorted by distance to `xy`. See [`pick`](@ref) for more details.
 """
 function pick_sorted(scene::Scene, xy, range)
+    PICK_TRACKING[] && (_PICK_COUNTER[] += 1)
     screen = getscreen(scene)
     screen === nothing && return Tuple{AbstractPlot, Int}[]
+    if PICK_TRACKING[]
+        # if the Makie implementation is used we'd double count if we just increment here
+        last = _PICK_COUNTER[]
+        result = pick_sorted(scene, screen, xy, range)
+        _PICK_COUNTER[] = last
+        return result
+    else
+        return pick_sorted(scene, screen, xy, range)
+    end
     return pick_sorted(scene, screen, xy, range)
 end
 
 function pick_sorted(scene::Scene, screen, xy, range)
+    PICK_TRACKING[] && (_PICK_COUNTER[] += 1)
     w, h = size(scene)
     if !((1.0 <= xy[1] <= w) && (1.0 <= xy[2] <= h))
         return Tuple{AbstractPlot, Int}[]
@@ -168,6 +193,7 @@ screen boundaries.
 """
 pick(x, rect::Rect2i) = pick(get_scene(x), rect)
 function pick(scene::Scene, rect::Rect2i)
+    PICK_TRACKING[] && (_PICK_COUNTER[] += 1)
     screen = getscreen(scene)
     screen === nothing && return Tuple{AbstractPlot, Int}[]
     return pick(scene, screen, rect)

src/makielayout/blocks/checkbox.jl

@@ -54,7 +54,7 @@ function initialize_block!(c::Checkbox)
         visible = @lift($ischecked || $ishovered),
     )
 
-    mouseevents = addmouseevents!(scene, shp, sc)
+    mouseevents = addmouseevents!(scene, checkboxrect)
 
     onmouseleftclick(mouseevents) do _
         newstatus = c.onchange[](c.checked[])
@@ -72,7 +72,7 @@ function initialize_block!(c::Checkbox)
     on(scene, c.size; update = true) do sz
         c.layoutobservables.autosize[] = Float64.((sz, sz))
     end
-    
+
     return
 end
 

src/makielayout/blocks/menu.jl

@@ -23,18 +23,25 @@ function initialize_block!(m::Menu; default = 1)
         end
     end
 
-    scenearea = lift(blockscene, m.layoutobservables.computedbbox, listheight, _direction, m.is_open;
-                     ignore_equal_values=true) do bbox, h, d, open
-        !open ?
-            round_to_IRect2D(BBox(left(bbox), right(bbox), 0, 0)) :
-            round_to_IRect2D(BBox(
+    scenearea = Observable(Rect2i(0,0,0,0), ignore_equal_values=true)
+    map!(blockscene, scenearea, m.layoutobservables.computedbbox, listheight, _direction, m.is_open;
+                     update = true) do bbox, h, d, open
+        if open
+            return round_to_IRect2D(BBox(
                 left(bbox),
                 right(bbox),
                 d === :down ? max(0, bottom(bbox) - h) : top(bbox),
-                d === :down ? bottom(bbox) : min(top(bbox) + h, top(blockscene.viewport[]))))
+                d === :down ? bottom(bbox) : min(top(bbox) + h, top(blockscene.viewport[]))
+            ))
+        else
+            # If the scene is not visible the scene placement and size does not
+            # matter for rendering. We still need to set the size to 0 for
+            # interactions though.
+            return Rect2i(0,0,0,0)
+        end
     end
 
-    menuscene = Scene(blockscene, scenearea, camera = campixel!, clear=true)
+    menuscene = Scene(blockscene, scenearea, camera = campixel!, clear=true, visible = m.is_open)
     translate!(menuscene, 0, 0, 200)
 
     onany(blockscene, scenearea, listheight) do area, listheight
@@ -110,26 +117,39 @@ function initialize_block!(m::Menu; default = 1)
     optiontexts = text!(menuscene, textpositions, text = optionstrings, align = (:left, :center),
         fontsize = m.fontsize, inspectable = false)
 
-    onany(blockscene, optionstrings, m.textpadding, m.layoutobservables.computedbbox) do _, pad, bbox
+    # listheight needs to be up to date before showing the menuscene so that its
+    # direction is correct
+    gc_heights = map(blockscene, optiontexts.plots[1][1], m.textpadding) do gcs, pad
         gcs = optiontexts.plots[1][1][]::Vector{GlyphCollection}
         bbs = map(x -> string_boundingbox(x, zero(Point3f), Quaternion(0, 0, 0, 0)), gcs)
         heights = map(bb -> height(bb) + pad[3] + pad[4], bbs)
-        heights_cumsum = [zero(eltype(heights)); cumsum(heights)]
         h = sum(heights)
+        listheight[] = h
+        return (heights, h)
+    end
+
+    onany(blockscene, gc_heights, scenearea) do (heights, h), bbox
+        # No need to update when the scene is hidden
+        widths(bbox) == Vec2i(0) && return
+
+        pad = m.textpadding[] # gc_heights triggers on padding, so we don't need to react to it
+        # listheight[] = h
+
+        heights_cumsum = [zero(eltype(heights)); cumsum(heights)]
         list_y_bounds[] = h .- heights_cumsum
         texts_y = @views h .- 0.5 .* (heights_cumsum[1:end-1] .+ heights_cumsum[2:end])
         textpositions[] = Point2f.(pad[1], texts_y)
-        listheight[] = h
         w_bbox = width(bbox)
         # need to manipulate the vectors themselves, otherwise update errors when lengths change
-        resize!(optionrects.val, length(bbs))
+        resize!(optionrects.val, length(heights))
 
-        optionrects.val .= map(eachindex(bbs)) do i
+        optionrects.val .= map(eachindex(heights)) do i
             BBox(0, w_bbox, h - heights_cumsum[i+1], h - heights_cumsum[i])
         end
 
         update_option_colors!(0)
         notify(optionrects)
+        return
     end
     notify(optionstrings)
 
@@ -178,8 +198,9 @@ function initialize_block!(m::Menu; default = 1)
         is_over_button = false
 
         if Makie.is_mouseinside(menuscene) # the whole scene containing all options
-            # Is inside the expanded menu selection
-            if mouseover(menuscene, optionpolys, optiontexts)
+            # Is inside the expanded menu selection (the polys cover the whole
+            # selectable area and are in pixel space relative to menuscene)
+            if any(r -> mp in r, optionpolys[1][])
                 is_over_options = true
                 was_inside_options = true
                 # we either clicked on an item or hover it
@@ -198,7 +219,8 @@ function initialize_block!(m::Menu; default = 1)
             return Consume(true)
         else
             # If not inside menuscene, we check the state for the menu button
-            if mouseover(blockscene, selectiontext, selectionpoly)
+            # (use position because selectionpoly is in blockscene)
+            if position in selectionpoly.converted[1][]
                 # If over, we either click it to open/close the menu, or we just hover it
                 is_over_button = true
                 was_inside_button = true