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

Add support for HiDPI in TkAgg on Windows#19167

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
tacaswell merged 7 commits intomatplotlib:masterfromQuLogic:tk-hidpi
May 14, 2021

Conversation

QuLogic
Copy link
Member

@QuLogicQuLogic commentedDec 23, 2020
edited
Loading

PR Summary

At the moment, Tk does not support updating the 'scaling' value when the monitor pixel ratio changes, or the window is moved to a different one. Thus this needs a patch in the C extension to catch that window message and propagate it to Python. Due to thread locking issues, this needs to be done via a polled queue.

This is waiting for the pixel ratio refactor,#19126.

PR Checklist

  • Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (runflake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
  • Conforms to Matplotlib style conventions (installflake8-docstrings and runflake8 --docstring-convention=all).
  • New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • [n/a] API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).

@QuLogic
Copy link
MemberAuthor

I have not tested on macOS, but this may work if Tk supports it. I tried on Linux and it doesn't seem to work, but I don't have a true HiDPI system to test.

@QuLogicQuLogicforce-pushed thetk-hidpi branch 2 times, most recently from0e3cce3 to52c07feCompareDecember 25, 2020 03:48
@QuLogicQuLogicforce-pushed thetk-hidpi branch 2 times, most recently from5f1b3d1 toaebec04CompareApril 15, 2021 00:53
@QuLogicQuLogic changed the titleAdd initial support for HiDPI in TkAgg on WindowsAdd support for HiDPI in TkAgg on WindowsApr 23, 2021
@QuLogic
Copy link
MemberAuthor

After quite a bit of trial-and-error, I think this now fully supports HiDPI as much as Windows does.

@QuLogicQuLogic marked this pull request as ready for reviewApril 23, 2021 03:39
{
switch (uMsg) {
case WM_DPICHANGED:
// This function is a subclassed window procedure, and so is run during
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider usingTcl_QueueEvent orTcl_EvalEx to queue up a call to a tkinter-registered python function to avoid the polling loop. Failing that, I think this all occurs in the main thread, so you can use adeque instead of aSimpleQueue.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

It is not possible to call any Tk functions due to the Tkinter-internal Tcl thread lock. I'm fairly certain I triedTcl_QueueEvent as well, but can do so again.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was under the impression that these windowprocs are called withinTcl_DoOneEvent, usually here:

https://github.com/python/cpython/blob/1e9f0933095403b215c2c4a0be7915d034ff7026/Modules/_tkinter.c#L2938

so I was expecting it to hold thetcl_lock already. If that's not the case... sorry for the distraction!

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Actually no, I think I triedTk_QueueEvent which does not work because of said locks. Looking at the API forTcl_QueueEvent, I'm not sure how it could be used. It takes a pointer a function to call later, which would still be my code and unable to (un)lock anything in the Tkinter library.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Note that Tk/Tcl is threaded on Windows, so it doesn't go through that route, but this one:
https://github.com/python/cpython/blob/1e9f0933095403b215c2c4a0be7915d034ff7026/Modules/_tkinter.c#L2931

As an alternative, I tried callingTcl_SetVar on a variable named per-window, and this might work without crashing. I need to do a bit more testing to be sure, since concurrency issues can be heisenbugs.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

OK, I used a Tk variable now; this is nice as it cleans up the queue polling.

I think what happened originally is that I tried the variable approach from Python, and that failed due to the competing locks which I only found out about some time later.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like it, way better than the original and less magical than what I was imagining.

@QuLogicQuLogicforce-pushed thetk-hidpi branch 2 times, most recently from649606a tod22bdd1CompareApril 27, 2021 04:51
@tacaswelltacaswell added this to thev3.5.0 milestoneApr 29, 2021
@jklymakjklymak requested a review fromanntzerMay 10, 2021 19:32
@jklymak
Copy link
Member

@richardsheridan are you OK with this now? I can "officially" approve for you if you are... Thanks!

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.

Didn't know you were waiting on an explicit approval from me! I'm happy with the new way the DPI change is triggered and with the change overall.

@jklymak
Copy link
Member

.... I don't think we were, but wanted to make sure you still thought it was on the right track. This still needs another person to approve.

@anntzer
Copy link
Contributor

I can't really claim any expertise on the topic, but I can report that on my (non-hidpi) laptop, this results in toolbar icons being clearly too big (by ~30%, just eyeballing it).

@anntzeranntzer removed their request for reviewMay 10, 2021 23:22
@QuLogic
Copy link
MemberAuthor

Good point; it's 24 points, but I should probably scale that down to be whatever the equivalent is.

@QuLogic
Copy link
MemberAuthor

The buttons are back to the previous size, though the spacers are a bit smaller just because it's a rounder number.

On Windows, the toolbar is a bit bigger due to choosing a larger font. But the original font is so small, I think this is fine.

@anntzer
Copy link
Contributor

Indeed, the new version doesn't show the issue.

@tacaswelltacaswell merged commit86d6a0d intomatplotlib:masterMay 14, 2021
@QuLogicQuLogic deleted the tk-hidpi branchMay 14, 2021 03:33
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@anntzeranntzeranntzer left review comments

@jklymakjklymakjklymak approved these changes

@richardsheridanrichardsheridanrichardsheridan approved these changes

Assignees
No one assigned
Labels
Projects
None yet
Milestone
v3.5.0
Development

Successfully merging this pull request may close these issues.

5 participants
@QuLogic@jklymak@anntzer@richardsheridan@tacaswell

[8]ページ先頭

©2009-2025 Movatter.jp