Segfault from path editor example with QtAgg

@rcomer

I was playing with the path editor example and found that with QtAgg if I resize the window I get

QWidget::repaint: Recursive repaint detected
QWidget::paintEngine: Should no longer be called
QPainter::begin: Paint device returned engine == 0, type: 1
QPainter::end: Painter not active, aborted
Segmentation fault (core dumped)

Removing the blit line from draw as proposed at #11431 fixes this for me. TkAgg works fine whether the blit line is there or not.

I am on Ubuntu 22.04 and tested this with main.

Originally posted by @rcomer in #11431 (comment)

Wow! I came across this too right now!

Which Qt bindings are you using? I do see the extra prints, but do not see any seg faults on my machine (I tried with both pyqt and pyside, qt6 and qt5).

From a quick scan of the linked PR, it looks like this is a race condition, so that may explain why I'm not seeing it but you are, so that is extra fun.

I have pyqt 5.15.9. I created the environment using the conda yml.

edit: though I created the env in December so perhaps I should make a new one and try again.

edit 2: confirmed same result with a fresh environment.

Uninstalling PySide2 somehow fixed this for me

I see this too with pyqt6.

Also confirmed that #27913 does not fix the problem.

I see this too with pyqt6.

I don’t know whether to 🎉 or ☹️

On the plus side, I can get rid of the segfault 🎉

On the down side, the figure does not repaint properly on resize until you hit a keyboardkey....

This is what I have been playing with before I sign off for the night:

diff --git a/galleries/examples/event_handling/path_editor.py b/galleries/examples/event_handling/path_editor.py
index d6e84b4540..f08637a188 100644
--- a/galleries/examples/event_handling/path_editor.py
+++ b/galleries/examples/event_handling/path_editor.py
@@ -91,6 +91,7 @@ class PathInteractor:
 
     def on_draw(self, event):
         """Callback for draws."""
+        print('in on draw cb')
         self.background = self.canvas.copy_from_bbox(self.ax.bbox)
         self.ax.draw_artist(self.pathpatch)
         self.ax.draw_artist(self.line)
@@ -120,7 +121,7 @@ class PathInteractor:
             self.line.set_visible(self.showverts)
             if not self.showverts:
                 self._ind = None
-        self.canvas.draw()
+        self.canvas.draw_idle()
 
     def on_mouse_move(self, event):
         """Callback for mouse movements."""
@@ -146,4 +147,5 @@ ax.set_title('drag vertices to update path')
 ax.set_xlim(-3, 4)
 ax.set_ylim(-3, 4)
 
+fig.canvas.draw()
 plt.show()
diff --git a/lib/matplotlib/backend_bases.py b/lib/matplotlib/backend_bases.py
index e90c110c19..2479a07322 100644
--- a/lib/matplotlib/backend_bases.py
+++ b/lib/matplotlib/backend_bases.py
@@ -1806,9 +1806,11 @@ class FigureCanvasBase:
     def _idle_draw_cntx(self):
         self._is_idle_drawing = True
         try:
+            print("enter idle conext")
             yield
         finally:
             self._is_idle_drawing = False
+            print("exit idle conext")
 
     def is_saving(self):
         """
@@ -1892,6 +1894,7 @@ class FigureCanvasBase:
         before saving output to disk. For example computing limits,
         auto-limits, and tick values.
         """
+        print("womp womp")
 
     def draw_idle(self, *args, **kwargs):
         """
diff --git a/lib/matplotlib/backends/backend_agg.py b/lib/matplotlib/backends/backend_agg.py
index 92253c02c1..b1d7ec8f6d 100644
--- a/lib/matplotlib/backends/backend_agg.py
+++ b/lib/matplotlib/backends/backend_agg.py
@@ -379,6 +379,7 @@ class FigureCanvasAgg(FigureCanvasBase):
 
     def draw(self):
         # docstring inherited
+        print("In the real Agg draw")
         self.renderer = self.get_renderer()
         self.renderer.clear()
         # Acquire a lock on the shared font cache.
diff --git a/lib/matplotlib/backends/backend_qt.py b/lib/matplotlib/backends/backend_qt.py
index db593ae77d..c288af2150 100644
--- a/lib/matplotlib/backends/backend_qt.py
+++ b/lib/matplotlib/backends/backend_qt.py
@@ -385,6 +385,7 @@ class FigureCanvasQT(FigureCanvasBase, QtWidgets.QWidget):
             QtWidgets.QWidget.resizeEvent(self, event)
             # emit our resize events
             ResizeEvent("resize_event", self)._process()
+            print("in resize event")
             self.draw_idle()
         finally:
             self._in_resize_event = False
@@ -457,17 +458,13 @@ class FigureCanvasQT(FigureCanvasBase, QtWidgets.QWidget):
             self._event_loop.quit()
 
     def draw(self):
-        """Render the figure, and queue a request for a Qt draw."""
-        # The renderer draw is done here; delaying causes problems with code
-        # that uses the result of the draw() to update plot elements.
-        if self._is_drawing:
-            return
-        with cbook._setattr_cm(self, _is_drawing=True):
-            super().draw()
+        print("hit og draw")
+        self._draw()
         self.update()
 
     def draw_idle(self):
         """Queue redraw of the Agg buffer and request Qt paintEvent."""
+        print("in draw idle")
         # The Agg draw needs to be handled by the same thread Matplotlib
         # modifies the scene graph from. Post Agg draw request to the
         # current event loop in order to ensure thread affinity and to
@@ -475,11 +472,17 @@ class FigureCanvasQT(FigureCanvasBase, QtWidgets.QWidget):
         # TODO: queued signal connection might be safer than singleShot
         if not (getattr(self, '_draw_pending', False) or
                 getattr(self, '_is_drawing', False)):
+            print("   and did something")
             self._draw_pending = True
-            QtCore.QTimer.singleShot(0, self._draw_idle)
+
+            print(f"   {self._draw_pending=}")
+            #QtCore.QTimer.singleShot(0, self._draw_idle)
+        self.update()
+
 
     def blit(self, bbox=None):
         # docstring inherited
+        print("in blit")
         if bbox is None and self.figure:
             bbox = self.figure.bbox  # Blit the entire canvas if bbox is None.
         # repaint uses logical pixels, not physical pixels like the renderer.
@@ -488,14 +491,18 @@ class FigureCanvasQT(FigureCanvasBase, QtWidgets.QWidget):
         self.repaint(l, self.rect().height() - t, w, h)
 
     def _draw_idle(self):
+        print(f"called with {self._draw_pending=}")
+        if not self._draw_pending:
+            return
+        if self._is_idle_drawing:
+            return
+
         with self._idle_draw_cntx():
-            if not self._draw_pending:
-                return
             self._draw_pending = False
             if self.height() < 0 or self.width() < 0:
                 return
             try:
-                self.draw()
+                self._draw()
             except Exception:
                 # Uncaught exceptions are fatal for PyQt5, so catch them.
                 traceback.print_exc()
diff --git a/lib/matplotlib/backends/backend_qtagg.py b/lib/matplotlib/backends/backend_qtagg.py
index 256e50a3d1..afca423520 100644
--- a/lib/matplotlib/backends/backend_qtagg.py
+++ b/lib/matplotlib/backends/backend_qtagg.py
@@ -5,6 +5,7 @@ Render to qt from agg.
 import ctypes
 
 from matplotlib.transforms import Bbox
+from matplotlib import cbook
 
 from .qt_compat import QT_API, QtCore, QtGui
 from .backend_agg import FigureCanvasAgg
@@ -15,6 +16,18 @@ from .backend_qt import (  # noqa: F401 # pylint: disable=W0611
 
 class FigureCanvasQTAgg(FigureCanvasAgg, FigureCanvasQT):
 
+    def _draw(self):
+        """Render the figure, and queue a request for a Qt draw."""
+        # The renderer draw is done here; delaying causes problems with code
+        # that uses the result of the draw() to update plot elements.
+        print("entered real draw")
+        if self._is_drawing:
+            return
+        with cbook._setattr_cm(self, _is_drawing=True):
+            print(type(self).mro())
+            super().draw()
+        print("finished real draw")
+
     def paintEvent(self, event):
         """
         Copy the image from the Agg canvas to the qt.drawable.
@@ -22,11 +35,13 @@ class FigureCanvasQTAgg(FigureCanvasAgg, FigureCanvasQT):
         In Qt, all drawing should be done inside of here when a widget is
         shown onscreen.
         """
+        print("in paintEvent")
         self._draw_idle()  # Only does something if a draw is pending.
 
         # If the canvas does not have a renderer, then give up and wait for
         # FigureCanvasAgg.draw(self) to be called.
         if not hasattr(self, 'renderer'):
+            print("no renderer")
             return
 
         painter = QtGui.QPainter(self)
@@ -61,7 +76,9 @@ class FigureCanvasQTAgg(FigureCanvasAgg, FigureCanvasQT):
             qimage.setDevicePixelRatio(self.device_pixel_ratio)
             # set origin using original QT coordinates
             origin = QtCore.QPoint(rect.left(), rect.top())
+            print("about to paint image")
             painter.drawImage(origin, qimage)
+            print("painted image")
             # Adjust the buf reference count to work around a memory
             # leak bug in QImage under PySide.
             if QT_API == "PySide2" and QtCore.__version_info__ < (5, 12):

I think the current problem is some multiple inheritance issue


class A:
    def foo(self):
        print("A foo")

class B(A):
    def foo(self):
        print("B foo")
        super().foo()


class C(A):
    def bar(self):
        print("C bar")
        super().foo()

class D(B, C):
    ...



d = D()

gives

In [26]: d.bar()
C bar
A foo

but

class C(A):
    def bar(self):
        print("C bar")
        self.foo()

works in the toy.

Something is not right with the draw/draw_idle/their private versions / update but I have not sort it out yet. It looks like we have two generations of attempts to make draw_idle work correctly layered on top of eachother (and neither fully works).

Related: For those trying to use the path editor example in current JupyterLab and Jupyter Notebook 7+, see here.

DOC: we do not need the blit call in on_draw

Good news! The simplest fix is to remove self.canvas.blit(...) from on_draw. There may still be improvements that can be made to how we interact with the GUI backends, but the immediate problem is to not do useless work.

Navigation Menu

Repository navigation

Segfault from path editor example with QtAgg #28002

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Segfault from path editor example with QtAgg #28002

Description

Activity

Impaler343 commented on Apr 1, 2024

ksunden commented on Apr 1, 2024

rcomer commented on Apr 1, 2024

Impaler343 commented on Apr 2, 2024

tacaswell commented on Apr 5, 2024

tacaswell commented on Apr 5, 2024

rcomer commented on Apr 5, 2024

tacaswell commented on Apr 5, 2024

tacaswell commented on Apr 5, 2024

fomightez commented on May 4, 2024

tacaswell commented on May 15, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions