-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
skip QImage leak workaround for PySide2 >= 5.12 #21172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
the leak was addressed in PYSIDE-140
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers
or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
Is it at all possible to add corresponding tests for our test suite? |
I am not sure how the test should look like because:
|
A minimal example demonstrating the issue. Expected behavior: A few windows are drawn in succession.
Actual behavior with PySide2 on master: Crashes after the first window Tested on Windows 10, Python 3.8, PySide2 5.15 from PySide2 import QtCore, QtWidgets
import numpy as np
from matplotlib.backends.backend_qt5agg import FigureCanvasQTAgg as FigureCanvas
from matplotlib.figure import Figure
class MatplotlibWidget(QtWidgets.QWidget):
def __init__(self, size=(5.0, 4.0), dpi=100):
QtWidgets.QWidget.__init__(self)
self.figure = Figure(size, dpi=dpi)
self.canvas = FigureCanvas(self.figure)
self.canvas.setParent(self)
self.subplot = self.figure.add_subplot(111)
self.show()
def plot(self, x, y):
self.subplot.plot(x, y)
self.canvas.draw()
app = QtWidgets.QApplication([])
timer = QtCore.QTimer()
timer.setInterval(1000)
max_cnt = 5
cnt = 0
mw = None
def on_timeout():
global cnt, mw
if mw is not None:
mw.close()
if cnt >= max_cnt:
app.exit()
return
cnt += 1
mw = MatplotlibWidget()
x = np.linspace(0, 1, 1000)
y = np.sin(2*np.pi*cnt*x)
mw.plot(x, y)
timer.timeout.connect(on_timeout)
timer.start()
app.exec() if hasattr(app, 'exec') else app.exec_() |
We may be able to remove the skip of installing pyside2 on OSX on GHA with this patch |
…172-on-v3.5.x Backport PR #21172 on branch v3.5.x (skip QImage leak workaround for PySide2 >= 5.12)
I was curious to understand why versions <= 3.4.3 didn't cause a segfault. if QT_API == "PyQt6":
from PyQt6 import sip
ptr = int(sip.voidptr(buf))
else:
ptr = buf As the code is now with this PR merged, PySide2 < 5.12 should (continue to) segfault. I guess that's probably not an issue, given that PySide2 < 5.12 is EOL. |
…ixed skip QImage leak workaround for PySide2 >= 5.12
skip QImage leak workaround for PySide2 >= 5.12
PR Summary
While testing out Matplotlib 3.5.0b1 with pyqtgraph, pyqtgraph CI pipelines were failing for PySide2 bindings.
pyqtgraph/pyqtgraph#1999
The segfault was due to an old PySide2 QImage leak workaround. This leak was addressed in https://bugreports.qt.io/browse/PYSIDE-140 and versions of PySide2 >= 5.12 no longer need this workaround.
This PR skips the workaround for PySide2 >= 5.12.
The other change of casting to int for PyQt6 avoids the following warning:
Note: pyqtgraph only exercises the changes made to
backend_qtagg.py
. The same changes made tobackend_qtcairo.py
were not tested.PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).