Skip to content

Improve initial macosx device scale #18274

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Improve initial macosx device scale #18274

wants to merge 2 commits into from
+15 −14

Conversation

dopplershift
Copy link
Contributor

PR Summary

Fixes #18213. The problem is that the initial device scale in the macosx backend was set to 1.0--it would then update on the first display of a figure. This calls an API on window creation to update the device scale. This fixes the failing test for me locally.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

@dopplershift dopplershift added this to the v3.4.0 milestone Aug 16, 2020
@jklymak
Copy link
Member

jklymak commented Aug 16, 2020

Is this robust across mixed dpi devices? I seem to remember some issue with folks who had retina and non retina setups

@dopplershift
Copy link
Contributor Author

dopplershift commented Aug 17, 2020 •
edited
Loading

Well, the goal here is to be safe across those, that's the reason for the whole device_scale song and dance. Whether we're actually doing that robustly 🤷‍♂️ . The Apple Docs lead me to believe we're not exactly doing things the way they want, but I'm not signing up to rewrite all of that--some brief testing with some more APIs has led me to conclude I don't understand this well enough.

This PR only changes things to use an alternate API initially to try to get the scale factor from the Window class rather than requiring a full graphics context. The rest of the device scale handling is unchanged from before. The change now means it gives me the proper value that we eventually want, at least on my Retina system, of 2.0 at a time in the init sequence that allows us to avoid ever making a figure with a lower DPI. This makes the test that was constantly failing on my machine now pass. It also is passing Travis, at least this time.

@QuLogic
Copy link
Member

QuLogic commented Aug 17, 2020

To confirm, what DPI is the figure saved as now?

@dopplershift
Copy link
Contributor Author

Before: 100 DPI initially, 200 DPI after display
After: 200 DPI

@QuLogic
Copy link
Member

QuLogic commented Aug 17, 2020

Has the macosx backend been wrong all this time? Figures should be saved at the logical DPI, so 100, not 200.

@dopplershift dopplershift marked this pull request as draft August 18, 2020 03:29
@dopplershift
Copy link
Contributor Author

It's possible I introduced the shift while fixing issues. Let me see if I can carve out some time how this should work (Apple's view of things aside).

@dopplershift dopplershift mentioned this pull request Dec 16, 2020
Merged
6 tasks
@QuLogic QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 21, 2021
@QuLogic QuLogic modified the milestones: v3.5.0, v3.6.0 Aug 23, 2021
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Oct 15, 2021
@QuLogic
Convert macosx backend to use device_pixel_ratio
55a16ef 
This was not originally implemented in matplotlib#19126, but causes some
inconsistencies with other backends.

This also sets the initial scale as implemented in matplotlib#18274.
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Oct 15, 2021
@QuLogic
Convert macosx backend to use device_pixel_ratio
e7c5fe7 
This was not originally implemented in matplotlib#19126, but causes some
inconsistencies with other backends.

This also sets the initial scale as implemented in matplotlib#18274.
@QuLogic QuLogic mentioned this pull request Oct 15, 2021
Merged
3 tasks
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Oct 20, 2021
@QuLogic
Convert macosx backend to use device_pixel_ratio
ab61cbe 
This was not originally implemented in matplotlib#19126, but causes some
inconsistencies with other backends.

This also sets the initial scale as implemented in matplotlib#18274.
@QuLogic QuLogic modified the milestones: v3.6.0, v3.5.0 Oct 20, 2021
@QuLogic
Copy link
Member

QuLogic commented Oct 20, 2021

Should be replaced by #21365.

@QuLogic QuLogic closed this Oct 20, 2021
@timhoffm timhoffm mentioned this pull request Nov 18, 2021
Open
ccaprani pushed a commit to ccaprani/matplotlib that referenced this pull request Jan 23, 2023
@QuLogic @ccaprani
Convert macosx backend to use device_pixel_ratio
82053b7 
This was not originally implemented in matplotlib#19126, but causes some
inconsistencies with other backends.

This also sets the initial scale as implemented in matplotlib#18274.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
GUI: MacOSX topic: testing
Projects
None yet
Milestone
v3.5.0
Development

Successfully merging this pull request may close these issues.

Figure out why test_interactive_backend fails on Travis macOS
3 participants
@dopplershift @jklymak @QuLogic