Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix scaling in Tk on non-Windows systems#28588

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
QuLogic merged 1 commit intomatplotlib:mainfromrnhmjoj:main
Sep 21, 2024
Merged

Conversation

rnhmjoj
Copy link
Contributor

PR summary

Fix scaling in Tk on non-Windows systems,closes#10388

PR checklist

cc:@tacaswell

Copy link

@github-actionsgithub-actionsbot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join uson gitter for real-time discussion.

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

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

@tacaswelltacaswell added this to thev3.10.0 milestoneJul 18, 2024
@rnhmjoj
Copy link
ContributorAuthor

ping

@tacaswell
Copy link
Member

Thank you for the ping!

attn@richardsheridan as our Tk expert.

@richardsheridan
Copy link
Contributor

This needs to be tested manually on Linux with two monitors with different dpi or scaling. I can't help with that though!

@rnhmjoj
Copy link
ContributorAuthor

Here's a comparison:

comparison

@rnhmjoj
Copy link
ContributorAuthor

I took the screenshots of the windows of the same plot at about the same physical dimensions on two different screens.
One is a full HD display with 96 dpi, the other a 4K display with 200 dpi. For comparison I upscaled the 96dpi screenshot to match those at 200dpi without an antialiasing filter (nearest-neighbour method).
As you can see by comparing the plot labels and the buttons this patch gives roughly the same results for both screens.

I tested everything on GNU/Linux with X.org. Someone should try a Wayland compositor and maybe macOS.

timhoffm reacted with thumbs up emoji

@timhoffm
Copy link
Member

Thanks for the comparison images. Positioning looks correct. I can't tell for the linewidths. Obviously, that depends on the upscaling. Probably an additional helpful comparison would be hidpi Qt backend vs hidpi Tk backend. I'd expect that the plot area would be pixel identical if we properly detect hidpi/magnification and feed it to the agg renderer.

@rnhmjoj
Copy link
ContributorAuthor

There you go, Qt vs Tk on a 200dpi display:
comparison2

timhoffm reacted with rocket emoji

Copy link
Contributor

@richardsheridanrichardsheridan left a comment

Choose a reason for hiding this comment

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

The scaling looks right, and if the window maintains about the same real-world dimensions when you drag it between monitors with different dpi, then I think it is good to go.

@timhoffm
Copy link
Member

There are two Tk tests on MacOS failing. I'm unclear whether this is related to the PR.

@rnhmjoj
Copy link
ContributorAuthor

rnhmjoj commentedAug 24, 2024
edited
Loading

if the window maintains about the same real-world dimensions when you drag it between monitors with different dpi, then I think it is good to go.

It doesn't for me, but for neither toolkit. When dragging the window from the 200dpi to 96dpi screen the physical dimensions increase (because my window manager doesn't really do anything about DPI) and the canvas remains scaled at 2x.
With Qt, only if I forceQT_SCREEN_SCALE_FACTORS='2;1' (despite both screens correctly reporting their physical dimensions) I can get the canvas to be re-rendered at 1x once the windows is fully contained in the 96dpi screen.

@tacaswell
Copy link
Member

I could not get to the build logs, but re-started the failed job.

@tacaswell
Copy link
Member

"power cycled" to get the tests to run again.

@rnhmjoj
Copy link
ContributorAuthor

Did anyone test this on macOS? If it's a concern I can enable it only forsys.platform == "linux".

Copy link
Member

@timhoffmtimhoffm left a comment

Choose a reason for hiding this comment

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

While I cannot test on OSX, I'm very much convinced that the non-windows way is "more correct" than the windows specific solution - Linux and OSX are typically similar on those matters.

If nobody can test in the next couple of days, IMHO we should be bold enough to merge as is.

@greglucas
Copy link
Contributor

I tested this on osx and it looks good locally to me. There are failing tests on osx with the tkagg backend which seem like they could be related.

@timhoffm
Copy link
Member

We can also be more defensive and only change for linux as proposedabove and then create a followup issue to investigate OSX. It would be good to have at least the linux version included in 3.10.

@tacaswell
Copy link
Member

The test failure is real and we can reproduce it locally.

Debugging on a mac, it seems that._tkcanvas.winfo_fpixels('1i') willalways return something that is almost 72. It seems to depend on the screen resolution you use:

In [4]: fig.canvas._tkcanvas.winfo_fpixels('1i')Out[4]: 72.0354505169867In [5]: fig.canvas._tkcanvas.winfo_fpixels('1i')Out[5]: 71.98228782287823In [6]: fig.canvas._tkcanvas.winfo_fpixels('1i')Out[6]: 71.98228782287823In [7]: fig.canvas._tkcanvas.winfo_fpixels('1i')Out[7]: 72.0354505169867In [8]: fig.canvas._tkcanvas.winfo_fpixels('1i')Out[8]: 72.04875346260387In [9]: fig.canvas._tkcanvas.winfo_fpixels('1i')Out[9]: 72.0354505169867In [10]: fig.canvas._tkcanvas.winfo_fpixels('1i')Out[10]: 71.98228782287823In [11]: fig.canvas._tkcanvas.winfo_fpixels('1i')Out[11]: 72.00885935769656

in between the calls I changed the resolution OSX was using. I think we need to exclude OSX here as this method does not actually work there as it tries to "help" a bit too much.

rnhmjoj and timhoffm reacted with thumbs up emoji

# scaled vs 96 dpi, and pixel ratio settings are given in whole
# percentages, so round to 2 digits.
ratio = round(self._tkcanvas.tk.call('tk', 'scaling') / (96 / 72), 2)
else:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
else:
elifsys.platform=="linux":

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thank you for testing it out! I limited the change to just "linux" and made sure_set_device_pixel_ratio is called only on supported platforms.

@QuLogicQuLogic merged commit8552c7a intomatplotlib:mainSep 21, 2024
40 checks passed
@QuLogic
Copy link
Member

Thanks@rnhmjoj! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.

@rnhmjoj
Copy link
ContributorAuthor

Thank you all for reviewing my patch!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell left review comments

@github-actionsgithub-actions[bot]github-actions[bot] left review comments

@QuLogicQuLogicQuLogic approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

@richardsheridanrichardsheridanrichardsheridan approved these changes

Assignees
No one assigned
Labels
Projects
Milestone
v3.10.0
Development

Successfully merging this pull request may close these issues.

Add retina screen support for TkAgg backend
6 participants
@rnhmjoj@tacaswell@richardsheridan@timhoffm@greglucas@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp