Fix Hidpi scaling for GTK4Cairo

[sstendahl]

PR summary

Closes #25847

There's a regression bug from Matplotlib 3.5.x to 3.6 and higher that breaks Matplotlib for HiDPI displays using the GTK4Cairo backend. I found that the reason behind this is that the renderer width is no longer set, as the associated function is removed from the general Cairo renderer.

This PR solves the named issue with two very simple lines of code, it simply just sets the same allocated width and height in the same way as in the 3.5.x series. Difference is that it no longer uses the class function to do this (as this function was removed), but I didn't want to touch too much code.

Unfortunately, I haven't been able to run tests. This may be due to my development environment (Fedora Silverblue in a Fedora 38 Toolbox container), but these two lines of codes are the only changes compared to the main branch. I've tried to run the command in the docs, but it tells me

OSError: The baseline image directory does not exist. This is most likely because the test data is not installed. You may need to install matplotlib from source to get the test data.
================================================================================= short test summary info ==================================================================================
ERROR  - OSError: The baseline image directory does not exist. This is most likely because the test data is not installed. You may need to install matplotlib from source to get the test data.

If anyone has any pointers about how to do this testing more properly let me know. Needless to say, I have done testing in the sense of running the build and checking the behaviour.

Here's some screencasts to illustrate the issue. This is done from a separate program using this backend, but the same behaviour happens when using matplotlib directly from the CLI, see related issue.

Behaviour before this PR:

Schermopname.van.2023-05-09.17-19-13.webm

Behaviour after this PR:

Screencast.from.2023-05-11.14-14-01.webm

PR checklist

• "closes #0000" is in the body of the PR description to link the related issue
• [N/A] new and changed code is tested
• [N/A] Plotting related features are demonstrated in an example
• [N/A] New Features and API Changes are noted with a directive and release note
• Documentation complies with general and docstring guidelines

[github-actions]

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

[sstendahl]

I am not sure what causes the failing test on MacOS, but this PR is based on the main branch, where this fails as well. Given the simplicity of this PR, I'm pretty certain it has the same cause. Little I can do about that in this PR without rebasing to a different branch.

[ksunden]

The macOS failure is due to inconsistent tk headers provided by the Azure image. It has been reported to Azure, but we are waiting in the fix

[tacaswell]

If anyone has any pointers about how to do this testing more properly let me know. Needless to say, I have done testing in the sense of running the build and checking the behaviour.

how did you checkout out and install the dev version of Matplotlib? pip install -ve . (the -e is important) may be required to ensure pytest finds the right version of things....

[anntzer]

Apologies for the breakage. Looking at #22004 it seems that gtk3cairo also needs the same fix.

I cannot test this right now but I wonder if it would be better for the dpi scale to be set via cairo_surface_set_device_scale (https://www.cairographics.org/manual/cairo-cairo-surface-t.html#cairo-surface-set-device-scale) (instead of cairo_scale as is currently the case https://www.cairographics.org/manual/cairo-Transformations.html#cairo-scale) before calling self._renderer.set_context, and directly handle such scaling (which can be read via cairo_surface_get_device_scale) when inferring the size in RendererCairo.set_context?

[sstendahl]

If anyone has any pointers about how to do this testing more properly let me know. Needless to say, I have done testing in the sense of running the build and checking the behaviour.

how did you checkout out and install the dev version of Matplotlib? pip install -ve . (the -e is important) may be required to ensure pytest finds the right version of things....

Thank you, that indeed worked! Initially I installed matplotlib into my own application by building the dev build from git using the Flatpak manifest. This is however a sandboxed environment, and cannot communicate with external tools. So that obviously doesn't work with Pytest. The main (selfish) goal here was to get rid of this bug in my application, and after the fix worked I thought it'd be good to submit it upstream.

When I tried to run the actual tests, I installed it using the remote git archive directly (python -m pip install matplotlib @ git+url) which gave me a workable install but it couldn't be found by pytest. Your suggestion worked however, and let me test it. There's a few errors that do not seem to be related to the changes here. Here's the full dump of the error output:

================================================================================================ FAILURES =================================================================================================
___________________________________________________________________________________________ test_pdflatex[pdf] ____________________________________________________________________________________________

args = (), kwds = {'extension': 'pdf', 'request': <FixtureRequest for <Function test_pdflatex[pdf]>>}

    @wraps(func)
    def inner(*args, **kwds):
        with self._recreate_cm():
>           return func(*args, **kwds)
E           matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 13.194):
E           	result_images/test_backend_pgf/pgf_pdflatex_pdf.png
E           	result_images/test_backend_pgf/pgf_pdflatex-expected_pdf.png
E           	result_images/test_backend_pgf/pgf_pdflatex_pdf-failed-diff.png

/usr/lib64/python3.11/contextlib.py:81: ImageComparisonFailure
______________________________________________________________________________________________ test_rcupdate ______________________________________________________________________________________________

    @needs_pgf_xelatex
    @needs_pgf_pdflatex
    @mpl.style.context('default')
    @pytest.mark.backend('pgf')
    def test_rcupdate():
        rc_sets = [{'font.family': 'sans-serif',
                    'font.size': 30,
                    'figure.subplot.left': .2,
                    'lines.markersize': 10,
                    'pgf.rcfonts': False,
                    'pgf.texsystem': 'xelatex'},
                   {'font.family': 'monospace',
                    'font.size': 10,
                    'figure.subplot.left': .1,
                    'lines.markersize': 20,
                    'pgf.rcfonts': False,
                    'pgf.texsystem': 'pdflatex',
                    'pgf.preamble': ('\\usepackage[utf8x]{inputenc}'
                                     '\\usepackage[T1]{fontenc}'
                                     '\\usepackage{sfmath}')}]
        tol = [0, 13.2] if _old_gs_version else [0, 0]
        for i, rc_set in enumerate(rc_sets):
            with mpl.rc_context(rc_set):
                for substring, pkg in [('sfmath', 'sfmath'), ('utf8x', 'ucs')]:
                    if (substring in mpl.rcParams['pgf.preamble']
                            and not _has_tex_package(pkg)):
                        pytest.skip(f'needs {pkg}.sty')
                create_figure()
>               compare_figure(f'pgf_rcupdate{i + 1}.pdf', tol=tol[i])

lib/matplotlib/tests/test_backend_pgf.py:141: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

fname = 'pgf_rcupdate2.pdf', savefig_kwargs = {}, tol = 0

    def compare_figure(fname, savefig_kwargs={}, tol=0):
        actual = os.path.join(result_dir, fname)
        plt.savefig(actual, **savefig_kwargs)
    
        expected = os.path.join(result_dir, "expected_%s" % fname)
        shutil.copyfile(os.path.join(baseline_dir, fname), expected)
        err = compare_images(expected, actual, tol=tol)
        if err:
>           raise ImageComparisonFailure(err)
E           matplotlib.testing.exceptions.ImageComparisonFailure: Error: Image files did not match.
E             RMS Value: 13.649280799729164
E             Expected:  
E               /var/home/sjoerd/Projects/matplotlib/result_images/test_backend_pgf/expected_pgf_rcupdate2_pdf.png
E             Actual:    
E               /var/home/sjoerd/Projects/matplotlib/result_images/test_backend_pgf/pgf_rcupdate2_pdf.png
E             Difference:
E               /var/home/sjoerd/Projects/matplotlib/result_images/test_backend_pgf/pgf_rcupdate2_pdf-failed-diff.png
E             Tolerance: 
E               0

lib/matplotlib/tests/test_backend_pgf.py:33: ImageComparisonFailure
___________________________________________________________________________________________ test_get_font_names ___________________________________________________________________________________________

    @pytest.mark.skipif(sys.platform == 'win32', reason='Linux or OS only')
    def test_get_font_names():
        paths_mpl = [cbook._get_data_path('fonts', subdir) for subdir in ['ttf']]
        fonts_mpl = findSystemFonts(paths_mpl, fontext='ttf')
        fonts_system = findSystemFonts(fontext='ttf')
        ttf_fonts = []
        for path in fonts_mpl + fonts_system:
            try:
                font = ft2font.FT2Font(path)
                prop = ttfFontProperty(font)
                ttf_fonts.append(prop.name)
            except Exception:
                pass
        available_fonts = sorted(list(set(ttf_fonts)))
        mpl_font_names = sorted(fontManager.get_font_names())
>       assert set(available_fonts) == set(mpl_font_names)
E       AssertionError: assert {'ALM Fixed',...wskiego', ...} == {'Agency FB',...ld Face', ...}
E         Extra items in the left set:
E         'drmdozl24'
E         'drmdozit10'
E         'drmdoz14'
E         'drmsc9'
E         'FandolHei'
E         'Yinit'...
E         
E         ...Full output truncated (624 lines hidden), use '-vv' to show

lib/matplotlib/tests/test_font_manager.py:324: AssertionError
________________________________________________________________________________________ test_openin_any_paranoid _________________________________________________________________________________________

    @needs_usetex
    def test_openin_any_paranoid():
        completed = subprocess.run(
            [sys.executable, "-c",
             'import matplotlib.pyplot as plt;'
             'plt.rcParams.update({"text.usetex": True});'
             'plt.title("paranoid");'
             'plt.show(block=False);'],
            env={**os.environ, 'openin_any': 'p'}, check=True, capture_output=True)
>       assert completed.stderr == b""
E       assert b'qt.qpa.plug...land" in ""\n' == b''
E         Use -v to get more diff

lib/matplotlib/tests/test_texmanager.py:74: AssertionError
========================================================================================= short test summary info =========================================================================================
FAILED lib/matplotlib/tests/test_backend_pgf.py::test_pdflatex[pdf] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 13.194):
FAILED lib/matplotlib/tests/test_backend_pgf.py::test_rcupdate - matplotlib.testing.exceptions.ImageComparisonFailure: Error: Image files did not match.
FAILED lib/matplotlib/tests/test_font_manager.py::test_get_font_names - AssertionError: assert {'ALM Fixed',...wskiego', ...} == {'Agency FB',...ld Face', ...}
FAILED lib/matplotlib/tests/test_texmanager.py::test_openin_any_paranoid - assert b'qt.qpa.plug...land" in ""\n' == b''
============================================================= 4 failed, 9039 passed, 299 skipped, 14 xfailed, 2 xpassed in 521.38s (0:08:41) ==============================================================

Note that this is run in a containerized environment (it's a Podman container with Fedora 38 Workstation, afaik it's somewhat comparable to Docker). I'm not sure if that's related to these errors, but compiling from source can sometimes a bit iffy as not all system tools are pre-installer or where they are expected to be.

Edit: Full disclosure, I've updated the mesa drivers in my container and installed texlive-scheme-full, this got rid of most of the errors in the test. So it mostly seems local issues. Still issues, mostly related to LaTeX it seems, which I expect to be local as well.

[sstendahl]

Apologies for the breakage. Looking at #22004 it seems that gtk3cairo also needs the same fix.

Thanks for the context, it's good to be able to trace back the origin of this bug. I honestly would never have noticed it as it requires relatively specific settings for it to be triggered. You need to use a scaled resolution, use one of the gtk backends and use the cairo backend before being confronted with this bug.

There's another issue in the Agg backend, where the plot is completely white during the resizing event (is this a known issue, should I file an issue about this). This is not the case with Cairo, so that's why we switched to the Cairo backend in Graphs. See related issue about this white background during the scaling here.

For this specific issue in this PR, I only noticed this after somebody who does use a HiDPI display filed an issue about this in Graphs. Otherwise, I would never have noticed it myself either. The fact that it's apparently not documented for about two years also shows how it's a bit of an obscure issue (probably since matplotlib in gtk4 defaults to the agg backend iirc). This all doesn't contribute a lot to the solution here (I tend to write too much), but my point is that I can understand how this bug went unnoticed with that patch.

I indeed see the same issue is true in gtk3cairo, so I added this fix there as well. Regarding the suggestion to use cairo_surface_set_device_scale before calling self._renderer.set_context, I'm not sure what is deemed as most reasonable by the matplotlib developers here. I am not familiar with Cairo's python bindings and have a hard time finding documentation about this. So I kept this PR like this for now, as it basically just implements the previous way to set the scaling as was done in Matplotlib 3.5.x. But if a different method is preferred by a reviewer, I'd be happy to dive a bit deeper into this and adjust the PR accordingly.

[anntzer]

I can't test this right now but I'm basically thinking of something along the lines of

diff --git i/lib/matplotlib/backends/backend_cairo.py w/lib/matplotlib/backends/backend_cairo.py
index 9ccadcdf1c..88830a61ca 100644
--- i/lib/matplotlib/backends/backend_cairo.py
+++ w/lib/matplotlib/backends/backend_cairo.py
@@ -85,6 +85,7 @@ class RendererCairo(RendererBase):

     def set_context(self, ctx):
         surface = ctx.get_target()
+        inv_x_dpi_ratio, inv_y_dpi_ratio = surface.get_device_scale()
         if hasattr(surface, "get_width") and hasattr(surface, "get_height"):
             size = surface.get_width(), surface.get_height()
         elif hasattr(surface, "get_extents"):  # GTK4 RecordingSurface.
@@ -100,6 +101,8 @@ class RendererCairo(RendererBase):
             ctx.restore()
         self.gc.ctx = ctx
         self.width, self.height = size
+        self.width /= inv_x_dpi_ratio
+        self.height /= inv_y_dpi_ratio

     def _fill_and_stroke(self, ctx, fill_c, alpha, alpha_overrides):
         if fill_c is not None:
diff --git i/lib/matplotlib/backends/backend_gtk4cairo.py w/lib/matplotlib/backends/backend_gtk4cairo.py
index d57f53fb28..d2cd2a8e33 100644
--- i/lib/matplotlib/backends/backend_gtk4cairo.py
+++ w/lib/matplotlib/backends/backend_gtk4cairo.py
@@ -10,10 +10,10 @@ class FigureCanvasGTK4Cairo(FigureCanvasCairo, FigureCanvasGTK4):
     def on_draw_event(self, widget, ctx):
         with (self.toolbar._wait_cursor_for_draw_cm() if self.toolbar
               else nullcontext()):
-            self._renderer.set_context(ctx)
             scale = self.device_pixel_ratio
             # Scale physical drawing to logical size.
-            ctx.scale(1 / scale, 1 / scale)
+            ctx.get_target().set_device_scale(1 / scale, 1 / scale)
+            self._renderer.set_context(ctx)
             allocation = self.get_allocation()
             Gtk.render_background(
                 self.get_style_context(), ctx,

[sstendahl]

I can't test this right now but I'm basically thinking of something along the lines of

diff --git i/lib/matplotlib/backends/backend_cairo.py w/lib/matplotlib/backends/backend_cairo.py
index 9ccadcdf1c..88830a61ca 100644
--- i/lib/matplotlib/backends/backend_cairo.py
+++ w/lib/matplotlib/backends/backend_cairo.py
@@ -85,6 +85,7 @@ class RendererCairo(RendererBase):

     def set_context(self, ctx):
         surface = ctx.get_target()
+        inv_x_dpi_ratio, inv_y_dpi_ratio = surface.get_device_scale()
         if hasattr(surface, "get_width") and hasattr(surface, "get_height"):
             size = surface.get_width(), surface.get_height()
         elif hasattr(surface, "get_extents"):  # GTK4 RecordingSurface.
@@ -100,6 +101,8 @@ class RendererCairo(RendererBase):
             ctx.restore()
         self.gc.ctx = ctx
         self.width, self.height = size
+        self.width /= inv_x_dpi_ratio
+        self.height /= inv_y_dpi_ratio

     def _fill_and_stroke(self, ctx, fill_c, alpha, alpha_overrides):
         if fill_c is not None:
diff --git i/lib/matplotlib/backends/backend_gtk4cairo.py w/lib/matplotlib/backends/backend_gtk4cairo.py
index d57f53fb28..d2cd2a8e33 100644
--- i/lib/matplotlib/backends/backend_gtk4cairo.py
+++ w/lib/matplotlib/backends/backend_gtk4cairo.py
@@ -10,10 +10,10 @@ class FigureCanvasGTK4Cairo(FigureCanvasCairo, FigureCanvasGTK4):
     def on_draw_event(self, widget, ctx):
         with (self.toolbar._wait_cursor_for_draw_cm() if self.toolbar
               else nullcontext()):
-            self._renderer.set_context(ctx)
             scale = self.device_pixel_ratio
             # Scale physical drawing to logical size.
-            ctx.scale(1 / scale, 1 / scale)
+            ctx.get_target().set_device_scale(1 / scale, 1 / scale)
+            self._renderer.set_context(ctx)
             allocation = self.get_allocation()
             Gtk.render_background(
                 self.get_style_context(), ctx,

Thanks, I am able to test this on my machine but need some free time to do some proper testing. Hoping to give it a look tomorrow as today seems pretty booked.

[sstendahl]

Apologies for the late reply. The scaling indeed is different with the suggested solution from @anntzer, but there's something slightly off with the mathematics I think. Here's how a full plot looks like:

While it should look like

Again, I am not sure which solution is generally preferred by whomever reviews this. But I am willing to take a closer look at where the mismatch comes from if this latter approach is preferred. May have to look at after june 9 or so though as I'm a bit busy coming time. with my upcoming PhD defense

[anntzer]

No hurries, get through your defense first (quite a few people here started working on matplotlib during their phd, so we know how it is...)
I would prefer something along the lines of what I proposed, if you can make it work, but won't insist on it either.

[melissawm]

Would you mind moving this to Draft status then? Just makes it easier for maintainers to wait for your return. Thanks and good luck 😄