Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: Enable DPI awareness #611

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

guihkx
Copy link
Contributor

@guihkx guihkx commented Nov 11, 2024

I'm not sure if this is the best place to do this (or even the correct way to do this), but this does fix blurry fonts on Windows on displays with scaling factors higher than 100%.

Before (125% scaling factor):

blurry

After (125% scaling factor):

sharp

This fixes blurry fonts on Windows on displays with scaling factors
higher than 100%.
@DevilXD
Copy link
Owner

DevilXD commented Nov 11, 2024

Hello,

Interesting. I've read more into the description of what it does and what are the possibilities, and it doesn't exactly seem it's something that this application needs, at least not in this form. Setting anything other than 0 implies the application will actually handle and scale itself based on the DPI setting. 0 means that it won't do that (the current state), so the system rescales everything automatically. A non-zero setting apparently may (unconfirmed) also cause checkboxes to shrink down in size, which would make it harder to click on them, and I guess it'd look ugly too.

The good news is that TkInter apparently has a relatively easy way to scale itself, via the scaling call: https://wiki.tcl-lang.org/page/tk+scaling The call looks like this: root.tk.call('tk', 'scaling', 1.25), where 1.25 is the factor used to scale everything with, and root would be the main GUI window, found somewhere in the main interface creation flow in gui.py.

Refs I found, might be worth looking into:

What do you think? I guess I could get into testing all this, but I'm currently working on #586 and #590, and they're kinda more important than this. If not, I can simply test this at a later date.

@DevilXD DevilXD added the Enhancement New feature or request label Nov 11, 2024
@guihkx
Copy link
Contributor Author

guihkx commented Nov 14, 2024

Thanks for the answer. The timing of this is a bit unfortunate, though. I just lost access to the testing laptop that had a HiDPI display, so now I'll have to test this on a regular display.... :P

Setting anything other than 0 implies the application will actually handle and scale itself based on the DPI setting. 0 means that it won't do that (the current state), so the system rescales everything automatically

That's what I expected, too, but for some reason, Windows doesn't to scale the fonts automatically, so everything looks blurry. :(

I don't know Windows enough to understand why simply calling SetProcessDpiAwareness(1) is enough to make Windows scale it properly, but this approach seems quite popular. It was even recently added to Python's IDLE IDE to deal with the font scaling issue all Tkinter apps seem to suffer from on Windows.

A non-zero setting apparently may (unconfirmed) also cause checkboxes to shrink down in size, which would make it harder to click on them, and I guess it'd look ugly too.

At least that one I can confirm it doesn't happen (I'm using a 150% scaling factor now because I don't have my testing laptop anymore):

Screenshot pr-branch-looks-good

https://wiki.tcl-lang.org/page/tk+scaling The call looks like this: root.tk.call('tk', 'scaling', 1.25)

Maybe I'm just dumb, but that wiki page looks rather complicated. It mentions having to know the physical dimensions of the screen to calculate the DPI correctly:

If you want to set the value correctly, so that an inch or a centimeter given as a Tk distance corresponds to an inch or centimeter on the screen/panel of the monitor, you can only do this correctly when knowing the actual height or with of the screen/panel and the number of pixels in that direction

I tried a naive approach, and simply hardcoded the scaling to 2.0 (with a 150% scaling factor set on Windows):

diff --git a/main.py b/main.py
index 00d536f..a69cfab 100644
--- a/main.py
+++ b/main.py
@@ -87,6 +87,7 @@ if __name__ == "__main__":
     # NOTE: parser output is shown via message box
     # we also need a dummy invisible window for the parser
     root = tk.Tk()
+    root.tk.call("tk", "scaling", 2.0)
     root.overrideredirect(True)
     root.withdraw()
     set_root_icon(root, resource_path("icons/pickaxe.ico"))

The result does not look great (everything still looks blurry, and I had to maximize the window so it would fit into my non-HiDPI screen):

Screenshot naive-tk-scaling-only

I guess I could get into testing all this, but I'm currently working on #586 and #590, and they're kinda more important than this. If not, I can simply test this at a later date.

It'd be great if you could. This HiDPI territory is new to me. In any case, let me know if/when you can, this is not high priority by any means.

@DevilXD
Copy link
Owner

DevilXD commented Nov 16, 2024

for some reason, Windows doesn't to scale the fonts automatically, so everything looks blurry.
I don't know Windows enough to understand why simply calling SetProcessDpiAwareness(1) is enough to make Windows scale it properly

Maybe I wasn't exactly clear in my original response about this, but as far as I understand it, 1 means that no automatic scaling occurs, and that's why the fonts appear to be not blurry, while 0 makes it so that Windows scales everything, and enlarging the fonts ends up looking blurry. "Makes it work" is only partially correct in that case, as the fonts should be enlarged/scaled by the application with 1 set, to actually follow the scaling setting of the system. Otherwise, simply setting 1 makes the application look the same regardless of the scale set. The decision on whether or not this is what we want is up for debate. That's what I've meant when I said "it doesn't exactly seem it's something that this application needs, at least not in this form" - using 1 would probably fix the fonts, yes, but 1 also expects the application to do the actual scaling instead, which it doesn't do so right now.

The end result should look akin of what the https://stackoverflow.com/a/74105119 shows in it's test and pictures, with window #1 showing the 0 setting, and window #3 showing the 1 setting, after the application does the scaling properly. #2 shows that window size is also something that needs to be scaled, but that's only applicable to fixed-size windows. The miner uses a "flexbox" approach, that automatically expands the window to the size of the window's contents. If the contents are made bigger, the window should resize itself bigger accordingly.

At least that one I can confirm it doesn't happen (I'm using a 150% scaling factor now because I don't have my testing laptop anymore)

That's good then. They'll require scaling just like any other GUI element anyway though.

Maybe I'm just dumb, but that wiki page looks rather complicated. It mentions having to know the physical dimensions of the screen to calculate the DPI correctly
I tried a naive approach, and simply hardcoded the scaling to 2.0 (with a 150% scaling factor set on Windows):

As far I understand it, the description refers to HiDPI displays, and the calculations are done solely to maintain the same physical size of any graphical object (font size for ex.) between a normal and HiDPI monitor. This needs to be taken care of separately from Windows GUI scale. For a normal monitor and the Windows GUI scale only, you're supposed to the scale factor to whatever the scaling factor is currently set in Windows, so for 150% scale, the factor should be 1.5, not 2.0. Not sure where you've got the 2.0 figure from.

I'm still yet to test everything on my side, but this is more or less a backburner issue for now. I'll need to start doing tests myself to push it anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants