From 833be1d4b53e48337ed38c06cdc6ff845502b79b Mon Sep 17 00:00:00 2001 From: Frederic Freyer Date: Wed, 16 Oct 2024 23:07:18 +0200 Subject: [PATCH] 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 | 1 + CairoMakie/test/runtests.jl | 3 +- GLMakie/src/picking.jl | 2 +- GLMakie/test/picking.jl | 272 ------------------ GLMakie/test/runtests.jl | 1 - .../src/tests/generic_components.jl | 34 ++- ReferenceTests/src/tests/refimages.jl | 3 + WGLMakie/src/picking.jl | 4 + WGLMakie/test/runtests.jl | 2 - 9 files changed, 41 insertions(+), 281 deletions(-) delete mode 100644 GLMakie/test/picking.jl rename WGLMakie/test/picking.jl => ReferenceTests/src/tests/generic_components.jl (89%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1b8269163..250773f4339 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/CairoMakie/test/runtests.jl b/CairoMakie/test/runtests.jl index b131a1dff32..01b2f4283a9 100644 --- a/CairoMakie/test/runtests.jl +++ b/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] diff --git a/GLMakie/src/picking.jl b/GLMakie/src/picking.jl index 573a52dd26b..47270f9df8e 100644 --- a/GLMakie/src/picking.jl +++ b/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) diff --git a/GLMakie/test/picking.jl b/GLMakie/test/picking.jl deleted file mode 100644 index 3b0fd6aadfa..00000000000 --- a/GLMakie/test/picking.jl +++ /dev/null @@ -1,272 +0,0 @@ -@testset "picking" begin - scene = Scene(size = (230, 370)) - campixel!(scene) - - sc1 = scatter!(scene, [20, NaN, 20], [20, NaN, 50], marker = Rect, markersize = 20) - sc2 = scatter!(scene, [50, 50, 20, 50], [20, 50, 80, 80], marker = Circle, markersize = 20, color = [:red, :red, :transparent, :red]) - ms = meshscatter!(scene, [20, NaN, 50], [110, NaN, 110], markersize = 10) - l1 = lines!(scene, [20, 50, 50, 20, 20], [140, 140, 170, 170, 140], linewidth = 10) - l2 = lines!(scene, [20, 50, NaN, 20, 50], [200, 200, NaN, 230, 230], linewidth = 20, linecap = :round) - ls = linesegments!(scene, [20, 50, NaN, NaN, 20, 50], [260, 260, NaN, NaN, 290, 290], linewidth = 20, linecap = :square) - tp = text!(scene, Point2f[(15, 320), (NaN, NaN), (15, 350)], text = ["█ ●", "hi", "●"], fontsize = 20, align = (:left, :center)) - t = tp.plots[1] - - i = image!(scene, 80..140, 20..50, rand(RGBf, 3, 2), interpolate = false) - s = surface!(scene, 80..140, 80..110, rand(3, 2), interpolate = false) - hm = heatmap!(scene, [80, 110, 140], [140, 170], [1 4; 2 5; 3 6]) - # mesh coloring should match triangle placements - m = mesh!(scene, Point2f.([80, 80, 110, 110], [200, 230, 200, 230]), [1 2 3; 2 3 4], color = [1,1,1,2]) - vx = voxels!(scene, [65, 155], [245, 305], [-1, 1], reshape([1,2,3,4,5,6], (3,2,1)), shading = NoShading) - vol = volume!(scene, 80..110, 320..350, -1..1, rand(2,2,2)) - - # reversed axis - i2 = image!(scene, 210..180, 20..50, rand(RGBf, 2, 2)) - s2 = surface!(scene, 210..180, 80..110, rand(2, 2)) - hm2 = heatmap!(scene, [210, 180], [140, 170], [1 2; 3 4]) - - scene - - # render one frame to generate picking texture - colorbuffer(scene); - - # verify that heatmap doesn't get optimized away - @test begin - screen = scene.current_screens[1] - robj = screen.renderlist[11][3] # text generates a text + line plot - shaders = robj.vertexarray.program.shader - names = [string(shader.name) for shader in shaders] - any(name -> endswith(name, "heatmap.vert"), names) && any(name -> endswith(name, "heatmap.frag"), names) - end - - @testset "scatter" begin - @test pick(scene, Point2f(20, 20)) == (sc1, 1) - @test pick(scene, Point2f(30, 60)) == (sc1, 3) - @test pick(scene, Point2f(57, 58)) == (nothing, 0) # maybe fragile - @test pick(scene, Point2f(57, 13)) == (sc2, 1) # maybe fragile - @test pick(scene, Point2f(20, 80)) == (nothing, 0) - @test pick(scene, Point2f(50, 80)) == (sc2, 4) - end - - @testset "meshscatter" begin - @test pick(scene, (20, 110)) == (ms, 1) - @test pick(scene, (44, 117)) == (ms, 3) - @test pick(scene, (57, 117)) == (nothing, 0) - end - - @testset "lines" begin - # Bit less precise since joints aren't strictly one segment or the other - @test pick(scene, 22, 140) == (l1, 2) - @test pick(scene, 48, 140) == (l1, 2) - @test pick(scene, 50, 142) == (l1, 3) - @test pick(scene, 50, 168) == (l1, 3) - @test pick(scene, 48, 170) == (l1, 4) - @test pick(scene, 22, 170) == (l1, 4) - @test pick(scene, 20, 168) == (l1, 5) - @test pick(scene, 20, 142) == (l1, 5) - - # more precise checks around borders (these maybe off by a pixel due to AA) - @test pick(scene, 20, 200) == (l2, 2) - @test pick(scene, 30, 210) == (l2, 2) - @test pick(scene, 30, 211) == (nothing, 0) - @test pick(scene, 60, 200) == (l2, 2) - @test pick(scene, 61, 200) == (nothing, 0) - @test pick(scene, 57, 207) == (l2, 2) - @test pick(scene, 57, 208) == (nothing, 0) - @test pick(scene, 40, 230) == (l2, 5) # nan handling - end - - @testset "linesegments" begin - @test pick(scene, 8, 260) == (nothing, 0) # off by a pixel due to AA - @test pick(scene, 10, 260) == (ls, 2) - @test pick(scene, 30, 270) == (ls, 2) - @test pick(scene, 30, 271) == (nothing, 0) - @test pick(scene, 60, 260) == (ls, 2) - @test pick(scene, 61, 260) == (nothing, 0) - - @test pick(scene, 8, 290) == (nothing, 0) # off by a pixel due to AA - @test pick(scene, 10, 290) == (ls, 6) - @test pick(scene, 30, 280) == (ls, 6) - @test pick(scene, 30, 278) == (nothing, 0) # off by a pixel due to AA - @test pick(scene, 60, 290) == (ls, 6) - @test pick(scene, 61, 290) == (nothing, 0) - end - - @testset "text" begin - @test pick(scene, 15, 320) == (t, 1) - @test pick(scene, 13, 320) == (nothing, 0) - # edge checks, further outside due to AA - @test pick(scene, 20, 306) == (nothing, 0) - @test pick(scene, 20, 320) == (t, 1) - @test pick(scene, 20, 333) == (nothing, 0) - # space is counted - @test pick(scene, 43, 320) == (t, 3) - @test pick(scene, 48, 325) == (t, 3) - @test pick(scene, 49, 326) == (nothing, 0) - # characters at nan position are counted - @test pick(scene, 20, 350) == (t, 6) - end - - @testset "image" begin - # outside border - for p in vcat( - [(x, y) for x in (79, 141) for y in (21, 49)], - [(x, y) for x in (81, 139) for y in (19, 51)] - ) - @test pick(scene, p) == (nothing, 0) - end - - # cell centered checks - @test pick(scene, 90, 30) == (i, 1) - @test pick(scene, 110, 30) == (i, 2) - @test pick(scene, 130, 30) == (i, 3) - @test pick(scene, 90, 40) == (i, 4) - @test pick(scene, 110, 40) == (i, 5) - @test pick(scene, 130, 40) == (i, 6) - - # precise check (around cell intersection) - @test pick(scene, 100-1, 35-1) == (i, 1) - @test pick(scene, 100+1, 35-1) == (i, 2) - @test pick(scene, 100-1, 35+1) == (i, 4) - @test pick(scene, 100+1, 35+1) == (i, 5) - - @test pick(scene, 120-1, 35-1) == (i, 2) - @test pick(scene, 120+1, 35-1) == (i, 3) - @test pick(scene, 120-1, 35+1) == (i, 5) - @test pick(scene, 120+1, 35+1) == (i, 6) - - # reversed axis check - @test pick(scene, 200, 30) == (i2, 1) - @test pick(scene, 190, 30) == (i2, 2) - @test pick(scene, 200, 40) == (i2, 3) - @test pick(scene, 190, 40) == (i2, 4) - end - - @testset "surface" begin - # outside border - for p in vcat( - [(x, y) for x in (79, 141) for y in (81, 109)], - [(x, y) for x in (81, 139) for y in (79, 111)] - ) - @test pick(scene, p) == (nothing, 0) - end - - # cell centered checks - @test pick(scene, 90, 90) == (s, 1) - @test pick(scene, 110, 90) == (s, 2) - @test pick(scene, 130, 90) == (s, 3) - @test pick(scene, 90, 100) == (s, 4) - @test pick(scene, 110, 100) == (s, 5) - @test pick(scene, 130, 100) == (s, 6) - - # precise check (around cell intersection) - @test pick(scene, 95-1, 95-1) == (s, 1) - @test pick(scene, 95+1, 95-1) == (s, 2) - @test pick(scene, 95-1, 95+1) == (s, 4) - @test pick(scene, 95+1, 95+1) == (s, 5) - - @test pick(scene, 125-1, 95-1) == (s, 2) - @test pick(scene, 125+1, 95-1) == (s, 3) - @test pick(scene, 125-1, 95+1) == (s, 5) - @test pick(scene, 125+1, 95+1) == (s, 6) - - # reversed axis check - @test pick(scene, 200, 90) == (s2, 1) - @test pick(scene, 190, 90) == (s2, 2) - @test pick(scene, 200, 100) == (s2, 3) - @test pick(scene, 190, 100) == (s2, 4) - end - - @testset "heatmap" begin - # outside border - for p in vcat( - [(x, y) for x in (64, 156) for y in (126, 184)], - [(x, y) for x in (66, 154) for y in (124, 186)] - ) - @test pick(scene, p) == (nothing, 0) - end - - # cell centered checks - @test pick(scene, 80, 140) == (hm, 1) - @test pick(scene, 110, 140) == (hm, 2) - @test pick(scene, 140, 140) == (hm, 3) - @test pick(scene, 80, 170) == (hm, 4) - @test pick(scene, 110, 170) == (hm, 5) - @test pick(scene, 140, 170) == (hm, 6) - - # precise check (around cell intersection) - @test pick(scene, 94, 154) == (hm, 1) - @test pick(scene, 96, 154) == (hm, 2) - @test pick(scene, 94, 156) == (hm, 4) - @test pick(scene, 96, 156) == (hm, 5) - - @test pick(scene, 124, 154) == (hm, 2) - @test pick(scene, 126, 154) == (hm, 3) - @test pick(scene, 124, 156) == (hm, 5) - @test pick(scene, 126, 156) == (hm, 6) - - # reversed axis check - @test pick(scene, 210, 140) == (hm2, 1) - @test pick(scene, 180, 140) == (hm2, 2) - @test pick(scene, 210, 170) == (hm2, 3) - @test pick(scene, 180, 170) == (hm2, 4) - end - - @testset "mesh" begin - @test pick(scene, 80, 200)[1] == m - @test pick(scene, 79, 200) == (nothing, 0) - @test pick(scene, 80, 199) == (nothing, 0) - @test pick(scene, 81, 201) == (m, 3) - @test pick(scene, 81, 225) == (m, 3) - @test pick(scene, 105, 201) == (m, 3) - @test pick(scene, 85, 229) == (m, 4) - @test pick(scene, 109, 205) == (m, 4) - @test pick(scene, 109, 229) == (m, 4) - @test pick(scene, 109, 229)[1] == m - @test pick(scene, 111, 230) == (nothing, 0) - @test pick(scene, 110, 231) == (nothing, 0) - end - - @testset "voxel" begin - # outside border - for p in vcat( - [(x, y) for x in (64, 246) for y in (126, 184)], - [(x, y) for x in (66, 244) for y in (124, 186)] - ) - @test pick(scene, p) == (nothing, 0) - end - - # cell centered checks - @test pick(scene, 80, 260) == (vx, 1) - @test pick(scene, 110, 260) == (vx, 2) - @test pick(scene, 140, 260) == (vx, 3) - @test pick(scene, 80, 290) == (vx, 4) - @test pick(scene, 110, 290) == (vx, 5) - @test pick(scene, 140, 290) == (vx, 6) - - # precise check (around cell intersection) - @test pick(scene, 94, 274) == (vx, 1) - @test pick(scene, 96, 274) == (vx, 2) - @test pick(scene, 94, 276) == (vx, 4) - @test pick(scene, 96, 276) == (vx, 5) - - @test pick(scene, 124, 274) == (vx, 2) - @test pick(scene, 126, 274) == (vx, 3) - @test pick(scene, 124, 276) == (vx, 5) - @test pick(scene, 126, 276) == (vx, 6) - end - - @testset "volume" begin - # volume doesn't produce indices because we can't resolve the depth of - # the pick - @test pick(scene, 80, 320)[1] == vol - @test pick(scene, 79, 320) == (nothing, 0) - @test pick(scene, 80, 319) == (nothing, 0) - @test pick(scene, 81, 321) == (vol, 0) - @test pick(scene, 81, 349) == (vol, 0) - @test pick(scene, 109, 321) == (vol, 0) - @test pick(scene, 109, 349) == (vol, 0) - @test pick(scene, 109, 349)[1] == vol - @test pick(scene, 111, 350) == (nothing, 0) - @test pick(scene, 110, 351) == (nothing, 0) - end -end \ No newline at end of file diff --git a/GLMakie/test/runtests.jl b/GLMakie/test/runtests.jl index f4047d25865..2aa9d1d5765 100644 --- a/GLMakie/test/runtests.jl +++ b/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 diff --git a/WGLMakie/test/picking.jl b/ReferenceTests/src/tests/generic_components.jl similarity index 89% rename from WGLMakie/test/picking.jl rename to ReferenceTests/src/tests/generic_components.jl index dac522e26ad..1e55b31af39 100644 --- a/WGLMakie/test/picking.jl +++ b/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 \ No newline at end of file diff --git a/ReferenceTests/src/tests/refimages.jl b/ReferenceTests/src/tests/refimages.jl index a73defc502a..1f425a04f76 100644 --- a/ReferenceTests/src/tests/refimages.jl +++ b/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 diff --git a/WGLMakie/src/picking.jl b/WGLMakie/src/picking.jl index 37b57feb392..74c06e227f6 100644 --- a/WGLMakie/src/picking.jl +++ b/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) diff --git a/WGLMakie/test/runtests.jl b/WGLMakie/test/runtests.jl index 346e1ff5ee8..f625dbe893d 100644 --- a/WGLMakie/test/runtests.jl +++ b/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)