-
-
Notifications
You must be signed in to change notification settings - Fork 449
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
Font/UI scaling improvements #3690
Conversation
@LosFarmosCTL are you able to do some macOS testing for this one? |
Sure, I'll do some testing sometime today. |
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.
Changes look overall very good to me on macOS, huge improvement across basically the entire application compared to before.
I did find a few cases where the scaling is unchanged though, and still too small compared to testing on a windows machine though:
1. The chatter list dialog:
2. The hotkey table in settings:
3. The account list in settings:
All other settings pages and tables (i.e. for filters, etc.) have always been fine though even before this PR, it's only those 2 that are off.
Thanks for testing! |
No problem, the macOS specific ones are all fixed by this yeah (#2544 #2351 #1076 #1075). Not entirely sure if this addresses all high DPI ones though, i.e. #750 is should definitely be unaffected right? For the rest its probably a good idea if someone with a high DPI display takes another look at them. |
@RAnders00 Do you mind testing this PR - as you are the issue holder of 5/12 of these issues 😁 |
Did some testing, appears that messages + the container are unaffected. I'd imagine that we'd want to pass DPI scaling through right down to the message element from their container (which would end up fixing #750 and #1092 in the process). Also, live changes to the DPI cause the split's input to mangle its size on Linux (and I assume this will come up when moving between different DPI monitors): |
Decided to skip Message*.cpp files, because chat scaling seems to be working fine and even your 2 screens from different dpi looks fine in this aspect. Will open another PR for #1092, about #750 not sure if its still valid on qt 5.15 but will test it this week with other linked issues. Did your linux tests worked well outside of runtime dpi changes? or fonts on screen appeared wonky even at normal start. |
I recommand in the future abandon absolute sizing (px) but use a size pallet that's built with the window's dimension / the system's dimension, so if you set the font size to MySizePallet.Get(15) (15th element) it will automatically adaptive to the dimension of the enviornment |
I'd like to do some testing on Ubuntu. |
This is still not merged in the main branch right? I just have to wait? :p |
yeah, if this works as described, it will be a big deal for linux and macos. hope a merge will happen soon. |
What's the status of this PR? |
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.
clang-tidy made some suggestions
Would love to get this tested by folks with different setups On my X11/Arch Linux btw system, there's no difference at all. |
The context menus and labels are too big on Windows: It works (partially) if you remove https://github.com/kornes/chatterino2/blob/f2d5f60a74e58b3c16acf1d59f5c03c4b97d345e/src/singletons/Fonts.cpp#L97-L105. The context menus will still be too big, but that's not an issue with this PR (it's tracked in #4862 and fixed by #4868). |
Can you try and comment out https://github.com/kornes/chatterino2/blob/f2d5f60a74e58b3c16acf1d59f5c03c4b97d345e/src/singletons/Fonts.cpp#L97-L105, like I did in #3690 (comment)? |
That's definitely a big improvement, but there is still something different going on compared to other areas of the UI. Best example imo, the button scaling (and text in general) in the change channel dialog, compared to the settings: That change is enough to make everything legible though. |
Hmm... I also have the bug that stuff in the select channel popup isn't sized correctly. Except for me, it's slightly too big. Changing https://github.com/kornes/chatterino2/blob/f2d5f60a74e58b3c16acf1d59f5c03c4b97d345e/src/widgets/BaseWindow.cpp#L779 to use |
stalled and most issues were already addressed |
Pull request checklist:
CHANGELOG.md
was updated, if applicableDescription
This PR aims to improve scaling of fonts and UI in high DPI.
MacOS will be mostly affected, but Windows also had few small fixes (tested with 175% windows scaling thing which is not real DPI but still exposed problems). Linux should be not affected, as its excluded from DPI modifier (for now at least).
All DPI calculations were moved to single place in Fonts::getOrCreateFontData() which now takes widget pointer for that purpose (as each child window can end up on different screen using different DPI). Windows and MacOS are set to use same scaling for now.
This PR also includes tooltip scaling fix #3687
To test on all platforms:
there will be more things to fix for sure, but heres start