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

Add HiDPI support in GTK. #20988

Merged
merged 4 commits into from
Sep 16, 2021
Merged

Add HiDPI support in GTK. #20988

merged 4 commits into from
Sep 16, 2021

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Sep 3, 2021

PR Summary

I had this written for GTK3 some time ago, but was waiting for GTK4 to go in so I could add that in as well. Mostly the same as Qt, but linking to the GTK version of events, etc.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@QuLogic QuLogic added this to the v3.5.0 milestone Sep 3, 2021
@QuLogic QuLogic force-pushed the gtk-hidpi branch 2 times, most recently from c72d331 to 8cf7bab Compare September 8, 2021 03:36
@anntzer
Copy link
Contributor

anntzer commented Sep 8, 2021

On gtk3agg/X11, I now get

Traceback (most recent call last):
  File ".../matplotlib/backends/backend_gtk3.py", line 234, in _screen_changed
    self._update_device_pixel_ratio()
TypeError: _update_device_pixel_ratio() missing 2 required positional arguments: 'instance' and 'param'

Removing the instance and param args makes things work.

On gtk3cairo the image is crisp, but too large and cropped out of the window.

On gtk4agg, _update_device_pixel_ratio still seems never called.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 8, 2021

Oops, sorry, juggling multiple machines to test this, and must have got some things crossed.

What versions of GTK3 & GTK4 are you using?

@anntzer
Copy link
Contributor

anntzer commented Sep 8, 2021

3.24.30/4.4.0.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 8, 2021

Ah sorry, it's not versions; I just keep forgetting to test with the X11 backend. Agg should work now.

I totally forgot, and should have mentioned initially, that I had no idea how to get this working with Cairo. In other Cairo backends, we draw to an image surface, and blit it to the backend's widget. Unlike those backends, here we render directly to the GTK cairo context. I can fake this a bit with some scaling and translation, but the line width, text size, etc. are then smaller than they should be. But I just realized that QtCairo is wrong the same way, so this might be fine as a stopgap in the meantime.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 8, 2021

OK, this should now "work" for Cairo as well in that the canvas is not giant and clipped. However, there is a bug across all Cairo backends with HiDPI not rendering elements at the correct size, for which I will open a separate issue.

Also, GTK3Cairo on X11 (but not Wayland) is broken for me and doesn't open any windows. This appears unrelated to this change.

@anntzer
Copy link
Contributor

anntzer commented Sep 9, 2021

This mostly works for me now; there's only one single issue left (again, always on X11 and always on environment-variable simulated high-dpi): the initial window size on gtk3 (not on gtk4) is too large (scaled by the dpi factor, I think).

@QuLogic
Copy link
Member Author

QuLogic commented Sep 9, 2021

I think that might be bugginess in GTK. We get configure-event with already scaled up sizes in that case, whereas on a Wayland HiDPI screen, the sizes are still in logical pixels. Thus when we do dim_in_pixels*device_pixel_ratio/dpi, we get a doubled dimension in inches. And even if we ignore that, the window itself is already doubled in size and we'd just be drawing in a corner.

@anntzer
Copy link
Contributor

anntzer commented Sep 9, 2021

To be honest that last point is a bit beyond my pay grade (or rather beyond the pay grade of my screen) but the rest looks good to me.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 9, 2021

Well, to be clear, it's the same in GTK4, the resize event comes in with the size in logical pixels, so that's why we don't get a double-sized window.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 9, 2021

Hah, actually, I take that back; it is our bug. Grabbing the pixel ratio earlier meant that the DPI was updated before the window itself was created. And the initial window size calculation uses the physical size instead of the logical size.

Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

Yup, that works :)

It is deprecated since GTK 3.8, and event compression occurs in other
ways which don't seem relevant to us.
Otherwise, if the pixel ratio is changed before the window is created,
the window will be scaled up an extra time. This occurs on GTK3, but not
GTK4, due to some different order of events, but I changed the GTK4
backend to do the same for consistency.
@tacaswell
Copy link
Member

tacaswell commented Sep 15, 2021

@QuLogic can self merge on rebase and green.

@QuLogic QuLogic merged commit c58564e into matplotlib:master Sep 16, 2021
@QuLogic QuLogic deleted the gtk-hidpi branch September 16, 2021 03:01
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 16, 2021
QuLogic added a commit that referenced this pull request Sep 16, 2021
…988-on-v3.5.x

Backport PR #20988 on branch v3.5.x (Add HiDPI support in GTK.)
tacaswell pushed a commit that referenced this pull request Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants