-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix a bug with the Qt5 backend with mixed resolution displays #8931
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
Changes from all commits
f8cb5c9
7f4a73a
31408af
6b15984
591a9e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,15 @@ def __init__(self, figure): | |
self._bbox_queue = [] | ||
self._drawRect = None | ||
|
||
# In cases with mixed resolution displays, we need to be careful if the | ||
# dpi_ratio changes - in this case we need to resize the canvas | ||
# accordingly. We could watch for screenChanged events from Qt, but | ||
# the issue is that we can't guarantee this will be emitted *before* | ||
# the first paintEvent for the canvas, so instead we keep track of the | ||
# dpi_ratio value here and in paintEvent we resize the canvas if | ||
# needed. | ||
self._dpi_ratio_prev = None | ||
|
||
def drawRectangle(self, rect): | ||
if rect is not None: | ||
self._drawRect = [pt / self._dpi_ratio for pt in rect] | ||
|
@@ -56,11 +65,29 @@ def paintEvent(self, e): | |
In Qt, all drawing should be done inside of here when a widget is | ||
shown onscreen. | ||
""" | ||
|
||
# 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'): | ||
return | ||
|
||
# As described in __init__ above, we need to be careful in cases with | ||
# mixed resolution displays if dpi_ratio is changing between painting | ||
# events. | ||
if (self._dpi_ratio_prev is None or | ||
self._dpi_ratio != self._dpi_ratio_prev): | ||
# We need to update the figure DPI | ||
self._update_figure_dpi() | ||
# The easiest way to resize the canvas is to emit a resizeEvent | ||
# since we implement all the logic for resizing the canvas for | ||
# that event. | ||
event = QtGui.QResizeEvent(self.size(), self.size()) | ||
# We use self.resizeEvent here instead of QApplication.postEvent | ||
# since the latter doesn't guarantee that the event will be emitted | ||
# straight away, and this causes visual delays in the changes. | ||
self.resizeEvent(event) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this also trigger a paint event? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might do - I'm investigating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apparently not - at least the paintEvent method does not get called again as a result. |
||
self._dpi_ratio_prev = self._dpi_ratio | ||
|
||
painter = QtGui.QPainter(self) | ||
|
||
if self._bbox_queue: | ||
|
@@ -167,9 +194,12 @@ def __init__(self, figure): | |
super(FigureCanvasQTAgg, self).__init__(figure=figure) | ||
# We don't want to scale up the figure DPI more than once. | ||
# Note, we don't handle a signal for changing DPI yet. | ||
if not hasattr(self.figure, '_original_dpi'): | ||
self.figure._original_dpi = self.figure.dpi | ||
self.figure.dpi = self._dpi_ratio * self.figure._original_dpi | ||
self.figure._original_dpi = self.figure.dpi | ||
self._update_figure_dpi() | ||
|
||
def _update_figure_dpi(self): | ||
dpi = self._dpi_ratio * self.figure._original_dpi | ||
self.figure._set_dpi(dpi, forward=False) | ||
|
||
|
||
@_BackendQT5.export | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,13 @@ | ||
from __future__ import (absolute_import, division, print_function, | ||
unicode_literals) | ||
|
||
import copy | ||
|
||
import matplotlib | ||
from matplotlib import pyplot as plt | ||
from matplotlib._pylab_helpers import Gcf | ||
import matplotlib | ||
import copy | ||
|
||
from numpy.testing import assert_equal | ||
|
||
import pytest | ||
try: | ||
|
@@ -95,3 +98,60 @@ def receive(event): | |
|
||
qt_canvas.mpl_connect('key_press_event', receive) | ||
qt_canvas.keyPressEvent(event) | ||
|
||
|
||
@pytest.mark.backend('Qt5Agg') | ||
def test_dpi_ratio_change(): | ||
""" | ||
Make sure that if _dpi_ratio changes, the figure dpi changes but the | ||
widget remains the same physical size. | ||
""" | ||
|
||
prop = 'matplotlib.backends.backend_qt5.FigureCanvasQT._dpi_ratio' | ||
|
||
with mock.patch(prop, new_callable=mock.PropertyMock) as p: | ||
|
||
p.return_value = 3 | ||
|
||
fig = plt.figure(figsize=(5, 2), dpi=120) | ||
qt_canvas = fig.canvas | ||
qt_canvas.show() | ||
|
||
from matplotlib.backends.backend_qt5 import qApp | ||
|
||
# Make sure the mocking worked | ||
assert qt_canvas._dpi_ratio == 3 | ||
|
||
size = qt_canvas.size() | ||
|
||
qt_canvas.manager.show() | ||
qApp.processEvents() | ||
|
||
# The DPI and the renderer width/height change | ||
assert fig.dpi == 360 | ||
assert qt_canvas.renderer.width == 1800 | ||
assert qt_canvas.renderer.height == 720 | ||
|
||
# The actual widget size and figure physical size don't change | ||
assert size.width() == 200 | ||
assert size.height() == 80 | ||
assert_equal(qt_canvas.get_width_height(), (600, 240)) | ||
assert_equal(fig.get_size_inches(), (5, 2)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A regular |
||
|
||
p.return_value = 2 | ||
|
||
assert qt_canvas._dpi_ratio == 2 | ||
|
||
qt_canvas.draw() | ||
qApp.processEvents() | ||
|
||
# The DPI and the renderer width/height change | ||
assert fig.dpi == 240 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't passing for me here; I think you need to do a little more to trigger an update. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, this seems to pass on the CI though - or does the test not get run on the CI? What platform and exact Qt version are you using? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see any obvious skips in the CI, but you can confirm with @anntzer that the tests should be running. The odd thing is I don't see any Qt4 skips and one would normally preclude the other; maybe they just luckily ran on different processes? I am running on Fedora+conda using the latest There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Almost certainly related to #8659 (causing some of the tests not being run). |
||
assert qt_canvas.renderer.width == 1200 | ||
assert qt_canvas.renderer.height == 480 | ||
|
||
# The actual widget size and figure physical size don't change | ||
assert size.width() == 200 | ||
assert size.height() == 80 | ||
assert_equal(qt_canvas.get_width_height(), (600, 240)) | ||
assert_equal(fig.get_size_inches(), (5, 2)) |
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.
Under-indented.