Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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. |
0e3cce3
to52c07fe
CompareUh oh!
There was an error while loading.Please reload this page.
5f1b3d1
toaebec04
CompareAfter quite a bit of trial-and-error, I think this now fully supports HiDPI as much as Windows does. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
{ | ||
switch (uMsg) { | ||
case WM_DPICHANGED: | ||
// This function is a subclassed window procedure, and so is run during |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
so I was expecting it to hold thetcl_lock
already. If that's not the case... sorry for the distraction!
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
649606a
tod22bdd1
Compare@richardsheridan are you OK with this now? I can "officially" approve for you if you are... Thanks! |
There was a problem hiding this 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.
.... 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. |
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). |
Good point; it's 24 points, but I should probably scale that down to be whatever the equivalent is. |
Also, rename variable to match GTK3 backend.
At the moment, Tk does not support updating the 'scaling' value when themonitor pixel ratio changes, or the window is moved to a different one.
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. |
Indeed, the new version doesn't show the issue. |
Uh oh!
There was an error while loading.Please reload this page.
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
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).