Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Support fractional HiDpi scaling with Qt backends#15656
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
11a1a57
toea8dfdb
CompareThere's an issue with
While I will try to fix this in isolation, we must check if other places also need an integer conversion. Also, should we cast to int or round to int? |
Probably do the same as what the qt version does? |
ea8dfdb
toc205b60
Compareobj.setDevicePixelRatioF(val) | ||
if hasattr(obj, 'setDevicePixelRatio'): | ||
# Not available on Qt4 or some older Qt5. | ||
obj.setDevicePixelRatio(val) |
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.
do we want to cast to int here?
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.
I would think so, given your other Qt PR, though that's for new Qt that wouldn't call this. Though I think it would come fromgetDevicePixelRatio
, so we may expectint
anyway?
@@ -19,8 +19,8 @@ def draw(self): | |||
def paintEvent(self, event): | |||
self._update_dpi() | |||
dpi_ratio = self._dpi_ratio | |||
width = dpi_ratio * self.width() | |||
height = dpi_ratio * self.height() | |||
width = int(dpi_ratio * self.width()) |
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.
We may want to useround
here rather thanint
to move the jumps away from the integers.
However, this may have issues with not being in sync with what we do else were in the code (which isint
).
It turns out Qt now has an enum to control how it rounds to pixels (https://doc.qt.io/qt-5/qt.html#HighDpiScaleFactorRoundingPolicy-enum). You can test this behavior by setting the importmatplotlib.pyplotaspltimportnumpyasnpth=np.linspace(0,2*np.pi,512)fig,ax=plt.subplots()ax.plot(th,np.sin(th))ax.plot(th,np.cos(th))ax.set_title(fig.canvas._dpi_ratio)plt.show() Without this patch it rounds down via truncating for non-integer ratios. I think what is going on is that we are getting the integer version of the dpi scaling and render the figure at (N, M) size. However, the widgets are seeing the fractional part and actually taking up (N1.2, M1.2) pixels on screen. When we paint the rendered figure Qt helpfully re-samples to the actual number of pixels and our anti-aliasing interacts really badly with that. |
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.
Take my review as only 1/2 as I did the rebase.
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.
This test always fails for me when run with everything else, so I'm not 100% on this, but it does pass when run by itself.
Uh oh!
There was an error while loading.Please reload this page.
obj.setDevicePixelRatioF(val) | ||
if hasattr(obj, 'setDevicePixelRatio'): | ||
# Not available on Qt4 or some older Qt5. | ||
obj.setDevicePixelRatio(val) |
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.
I would think so, given your other Qt PR, though that's for new Qt that wouldn't call this. Though I think it would come fromgetDevicePixelRatio
, so we may expectint
anyway?
I don‘t have the time to look into this in the next couple of days. Anybody can take over and push to this PR if necessary. |
Author: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
if hasattr(obj, 'setDevicePixelRatioF'): | ||
# Not available on Qt<5.6 | ||
obj.setDevicePixelRatioF(val) | ||
if hasattr(obj, 'setDevicePixelRatio'): |
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.
This should beelif
, no? Or you'll set pixel ratio two different ways.
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.
I opened#17640 with this change.
arghh.sorry@QuLogic |
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon! If these instruction are inaccurate, feel free tosuggest an improvement. |
…t backendsMerge pull requestmatplotlib#15656 from timhoffm/qt-fractional-hidpiSupport fractional HiDpi scaling with Qt backendsConflicts:lib/matplotlib/backends/backend_qt5.pylib/matplotlib/backends/qt_compat.py - only backport relevant changes
@timhoffm Actually, is the setDevicePixelRatioF shim ever used? AFAICT there's only |
timhoffm commentedOct 20, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@anntzer You are probably right. I probably did the getter first and assumed that the setter follows the same pattern. td;dr: Additional findings on the getter Digging a bit deeper: It seems that However, the derived classes I was about to propose that we could replace So indeed we could use |
PR Summary
We need to use the floating point versions
devicePixelRatioF
instead ofdevicePixelRatio
to scale the canvas correctly when the system uses a fractional scaling.Since we support older Qt versions, setting and getting
devicePixelRatioF
is wrapped in helper functions that fall back to the integer version or even no scaling.Tested with a scaling factor of 1.2:
before:

after:

PR Checklist