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

Qt5Agg HiDPI support #7507

Merged
merged 6 commits into from Jan 2, 2017
Merged

Qt5Agg HiDPI support #7507

merged 6 commits into from Jan 2, 2017

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Nov 24, 2016

This completes the work by @piannucci in #6389. Scaling is necessary on the figure DPI, and inverse scaling on rubberband and blitting locations.

I don't have a HiDPI display, but I tested by setting QT_SCALE_FACTOR=2. There is one bug with resizing if I use the grip in the status bar because it produces the wrong size. I'm not sure where that is created, so I haven't fixed it yet. Resizing using the OS's size works fine.

Ping @tacaswell, @astrofrog, @adrn, and @jenshnielsen who appear to have HiDPI screens based on other issues.

@QuLogic QuLogic added this to the 2.0.1 (next bug fix release) milestone Nov 24, 2016
@astrofrog
Copy link
Contributor

Seems to work nicely!

@tacaswell
Copy link
Member

There is a bug with fig.set_size_inches, it uses the scaled DPI to resize the canvas.

@QuLogic
Copy link
Member Author

QuLogic commented Nov 24, 2016

I'm not seeing the WebAgg backend (#5383) do anything to handle that either; is it really needed?

@tacaswell
Copy link
Member

The set_size_inches bug is probably related to the mouse-resize issue?

@QuLogic
Copy link
Member Author

QuLogic commented Dec 1, 2016

On resize events, we scale the logical pixels to physical pixels and then calculate inches via the scaled DPI, so I'm pretty sure fig.set_size_inches should use the scaled DPI.

Looking at the resizeEvent in FigureCanvasQT, I get logical sizes of ~480x80 when resizing using the OS handles. When using the grip in the toolbar, I get logical sizes of ~729x187, which aren't even double! I assume these values come directly from Qt as I see nothing else in the traceback.

So I'm wondering whether this bug occurs with true HiDPI displays? Is it a bug in Qt or just Qt's QT_SCALE_FACTOR?

@QuLogic
Copy link
Member Author

QuLogic commented Dec 1, 2016

Found QTBUG-51899 and QTBUG-53389, so this is not a bug in the matplotlib code.

@QuLogic QuLogic changed the title Qt5Agg HiDPI support [MRG] Qt5Agg HiDPI support Dec 1, 2016
@NelleV NelleV changed the title [MRG] Qt5Agg HiDPI support [MRG+1] Qt5Agg HiDPI support Dec 30, 2016
@NelleV
Copy link
Member

NelleV commented Dec 30, 2016

This looks good. I am not very familiar with these backends, so another pair of eye should look at this PR.

@MAKOMO
Copy link
Contributor

MAKOMO commented Dec 30, 2016

Works for me as expected. Would be cool to have this in the final v2.0 already.

@jenshnielsen
Copy link
Member

Tested it too and seems to work fine. The only question is if it's to late to merge into 2.0 @tacaswell

If the underlying canvas as a `_dpi_ratio`, scale the dpi before
computing the new size.
@efiring efiring merged commit a468e31 into matplotlib:v2.x Jan 2, 2017
@tacaswell tacaswell modified the milestones: 2.0 (style change major release), 2.0.1 (next bug fix release) Jan 2, 2017
@QuLogic QuLogic deleted the qt5agg_hidpi branch January 4, 2017 00:20
@QuLogic QuLogic changed the title [MRG+1] Qt5Agg HiDPI support Qt5Agg HiDPI support Jan 4, 2017
@anntzer anntzer mentioned this pull request Dec 13, 2017
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants