Fwd: [Matplotlib-devel] need wxPython reviewers for PR

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Fwd: [Matplotlib-devel] need wxPython reviewers for PR

Chris Barker - NOAA Federal
Folks:

This from the matplotlib people. I don't think I'm going to have the time to do it, so if any of you use MPL, it would be great if you could do some testing.

-CHB

---------- Forwarded message ----------
From: Eric Firing <[hidden email]>
Date: Tue, Dec 20, 2016 at 9:40 PM
Subject: [Matplotlib-devel] need wxPython reviewers for PR
To: matplotlib development list <[hidden email]>


We have a PR to fix a problem in backend_wx.py, and as far as I know most of us who have been reviewing PRs recently are not WX users.  Hence this call for assistance: if you use the wxagg backend, and can build matplotlib from source, please try out the patch and review it:

https://github.com/matplotlib/matplotlib/pull/7652

Thanks.

Eric
_______________________________________________
Matplotlib-devel mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/matplotlib-devel



--

Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R            (206) 526-6959   voice
7600 Sand Point Way NE   (206) 526-6329   fax
Seattle, WA  98115       (206) 526-6317   main reception

[hidden email]

--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [Matplotlib-devel] need wxPython reviewers for PR

Matt Newville
Hi Chris, All,

On Wed, Dec 21, 2016 at 5:20 PM, Chris Barker <[hidden email]> wrote:
Folks:

This from the matplotlib people. I don't think I'm going to have the time to do it, so if any of you use MPL, it would be great if you could do some testing.


Author of that PR here. 

The basic issue is that the matplotlib backend provides a plotting canvas that subclasses wx.Panel() and does `self.CaptureMouse()` in the "mouse down" event handlers, and  "if self.HasCapture(): self.ReleaseMouse()"  in the "mouse up" event handlers.  Importantly, `wx.MouseCaptureLostEvent` events (neither EVT_MOUSE_CAPTURE_CHANGED nor EVT_MOUSE_CAPTURE_LOST) are not handled at all.  

The inclusion of the "self.CaptureMouse()" seems to cause intermittent C++ assertions that kills any subsequent mouse events on Mac OSX and Linux.  I never see these problems on Windows. 

What is slightly confusing to me is that the docs say that if CaptureMouse() is used that these events must be handled, but the docs for these events suggest that they operate only on MSW.   

Monkey-patching "self.CaptureMouse()" to be a null function avoids the assertions and all mouse events seem to work fine without this.  Therefore, the PR simply removes the call from the mouse down event handlers.

Does anyone actually use `CaptureMouse()`?

 --Matt

 
-CHB

---------- Forwarded message ----------
From: Eric Firing <[hidden email]>
Date: Tue, Dec 20, 2016 at 9:40 PM
Subject: [Matplotlib-devel] need wxPython reviewers for PR
To: matplotlib development list <[hidden email]>


We have a PR to fix a problem in backend_wx.py, and as far as I know most of us who have been reviewing PRs recently are not WX users.  Hence this call for assistance: if you use the wxagg backend, and can build matplotlib from source, please try out the patch and review it:

https://github.com/matplotlib/matplotlib/pull/7652

Thanks.

Eric
_______________________________________________
Matplotlib-devel mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/matplotlib-devel



--

Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R            <a href="tel:%28206%29%20526-6959" value="+12065266959" target="_blank">(206) 526-6959   voice
7600 Sand Point Way NE   <a href="tel:%28206%29%20526-6329" value="+12065266329" target="_blank">(206) 526-6329   fax
Seattle, WA  98115       <a href="tel:%28206%29%20526-6317" value="+12065266317" target="_blank">(206) 526-6317   main reception

[hidden email]
--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [Matplotlib-devel] need wxPython reviewers for PR

Chris Barker - NOAA Federal
On Wed, Dec 21, 2016 at 7:36 PM, Matt Newville <[hidden email]> wrote:

