digitaltrails / vdu_controls

VDU controls - a control panel for monitor brightness/contrast/...
GNU General Public License v3.0
103 stars 4 forks source link

High DPI versus normal DPI customisations #63

Closed digitaltrails closed 8 months ago

digitaltrails commented 9 months ago

While Qt does support automatically coping with High DPI and Normal DPI displays, when the application overrides some icon and pixmap sizes, the sizing then becomes fixed no matter what the DPI.

For example, vdu_controls was developed on a High-DPI desktop, and some icons and the splash screen were sized specifically for that environment. Sizes needed to be specified because the Qt defaults weren't quite what was needed.

Where vdu_controls explicitly sets icon and pixmaps sizes, it needs to account for High and Normal DPI in order to be well laid out in both DPI environments.

Currently, for Normal DPI:

  1. The splash screen is excessively large.
  2. The main screen control icons are larger than seems proportionate.
  3. The Preset Dialog Activate-Edit button icons are also too large.

Where the code makes choices about sizes, it needs to test for High-DPI and make appropriate choices for Normal versus High DPI.

digitaltrails commented 9 months ago

When icons and pixmaps have custom sizes, the size chosen will now depend on whether a screen is Normal or High DPI. Here are the before and after screen-captures for Normal-DPI:

high-dpi-before

high-dpi-after

digitaltrails commented 9 months ago

With High-DPI, it's difficult to know what's appropriate, some folk may have scaled their displays, others may have just adjusted font sizes (me), and some may have done both. I've added adjustments that make things look right for me, but that might not suit everyone, so I've also added an adjust-for-dpi setting that can turn off these adjustments off.

It's also possible some may not like the adjustments I've made to Normal-DPI. For Normal-DPI, I've made the icons a bit smaller to match the proportions I see in High-DPI. For now that's just tough, but if anyone objects, I guess I could add another setting.

digitaltrails commented 9 months ago

Closing, will reopen if anyone raises a related problem.

digitaltrails commented 8 months ago

The choice of icon sizes might be better proportioned by interrogating the default font size in use. This would have the advantage of being fully automatic with no need to be aware of DPI (the new setting could be removed).

The approach would be to create QLabel containing some text and either use the height of the label or height of the text to derive an icon size.

RokeJulianLockhart commented 8 months ago
  1. Screenshot_20231019_160103

  2. Screenshot_20231019_160338

The new version now renders icons too small regardless of whether the DPI awareness option is enabled or not.

digitaltrails commented 8 months ago

To generalise DPI adjustments, I need to detect High DPI in a way that also to accounts for any desktop scaling.

As noted above, I thought measuring a rendered font might be a good way to find out a true measure of font versus icon size that would account for both DPI and scaling. The approach is confirmed as workable by experimentation and by https://forum.qt.io/topic/131666/high-dpi-scaling-detection.

I've checked in 210be3e which bumps the version to 1.12.1 and implements this approach, in essence I just create a label and measure it's realised text height in pixels:

    font_height = QFontMetrics(QLabel("ABC").font()).height()
    high_dpi = font_height > 30
    log_info(f"HighDPI: {high_dpi} (standard font height={font_height})")

As far as I can tell most scaled displays have a general font size around 20-22 and unscaled 4K displays seem to be about 30.

This should stop high DPI icons from being used if a high-DPI desktop is scaled. It's possible adjust-for-dpi flag is no longer necessary, but I can't be absolutely sure, so I'll leave it in place.

I missed this issue because I don't use scaling. I don't use scaling because some photo-editing GUI's and browsers do not show the image at full DPI when scaled.

digitaltrails commented 8 months ago
  1. Screenshot_20231019_160103

  2. Screenshot_20231019_160338

The new version now renders icons too small regardless of whether the DPI awareness option is enabled or not.

Can you run the latest code in a console, it should report itself to be v1.2.1 and report what it outputs for:

INFO: HighDPI: True (standard font height=31)

I only have a 4K and a FHD monitor, so those are the only two allowed for - which two possible icon sizes. Maybe I need to add a third size.

digitaltrails commented 8 months ago

The slider icon sizes are custom, either 40 or 52.

The default bottom panel icon size appears to be 22, and I bump that to 32.

In your screen-capture, the bottom icons look to small compared to the slider too big. Do you think the slider icon is too small too?

But you also appear to be using a (custom?) fixed font, that might also have some affect.

All this stuff is a bit delicate. There's gnome and deepin to consider as well, Wayland as well, but hopefully three steps might cover FHD, 2k and 4K - but scaling may be a spanner in the works.

digitaltrails commented 8 months ago

Maybe I should just compute icon sizes based on font height - have to derive the right algorithm though, e.g. icon_height = current_icon_height * font_hight/standard_font_height

RokeJulianLockhart commented 8 months ago

https://github.com/digitaltrails/vdu_controls/issues/63#issuecomment-1771613747

@digitaltrails, vdu_controls outputs

```log RokeJulianLockhart@s1e8h4:~> vdu_controls 22:59:21 INFO: VDU Controls 1.12.0 /usr/bin/vdu_controls 22:59:21 INFO: python-locale: ('en_GB', 'UTF-8') Qt-locale: en_GB 22:59:21 INFO: app-style: breeze (detected a dark theme) 22:59:21 INFO: Looking for config file '/home/RokeJulianLockhart/.config/vdu_controls/vdu_controls.conf' 22:59:21 INFO: Using config file '/home/RokeJulianLockhart/.config/vdu_controls/vdu_controls.conf' ```

vdu_controls --adjust-for-dpi

  1. ```log RokeJulianLockhart@s1e8h4:~> vdu_controls --adjust-for-dpi 23:06:03 INFO: VDU Controls 1.12.0 /usr/bin/vdu_controls 23:06:03 INFO: python-locale: ('en_GB', 'UTF-8') Qt-locale: en_GB 23:06:03 INFO: app-style: breeze (detected a dark theme) 23:06:03 INFO: Looking for config file '/home/RokeJulianLockhart/.config/vdu_controls/vdu_controls.conf' 23:06:03 INFO: Using config file '/home/RokeJulianLockhart/.config/vdu_controls/vdu_controls.conf' 23:06:03 INFO: Logging to stdout 23:06:03 INFO: Using system tray. 23:06:03 INFO: Configuring application (reconfiguring=False)... 23:06:03 INFO: ddcutil version (1, 4, 1) (dynamic-sleep=False) 23:06:06 INFO: Number of detected monitors is stable at 1 (loop=2) 23:06:06 INFO: Initializing controls for vdu_number='1' vdu_model_name='Q3279WG5B' self.vdu_stable_id='Q3279WG5B_40899' 23:06:06 INFO: Using config file '/home/RokeJulianLockhart/.config/vdu_controls/Q3279WG5B_40899.conf' 23:06:06 INFO: splash_message: 'DDC ID 1\nQ3279WG5B:40899' 23:06:07 INFO: splash_message: 'Checking Presets' 23:06:07 INFO: Scheduling presets reconfiguring=True 23:06:07 INFO: Will update solar elevation activations tomorrow at 2023-10-20 00:00:00+01:00 (in 54 minutes) 23:06:07 INFO: Completed configuring application 23:06:38 INFO: App exit rc=0 RokeJulianLockhart@s1e8h4:~> ```
  2. image

vdu_controls --no-adjust-for-dpi

  1. ```log RokeJulianLockhart@s1e8h4:~> vdu_controls --no-adjust-for-dpi 23:07:13 INFO: VDU Controls 1.12.0 /usr/bin/vdu_controls 23:07:13 INFO: python-locale: ('en_GB', 'UTF-8') Qt-locale: en_GB 23:07:13 INFO: app-style: breeze (detected a dark theme) 23:07:13 INFO: Looking for config file '/home/RokeJulianLockhart/.config/vdu_controls/vdu_controls.conf' 23:07:13 INFO: Using config file '/home/RokeJulianLockhart/.config/vdu_controls/vdu_controls.conf' 23:07:13 WARNING: Command line adjust-for-dpi=no overrides vdu-controls-globals.adjust-for-dpi=yes (in /home/RokeJulianLockhart/.config/vdu_controls/vdu_controls.conf) 23:07:13 INFO: Logging to stdout 23:07:13 INFO: Using system tray. 23:07:13 INFO: Configuring application (reconfiguring=False)... 23:07:13 INFO: ddcutil version (1, 4, 1) (dynamic-sleep=False) 23:07:16 INFO: Number of detected monitors is stable at 1 (loop=2) 23:07:16 INFO: Initializing controls for vdu_number='1' vdu_model_name='Q3279WG5B' self.vdu_stable_id='Q3279WG5B_40899' 23:07:16 INFO: Using config file '/home/RokeJulianLockhart/.config/vdu_controls/Q3279WG5B_40899.conf' 23:07:16 INFO: splash_message: 'DDC ID 1\nQ3279WG5B:40899' 23:07:17 INFO: splash_message: 'Checking Presets' 23:07:17 INFO: Scheduling presets reconfiguring=True 23:07:17 INFO: Will update solar elevation activations tomorrow at 2023-10-20 00:00:00+01:00 (in 53 minutes) 23:07:17 INFO: Completed configuring application 23:07:37 INFO: App exit rc=0 RokeJulianLockhart@s1e8h4:~> ```
  2. image

