From 225d0ae0e7c9db3bc43b2642327ee9a8ee84fed4 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 19 Jun 2024 22:31:51 +0200 Subject: [PATCH] Fixes for WGLMakie (#3975) * fix #3781 * fix text picking for #3752 * check for !isempty at correct place * add changelog --- CHANGELOG.md | 1 + WGLMakie/assets/sprites.frag | 3 +- WGLMakie/src/picking.jl | 7 +++-- WGLMakie/src/wglmakie.bundled.js | 30 +++++++++++++++++++ WGLMakie/src/wglmakie.js | 30 +++++++++++++++++++ src/interaction/inspector.jl | 45 +++++++++++++++------------- src/makielayout/mousestatemachine.jl | 3 +- 7 files changed, 92 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62241a16bd3..4adc2bb487d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/WGLMakie/assets/sprites.frag b/WGLMakie/assets/sprites.frag index 468b75366ff..a4befe2b936 100644 --- a/WGLMakie/assets/sprites.frag +++ b/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; } diff --git a/WGLMakie/src/picking.jl b/WGLMakie/src/picking.jl index 38a4f3fc8e6..1e069317a3c 100644 --- a/WGLMakie/src/picking.jl +++ b/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 diff --git a/WGLMakie/src/wglmakie.bundled.js b/WGLMakie/src/wglmakie.bundled.js index 41acc44390b..d0668a19b6a 100644 --- a/WGLMakie/src/wglmakie.bundled.js +++ b/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 }; diff --git a/WGLMakie/src/wglmakie.js b/WGLMakie/src/wglmakie.js index ec7dd3a24a9..110273d7883 100644 --- a/WGLMakie/src/wglmakie.js +++ b/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]; diff --git a/src/interaction/inspector.jl b/src/interaction/inspector.jl index 0c66f001b00..0331e39eb07 100644 --- a/src/interaction/inspector.jl +++ b/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 diff --git a/src/makielayout/mousestatemachine.jl b/src/makielayout/mousestatemachine.jl index b3fe3da35a7..42d09b54365 100644 --- a/src/makielayout/mousestatemachine.jl +++ b/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