Author of that PR here. 


I should have known :-)

 
The basic issue is that the matplotlib backend provides a plotting canvas that subclasses wx.Panel() and does `self.CaptureMouse()` in the "mouse down" event handlers, and  "if self.HasCapture(): self.ReleaseMouse()"  in the "mouse up" event handlers.  Importantly, `wx.MouseCaptureLostEvent` events (neither EVT_MOUSE_CAPTURE_CHANGED nor EVT_MOUSE_CAPTURE_LOST) are not handled at all.  

The inclusion of the "self.CaptureMouse()" seems to cause intermittent C++ assertions that kills any subsequent mouse events on Mac OSX and Linux.  I never see these problems on Windows. 

I've used CaptureMouse() in non MPL apps with some success in the past.
 
Monkey-patching "self.CaptureMouse()" to be a null function avoids the assertions and all mouse events seem to work fine without this.  Therefore, the PR simply removes the call from the mouse down event handlers.

Why not just remove the CaptureMouse call from MPL?
 
Does anyone actually use `CaptureMouse()`?

It's really useful in some instances where you want to click down in a Panel, and use the mouse to drag around something -- maybe going off the Panel a bit, and want it to work like there is a larger panel. MAybe I didn't explain that right -- but it is useful for usability.

Though if it's causing problems, maybe better not to use it for every Mouse event.

could an end user application add it if they need it?

-CHB






 
 --Matt

 
-CHB

---------- Forwarded message ----------
From: Eric Firing <[hidden email]>
Date: Tue, Dec 20, 2016 at 9:40 PM
Subject: [Matplotlib-devel] need wxPython reviewers for PR
To: matplotlib development list <[hidden email]>


We have a PR to fix a problem in backend_wx.py, and as far as I know most of us who have been reviewing PRs recently are not WX users.  Hence this call for assistance: if you use the wxagg backend, and can build matplotlib from source, please try out the patch and review it:

https://github.com/matplotlib/matplotlib/pull/7652

Thanks.

Eric
_______________________________________________
Matplotlib-devel mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/matplotlib-devel



--

Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R            <a href="tel:%28206%29%20526-6959" value="+12065266959" target="_blank">(206) 526-6959   voice
7600 Sand Point Way NE   <a href="tel:%28206%29%20526-6329" value="+12065266329" target="_blank">(206) 526-6329   fax
Seattle, WA  98115       <a href="tel:%28206%29%20526-6317" value="+12065266317" target="_blank">(206) 526-6317   main reception

[hidden email]
--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.



--

Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R            (206) 526-6959   voice
7600 Sand Point Way NE   (206) 526-6329   fax
Seattle, WA  98115       (206) 526-6317   main reception

[hidden email]

--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [Matplotlib-devel] need wxPython reviewers for PR

Matt Newville


On Wed, Dec 21, 2016 at 10:44 PM, Chris Barker <[hidden email]> wrote:
On Wed, Dec 21, 2016 at 7:36 PM, Matt Newville <[hidden email]> wrote:

Author of that PR here. 


I should have known :-)

 
The basic issue is that the matplotlib backend provides a plotting canvas that subclasses wx.Panel() and does `self.CaptureMouse()` in the "mouse down" event handlers, and  "if self.HasCapture(): self.ReleaseMouse()"  in the "mouse up" event handlers.  Importantly, `wx.MouseCaptureLostEvent` events (neither EVT_MOUSE_CAPTURE_CHANGED nor EVT_MOUSE_CAPTURE_LOST) are not handled at all.  

The inclusion of the "self.CaptureMouse()" seems to cause intermittent C++ assertions that kills any subsequent mouse events on Mac OSX and Linux.  I never see these problems on Windows. 

I've used CaptureMouse() in non MPL apps with some success in the past.
Monkey-patching "self.CaptureMouse()" to be a null function avoids the assertions and all mouse events seem to work fine without this.  Therefore, the PR simply removes the call from the mouse down event handlers.