> ![Screenshot_20231019_230942](https://github.com/digitaltrails/vdu_controls/assets/42837531/b5ab5242-9697-444d-aab7-d65f0a4f9eba)

I wonder whether the preference might not be affecting my device.

digitaltrails commented 8 months ago

Thanks, I should have given more detailed instructions. Please download v1.12.1, I've added some logging INFO on what the program decides about DPI, which is now based on font height:

08:49:32 INFO: VDU Controls 1.12.1 vdu_controls.py  
08:49:32 INFO: python-locale: ('en_NZ', 'UTF-8') Qt-locale: en_GB
08:49:32 INFO: app-style: breeze (detected a light theme)
08:49:32 INFO: Looking for config file '/home/michael/.config/vdu_controls/vdu_controls.conf'
08:49:32 INFO: Using config file '/home/michael/.config/vdu_controls/vdu_controls.conf'
08:49:32 INFO: Logging to stdout
08:49:33 INFO: HighDPI: True (standard font height=31)   <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
08:49:33 INFO: Using system tray.
08:49:33 INFO: Configuring application (reconfiguring=False)...
08:49:33 INFO: ddcutil version (2, 0, 0) (dynamic-sleep=True)
08:49:38 INFO: Number of detected monitors is stable at 2 (loop=2)
08:49:38 INFO: Initializing controls for vdu_number='1' vdu_model_name='HP_ZR24w' self.vdu_stable_id='HP_ZR24w_CNT00811J6'
08:49:38 INFO: Using config file '/home/michael/.config/vdu_controls/HP_ZR24w_CNT00811J6.conf'
08:49:38 INFO: Initializing controls for vdu_number='2' vdu_model_name='LG_HDR_4K' self.vdu_stable_id='LG_HDR_4K_86395'
08:49:38 INFO: Using config file '/home/michael/.config/vdu_controls/LG_HDR_4K_86395.conf'
08:49:38 INFO: splash_message: 'DDC ID 1\nHP_ZR24w:CNT00811J6'
08:49:39 INFO: splash_message: 'DDC ID 2\nLG_HDR_4K:86395'
08:49:41 INFO: splash_message: 'Checking Presets'
08:49:41 INFO: Reading autolux file '/home/michael/.config/vdu_controls/AutoLux.conf'
08:49:41 INFO: Lux auto-brightness settings refresh - monitoring is off.
08:49:41 INFO: Scheduling is disabled
08:49:41 INFO: Completed configuring application

Once we know what font height (in pixels) is being used, we might be able to come up with a suitable icon size and I could use that to help code an algorithm to dynamically set the icon size.

If you want to experiment with what icon size suits your desktop, you could edit you local vdu_controls.py and change the line that sets the icon size for sliders:

            svg_icon.setFixedSize(52, 52) if is_high_dpi() else svg_icon.setFixedSize(40, 40)
digitaltrails commented 8 months ago

I've pushed changes to v1.12.1 that scales icons and the splash-screen based on the pixel height of the default font, the font that would be used for any Qt QLabel, which is presumably set by some desktop setting. I think DPI is inherently expressed in the font pixel height, so the code no longer does anything based on for DPI.

I will likely remove the new adjust-for-dpi setting which now does nothing. But I need to see what this looks like on deepin, gnome, xfce, and also for a non-4k display. I've tested on a 4K display running with high-DPI and in VirtualBox running in normal DPI - to my eyes the sizing looks OK.

On startup, there is be a message that lists the standard font height in pixels:

20:01:13 INFO: standard_font_pixel_height=32

I actually round it up to a value divisible by two (trying to avoid fractional rounding in some of the later computation):

        standard_font_pixel_height = QFontMetrics(QLabel("ABC").font()).height()
        standard_font_pixel_height += standard_font_pixel_height % 2

There are various calls to a function called standard_height() in the code, should you wish to see what the scale factors applied to compute the sizes of anything, just search for it.

If a non-standard font is used as the default, I suppose it might cause issues, but I hope not.

This is tricky/messy stuff, no wonder so many applications look wrong. I also now think that allow the setting of scale-factors for the desktop doesn't provide much incentive for developers to take advantage of high-DPI. So the poor old end user gets a poorer experience, even after investing in a nice high-DPI monitor.

digitaltrails commented 8 months ago

Native_font_height is a better name for this than standard_font_height. I'm going to generalise the use of native_font_height to solve some layout issues for the different desktop DPI: 4K, FHD, 2K, etcetera.

In many places in the code, pixel dimensions, such as minimum widget sizes, have been selected based on how they look on my desktop, but I use a font height of 32 pixels (on a 4K unscaled desktop). Many of these layouts look a bit empty on KDE FHD which defaults to 22 pixels. Scaling dimensions in proportion to the native_font_height seems to result tighter layouts without breaking them. I'll go down this rabbit hole and see what happens.

digitaltrails commented 8 months ago

BTW, in researching this topic I came across http://invent.kde.org/plasma/plasma-desktop/-/issues/58. This seems to be an attempt to force a scaling only solution for aesthetic purposes. I think that's might be a bad idea in a multi-platform (gnome, kde, kde3, ...) environment where not all applications behave properly when scaled. Aesthetic goals are only part of the picture.

RokeJulianLockhart commented 8 months ago

https://github.com/digitaltrails/vdu_controls/issues/63#issuecomment-1773383497

@digitaltrails, I agree and tried to make my case there at https://invent.kde.org/plasma/plasma-desktop/-/issues/58#note_601915, but Nate was not having any of it (not that it was popular, per https://invent.kde.org/plasma/plasma-desktop/-/issues/58#note_675645). You could always try chipping in, but it might not do much to sway anyone making the decision.

digitaltrails commented 8 months ago

https://github.com/digitaltrails/vdu_controls/issues/63#issuecomment-1773383497

@digitaltrails, I agree and tried to make my case there at https://invent.kde.org/plasma/plasma-desktop/-/issues/58#note_601915, but Nate was not having any of it (not that it was popular, per https://invent.kde.org/plasma/plasma-desktop/-/issues/58#note_675645). You could always try chipping in, but it might not do much to sway anyone making the decision.

I think you made some good points. I could also see Nate's got his mind made up on this. I think it has to play out. Wait and see if it generates much push-back when it arrives on everyone's desktop.

RokeJulianLockhart commented 8 months ago

Yeah, probably. Considering that we're looking KDE-specific solutions instead of Qt ones, doesn't this demonstrate a rather severe lack of cohesion in the Qt framework? I feel like there should be a toolkit-wide way to call some standard icon sizes that relate to the current text size, since surely Qt itself renders both at some point in the pipeline. KDE Frameworks only appears to have the QML option too now that http://invent.kde.org/plasma/plasma-desktop/-/issues/58 has passed, though I'm no Qt dev.

digitaltrails commented 8 months ago

Yeah, probably. Considering that we're looking KDE-specific solutions instead of Qt ones, doesn't this demonstrate a rather severe lack of cohesion in the Qt framework? I feel like there should be a toolkit-wide way to call some standard icon sizes that relate to the current text size, since surely Qt itself renders both at some point in the pipeline. KDE Frameworks only appears to have the QML option too now that http://invent.kde.org/plasma/plasma-desktop/-/issues/58 has passed, though I'm no Qt dev.

I think High DPI has kind of thrown a spanner in the works for Qt. It doesn't quite feel right that I'm measuring text height in pixels that might not be real pixels. There are calls I can make to find out if the display is High DPI, but then there's scaling. It's been a while since I've done any Android coding, but at least that solidly distinguished pixels and density-independent-pixels - it made it clear what unit you're using.

digitaltrails commented 8 months ago

Released in v1.20.0.