Skip to content
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

PR: Update pixmap height calculation #4738

Merged
merged 3 commits into from
Jul 26, 2017
Merged

PR: Update pixmap height calculation #4738

merged 3 commits into from
Jul 26, 2017

Conversation

DStauffman
Copy link
Contributor

@DStauffman DStauffman commented Jul 13, 2017

Fixes #4734


The font height and pixmap height were in incompatible units when DPI scaling is on. Dividing the qpixmap height by self.devicePixelRatio() converts it into device independent units. See http://doc.qt.io/qt-5/qpixmap.html#devicePixelRatio

@ccordoba12 ccordoba12 added this to the v3.2.1 milestone Jul 13, 2017
@@ -1179,7 +1179,8 @@ def linenumberarea_paint_event(self, event):
active_line_number = active_block.blockNumber() + 1

def draw_pixmap(ytop, pixmap):
painter.drawPixmap(0, ytop + (font_height-pixmap.height()) / 2,
pixmap_height = pixmap.height() / pixmap.devicePixelRatio()
Copy link
Member

Choose a reason for hiding this comment

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

devicePixelRatio was introduced in Qt5.5, that is making tests in Qt4 fail, maybe you could add a try.. except and leave the old behavior for Qt4

Copy link
Member

Choose a reason for hiding this comment

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

We have variables for checking Qt4 or Qt5 let's use those!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the logic to use the existing is_pyqt46 flag and restored the original behavior for PyQt4. It's as of yet untested on PyQt4, as I don't have that installed right now. Let me know if it works, or I can test it later.

@@ -1179,7 +1179,12 @@ def linenumberarea_paint_event(self, event):
active_line_number = active_block.blockNumber() + 1

def draw_pixmap(ytop, pixmap):
painter.drawPixmap(0, ytop + (font_height-pixmap.height()) / 2,
if is_pyqt46:
Copy link
Member

Choose a reason for hiding this comment

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

I think we need

from qtpy import PYQT4, PYQT5

 if PYQT4:

https://github.com/spyder-ide/qtpy/blob/master/qtpy/__init__.py#L94

Copy link
Member

Choose a reason for hiding this comment

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

Nop, it needs to be more fine-grained because we support PyQt >=5.2. So the solution is something along the lines

from qtpy import QT_VERSION
QT55_VERSION = programs.check_version(QT_VERSION, "5.5", ">=")

if not QT55_VERSION:
    pixmap_height = pixmap.height()
else:
    ....

Of course, you need to add the imports and constants in the right places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, the more fine grained option is now in there. Thanks for the help in knowing what to import.

@ccordoba12 ccordoba12 changed the title Update pixmap height calculation PR: Update pixmap height calculation Jul 26, 2017
@ccordoba12 ccordoba12 merged commit be0568a into spyder-ide:3.x Jul 26, 2017
ccordoba12 added a commit that referenced this pull request Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants