[Matplotlib-devel] need wxPython reviewers for PR

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

···

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

Chris.Barker@noaa.gov

Hi Chris, All,

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

···

On Wed, Dec 21, 2016 at 5:20 PM, Chris Barker <chris.barker@noaa.gov> wrote:

-CHB

---------- Forwarded message ----------
From: Eric Firing <efiring@hawaii.edu>
Date: Tue, Dec 20, 2016 at 9:40 PM
Subject: [Matplotlib-devel] need wxPython reviewers for PR
To: matplotlib development list <matplotlib-devel@python.org>

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:

backend_wx.py: ReleaseMouse() when the capture is lost by newville · Pull Request #7652 · matplotlib/matplotlib · GitHub

Thanks.

Eric
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@python.org
Matplotlib-devel Info Page

--

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

Chris.Barker@noaa.gov
--
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 wxpython-users+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Author of that PR here.

I should have known :slight_smile:

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

···

On Wed, Dec 21, 2016 at 7:36 PM, Matt Newville <newville@cars.uchicago.edu> wrote:

--Matt

-CHB

---------- Forwarded message ----------
From: Eric Firing <efiring@hawaii.edu>
Date: Tue, Dec 20, 2016 at 9:40 PM
Subject: [Matplotlib-devel] need wxPython reviewers for PR
To: matplotlib development list <matplotlib-devel@python.org>

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:

backend_wx.py: ReleaseMouse() when the capture is lost by newville · Pull Request #7652 · matplotlib/matplotlib · GitHub

Thanks.

Eric
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@python.org
Matplotlib-devel Info Page

--

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

Chris.Barker@noaa.gov
--
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 wxpython-users+unsubscribe@googlegroups.com.
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 wxpython-users+unsubscribe@googlegroups.com.
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

Chris.Barker@noaa.gov

Author of that PR here.

I should have known :slight_smile:

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

···

On Wed, Dec 21, 2016 at 10:44 PM, Chris Barker <chris.barker@noaa.gov> wrote:

On Wed, Dec 21, 2016 at 7:36 PM, Matt Newville <newville@cars.uchicago.edu > > 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 :frowning:

-CHB

···

On Thu, Dec 22, 2016 at 10:22 AM, Matt Newville <newville@cars.uchicago.edu> wrote:

--

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

Chris.Barker@noaa.gov

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 _onButtonDown() methods and the three _onButtonDClick() 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](http://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.

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

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.

More testing would be helpful.

Cheers,

yup – that is the point of CaptureMouse()

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.

Why not just remove the CaptureMouse call from MPL?

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.

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.

–Matt

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](http://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 wxpython-users+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.