Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Simplify the qt backend by using buffers to construct the image to be restored#27913

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
pelson wants to merge4 commits intomatplotlib:main
base:main
Choose a base branch
Loading
frompelson:feature/qtbackend-copy_from_boox-simplification
Open
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 12 additions & 19 deletionslib/matplotlib/backends/backend_qtagg.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,15 +2,14 @@
Render to qt from agg.
"""

importctypes
importnumpy as np

from matplotlib.transforms import Bbox

from .qt_compat import QT_API, QtCore, QtGui
from .qt_compat import QtGui
from .backend_agg import FigureCanvasAgg
from .backend_qt import _BackendQT, FigureCanvasQT
from .backend_qt import ( # noqa: F401 # pylint: disable=W0611
FigureManagerQT, NavigationToolbar2QT)
from ..transforms import Bbox


class FigureCanvasQTAgg(FigureCanvasAgg, FigureCanvasQT):
Expand DownExpand Up@@ -47,25 +46,19 @@ def paintEvent(self, event):
right = left + width
# create a buffer using the image bounding box
bbox = Bbox([[left, bottom], [right, top]])
buf =memoryview(self.copy_from_bbox(bbox))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think it would work to simply dobuf = memoryview(np.asarray(self.copy_from_bbox(bbox))) here and not touch anything else? This way from the pure python side you just need something that implements__array__ (a standard numpy interface) and there's no need to introduce a new custom API (get_bounds).

See also#23882 and the linked discourse thread.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for the linked issue, very relevant. FWIW, I'm fully in favour of removingBufferRegion (a non-public API), but not so hot on removingrestore_region (a public API). Ifrestore_region is simply redirected todraw_image then fair enough, though I don't especially like the idea ofgc=None, nor changing the blitting blending model.

That linked discussion was super interesting - I had assumed that there was nobody using a subset bbox forrestore_region - it seems like an obscure API to me. I was going to propose it be removed - I would still do that personally (and make it easy to subset the copied region instead).

Ultimately, I think we share the same objective here... I'm guessing this is about standardising the (rasterising) renderer such that in the future we could have backends which are not renderer specific (e.g. a common implementation of a qt backend for both agg and cairo (and other potential renderers)).

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I took a look at this, and you are right. I like it because it avoids extending the existing inaccessible type. It doesn't solve the fact that we do need to return more than just an image (we need the origin of the image too), but I'm confident we could removeBufferRegion at this point and replace it with a simple Python object (currently a numpy subclass). Ideally, it would not need to be a numpy subclass in the future (containment over inheritance, and all)

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I should say: I don't see a need formemoryview at all. I just used yourasarray trick.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I realised that this PR should also cover qtcairo, which has virtually the same implementation (but uses the pre-multiplied format)

img =np.asarray(self.copy_from_bbox(bbox), dtype=np.uint8)

if QT_API == "PyQt6":
from PyQt6 import sip
ptr = int(sip.voidptr(buf))
else:
ptr = buf
# Clear the widget canvas, to avoid issues as seen in
# https://github.com/matplotlib/matplotlib/issues/13012
painter.eraseRect(rect)

painter.eraseRect(rect) # clear the widget canvas
qimage = QtGui.QImage(ptr, buf.shape[1], buf.shape[0],
QtGui.QImage.Format.Format_RGBA8888)
qimage = QtGui.QImage(
img, img.shape[1], img.shape[0],
QtGui.QImage.Format.Format_RGBA8888,
)
qimage.setDevicePixelRatio(self.device_pixel_ratio)
# set origin using original QT coordinates
origin = QtCore.QPoint(rect.left(), rect.top())
painter.drawImage(origin, qimage)
# 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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looking at#25363 I guess this branch is never exercised anymore indeed, but the qt>=5.12 version bound doesn't seem to be documented anywhere but in that PR changelog note; perhaps it should be mentioned in dependencies.rst (even though it is already a consequence of wheel availability for py3.9)? (@oscargus)

ctypes.c_long.from_address(id(buf)).value = 1
painter.drawImage(rect.topLeft(), qimage)

self._draw_rect_callback(painter)
finally:
Expand Down
6 changes: 6 additions & 0 deletionssrc/_backend_agg_wrapper.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -472,6 +472,12 @@ static PyObject *PyRendererAgg_clear(PyRendererAgg *self, PyObject *args)

static PyObject *PyRendererAgg_copy_from_bbox(PyRendererAgg *self, PyObject *args)
{
// Note that whilst the copy_from_bbox call can technically return an image that
// is of a different rect than was requested, this is not used in the underlying
// backend. In the future, this copy_from_bbox will not return a PyBufferRegion,
// and instead simply return an image (the renderer interface may still expose a
// bbox in the response for convenience, but this doesn't need to be a special
// type at the C++ level).
agg::rect_d bbox;
BufferRegion *reg;
PyObject *regobj;
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp