Hi, I've been playing with the pick infrastructure in 0.90 and find that it doesn't meet my needs. The issue is that events from all artists go to the same callback, so you are forced write your callback as a series of if statements for each artist which may receive a pick event. I would rather have an infrastructure more like a canvas, where I can register callbacks to particular handles for particular mouse events (enter, leave, motion, press and release). I would like to rely on the zorder attribute of artists when processing the events. If the callback succeeds, then the event is consumed and no more handles are checked. If the callback fails then the next lower zorder handle is checked. I can simulate what I want with the current infrastructure by having the mouse events call figure.pick() and the pick event capture every object that was picked, but it is ugly and not very efficient (not every object wants to track enter/motion/leave events for example, but they get asked on every pick even if they only want to monitor mouse clicks). Instead I would like to start by splitting the current pick method into two parts: contains(event,picker) which returns truth value,details pick(event) which generates the current pick event Later I can construct the infrastructure I want on the contains() function for each artist. Does this approach seem reasonable to those who will approve the patches I will send? Thanks, Paul Kienzle pki...@ni...
On 7/6/07, Paul Kienzle <pki...@ni...> wrote: > Hi, > > I've been playing with the pick infrastructure in 0.90 and find that > it doesn't meet my needs. The issue is that events from all artists > go to the same callback, so you are forced write your callback as a > series of if statements for each artist which may receive a pick event. I definitely agree that having to write one callback for all the different objects you may want to pick on is a bad design, so I'd be happy to see something better. Do you think it would make sense to register callbacks with a given artist -- eg if you are picked call me -- or with a class (eg all Line3D picks call me), or do you have another approach in mind? > I would rather have an infrastructure more like a canvas, where I > can register callbacks to particular handles for particular > mouse events (enter, leave, motion, press and release). I would like > to rely on the zorder attribute of artists when processing the > events. If the callback succeeds, then the event is consumed and > no more handles are checked. If the callback fails then the next > lower zorder handle is checked. > > I can simulate what I want with the current infrastructure by having > the mouse events call figure.pick() and the pick event capture > every object that was picked, but it is ugly and not very > efficient (not every object wants to track enter/motion/leave > events for example, but they get asked on every pick even if > they only want to monitor mouse clicks). > > Instead I would like to start by splitting the current pick method > into two parts: > contains(event,picker) which returns truth value,details > pick(event) which generates the current pick event > > Later I can construct the infrastructure I want on the contains() > function for each artist. > > Does this approach seem reasonable to those who will approve the > patches I will send? I don't really understand the contains part -- can you elaborate a little bit? I would definitely be amenable to accepting patches that improve the current API :-) If you want to support things like mouse over, in which the pick will have to be computed on mouse motion, we'll want to do some caching, eg in line, to make this more efficient. The current approach is fairly expensive but that's fine for single click picks, but not for continuous mouse motion. JDH
On Fri, Jul 06, 2007 at 10:15:48AM -0500, John Hunter wrote: > On 7/6/07, Paul Kienzle <pki...@ni...> wrote: > > Hi, > > > > I've been playing with the pick infrastructure in 0.90 and find that > > it doesn't meet my needs. The issue is that events from all artists > > go to the same callback, so you are forced write your callback as a > > series of if statements for each artist which may receive a pick event. > > I definitely agree that having to write one callback for all the > different objects you may want to pick on is a bad design, so I'd be > happy to see something better. Do you think it would make sense to > register callbacks with a given artist -- eg if you are picked call me > -- or with a class (eg all Line3D picks call me), or do you have > another approach in mind? I definitely want to be able to associate different callbacks with different specific artists. For example, I might want to define a gaussian peak that I want to drag around and match to the peak in some data. The data and the peak will both be represented using lines, but I only want to drag the peak, not the data. Having a default callback for a class may be useful in many places as well, such as automatically highlighting the corresponding legend entry on mouseover. For now I have no opinion on whether the additional complexity is worthwhile. - Paul
On Fri, Jul 06, 2007 at 10:15:48AM -0500, John Hunter wrote: > On 7/6/07, Paul Kienzle <pki...@ni...> wrote: > > Instead I would like to start by splitting the current pick method > > into two parts: > > contains(event,picker) which returns truth value,details > > pick(event) which generates the current pick event > > I don't really understand the contains part -- can you elaborate a little bit? > > I would definitely be amenable to accepting patches that improve the > current API :-) I have an initial patch against CVS available which implements contains() for most classes. Legend is too complex for simple hacks. I have a number of questions regarding details of implementation, in particular determining sizes of things on the plot. The question now is what to do with it? I can post it to the patch tracker on sourceforge, but I'm hesitant to do so since it still has issues. Do you want it there or on the list? Let me know... - Paul
On Fri, Jul 06, 2007 at 05:31:58PM -0400, Paul Kienzle wrote: > On Fri, Jul 06, 2007 at 10:15:48AM -0500, John Hunter wrote: > > On 7/6/07, Paul Kienzle <pki...@ni...> wrote: > > > Instead I would like to start by splitting the current pick method > > > into two parts: > > > contains(event,picker) which returns truth value,details > > > pick(event) which generates the current pick event > > > > I don't really understand the contains part -- can you elaborate a little bit? > > > > I would definitely be amenable to accepting patches that improve the > > current API :-) > > I have an initial patch against CVS available which implements > contains() for most classes. Legend is too complex for simple hacks. > I have a number of questions regarding details of implementation, > in particular determining sizes of things on the plot. > > The question now is what to do with it? I can post it to the patch > tracker on sourceforge, but I'm hesitant to do so since it still > has issues. Do you want it there or on the list? I submitted the 'contains' patch to the patch tracker on sourceforge. I've worked out most of the issues, including selecting lines instead of just points. I still haven't addressed legend picking, and I don't know how to handle line offsets. I'm also unhappy with relying on a cached renderer for some of the size checks (e.g., text extents). One solution is to send this along with the 'contains' test, though that would mean changing the interface to pick. Any other suggestions? In backend_bases.FigureCanvasBase.__init__ there is an "if False" statement. Turn it to True and all objects on every graph will be highlighted as the mouse hovers over them and restored when it moves away. I worked through many of the demos in the repository to check that the various objects are recognized. Sorry about the size of the patch --- I wanted to keep the system in a working state so I had to do everything. Also I spent some time writing __str__ methods for a lot of the artists so that it was easier for me to debug. Let me know if you want these stripped and sent as a separate patch. - Paul
On Sun, Jul 08, 2007 at 03:04:41AM -0400, Paul Kienzle wrote:
> I submitted the 'contains' patch to the patch tracker on sourceforge.
I've updated the 'contains' patch to handle figure enter/leave, at
least on wx backends, and fixed a minor hit test bug in ellipse.
I've also removed the special status of the pick event notifier,
using the mpl_connect event processing framework to handle it.
You can attach the enter/leave artist highlighting demo to a
figure using:
figure.canvas.mpl_connect("motion_notify_event",figure.canvas.hilite)
You can disable pick on a figure using:
fig.canvas.mpl_disconnect(fig.canvas.button_pick_id)
fig.canvas.mpl_disconnect(fig.canvas.scroll_pick_id)
Now I can work on an event handling framework which connects
directly to the artists rather than filtering all callbacks to
the same pick function. I'll mostly follow tk canvas handling
conventions unless anyone has suggests for a better approach.
- Paul
On 7/9/07, Paul Kienzle <pki...@ni...> wrote:
> On Sun, Jul 08, 2007 at 03:04:41AM -0400, Paul Kienzle wrote:
> > I submitted the 'contains' patch to the patch tracker on sourceforge.
>
> I've updated the 'contains' patch to handle figure enter/leave, at
> least on wx backends, and fixed a minor hit test bug in ellipse.
> I've also removed the special status of the pick event notifier,
> using the mpl_connect event processing framework to handle it.
>
Hey Paul, this is a very impressive piece of work. You weren't
kidding when you said you had a lot of work ahead of you in your first
post on the subject. The recent pick framework was a significant
improvement on what we had before it, which was barely usable, but
your stuff is going to make it *much* better. It is impressive that
your code doesn't break the existing API, but don't kill yourself over
maintaining API compatibiltiy. The pick_even was only added in the
0.90 release, and is still very lightly used. I would like to
preserve most of the functionality that is there (eg custom hit
testing functions, as you have) but if we need to tweak the API that
will be fine.
When you finish the architectural changes, I hope you'll provide us
with a nice example or two, and or modify the existing pick_event_demo
and data_browser code to use the cleaner API , eg registering with the
artist directly).
One minor bug I fixed for non wx backends in the (very cool)
figure.canvas.mpl_connect("motion_notify_event",figure.canvas.hilite)
functionality. gui_repaint is a wx only method, and I just commented
it out and replaced the "draw" with a draw_idle. This should work
across backends. Note draw_idle by default in backend_bases just
calls draw, but backends can override it to use their GUIs idle
handler. This is usually a good idea so you don't get an event pile
up. In particular, it is not implemented for backend_wx, so one of
you wx gurus/users may want to consider adding it, because for
interactive stuff like motion events, it can be a big win.
JDH
On Mon, Jul 09, 2007 at 10:37:35AM -0500, John Hunter wrote:
> One minor bug I fixed for non wx backends in the (very cool)
> figure.canvas.mpl_connect("motion_notify_event",figure.canvas.hilite)
> functionality. gui_repaint is a wx only method, and I just commented
> it out and replaced the "draw" with a draw_idle. This should work
> across backends. Note draw_idle by default in backend_bases just
> calls draw, but backends can override it to use their GUIs idle
> handler. This is usually a good idea so you don't get an event pile
> up. In particular, it is not implemented for backend_wx, so one of
> you wx gurus/users may want to consider adding it, because for
> interactive stuff like motion events, it can be a big win.
This change works for me under wx os OS X.
I got the gui_paint idea from examples/dynamic_demo_wx.py. Replacing
gui_paint with draw_idle seems to work okay there as well. I'll
create a patch. Do you small patches like this on tracker as well?
- Paul
On 7/9/07, Paul Kienzle <pki...@ni...> wrote: > This change works for me under wx os OS X. > I got the gui_paint idea from examples/dynamic_demo_wx.py. Replacing > gui_paint with draw_idle seems to work okay there as well. I'll > create a patch. Do you small patches like this on tracker as well? Sorry if I wasn't clear, I already made the change when I committed it, so all is well. As for patches, small patches are fine just sent to the site. If you are worried about them getting lost, post them to the sf site. But if you send me a sf id, I'll add you to the developers list and you can commit directly. You may want to check out the CODING_GUIDE i the svn repository. Thanks, JDH