Why not just remove the CaptureMouse call from MPL?

That's exactly what the original PR did.    After testing on Windows, I did notice that some mouse events get lost without CaptureMouse().  Specifically, on Windows, doing LeftDown on the wx.Panel with the MPL plot canvas then dragging the mouse out of that Window means that subsequent Motion or LeftUp events are not sent to the MPL plot Panel.  

So, I changed the PR for the MPL backed to explicitly handle the MOUSE_CAPTURE events, with an event handler that just does

    if self.HasCapture(): self.ReleaseMouse()

This appears to work better (that is, not loose Mouse events when they stray out of the Window) on Windows as well as on Linux and Mac OSX.  With this change, I was not able to generate C++ assertions on any platform, though I am not certain that these cannot happen.

 
 
Does anyone actually use `CaptureMouse()`?

It's really useful in some instances where you want to click down in a Panel, and use the mouse to drag around something -- maybe going off the Panel a bit, and want it to work like there is a larger panel. MAybe I didn't explain that right -- but it is useful for usability.

 
Though if it's causing problems, maybe better not to use it for every Mouse event.

could an end user application add it if they need it?


I think this would be complicated for a MPL backend which is most typically invoked without the end users awareness and often even without the script writer explicitly doing anything beyond

    import matplotlib
    matplotlib.use("WXAgg")
 
so I think it has to be solved in the backend code.  My hope is that handling MOUSE_CAPTURE events as explicitly required in the documentation for CaptureMouse() will reliable avoid the errors.

--Matt

--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [Matplotlib-devel] need wxPython reviewers for PR

Chris Barker - NOAA Federal
On Thu, Dec 22, 2016 at 10:22 AM, Matt Newville <[hidden email]> wrote:
Why not just remove the CaptureMouse call from MPL?

That's exactly what the original PR did.    After testing on Windows, I did notice that some mouse events get lost without CaptureMouse().  Specifically, on Windows, doing LeftDown on the wx.Panel with the MPL plot canvas then dragging the mouse out of that Window means that subsequent Motion or LeftUp events are not sent to the MPL plot Panel.  

yup -- that is the point of CaptureMouse()

So, I changed the PR for the MPL backed to explicitly handle the MOUSE_CAPTURE events, with an event handler that just does

    if self.HasCapture(): self.ReleaseMouse()

This appears to work better (that is, not loose Mouse events when they stray out of the Window) on Windows as well as on Linux and Mac OSX. 

Yup -- I think that's the way to handle it. Oddly, you can get problems calling ReleaseMouse() if it's not currently captured -- I'd think wx would have a guard against that, but I guess not. This approach has worked for me in other apps -- so I think we're good.
 
With this change, I was not able to generate C++ assertions on any platform, though I am not certain that these cannot happen.

which is why some other testers would be good -- I don't think I"ll have time to do that, though :-(

-CHB



--

Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R            (206) 526-6959   voice
7600 Sand Point Way NE   (206) 526-6329   fax
Seattle, WA  98115       (206) 526-6317   main reception

[hidden email]

--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [Matplotlib-devel] need wxPython reviewers for PR

Matt Newville
Hi Chris, All,

OK, certain things have come to light.  I found a problem on OSX (Python 2.7.13 and wxPython 3.0.3.dev2486), even when handling EVT_MOUSE_CAPTURE_LOST.

In a matplotlib Panel, I have RIGHT_DOWN bring up a Popup window which might then cause a separate Frame to be created and raised.  When this happens on OSX )at least), the mouse capture appears to not be released by the Panel, so that a subsequent *_DOWN event in the Panel will try to capture the mouse, raising a C++ assertion error and effectively killing interactivity with the application.

To be clear, in MPL FigureCanvasWx subclasses wx.Panel, and includes (among other) event bindings:
        self.Bind(wx.EVT_RIGHT_DOWN, self._onRightButtonDown)
        self.Bind(wx.EVT_RIGHT_DCLICK, self._onRightButtonDClick)
        self.Bind(wx.EVT_RIGHT_UP, self._onRightButtonUp)

        self.Bind(wx.EVT_LEFT_DOWN, self._onLeftButtonDown)
        self.Bind(wx.EVT_LEFT_DCLICK, self._onLeftButtonDClick)
        self.Bind(wx.EVT_LEFT_UP, self._onLeftButtonUp)

        self.Bind(wx.EVT_MIDDLE_DOWN, self._onMiddleButtonDown)
        self.Bind(wx.EVT_MIDDLE_DCLICK, self._onMiddleButtonDClick)
        self.Bind(wx.EVT_MIDDLE_UP, self._onMiddleButtonUp)

In the original version of the PR, I added
        self.Bind(wx.EVT_MOUSE_CAPTURE_CHANGED, self._onCaptureLost)
        self.Bind(wx.EVT_MOUSE_CAPTURE_LOST, self._onCaptureLost)

to release the mouse, as suggested in the wxPython docs.

For simplicity, I have consolidated the calls to CaptureMouse() and RelaseMouse() to a single method:

    def _set_capture(self, capture=True): # Version 1: capture or release
        """control wx mouse capture """
        if capture:
            self.CaptureMouse()
        elif self.HasCapture():
            self.ReleaseMouse()

and then have `self._set_capture(True)` in the three _on*ButtonDown() methods and the three _on*ButtonDClick() methods, and have `self._set_capture(False)` in the three _on*ButtonUp() methods, and in the_onCaptureLost() method.  All of these event handlers also include 'event.Skip()'.

Again, with the actions described above, this version of _set_capture fails on Mac OSX, with
       wx._core.wxAssertionError: C++ assertion "!wxMouseCapture::IsInCaptureStack(this)" failed at /Users/robind/projects/buildbots/macosx-vm4/dist-osx-py27/Phoenix/ext/wxWidgets/src/common/wincmn.cpp(3271) in CaptureMouse(): Recapturing the mouse in the same window?

To recap the situation:

On Windows: Capturing the mouse is definitely needed in order for "out of window mouse Motion and Up events to be restored on the combination of ("MouseDown", "Mouse move out of Window", "Mouse move back into Window", "MouseUp").  The C++ assertions seem to never happen, and handling MOUSE_CAPTURE_LOST is not necessary.

On OSX (and I believe Linux): Capturing the mouse is not necessary for the "move mouse out of window and back" events to work as desired, and handling MOUSE_CAPTURE_LOST is not sufficient to prevent C++ assertions.

For an improved solution, this modified version of _set_capture()  works on OSX:

    def _set_capture(self, capture=True):  # Version 2: always release before capture
        """control wx mouse capture """
        if self.HasCapture():
            self.ReleaseMouse()
        if capture:
            self.CaptureMouse()

That is, *ALWAYS* release a captured mouse, and re-acquire if capture is requested.  Given the above information and previous experience, this will also always wotk on Windows.  Being away from a Windows machine, it is proven, but not tested ;).  But, to be clear, this also works:

    def _set_capture(self, capture=True):  # Version 3: don't bother with Capture/Release on posix
        """control wx mouse capture """
        if os.name == 'posix':
            return
        if capture:
            self.CaptureMouse()
        elif self.HasCapture():
            self.ReleaseMouse()

In fact, handling MOUSE_CAPTURE_LOST events appears to be unimportant.

The current version of the PR for MPL includes handling MOUSE_CAPTURE_LOST events (I believe they are unimportant, but having them in the code ought to prevent someone from making the mistake I made of thinking that adding them will solve some problem).  It uses the 2nd version of _set_capture() above, always releasing the mouse before trying to capture it.



Why not just remove the CaptureMouse call from MPL?

That's exactly what the original PR did.    After testing on Windows, I did notice that some mouse events get lost without CaptureMouse().  Specifically, on Windows, doing LeftDown on the wx.Panel with the MPL plot canvas then dragging the mouse out of that Window means that subsequent Motion or LeftUp events are not sent to the MPL plot Panel.  

yup -- that is the point of CaptureMouse()


Except on OSX on Linux, where CaptureMouse() is not necessary for this behavior.

 
So, I changed the PR for the MPL backed to explicitly handle the MOUSE_CAPTURE events, with an event handler that just does

    if self.HasCapture(): self.ReleaseMouse()

This appears to work better (that is, not loose Mouse events when they stray out of the Window) on Windows as well as on Linux and Mac OSX. 

Yup -- I think that's the way to handle it. Oddly, you can get problems calling ReleaseMouse() if it's not currently captured -- I'd think wx would have a guard against that, but I guess not. This approach has worked for me in other apps -- so I think we're good.
 

Actually, it wasn't good enough.  The panel was only releasing if captured, but it is also important to ensure the mouse is released before trying to re-capture it, at least on OSX.
 

With this change, I was not able to generate C++ assertions on any platform, though I am not certain that these cannot happen.

More testing would be helpful.

Cheers,
 
--Matt

--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [Matplotlib-devel] need wxPython reviewers for PR

Chris Barker - NOAA Federal
Thanks for doing all this Matt, sounds like you've found a good solution. Sorry I haven't had time to get in and test this.

One thought:

If you decide to go with a platform-checking version:

    def _set_capture(self, capture=True):  # Version 3: don't bother with Capture/Release on posix
        """control wx mouse capture """
        if os.name == 'posix':
            return
        if capture:
            self.CaptureMouse()
        elif self.HasCapture():
            self.ReleaseMouse()

Maybe check for example version instead of is version -- I can't remember the flag names, but something like wxGTK, wxWin, wxCocoa (or wxMac).

Also, while it sounds like (at least on some platforms) you need CaptureMouse to catch s mouse up after leaving the window, it's also often nice to have mouse motion events tracked even when outside the window -- so I'm all for capturing the mouse.

-CHB


CHB



In fact, handling MOUSE_CAPTURE_LOST events appears to be unimportant.

The current version of the PR for MPL includes handling MOUSE_CAPTURE_LOST events (I believe they are unimportant, but having them in the code ought to prevent someone from making the mistake I made of thinking that adding them will solve some problem).  It uses the 2nd version of _set_capture() above, always releasing the mouse before trying to capture it.



Why not just remove the CaptureMouse call from MPL?

That's exactly what the original PR did.    After testing on Windows, I did notice that some mouse events get lost without CaptureMouse().  Specifically, on Windows, doing LeftDown on the wx.Panel with the MPL plot canvas then dragging the mouse out of that Window means that subsequent Motion or LeftUp events are not sent to the MPL plot Panel.  

yup -- that is the point of CaptureMouse()


Except on OSX on Linux, where CaptureMouse() is not necessary for this behavior.

 
So, I changed the PR for the MPL backed to explicitly handle the MOUSE_CAPTURE events, with an event handler that just does

    if self.HasCapture(): self.ReleaseMouse()

This appears to work better (that is, not loose Mouse events when they stray out of the Window) on Windows as well as on Linux and Mac OSX. 

Yup -- I think that's the way to handle it. Oddly, you can get problems calling ReleaseMouse() if it's not currently captured -- I'd think wx would have a guard against that, but I guess not. This approach has worked for me in other apps -- so I think we're good.
 

Actually, it wasn't good enough.  The panel was only releasing if captured, but it is also important to ensure the mouse is released before trying to re-capture it, at least on OSX.
 

With this change, I was not able to generate C++ assertions on any platform, though I am not certain that these cannot happen.

More testing would be helpful.

Cheers,
 
--Matt

--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "wxPython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.