Fixes for WGLMakie (#3975)

* fix #3781

* fix text picking for #3752

* check for !isempty at correct place

* add changelog

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- Fixes for Menu and DataInspector [#3975](https://github.com/MakieOrg/Makie.jl/pull/3975)
 - Add line-loop detection and rendering to GLMakie and WGLMakie [#3907](https://github.com/MakieOrg/Makie.jl/pull/3907)
 
 ## [0.21.3] - 2024-06-17

WGLMakie/assets/sprites.frag

@@ -149,11 +149,12 @@ void main() {
     if (picking) {
         if (final_color.a > 0.1) {
             fragment_color = pack_int(object_id, frag_instance_id);
+        } else {
+            discard;
         }
         return;
     }
 
-
     if (final_color.a <= 0.0){
         discard;
     }

WGLMakie/src/picking.jl

@@ -16,8 +16,7 @@ function pick_native(screen::Screen, rect::Rect2i)
     if isempty(matrix)
         return empty
     else
-        all_children = Makie.collect_atomic_plots(scene)
-        lookup = Dict(Pair.(js_uuid.(all_children), all_children))
+        lookup = plot_lookup(scene)
         return map(matrix) do (uuid, index)
             !haskey(lookup, uuid) && return (nothing, 0)
             return (lookup[uuid], Int(index) + 1)
@@ -40,6 +39,7 @@ function Makie.pick_closest(scene::Scene, screen::Screen, xy, range::Integer)
         Promise.all([$(WGL), $(scene)]).then(([WGL, scene]) => WGL.pick_closest(scene, $(xy_vec), $(range)))
     """)
     lookup = plot_lookup(scene)
+    !haskey(lookup, selection[1]) && return (nothing, 0)
     return (lookup[selection[1]], selection[2] + 1)
 end
 
@@ -55,11 +55,12 @@ function Makie.pick_sorted(scene::Scene, screen::Screen, xy, range)
     isnothing(selection) && return Tuple{Union{Nothing,AbstractPlot},Int}[]
     lookup = plot_lookup(scene)
     return map(selection) do (plot_id, index)
+        !haskey(lookup, plot_id) && return (nothing, 0)
         return (lookup[plot_id], index + 1)
     end
 end
 
-function Makie.pick(scene::Scene, screen::Screen, xy)
+function Makie.pick(::Scene, screen::Screen, xy)
     plot_matrix = pick_native(screen, Rect2i(xy..., 1, 1))
     return plot_matrix[1, 1]
 end

WGLMakie/src/wglmakie.bundled.js

@@ -23124,6 +23124,35 @@ function pick_native(scene, _x, _y, _w, _h) {
         plots
     ];
 }
+function get_picking_buffer(scene) {
+    const { renderer , picking_target  } = scene.screen;
+    const [w, h] = [
+        picking_target.width,
+        picking_target.height
+    ];
+    renderer.setRenderTarget(picking_target);
+    set_picking_uniforms(scene, 1, true);
+    render_scene(scene, true);
+    renderer.setRenderTarget(null);
+    const nbytes = w * h * 4;
+    const pixel_bytes = new Uint8Array(nbytes);
+    renderer.readRenderTargetPixels(picking_target, 0, 0, w, h, pixel_bytes);
+    const reinterpret_view = new DataView(pixel_bytes.buffer);
+    const picked_plots_array = [];
+    for(let i = 0; i < pixel_bytes.length / 4; i++){
+        const id = reinterpret_view.getUint16(i * 4);
+        const index = reinterpret_view.getUint16(i * 4 + 2);
+        picked_plots_array.push([
+            id,
+            index
+        ]);
+    }
+    return {
+        picked_plots_array,
+        w,
+        h
+    };
+}
 function pick_closest(scene, xy, range) {
     const { renderer  } = scene.screen;
     const [width, height] = [
@@ -23282,6 +23311,7 @@ export { deserialize_scene as deserialize_scene, threejs_module as threejs_modul
 export { render_scene as render_scene };
 export { wglerror as wglerror };
 export { pick_native as pick_native };
+export { get_picking_buffer as get_picking_buffer };
 export { pick_closest as pick_closest };
 export { pick_sorted as pick_sorted };
 export { pick_native_uuid as pick_native_uuid };

WGLMakie/src/wglmakie.js

@@ -566,6 +566,36 @@ export function pick_native(scene, _x, _y, _w, _h) {
     return [plot_matrix, plots];
 }
 
+// For debugging the pixelbuffer
+export function get_picking_buffer(scene) {
+    const { renderer, picking_target } = scene.screen;
+    const [w, h] = [picking_target.width, picking_target.height];
+    // render the scene
+    renderer.setRenderTarget(picking_target);
+    set_picking_uniforms(scene, 1, true);
+    render_scene(scene, true);
+    renderer.setRenderTarget(null); // reset render target
+    const nbytes = w * h * 4;
+    const pixel_bytes = new Uint8Array(nbytes);
+    //read the pixel
+    renderer.readRenderTargetPixels(
+        picking_target,
+        0, // x
+        0, // y
+        w, // width
+        h, // height
+        pixel_bytes
+    );
+    const reinterpret_view = new DataView(pixel_bytes.buffer);
+    const picked_plots_array = []
+    for (let i = 0; i < pixel_bytes.length / 4; i++) {
+        const id = reinterpret_view.getUint16(i * 4);
+        const index = reinterpret_view.getUint16(i * 4 + 2);
+        picked_plots_array.push([id, index]);
+    }
+    return {picked_plots_array, w, h};
+}
+
 export function pick_closest(scene, xy, range) {
     const { renderer } = scene.screen;
     const [ width, height ] = [renderer._width, renderer._height];

src/interaction/inspector.jl

@@ -189,11 +189,12 @@ mutable struct DataInspector
     selection::AbstractPlot
 
     obsfuncs::Vector{Any}
+    lock::Threads.ReentrantLock
 end
 
 
 function DataInspector(scene::Scene, plot::AbstractPlot, attributes)
-    x = DataInspector(scene, attributes, AbstractPlot[], plot, plot, Any[])
+    x = DataInspector(scene, attributes, AbstractPlot[], plot, plot, Any[], Threads.ReentrantLock())
     # finalizer(cleanup, x) # doesn't get triggered when this is dereferenced
     x
 end
@@ -297,31 +298,33 @@ DataInspector(; kwargs...) = DataInspector(current_figure(); kwargs...)
 
 function on_hover(inspector)
     parent = inspector.root
-    (inspector.attributes.enabled[] && is_mouseinside(parent)) || return Consume(false)
-
-    mp = mouseposition_px(parent)
-    should_clear = true
-    for (plt, idx) in pick_sorted(parent, mp, inspector.attributes.range[])
-        if to_value(get(plt.attributes, :inspectable, true))
-            # show_data should return true if it created a tooltip
-            if show_data_recursion(inspector, plt, idx)
-                should_clear = false
-                break
+    lock(inspector.lock) do
+        (inspector.attributes.enabled[] && is_mouseinside(parent)) || return Consume(false)
+
+        mp = mouseposition_px(parent)
+        should_clear = true
+        for (plt, idx) in pick_sorted(parent, mp, inspector.attributes.range[])
+            if to_value(get(plt.attributes, :inspectable, true))
+                # show_data should return true if it created a tooltip
+                if show_data_recursion(inspector, plt, idx)
+                    should_clear = false
+                    break
+                end
             end
         end
-    end
 
-    if should_clear
-        plot = inspector.selection
-        if to_value(get(plot, :inspector_clear, automatic)) !== automatic
-            plot[:inspector_clear][](inspector, plot)
+        if should_clear
+            plot = inspector.selection
+            if to_value(get(plot, :inspector_clear, automatic)) !== automatic
+                plot[:inspector_clear][](inspector, plot)
+            end
+            inspector.plot.visible[] = false
+            inspector.attributes.indicator_visible[] = false
+            inspector.plot.offset.val = inspector.attributes.offset[]
         end
-        inspector.plot.visible[] = false
-        inspector.attributes.indicator_visible[] = false
-        inspector.plot.offset.val = inspector.attributes.offset[]
-    end
 
-    return Consume(false)
+        return Consume(false)
+    end
 end
 
 

src/makielayout/mousestatemachine.jl

@@ -286,10 +286,9 @@ function _addmouseevents!(scene, is_mouse_over_relevant_area, priority)
         # TODO: this could probably be simplified by just using event.button
         # though that would probably change the way this handles a bit
         pressed_buttons = events(scene).mousebuttonstate
-
         # mouse went down, this can either happen inside or outside the objects of interest
         # we also only react if one button is pressed, because otherwise things go crazy (pressed left button plus clicks from other buttons in between are not allowed, e.g.)
-        if event.action == Mouse.press && _isstandardmousebutton(first(pressed_buttons))
+        if event.action == Mouse.press && !isempty(pressed_buttons) && _isstandardmousebutton(first(pressed_buttons))
             if length(pressed_buttons) == 1
                 button = first(pressed_buttons)
                 mouse_downed_button[] = button