Skip to content

Issue with DPI corrections with Qt5 backend #8052

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

Closed
astrofrog opened this issue Feb 9, 2017 · 22 comments
Closed

Issue with DPI corrections with Qt5 backend #8052

astrofrog opened this issue Feb 9, 2017 · 22 comments
Labels
GUI: Qt Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Milestone

Comments

@astrofrog
Copy link
Contributor

I am running into issues with Matplotlib 2.0 with the Qt5 backend. The following script:

import matplotlib
matplotlib.use('Qt5Agg')
import matplotlib.pyplot as plt

fig = plt.figure()
ax = fig.add_axes([0, 0, 1, 1])

print(ax.transData.transform((0.0, 0.0)))
print(ax.transData.transform((0.5, 0.5)))
print(ax.transData.transform((1.0, 1.0)))
print(fig.canvas.width(), fig.canvas.height())

returns

[ 0.  0.]
[ 640.  480.]
[ 1280.   960.]
320 240

with Qt5, and:

[ 0. 0.]
[ 320. 240.]
[ 640. 480.]
(640, 480)

with Qt4. In Qt5 there is a factor of 4x difference between the display coordinates and the size of the widget, where I think there should only be a factor of 2x. A more visual way to see this issue is to run:

from PyQt5 import QtGui, QtWidgets
from matplotlib.figure import Figure
from matplotlib.backends.backend_qt5agg import FigureCanvasQTAgg as FigureCanvas


class MplCanvas(FigureCanvas):

    def __init__(self, fig):
        FigureCanvas.__init__(self, fig)
        self.renderer = None
        self.x = self.y = 0

    def paintEvent(self, event):

        if self.renderer is None:
            self.renderer = self.get_renderer()

        super(MplCanvas, self).paintEvent(event)

        p = QtGui.QPainter(self)
        pen = QtGui.QPen(QtGui.QColor('black'))
        pen.setWidth(20)
        p.setPen(pen)
        p.drawPoint(self.x, self.y)

    def mouse_press(self, event):
        x, y = ax.transData.transform((event.xdata, event.ydata))
        self.x, self.y = x, self.height() - y
        self.update()


app = QtWidgets.QApplication([''])

fig = Figure()
canvas = MplCanvas(fig)
ax = fig.add_axes([0, 0, 1, 1])

print(ax.transData.transform((0.0, 0.0)))
print(ax.transData.transform((0.5, 0.5)))
print(ax.transData.transform((1.0, 1.0)))
print(canvas.width(), canvas.height())

canvas.mpl_connect('button_press_event', canvas.mouse_press)
canvas.mpl_connect('motion_notify_event', canvas.mouse_press)

canvas.show()
app.exec_()

This shows a window where if you move around the cursor, a black square should follow it. With Qt5, the square is offset by a factor of 2x:

untitled

cc @tacaswell

@astrofrog
Copy link
Contributor Author

Looking into this a bit more - the difference of a factor of 4x in the first example changes to 2x after the plot is shown, which explains why the marker is off by a factor of 2 in the more complex case.

@tacaswell tacaswell added this to the 2.0.1 (next bug fix release) milestone Feb 10, 2017
@QuLogic
Copy link
Member

QuLogic commented Feb 21, 2017

Does the first example really fix itself after showing the plot? The second bug is unrelated, but easily fixed. I'm not really sure where the width/height come from in the first case.

@astrofrog
Copy link
Contributor Author

@QuLogic - the first example doesn't fix itself, because the display coordinates and canvas size are still mismatched by a factor of 2x after showing (which is basically the problem illustrated in the second example I think)

In any case, the only real issue for me is the second example, so just ignore the first example if it's confusing!

@QuLogic
Copy link
Member

QuLogic commented Feb 24, 2017

The first bug seems to be related to figure.Figure; it seems to work right with plt.figure. In fact, with the former code, the window does not become twice as big as it should, so the rendered figure is actually scaled down twice.

@QuLogic
Copy link
Member

QuLogic commented Feb 24, 2017

In the case of plt.figure, the figure is run through the FigureManagerQT, which adds the toolbar, but also resizes the window to the canvas.sizeHint, so adding:

cs = canvas.sizeHint()
canvas.resize(cs.width(), cs.height())

will get the sizes correct.

@efiring
Copy link
Member

efiring commented Apr 4, 2017

I am seeing a devastating bug with qt5 on a retina display, OSX Sierra, on both master and 2.0.x: zoom/pan, zoom-to-rect, and the cursor readout are completely wrong. I can't investigate right now, but I don't think this has always been the case.

@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Apr 4, 2017
@efiring
Copy link
Member

efiring commented Apr 5, 2017

Bisecting leads to #8144:

commit 419da798ce521b41d6b358a2bd79496caacb4035
Author: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Date:   Tue Feb 21 00:43:55 2017 -0500

    Qt5: Fix event positions on HiDPI screens.

    The event location is in logical pixels, which is the way it should stay
    for the rest of the stack.

@tacaswell
Copy link
Member

The problem is that internally mpl lives entirely in the 'real' pixel realm, but Qt lives (except for in the qpainter when we are rendering the plot) in 'logical' pixel space. I think that the two sizes are coming back different is the correct (but super confusing) behavior.

To fix the example:

from PyQt5 import QtGui, QtWidgets
from matplotlib.figure import Figure
from matplotlib.backends.backend_qt5agg import FigureCanvasQTAgg as FigureCanvas


class MplCanvas(FigureCanvas):

    def __init__(self, fig):
        FigureCanvas.__init__(self, fig)
        self.renderer = None
        self.x = self.y = 0

    def paintEvent(self, event):

        if self.renderer is None:
            self.renderer = self.get_renderer()

        super(MplCanvas, self).paintEvent(event)

        p = QtGui.QPainter(self)
        pen = QtGui.QPen(QtGui.QColor('black'))
        pen.setWidth(20)
        p.setPen(pen)
        p.drawPoint(self.x, self.y)

    def mouse_press(self, event):
        # convert from data -> screen pixels -> logical pixels
        x, y = ax.transData.transform((event.xdata, event.ydata)) / self._dpi_ratio
        self.x, self.y = x, self.height() - y
        self.update()


app = QtWidgets.QApplication([''])

fig = Figure()
canvas = MplCanvas(fig)
ax = fig.add_axes([0, 0, 1, 1])

print(ax.transData.transform((0.0, 0.0)))
print(ax.transData.transform((0.5, 0.5)))
print(ax.transData.transform((1.0, 1.0)))
print(canvas.width(), canvas.height())

canvas.mpl_connect('button_press_event', canvas.mouse_press)
canvas.mpl_connect('motion_notify_event', canvas.mouse_press)

canvas.show()
app.exec_()

There are still a couple too many ad-hoc self._dpi_ratio scalings in the code for me to be happy I understand what is going on, but I am pretty sure of:

  • we deal with hi-dpi by just upping the dpi the figure thinks it has
  • all of our internal transforms need to work in physical (screen) pixels or we will have to push the DPI much deeper into the library
  • qt always works in logical pixels
  • if you want to map mpl pixels <-> qt pixels you have to do the scaling the correct direction
  • the factor of 4 is really weird

@tacaswell
Copy link
Member

  • showing the window seems to change the size of the figure by a factor of 2, not sure why yet. Tracking that down seems key to all of this...

@tacaswell
Copy link
Member

and by size I mean looking at fig.bbox_inches

@tacaswell
Copy link
Member

It is in resizeEvent

@tacaswell
Copy link
Member

xlink to #8440

@tacaswell
Copy link
Member

🐉 🐲 🐉

@QuLogic QuLogic modified the milestones: 2.0.1 (next bug fix release), 2.0.2 (next bug fix release) May 3, 2017
@astrofrog
Copy link
Contributor Author

@tacaswell - the fixed example you showed in #8052 (comment) does not work correctly on my MacBook Pro with retina display with the latest developer version of Matplotlib. Does it still work ok for you?

@tacaswell
Copy link
Member

@astrofrog The changes just got merged up to master today, they were on the 2.0.x branch.

@tacaswell
Copy link
Member

but it is halving the size of figure again.....

@tacaswell
Copy link
Member

You would think keeping track of two coordinate frames would not be so hard....

@tacaswell tacaswell modified the milestones: 2.0.2 (critical bug fixes from 2.0.1), 2.0.3 (next bug fix release), 2.1 (next point release) May 9, 2017
@tacaswell
Copy link
Member

