MakieOrg / Makie.jl

Interactive data visualizations and plotting in Julia
https://docs.makie.org/stable
MIT License
2.42k stars 313 forks source link

mouse_selection / pick returns an incorrect index if the plot has more than 65535 points #703

Closed kragol closed 4 years ago

kragol commented 4 years ago

There seems to be a problem with the GLMakie implementation of pick that results in returned indices being capped at 65535. In practice, this means that pick does not work as expected for plots with more than 65535 points.

Here is a MWE, tested with the latest Makie release (0.11.1) and also the master branch.

using Makie

scene = scatter(1:100_000,1:100_000)
scatterplot = scene[end]

on(scene.events.mouseposition) do mp
    plot, idx = mouse_selection(scene)
    if plot == scatterplot
        @show idx
    end
end

display(scene)

This displays a simple scatter plot with 100_000 points and prints the index of the hovered point to stdout. Everything works as expected while hovering points with x,y coordinates <= 65535. However, the returned index is always 65535 for points with coordinates >= 65535.

The issue seems to be related to the implementation of pick_native in GLMakie ( at screen.jl:392 on master):

function pick_native(screen::Screen, xy::Vec{2, Float64})
    isopen(screen) || return SelectionID{Int}(0, 0)
    sid = Base.RefValue{SelectionID{UInt16}}()
    window_size = widths(screen)
    fb = screen.framebuffer
    buff = fb.objectid
    glBindFramebuffer(GL_FRAMEBUFFER, fb.id[1])
    glReadBuffer(GL_COLOR_ATTACHMENT1)
    x, y = floor.(Int, xy)
    w, h = window_size
    if x > 0 && y > 0 && x <= w && y <= h
        glReadPixels(x, y, 1, 1, buff.format, buff.pixeltype, sid)
        return convert(SelectionID{Int}, sid[])
    end
    return return SelectionID{Int}(0, 0)
end

Here, the returned index is typed as UInt16 which explains the 65535 threshold. However, simply using UInt32 instead here does not solve the problem (it seems to result in an error), which seems to indicate that glReadPixels somehow returns an UInt16. I gave up on further investigations as I am not familiar enough with OpenGL.

As for a workaround one can manually look for the point minimizing the distance to the mouse pointer. For instance, this can be done in data space by comparing the mouse coordinates with the data coordinates

on(AbstractPlotting.mouse_in_scene(scene)) do mp
    plot, idx = mouse_selection(scene)
    if plot == scatterplot
        if idx >= 65535
            mouse_coordinates_data = to_world(scene,Point(mp))
            data_coords = scatterplot[1][]
            _,idx = findmin( (x -> x[1]^2 + x[1]^2).( data_coords .- mouse_coordinates_data ) )
        end
        @show idx
    end
end

( using mouse_in_scene here instead of scene.events.mouseposition to avoid issues with subscenes. It doesn't actually change anything for this example. )

However I am not sure how to do the distance minimization in "pixel space", which would be a more intuitive behaviour. The problem is that I haven't found the function mapping coordinates to pixels (the inverse of to_world)...

ffreyer commented 4 years ago

The ids are written to a texture when the plot is rendered, which contains two UInt8s. Changing the UInt16 here to a UInt32 won't change that.

I think it might be enough to change https://github.com/JuliaPlots/GLMakie.jl/blob/3e9caf368ba3075a21252f58cadfb8bd521bb34e/src/glwindow.jl#L35 and https://github.com/JuliaPlots/GLMakie.jl/blob/3e9caf368ba3075a21252f58cadfb8bd521bb34e/src/glwindow.jl#L162 to GLuint with sid = Base.RefValue{SelectionID{UInt32}}().

kragol commented 4 years ago

Thanks @ffreyer , that seems to work (see the PR).