Skip to content

Add HiDPI support in GTK. - #20988

Merged
QuLogic merged 4 commits into
matplotlib:masterfrom
QuLogic:gtk-hidpi
Sep 16, 2021
Merged

QuLogic merged 4 commits into
matplotlib:masterfrom
QuLogic:gtk-hidpi

Conversation

@QuLogic

@QuLogic QuLogic commented Sep 3, 2021

Copy link
Copy Markdown
Member

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
Comment thread lib/matplotlib/backends/backend_gtk3.py Resolved
Comment thread lib/matplotlib/backends/backend_gtk3.py Resolved
@QuLogic
QuLogic force-pushed the gtk-hidpi branch 2 times, most recently from c72d331 to 8cf7bab Compare September 8, 2021 03:36
@anntzer

anntzer commented Sep 8, 2021 •

Copy link
Copy Markdown
Contributor

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

QuLogic commented Sep 8, 2021

Copy link
Copy Markdown
Member Author

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

What versions of GTK3 & GTK4 are you using?

@anntzer

anntzer commented Sep 8, 2021

Copy link
Copy Markdown
Contributor

3.24.30/4.4.0.

@QuLogic

QuLogic commented Sep 8, 2021

Copy link
Copy Markdown
Member Author

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

QuLogic commented Sep 8, 2021 •

Copy link
Copy Markdown
Member Author

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

anntzer commented Sep 9, 2021

Copy link
Copy Markdown
Contributor

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

QuLogic commented Sep 9, 2021

Copy link
Copy Markdown
Member Author

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

anntzer commented Sep 9, 2021

Copy link
Copy Markdown
Contributor

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

QuLogic commented Sep 9, 2021

Copy link
Copy Markdown
Member Author

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

QuLogic commented Sep 9, 2021

Copy link
Copy Markdown
Member Author

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.

@anntzer anntzer left a comment

Copy link
Copy Markdown
Contributor

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

tacaswell commented Sep 15, 2021 •

Copy link
Copy Markdown
Member

@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.

3 participants