On a bit more consideration, I think this is working again, but there is some weird order of operations going on here starting up the window (It pulls the size to use from the un-scaled DPI and then at some point triggers a resize event which in turn down-sizes the figure (via a self.figure.set_size_inches in the resizeEvent method), the following works correctly (and is within a fraction of an inch 6 inches wide if I hold a ruler up to the screen!).

By a code path that is not immediantly clear to me now, going through plt.subplots and friends triggers enough callbacks that the figures end up the right size.

from PyQt5 import QtGui, QtWidgets
from matplotlib.figure import Figure
from matplotlib.backends.backend_qt5agg import FigureCanvasQTAgg as FigureCanvas
# import matplotlib.pyplot as plt
# plt.ion()

class MplCanvas(FigureCanvas):

    def __init__(self, fig):
        FigureCanvas.__init__(self, fig)
        self.renderer = None
        self.x = self.y = 0

    def paintEvent(self, event):

        if self.renderer is None:
            self.renderer = self.get_renderer()

        super(MplCanvas, self).paintEvent(event)

        p = QtGui.QPainter(self)
        pen = QtGui.QPen(QtGui.QColor('black'))
        pen.setWidth(20)
        p.setPen(pen)
        p.drawPoint(self.x, self.y)

    def mouse_press(self, event):
        # convert from data -> screen pixels -> logical pixels
        x, y = ax.transData.transform((event.xdata, event.ydata)) / self._dpi_ratio

        self.x, self.y = x, self.height() - y
        self.update()


app = QtWidgets.QApplication([''])

# this dpi is 1/2 the dpi of my screen
fig = Figure(figsize=(6, 4), dpi=155/2)
canvas = MplCanvas(fig)
ax = fig.add_axes([0, 0, 1, 1])

print(ax.transData.transform((0.0, 0.0)))
print(ax.transData.transform((0.5, 0.5)))
print(ax.transData.transform((1.0, 1.0)))
print(canvas.width(), canvas.height())
print(fig.get_size_inches())

canvas.mpl_connect('button_press_event', canvas.mouse_press)
canvas.mpl_connect('motion_notify_event', canvas.mouse_press)

canvas.show()
fig.set_size_inches(6, 4)
canvas.resize(*canvas.get_width_height())
print(ax.transData.transform((0.0, 0.0)))
print(ax.transData.transform((0.5, 0.5)))
print(ax.transData.transform((1.0, 1.0)))
print(canvas.width(), canvas.height())
print(fig.get_size_inches())

app.exec_()

I have been invoking this via

QT_AUTO_SCREEN_SCALE_FACTOR=1 python /tmp/qt_dpi.py 

on a linux box (arch + awesomeWM + a 4k display that report

DP-2 connected 3840x2160+0+0 (normal left inverted right x axis y axis) 621mm x 341mm
   3840x2160     60.00*+  30.00    29.97    23.98  
   1920x1080     60.00    59.94    50.00    29.97    23.97    60.00    50.04  
   .....

from xrandr.

I also have a normal dpi monitor and moving the window back-and-forth still has issues, moving it to the other monitor seems to cut the size in half the first time (but not on future times) and does not double when moving back, but the mouse events work correctly on both screens and after moving back and forth. This does not seem to happen with figures wrapped in the FigureManager.

We need to hook up http://doc.qt.io/qt-5/qwindow.html#screenChanged and update the dpi accordingly, but I am not sure I want to hold 2.0.2 (which has a bunch of critical bug fixes) for that. This will be a pretty major bit of work as the _orginal_dpi and _dpi_ratio attributes should probably be pushed all the way up into the base-classes, and the dpi property on Figure should get a bit smarter (to update _original_dpi when required) .

@QuLogic
Copy link
Member

QuLogic commented May 9, 2017

IIRC, I believe it's the FigureManager where we insert the right stuff to trigger the dpi change.

@tacaswell
Copy link
Member

@astrofrog Is this fully addressed by #8931

@astrofrog
Copy link
Contributor Author

@tacaswell - good question, I didn't think of them as the same bug but it might be fixed! Will check and report back.

@tacaswell
Copy link
Member

I am going to say this is taken care of, possibly super-ceded by #9062

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI: Qt Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

No branches or pull requests

4 participants