Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

Change ranged pick to return all plots sorted by distance #177

Merged
merged 6 commits into from
May 7, 2021

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Apr 10, 2021

As an alternative (or addition) to #174.

To ignore a scatter plot

... with #174 :

fig, ax, p = scatter(rand(Point2f0, 10), pickable = false)
on(events(fig.scene).mouseposition) do mp
    plt, idx = pick(fig.scene, mp, 30)
    println(typeof(plt), " @ ", idx)
end

... with this :

fig, ax, p = scatter(rand(Point2f0, 10), e)
on(events(fig.scene).mouseposition) do mp
    for (plt, idx) in pick(fig.scene, mp, 30)
        if plt != p
            println(typeof(plt), " @ ", idx)
            break
        end
    end
end

I personally prefer #174. It makes more sense to me to for a plot to be pickable or not, than for that to be decided in pick. It's also simpler to use, especially if you want to avoid picking plots from, for example, axis. (For that you'd need to compare to fig.scene.plots)

@ffreyer
Copy link
Collaborator Author

ffreyer commented Apr 12, 2021

I have this as a draft atm because I'm still thinking about how to organize this. Currently we have:

  • pick(scene, xy) to pick the (plot_or_nothing, idx) at xy
  • pick(scene, xy, range) to pick the closest (plot, idx) to xy in a (xy .- range, xy .+ range) region
  • pick(screen, rect) to get all the (plot, idx) in rect. I'm not sure if this one has a scene method...

What I want to implement is a pick which returns the closest (plot, idx) to a xy in a given range or rect while ignoring some plots. The options I can think of are:

  1. Implement a pick(scene, xy, range, blacklist) that returns the closest (plot, idx) pair to xy that is not in blacklist. This is easier to optimize/to do without allocations, but not very general.
  2. Adjust pick(scene, xy, range) to return all plots in range, sorted by distance from xy. With this we could do handle blacklist (or whitelist) in AbstractPlotting or let the user deal with it.
  3. Adjust pick(screen, rect) to return a vector of (plot, idx, pos) triples. With this we could move pick(scene, xy, range) and blacklists (and whitelists) to AbstractPlotting entirely, if I'm not mistaken.
  4. Adjust pick(screen, rect) to map the matrix of SelectionID to a matrix of (plot, idx) pairs. This should allow moving ranged and blacklist (and whitelist) picks to AbstractPlotting. It would also make it easier to implement other things in AbstractPlotting, e.g. pick_most_frequent(scene, rect).

The pr currently has option 2 implemented. What do you think would be best @SimonDanisch ?

@ffreyer
Copy link
Collaborator Author

ffreyer commented Apr 17, 2021

I went with option 4 now, and made some changes in AbstractPlotting

@ffreyer ffreyer marked this pull request as ready for review April 17, 2021 17:00
@ffreyer
Copy link
Collaborator Author

ffreyer commented Apr 17, 2021

Tests fail because some methods require definitions from JuliaPlots/AbstractPlotting.jl#696. They should pass with those. (They fail locally for me at @test idx == N-1, but that has been the case before as well.)

@SimonDanisch SimonDanisch merged commit b01e8e5 into JuliaPlots:master May 7, 2021
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants