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

Font/UI scaling improvements #3690

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kornes
Copy link
Contributor

@kornes kornes commented Apr 22, 2022

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

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:

  • if linux is really not affected at all
  • if scaling on Retina/high DPI displays looks OK or initial font size is still too small and has to be bumped
  • if zoom between 0.5x - 4.0x doesn't break any UI elements right away
  • if scaling is uniform across all widgets and texts
  • is every text readable and fully visible (main window, viewers list, emote picker, user card) and doesn't go out of bounds
  • if multiple monitors at different DPI are available, move chatterino windows between them while changing zoom level

there will be more things to fix for sure, but heres start

@jupjohn
Copy link
Contributor

jupjohn commented Apr 23, 2022

@LosFarmosCTL are you able to do some macOS testing for this one?

@LosFarmosCTL
Copy link
Contributor

@LosFarmosCTL are you able to do some macOS testing for this one?

Sure, I'll do some testing sometime today.

Copy link
Contributor

@LosFarmosCTL LosFarmosCTL left a 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.

chatterino 2022-04-23 at 17 10 15

chatterino 2022-04-23 at 17 12 04 chatterino 2022-04-23 at 17 12 09

chatterino 2022-04-23 at 17 12 58 chatterino 2022-04-23 at 17 13 05

chatterino 2022-04-23 at 17 13 25 chatterino 2022-04-23 at 17 13 29

chatterino 2022-04-23 at 17 51 16 chatterino 2022-04-23 at 17 51 10

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:

chatterino 2022-04-23 at 17 14 41

2. The hotkey table in settings:

chatterino 2022-04-23 at 17 47 32

3. The account list in settings:

chatterino 2022-04-23 at 17 48 07

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.

@kornes
Copy link
Contributor Author

kornes commented Apr 23, 2022

Thanks for testing!
can you post screen of how Filters tab looks for reference? on windows Accounts and Filters tab appears to use same font size.
Settings window is explicitly excluded from all scaling rest of the app use so didn't look further inside, might be something to look into in next PR :P

@LosFarmosCTL
Copy link
Contributor

Filters:

image

Hotkeys:

image

@kornes
Copy link
Contributor Author

kornes commented Apr 23, 2022

both mentioned tabs were the only one using custom fonts, now removed so all settings tabs should look the same.

Now, categories need some clear visibility so i used background coloring instead (qt's dark gray):
img

img2

if people wont like this solution, could revert it and make bigger fonts work, but i think its not bad :D

@LosFarmosCTL
Copy link
Contributor

@LosFarmosCTL sorry for ping again, would you be able to verify the macOS ones (and any others you might be able to reproduce)?

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.

@Felanbird
Copy link
Collaborator

@RAnders00 Do you mind testing this PR - as you are the issue holder of 5/12 of these issues 😁

@jupjohn
Copy link
Contributor

jupjohn commented Apr 25, 2022

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):

160 DPI:
image

96 DPI (changed with app running):
image

@kornes
Copy link
Contributor Author

kornes commented Apr 26, 2022

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

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.
Runtime DPI change is still poorly handled (didn't touch that in this pr) and will need lot more research and testing, but impact is small as its mostly affecting users who dock their laptops and/or swap to external monitors with app running.
Moving windows between monitors worked fine from my testing so far and rescale between DPIs with no issues.

@kornes kornes marked this pull request as draft April 30, 2022 00:10
@kornes kornes changed the title Font/UI scaling improvements [WIP] Font/UI scaling improvements Apr 30, 2022
@Xyncgas
Copy link

Xyncgas commented Jul 2, 2022

image

@Xyncgas
Copy link

Xyncgas commented Jul 2, 2022

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

@Brog33
Copy link

Brog33 commented Aug 28, 2022

I'd like to do some testing on Ubuntu.

@Nerixyz Nerixyz mentioned this pull request Dec 3, 2022
1 task
@Caf
Copy link

Caf commented Mar 10, 2023

This is still not merged in the main branch right? I just have to wait? :p

@Brog33
Copy link

Brog33 commented Mar 12, 2023

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.

@dead-claudia
Copy link

What's the status of this PR?

Copy link
Contributor

@github-actions github-actions bot left a 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

src/controllers/hotkeys/HotkeyModel.cpp Show resolved Hide resolved
src/controllers/hotkeys/HotkeyModel.cpp Show resolved Hide resolved
src/singletons/Fonts.cpp Show resolved Hide resolved
src/singletons/Fonts.cpp Show resolved Hide resolved
src/singletons/Fonts.cpp Show resolved Hide resolved
src/widgets/Label.cpp Outdated Show resolved Hide resolved
src/widgets/helper/NotebookTab.cpp Show resolved Hide resolved
src/widgets/settingspages/GeneralPage.cpp Outdated Show resolved Hide resolved
src/widgets/splits/Split.cpp Show resolved Hide resolved
src/widgets/splits/SplitInput.cpp Show resolved Hide resolved
@pajlada pajlada marked this pull request as ready for review March 23, 2024 11:36
@pajlada pajlada changed the title [WIP] Font/UI scaling improvements Font/UI scaling improvements Mar 23, 2024
@pajlada
Copy link
Member

pajlada commented Mar 23, 2024

Would love to get this tested by folks with different setups

On my X11/Arch Linux btw system, there's no difference at all.
On my remote desktop mac running macOS sonoma with no scaling, everything looks great. The "Change channel" popup is now readable, tabs are now readable, split headers are now readable

@LosFarmosCTL
Copy link
Contributor

I recently bought a high-DPI MacBook, so I could finally test this on macOS for both low- and high-DPI displays.

On my old Hackintosh, with a standard 1080p monitor, the changes look great, everything is at a reasonable scale and legible.

For high-DPI, there still appear to be some issues though:

image

The tab titles and the change channel dialog are still way too small, while they appear fine on low-DPI screens.

Some stuff works completely fine on high-DPI as well though, such as the table cell content on settings pages:

image

Overall the only things I could find that don't work are the tab titles, message input box and the change channel dialog.

@Nerixyz
Copy link
Contributor

Nerixyz commented Mar 23, 2024

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).

@LosFarmosCTL
Copy link
Contributor

Tooltips appear fine on high-dpi macOS:

image

Labels have the same issues as tab titles, etc.

SCR-20240323-maou

Both are fine on a low-dpi screen.

@Nerixyz
Copy link
Contributor

Nerixyz commented Mar 23, 2024

Labels have the same issues as tab titles, etc.

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)?

@LosFarmosCTL
Copy link
Contributor

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)?

image

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:

image image

That change is enough to make everything legible though.

@Nerixyz
Copy link
Contributor

Nerixyz commented Mar 23, 2024

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 this->qtFontScale() instead of this->scale() fixes this, but the account-switch button is too small (probably fixed by #4868).

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

Successfully merging this pull request may close these issues.

None yet