Skip to content

Fix Cairo backends on HiDPI screens #21025

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

Merged
merged 3 commits into from
Sep 14, 2021
Merged

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Sep 9, 2021

PR Summary

Since Cairo renderers exist forever, we need to ensure they are using the correct DPI, or else physical sizes are incorrect.

Fixes #21024.

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 9, 2021
@jklymak
Copy link
Member

jklymak commented Sep 9, 2021

So Cairo doesn't support mixed dpi screen setups?

fig.savefig(buf, format='png')
ratio2 = Image.open(buf)

assert ratio1.size == ratio2.size
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess even the whole images should be the same?

Copy link
Member Author

Choose a reason for hiding this comment

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

True, but they're empty figures, so I didn't bother. I thought of using check_figures_equal, but I don't know how to force a different backend with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

You could actually plot something, and do a plain rgba array equality check?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure

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.

I don't have a mixed dpi setup to test, but at least for single-screen simulated highdpi, this does what it says on the tin.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 9, 2021

So Cairo doesn't support mixed dpi screen setups?

If you mean multiple screens, that's handled at the canvas level. If you mean something like PDF, I'm not sure, but I think the Agg/PDF backends are just as broken.

@jklymak
Copy link
Member

jklymak commented Sep 9, 2021

Yeah I meant screen one is non-retina and screen two is retina, and you drag the window from one to the other.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 9, 2021

Yes, that depends on the backend supporting it, but e.g., QtCairo does support that.

Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

Checked locally and confirmed that the fix works

Unlike the Agg renderer, the Cairo renderer exists forever and is at the
DPI when the figure was created. This needs to be updated before drawing
or things in physical sizes (e.g., text or line widths) will be the
wrong size.
Otherwise, `FigureCanvasBase.get_width_height` returns a size scaled
down by the current pixel ratio, even though the DPI is not scaled up.
This causes the saved figure to be cropped.
@anntzer anntzer merged commit 9bbef1e into matplotlib:master Sep 14, 2021
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 14, 2021
@QuLogic QuLogic deleted the cairo-hidpi branch September 14, 2021 00:32
QuLogic added a commit that referenced this pull request Sep 14, 2021
…025-on-v3.5.x

Backport PR #21025 on branch v3.5.x (Fix Cairo backends on HiDPI screens)
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH]: Cairo backends do not fully support HiDPI
4 participants