Improve picking tests (#4488)

* improve verification of heatmap path

* verify heatmap path for WGLMakie

* for picking tests to refimg tests + add refimg

* update changelog

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Changed image, heatmap and surface picking indices to correctly index the relevant matrix arguments. [#4459](https://github.com/MakieOrg/Makie.jl/pull/4459)
 - Improved performance of `record` by avoiding unnecessary copying in common cases [#4475](https://github.com/MakieOrg/Makie.jl/pull/4475).
 - Fix usage of `AggMean()` and other aggregations operating on 3d data for `datashader` [#4346](https://github.com/MakieOrg/Makie.jl/pull/4346).
+- Fixed `pick(scene, rect2)` in WGLMakie [#4488](https://github.com/MakieOrg/Makie.jl/pull/4488)
 
 ## [0.21.14] - 2024-10-11
 

CairoMakie/test/runtests.jl

@@ -184,7 +184,8 @@ excludes = Set([
     "Textured meshscatter", # not yet implemented
     "Voxel - texture mapping", # not yet implemented
     "Miter Joints for line rendering", # CairoMakie does not show overlap here
-    "Scatter with FastPixel" # almost works, but scatter + markerspace=:data seems broken for 3D
+    "Scatter with FastPixel", # almost works, but scatter + markerspace=:data seems broken for 3D
+    "picking", # Not implemented
 ])
 
 functions = [:volume, :volume!, :uv_mesh]

GLMakie/src/picking.jl

@@ -14,7 +14,7 @@ function pick_native(screen::Screen, rect::Rect2i)
     rw, rh = widths(rect)
     w, h = size(screen.root_scene)
     ppu = screen.px_per_unit[]
-    if rx > 0 && ry > 0 && rx + rw <= w && ry + rh <= h
+    if rx >= 0 && ry >= 0 && rx + rw <= w && ry + rh <= h
         rx, ry, rw, rh = round.(Int, ppu .* (rx, ry, rw, rh))
         sid = zeros(SelectionID{UInt32}, rw, rh)
         glReadPixels(rx, ry, rw, rh, buff.format, buff.pixeltype, sid)

GLMakie/test/runtests.jl

@@ -25,7 +25,6 @@ end
 
 # run the unit test suite
 include("unit_tests.jl")
-include("picking.jl")
 
 @testset "Reference Tests" begin
     @testset "refimages" begin

WGLMakie/test/picking.jl → ReferenceTests/src/tests/generic_components.jl

@@ -1,4 +1,6 @@
-@testset "picking" begin
+# For things that aren't as plot related
+
+@reference_test "picking" begin
     scene = Scene(size = (230, 370))
     campixel!(scene)
 
@@ -24,13 +26,29 @@
     s2 = surface!(scene, 210..180, 80..110, rand(2, 2))
     hm2 = heatmap!(scene, [210, 180], [140, 170], [1 2; 3 4])
 
-    scene
+    scene # for easy reviewing of the plot
 
     # render one frame to generate picking texture
-    colorbuffer(scene);
+    colorbuffer(scene, px_per_unit = 2);
 
-    # TODO: Can we verify slow heatmap path somehow?
+    # verify that heatmap path is used for heatmaps
+    if Symbol(Makie.current_backend()) == :WGLMakie
+        @test length(faces(WGLMakie.create_shader(scene, hm).vertexarray)) > 2
+        @test length(faces(WGLMakie.create_shader(scene, hm2).vertexarray)) > 2
+    elseif Symbol(Makie.current_backend()) == :GLMakie
+        screen = scene.current_screens[1]
+        for plt in (hm, hm2)
+            robj = screen.cache[objectid(plt)]
+            shaders = robj.vertexarray.program.shader
+            names = [string(shader.name) for shader in shaders]
+            @test any(name -> endswith(name, "heatmap.vert"), names) && any(name -> endswith(name, "heatmap.frag"), names)
+        end
+    else
+        error("picking tests are only meant to run on GLMakie & WGLMakie")
+    end
 
+    # raw picking tests
+
     @testset "scatter" begin
         @test pick(scene, Point2f(20, 20)) == (sc1, 1)
         @test pick(scene, Point2f(29, 59)) == (sc1, 3)
@@ -262,4 +280,12 @@
         @test pick(scene, 111, 350) == (nothing, 0)
         @test pick(scene, 110, 351) == (nothing, 0)
     end
+
+    # grab all indices and generate a plot for them (w/ fixed px_per_unit)
+    full_screen = last.(pick(scene, scene.viewport[]))
+
+    scene2 = Scene(size = 2.0 .* widths(scene.viewport[]))
+    campixel!(scene2)
+    image!(scene2, full_screen, colormap = :viridis)
+    scene2
 end

ReferenceTests/src/tests/refimages.jl

@@ -50,3 +50,6 @@ end
 @testset "updating_plots" begin
     include("updating.jl")
 end
+@testset "generic_components" begin
+    include("generic_components.jl")
+end

WGLMakie/src/picking.jl

@@ -72,6 +72,10 @@ function Makie.pick(::Scene, screen::Screen, xy)
     return plot_matrix[1, 1]
 end
 
+function Makie.pick(::Scene, screen::Screen, r::Rect2)
+    return pick_native(screen, Rect2i(round.(minimum(r)), round.(widths(r))))
+end
+
 """
     ToolTip(figurelike, js_callback; plots=plots_you_want_to_hover)
 

WGLMakie/test/runtests.jl

@@ -39,8 +39,6 @@ edisplay = Bonito.use_electron_display(devtools=true)
     ReferenceTests.test_comparison(scores; threshold = 0.05)
 end
 
-include("picking.jl")
-
 @testset "memory leaks" begin
     Makie.CURRENT_FIGURE[] = nothing
     app = App(nothing)