Skip to content

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

Merged
merged 5 commits into from
Jul 31, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion lib/matplotlib/backends/backend_qt5.py
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ def save_figure(self, *args):

fname, filter = _getSaveFileName(self.parent,
"Choose a filename to save to",
start, filters, selectedFilter)
start, filters, selectedFilter)
if fname:
if startpath == '':
# explicitly missing key or empty str signals to use cwd
Expand Down
36 changes: 33 additions & 3 deletions lib/matplotlib/backends/backend_qt5agg.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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):
Copy link
Member

Choose a reason for hiding this comment

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

Under-indented.

# 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)
Copy link
Member

Choose a reason for hiding this comment

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

Does this also trigger a paint event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might do - I'm investigating

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:
Expand Down Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions lib/matplotlib/figure.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,11 +418,16 @@ def _get_axes(self):
def _get_dpi(self):
return self._dpi

def _set_dpi(self, dpi):
def _set_dpi(self, dpi, forward=True):
"""
The forward kwarg is passed on to set_size_inches
"""
self._dpi = dpi
self.dpi_scale_trans.clear().scale(dpi, dpi)
self.set_size_inches(*self.get_size_inches())
w, h = self.get_size_inches()
self.set_size_inches(w, h, forward=forward)
self.callbacks.process('dpi_changed', self)

dpi = property(_get_dpi, _set_dpi)

def get_tight_layout(self):
Expand Down
64 changes: 62 additions & 2 deletions lib/matplotlib/tests/test_backend_qt5.py
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:
Expand Down Expand Up @@ -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))
Copy link
Member

Choose a reason for hiding this comment

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

A regular assert would be fine here; you didn't need to import assert_equal.


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
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Member

@QuLogic QuLogic Aug 2, 2017

Choose a reason for hiding this comment

The 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 defaults Qt package (conda-forge's Qt seems to be broken) 5.6.2-5 with Python 3.6.

Copy link
Contributor

@anntzer anntzer Aug 2, 2017

Choose a reason for hiding this comment

The 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))