[Bug]: scroll_event is broken after motion_notify_event in WXAgg #22211

komoto48g · 2022-01-12T09:18:23Z

Bug summary

Scrolling the mouse wheel returns a broken xy value.
After any key is pressed, the xy value will be correct during no motion.

Code for reproduction

import matplotlib
from matplotlib.backends.backend_wxagg import FigureCanvasWxAgg as FigureCanvas
from matplotlib.backends.backend_wxagg import NavigationToolbar2WxAgg as Toolbar
from matplotlib.figure import Figure
import wx

class Plugin(wx.Panel):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)

        self.figure = Figure(facecolor='white', figsize=(.1,.1)) # inches
        self.canvas = FigureCanvas(self, -1, self.figure)
        self.toolbar = Toolbar(self.canvas)

        self.SetSizer(wx.BoxSizer(wx.VERTICAL))
        self.Sizer.AddMany((
            (self.canvas, 1, wx.EXPAND|wx.ALL, 0),
            (self.toolbar, 0, wx.EXPAND|wx.ALL, 2),
        ))

        self.canvas.mpl_connect('scroll_event', print)
        self.canvas.mpl_connect('motion_notify_event', print)

        self.figure.clear()
        self.figure.add_subplot(111)

if __name__ == '__main__':
    app = wx.App()
    frm = wx.Frame(None)
    frm.panel = Plugin(frm)
    frm.Show()
    app.MainLoop()

Actual outcome

When you scroll on the canvas, you will see corrupted data such as the following:

scroll_event: xy=(32761, 231) xydata=(None, None) button=down dblclick=False inaxes=None`
                  ^^^^^ corrupted

Expected outcome

After any key is pressed (but no motion), you will see a message such as the following:

scroll_event: xy=(153, 107) xydata=(0.35..., 0.45...) button=up dblclick=False inaxes=AxesSubplot(0.125,0.11;0.775x0.77)
                  ^^^ ok

Additional information

wxPython 4.1.1 msw (phoenix) wxWidgets 3.1.5
backend_bases.py:1211: (3.5.x)
backend_bases.py:1229: (3.4.x)

class Event:
    def __init__(self, name, canvas, guiEvent=None):
        self.name = name
        self.canvas = canvas
        self.guiEvent = guiEvent
        ^^^^^^^^^^^^^^^^^^^^^^^^ Don't keep reference to the wx.Event object.
                                 !! Otherwise it would be a potential bug.

To fix this issue temporarily, change it to dummy data.
So far, self.guiEvent doesn't seem to be referenced anywhere.

-        self.guiEvent = guiEvent
+        self.guiEvent = None

A related issue is discussed in the Phoenix issue tracker.
wxWidgets/Phoenix#2034

Operating system

Windows 10

Matplotlib Version

3.4.0, 3.5.1

Matplotlib Backend

WXAgg

Python version

3.8.6, 3.9.9

Jupyter version

No response

Installation

pip

The text was updated successfully, but these errors were encountered:

anntzer · 2022-01-12T09:45:22Z

guiEvent is public API and would need a deprecation period; furthermore, looking at the linked wx issue, it seems like this is a bug on wx's side? Alternatively we can use the evt.__class__(evt) hack mentioned in the thread, but "official" guidance from the wx project would be nice.

komoto48g · 2022-01-12T12:35:03Z

Thank you for reviewing.

it seems like this is a bug on wx's side?

I am not sure about this, so I posted this topic in [Discuss wxPython] and asked for feedback or official guidance.

DietmarSchwertberger · 2022-03-11T19:29:03Z

Whether there is a bug involved is not yet clear. I have had a look at the C++ code, but that's very complex. There could be an issue in the wxWidgets <-> sip interaction, but that's beyond my capabilities...
Anyway, it's no good style to keep references to short lived objects. For PaintDC it's explicitely documented that no reference is to be stored. For Event, it's not (yet) documented.

The version evt.__class__(evt) or evt.Clone() would care for case that someone copies guiEvent, but I would say this is not a useful use case.

I think the best solution would be delete the reference at the end of the matplotlib event handling.

E.g. in backend_bases.py motion_notify_event add one of these lines at the end:

        event.guiEvent = None
or:
        del event.guiEvent

I would recommend the del call.
This should be done for all Event types, even if there is currently no issue.
Unfortunately there seems to be no hook which would cover all event handlers at once.
(Adding Event.__del__ would not work.)

komoto48g · 2022-03-15T14:18:38Z

Thank you for taking the time to review.

The version evt.__class__(evt) or evt.Clone() would care for case that someone copies guiEvent, but I would say this is not a useful use case.

I think the best solution would be delete the reference at the end of the matplotlib event handling.

If someone holds the reference, even if del event.guiEvent is called, it will only decrement the ref-counter.
I recognized that the behavior of wx is by design, not a bug.
I think the best solution would be to make guiEvent API deprecated.

There are resonable reasons to make (problematic) guiEvent API deprecated.

For developers, guiEvent is not referenced anywhere in the matplotlib/WXAgg module, thus it has little impact.
For users, guiEvent is not very useful because it is easy to chain the event and refer it, e.g.,

    self.canvas.mpl_connect('motion_notify_event', self.on_motion_notify)
    self.canvas.Bind(wx.EVT_MOTION, self.OnMotionNotify)
    ...

anntzer · 2022-03-15T14:27:24Z

FWIW I have relied on guiEvent e.g. at https://github.com/anntzer/mplinorm/blob/f0d340b8cc63b7b000e18a466f53671ac92e7b69/lib/mplinorm.py#L163-L194. There may be workarounds but some documentation on how to achieve them would be nice (e.g. in my case I need a correspondence between the motion_notify_event handled by Matplotlib and the EVT_MOTION from wx).

tacaswell · 2022-03-17T03:38:01Z

I am pretty 👎 on deprecating evt.guiEvent because that is our escape hatch to get the event from the underlying UI through to callbacks registered to Matplotlib. It is indeed a brittle API (as the type is dependent on which backend you are using), but it is still useful and present on all of the GUI backends.

The source of our problem is

matplotlib/lib/matplotlib/backend_bases.py

Line 1298 in 96af8c5

lastevent = None # the last event that was triggered before this one

and

matplotlib/lib/matplotlib/backend_bases.py

Lines 1332 to 1356 in 96af8c5

    
           def _update_enter_leave(self): 
        
               """Process the figure/axes enter leave events.""" 
        
               if LocationEvent.lastevent is not None: 
        
                   last = LocationEvent.lastevent 
        
                   if last.inaxes != self.inaxes: 
        
                       # process Axes enter/leave events 
        
                       try: 
        
                           if last.inaxes is not None: 
        
                               last.canvas.callbacks.process('axes_leave_event', last) 
        
                       except Exception: 
        
                           pass 
        
                           # See ticket 2901582. 
        
                           # I think this is a valid exception to the rule 
        
                           # against catching all exceptions; if anything goes 
        
                           # wrong, we simply want to move on and process the 
        
                           # current event. 
        
                       if self.inaxes is not None: 
        
                           self.canvas.callbacks.process('axes_enter_event', self) 
        
               else: 
        
                   # process a figure enter event 
        
                   if self.inaxes is not None: 
        
                       self.canvas.callbacks.process('axes_enter_event', self) 
        
               LocationEvent.lastevent = self

which cache the last event as a class level attribute on Location event when we enter / leave an Axes.

I think possible fixes here are:

clone the Event while stripping the GUI event when we stash LocationEvent.lastevent (bunch of work, maybe not easy to do given that this logic is currently called from the __init__ method...maybe that should move?)
do not forward WX guiEvents on to the user (odd in that it makes wx different than the other GUI backends, but would only affect wx users and keep the fallout contained)
deprecate LocationEvent.lastevent and only stash the last axes?

I am not a fan of deleting or setting to None the guiEvent attribute as if the user has held onto the mpl event object, mutating it under them seems rude.

anntzer · 2022-08-25T11:35:14Z

Deprecating lastevent certainly seems reasonable.

anntzer added GUI: wx status: upstream fix required labels Jan 12, 2022

This was referenced Mar 11, 2022

wxPython 4.1 does not propagate MouseWheel event with WxAgg #17513

Closed

Mouse scroll events are not correctly passed to matplotlib in 4.1.0 wxWidgets/Phoenix#1707

Open

tacaswell added this to the v3.6.0 milestone Mar 17, 2022

tacaswell added status: confirmed bug and removed status: upstream fix required labels Mar 17, 2022

QuLogic modified the milestones: v3.6.0, v3.7.0 Aug 24, 2022

matplotlib deleted a comment from anntzer Aug 25, 2022

anntzer mentioned this issue Jan 27, 2023

Deprecate LocationEvent.lastevent. #25101

Merged

6 tasks

ksunden modified the milestones: v3.7.0, v3.7.1 Feb 14, 2023

QuLogic modified the milestones: v3.7.1, v3.7.2 Mar 4, 2023

anntzer mentioned this issue Mar 26, 2023

Make guiEvent available only within the event handlers. #25559

Merged

6 tasks

jklymak closed this as completed in #25559 Mar 28, 2023

QuLogic modified the milestones: v3.7.2, v3.8.0 Mar 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug]: scroll_event is broken after motion_notify_event in WXAgg #22211

[Bug]: scroll_event is broken after motion_notify_event in WXAgg #22211

komoto48g commented Jan 12, 2022 •

edited

Loading

anntzer commented Jan 12, 2022

komoto48g commented Jan 12, 2022 •

edited

Loading

DietmarSchwertberger commented Mar 11, 2022

komoto48g commented Mar 15, 2022 •

edited

Loading

anntzer commented Mar 15, 2022

tacaswell commented Mar 17, 2022

anntzer commented Aug 25, 2022

[Bug]: scroll_event is broken after motion_notify_event in WXAgg #22211

[Bug]: scroll_event is broken after motion_notify_event in WXAgg #22211

Comments

komoto48g commented Jan 12, 2022 • edited Loading

Bug summary

Code for reproduction

Actual outcome

Expected outcome

Additional information

Operating system

Matplotlib Version

Matplotlib Backend

Python version

Jupyter version

Installation

anntzer commented Jan 12, 2022

komoto48g commented Jan 12, 2022 • edited Loading

DietmarSchwertberger commented Mar 11, 2022

komoto48g commented Mar 15, 2022 • edited Loading

anntzer commented Mar 15, 2022

tacaswell commented Mar 17, 2022

anntzer commented Aug 25, 2022

komoto48g commented Jan 12, 2022 •

edited

Loading

komoto48g commented Jan 12, 2022 •

edited

Loading

komoto48g commented Mar 15, 2022 •

edited

Loading