Fix set_size_inches on HiDPI and also GTK4 by QuLogic · Pull Request #21212 · matplotlib/matplotlib · GitHub

PR Summary

I tested GTK3 and GTK4, but could not test Tk because I don't have Windows now.

PR Checklist

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (run flake8 on changed files to check).
[n/a] 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).
[n/a] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
[n/a] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

This is causing Qt figures to double in size as the resize in the qt backends expects logical (not physical) pixels.

So Qt was also buggy because the resize call in backend_bases used physical pixels which it didn't expect. In fact, only WebAgg/nbAgg worked correctly, because it's been converting from physical to logical.

I've now told Qt to do the conversion as well, so they should all work correctly.

Note, this always returned 1 since the attribute was renamed, so the resize was always in physical pixels.

I propagated the cast to all the backends with the changes, instead of just Qt, as they all technically use ints.

This is broken for macosx right now. It keeps doubling in size when jumping between monitors. All other backends look good to me though.

I think the macosx backend is the only one that hasn't been updated to use the proper superclass pixel ratio in #19126, and so it actually does have _dpi_ratio.

This works for me locally to keep the figure size in check.

diff --git a/lib/matplotlib/backends/backend_macosx.py b/lib/matplotlib/backends/backend_macosx.py
index a692504afb..c9bf34f5e4 100644
--- a/lib/matplotlib/backends/backend_macosx.py
+++ b/lib/matplotlib/backends/backend_macosx.py
@@ -30,14 +30,6 @@ class FigureCanvasMac(_macosx.FigureCanvas, FigureCanvasAgg):
         FigureCanvasBase.__init__(self, figure)
         width, height = self.get_width_height()
         _macosx.FigureCanvas.__init__(self, width, height)
-        self._dpi_ratio = 1.0
-
-    def _set_device_scale(self, value):
-        if self._dpi_ratio != value:
-            # Need the new value in place before setting figure.dpi, which
-            # will trigger a resize
-            self._dpi_ratio, old_value = value, self._dpi_ratio
-            self.figure.dpi = self.figure.dpi / old_value * self._dpi_ratio
 
     def set_cursor(self, cursor):
         # docstring inherited
@@ -63,8 +55,8 @@ class FigureCanvasMac(_macosx.FigureCanvas, FigureCanvasAgg):
         dpi = self.figure.dpi
         width /= dpi
         height /= dpi
-        self.figure.set_size_inches(width * self._dpi_ratio,
-                                    height * self._dpi_ratio,
+        self.figure.set_size_inches(width,
+                                    height,
                                     forward=False)
         FigureCanvasBase.resize_event(self)
         self.draw_idle()
diff --git a/src/_macosx.m b/src/_macosx.m
index ef999ca9ea..2d666788a3 100755
--- a/src/_macosx.m
+++ b/src/_macosx.m
@@ -1658,7 +1658,7 @@ -(void)drawRect:(NSRect)rect
 
     if (device_scale != new_device_scale) {
         device_scale = new_device_scale;
-        if (!PyObject_CallMethod(canvas, "_set_device_scale", "d", device_scale, NULL)) {
+        if (!PyObject_CallMethod(canvas, "_set_device_pixel_ratio", "d", device_scale, NULL)) {
             PyErr_Print();
             goto exit;
         }

That makes sense; I have a bit more cleanup to do and will put that into a separate PR.

Dismissing this, expecting a follow-up PR to address the doubling size in the macosx backend.

@QuLogic, so you're saying this is ready to go in as-is, or did you have additional clean-up for this PR?

Not for this PR directly, unless you want to wait for the macosx backend fixes first?

I think your patch is effectively correct in that it stops the resizing, but not technically correct as the pixel ratio is not handled in the right place, and not in all locations. Possibly, macosx had some attribute wrong the entire time, but I'm not sure yet.

So this is on FigureCanvasBase, but it's actually unused. I will modify the docstring for FigureManagerBase.resize, and mark this one unused.

This currently still has an issue with the MacOSX backend, but #21365 fixes that, so it should go in first probably.

This passes physical pixels to the backend, as it will be more accurate due to the int-cast. Fixes matplotlib#21090

…212-on-v3.5.x Backport PR #21212 on branch v3.5.x (Fix set_size_inches on HiDPI and also GTK4)

QuLogic added GUI: gtk GUI: tk labels Sep 28, 2021

QuLogic added this to the v3.5.0 milestone Sep 28, 2021

QuLogic force-pushed the fix-set_size_inches branch from 7267d23 to a0a1294 Compare September 29, 2021 19:58

QuLogic commented Sep 29, 2021

View reviewed changes

QuLogic force-pushed the fix-set_size_inches branch from 9ea14b1 to 38691d7 Compare September 29, 2021 23:14

QuLogic force-pushed the fix-set_size_inches branch from 38691d7 to 9dc00b4 Compare September 29, 2021 23:18

tacaswell approved these changes Oct 14, 2021

View reviewed changes

greglucas previously requested changes Oct 14, 2021

View reviewed changes

QuLogic commented Oct 15, 2021

View reviewed changes

QuLogic force-pushed the fix-set_size_inches branch 2 times, most recently from 4635702 to 56fe546 Compare October 15, 2021 03:06

greglucas approved these changes Oct 15, 2021

View reviewed changes

tacaswell force-pushed the main branch from f6e0ee4 to a1eef38 Compare October 20, 2021 19:55

QuLogic added 2 commits October 20, 2021 16:10

GTK4: Fix forward application of set_size_inches

4b5328d

Fix set_size_inches on HiDPI screens

9869826

This passes physical pixels to the backend, as it will be more accurate due to the int-cast. Fixes matplotlib#21090

QuLogic force-pushed the fix-set_size_inches branch from 56fe546 to 9869826 Compare October 20, 2021 20:10

tacaswell merged commit 8a8dd90 into matplotlib:main Oct 20, 2021

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 20, 2021

Backport PR matplotlib#21212 : Fix set_size_inches on HiDPI and also GTK4

fbf5477

meeseeksmachine mentioned this pull request Oct 20, 2021

Backport PR #21212 on branch v3.5.x (Fix set_size_inches on HiDPI and also GTK4) #21406

Merged

QuLogic deleted the fix-set_size_inches branch October 20, 2021 22:07

QuLogic added a commit that referenced this pull request Oct 21, 2021

Merge pull request #21406 from meeseeksmachine/auto-backport-of-pr-21…

7e36e90

…212-on-v3.5.x Backport PR #21212 on branch v3.5.x (Fix set_size_inches on HiDPI and also GTK4)

                       if forward:
                           canvas = getattr(self, 'canvas')
                           if canvas is not None:
-                              dpi_ratio = getattr(canvas, '_dpi_ratio', 1)

Fix set_size_inches on HiDPI and also GTK4 by QuLogic · Pull Request #21212 · matplotlib/matplotlib · GitHub

Repository navigation

Fix set_size_inches on HiDPI and also GTK4 - #21212

Fix set_size_inches on HiDPI and also GTK4#21212
tacaswell merged 2 commits into
matplotlib:mainfrom
QuLogic:fix-set_size_inches

QuLogic commented Sep 28, 2021

Uh oh!

tacaswell commented Sep 29, 2021

Uh oh!

QuLogic commented Sep 29, 2021

Uh oh!

QuLogic Sep 29, 2021

Uh oh!

QuLogic commented Sep 29, 2021

Uh oh!

greglucas left a comment

Uh oh!

QuLogic commented Oct 14, 2021

Uh oh!

greglucas commented Oct 14, 2021

Uh oh!

QuLogic commented Oct 14, 2021

Uh oh!

greglucas commented Oct 14, 2021

Uh oh!

QuLogic commented Oct 14, 2021

Uh oh!

QuLogic Oct 15, 2021

Uh oh!

greglucas left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

                       """Blit the canvas in bbox (default entire canvas)."""
                   def resize(self, w, h):
-                      """Set the canvas size in pixels."""

Fix set_size_inches on HiDPI and also GTK4 by QuLogic · Pull Request #21212 · matplotlib/matplotlib · GitHub

Repository navigation

Fix set_size_inches on HiDPI and also GTK4 - #21212

Fix set_size_inches on HiDPI and also GTK4#21212tacaswell merged 2 commits intomatplotlib:mainmatplotlib/matplotlib:mainfrom QuLogic:fix-set_size_inchesQuLogic/matplotlib:fix-set_size_inchesCopy head branch name to clipboard

Conversation

QuLogic commented Sep 28, 2021

PR Summary

PR Checklist

Uh oh!

tacaswell commented Sep 29, 2021

Uh oh!

QuLogic commented Sep 29, 2021

Uh oh!

QuLogic Sep 29, 2021

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Sep 29, 2021

Uh oh!

greglucas left a comment

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Oct 14, 2021

Uh oh!

greglucas commented Oct 14, 2021

Uh oh!

QuLogic commented Oct 14, 2021

Uh oh!

greglucas commented Oct 14, 2021

Uh oh!

QuLogic commented Oct 14, 2021

Uh oh!

QuLogic Oct 15, 2021

Choose a reason for hiding this comment

Uh oh!

greglucas left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix set_size_inches on HiDPI and also GTK4#21212
tacaswell merged 2 commits into
matplotlib:mainfrom
QuLogic:fix-set_size_inches