Add support for HiDPI in TkAgg on Windows #19167
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 to
52c07fe
Compare
5f1b3d1 to
aebec04
Compare
|
After quite a bit of trial-and-error, I think this now fully supports HiDPI as much as Windows does. |
| { | ||
| switch (uMsg) { | ||
| case WM_DPICHANGED: | ||
| // This function is a subclassed window procedure, and so is run during |
There was a problem hiding this comment.
Consider using Tcl_QueueEvent or Tcl_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 a deque instead of a SimpleQueue.
There was a problem hiding this comment.
It is not possible to call any Tk functions due to the Tkinter-internal Tcl thread lock. I'm fairly certain I tried Tcl_QueueEvent as well, but can do so again.
There was a problem hiding this comment.
I was under the impression that these windowprocs are called within Tcl_DoOneEvent, usually here:
so I was expecting it to hold the tcl_lock already. If that's not the case... sorry for the distraction!
There was a problem hiding this comment.
Actually no, I think I tried Tk_QueueEvent which does not work because of said locks. Looking at the API for Tcl_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.
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 calling Tcl_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.
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.
I like it, way better than the original and less magical than what I was imagining.
649606a to
d22bdd1
Compare
|
@richardsheridan are you OK with this now? I can "officially" approve for you if you are... Thanks! |
|
.... 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 the monitor 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. |
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
pytestpasses).flake8on changed files to check).flake8-docstringsand 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).