Summary: | Search icon in search fields around Plasmashell is too large when NOT using Plasma scaling | ||
---|---|---|---|
Product: | [Frameworks and Libraries] libplasma | Reporter: | Jin Liu <ad.liu.jin> |
Component: | components | Assignee: | Plasma Bugs List <plasma-bugs> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | carl, kde, me, nate, noahadvs, notmart, qydwhotmail |
Priority: | HI | ||
Version: | 5.94.0 | ||
Target Milestone: | --- | ||
Platform: | Other | ||
OS: | Linux | ||
See Also: | https://bugs.kde.org/show_bug.cgi?id=356446 | ||
Latest Commit: | https://invent.kde.org/frameworks/plasma-framework/commit/26ae86dff198fbce05ba3824b89786224b3792a9 | Version Fixed In: | 5.96 |
Sentry Crash Report: | |||
Attachments: | search box in plasmashell vs. systemsettings |
Did you scale the system by changing the Font DPI instead of using the scaling slider in System Settings > Display and Monitor? If so that's not supported, because it causes issues like this. I would recommend using the scaling slider instead, and also use the Wayland session instead of the X11 session If you have to use the X11 session, use the scaling slider and set the `PLASMA_USE_QT_SCALING=1` environment variable in /etc/environment which will make HiDPI scaling in Plasma work better as long as you never need to connect a second screen with a different DPI. If you do, use the Wayland session. (In reply to Nate Graham from comment #1) > Did you scale the system by changing the Font DPI instead of using the > scaling slider in System Settings > Display and Monitor? No, I used the global scaling slider. But whenever I set the global scaling to 250%, it automatically enables the force font DPI and set it to 240dpi. I add a new user and reboot into it. (also deleted /etc/sddm.conf.d/kde_settings.conf, because it has a "ServerArguments=-dpi 240" which I'm not sure if matters) The default state is "force font DPI" disabled and the number is 96, and global scaling at 100%. Both Plasmashell and apps have tiny icons and normal-sized texts. But the too-large-search-icon problem is the same. Then I set the global scaling to 250%, still the same problem. > If so that's not supported, because it causes issues like this. I would > recommend using the scaling slider instead, and also use the Wayland session > instead of the X11 session I tried the Wayland session. Yes it works fine. > If you have to use the X11 session, use the scaling slider and set the > `PLASMA_USE_QT_SCALING=1` environment variable in /etc/environment which > will make HiDPI scaling in Plasma work better as long as you never need to > connect a second screen with a different DPI. If you do, use the Wayland > session. Also tried X11 session with PLASMA_USE_QT_SCALING=1, yes it works fine too. Thanks. OK, seems like an issue with the icon in the Plasma search field component that is caused by using Plasma scaling, and solved by using Qt scaling. But Plasma scaling is still supported and used by default on X11, so we should fix it. The icon has this set for its size policy: implicitHeight: PlasmaCore.Units.iconSizes.sizeForLabels implicitWidth: PlasmaCore.Units.iconSizes.sizeForLabels So I wonder if sizeForLabels is generally broken. Probably something here is incorrect: int Units::roundToIconSize(int size) { // If not using Qt scaling (e.g. on X11 by default), manually scale all sizes // according to the DPR because Qt hasn't done it for us. Otherwise all // returned sizes are too small and icons are only the right size by pure // chance for the larger sizes due to their large differences in size qreal multiplier = 1.0; QScreen *primary = QGuiApplication::primaryScreen(); if (primary) { // Note that when using Qt scaling, the multiplier will always be 1 // because Qt will scale everything for us. This is good and intentional. multiplier = bestIconScaleForDevicePixelRatio((qreal)primary->logicalDotsPerInchX() / (qreal)96); (In reply to Nate Graham from comment #4) > Probably something here is incorrect: > > > int Units::roundToIconSize(int size) > { > // If not using Qt scaling (e.g. on X11 by default), manually scale all > sizes > // according to the DPR because Qt hasn't done it for us. Otherwise all > // returned sizes are too small and icons are only the right size by pure > // chance for the larger sizes due to their large differences in size > qreal multiplier = 1.0; > QScreen *primary = QGuiApplication::primaryScreen(); > if (primary) { > // Note that when using Qt scaling, the multiplier will always be 1 > // because Qt will scale everything for us. This is good and > intentional. > multiplier = > bestIconScaleForDevicePixelRatio((qreal)primary->logicalDotsPerInchX() / > (qreal)96); Indeed. This roundToIconSize function is used here: https://github.com/KDE/plasma-framework/blob/b5f8095d96320bd14f724d6bb1da17e3d1dc8bbf/src/declarativeimports/core/units.cpp#L107 > void Units::iconLoaderSettingsChanged() > { > m_iconSizes->insert(QStringLiteral("desktop"), devicePixelIconSize(KIconLoader::global()->currentSize(KIconLoader::Desktop))); > > m_iconSizes->insert(QStringLiteral("tiny"), devicePixelIconSize(KIconLoader::SizeSmall) / 2); > m_iconSizes->insert(QStringLiteral("small"), devicePixelIconSize(KIconLoader::SizeSmall)); > m_iconSizes->insert(QStringLiteral("smallMedium"), devicePixelIconSize(KIconLoader::SizeSmallMedium)); > m_iconSizes->insert(QStringLiteral("medium"), devicePixelIconSize(KIconLoader::SizeMedium)); > m_iconSizes->insert(QStringLiteral("large"), devicePixelIconSize(KIconLoader::SizeLarge)); > m_iconSizes->insert(QStringLiteral("huge"), devicePixelIconSize(KIconLoader::SizeHuge)); > m_iconSizes->insert(QStringLiteral("enormous"), devicePixelIconSize(KIconLoader::SizeEnormous)); > // gridUnit is always the font height here > m_iconSizes->insert(QStringLiteral("sizeForLabels"), devicePixelIconSize(roundToIconSize(QFontMetrics(QGuiApplication::font()).height()))); > > m_iconSizeHints->insert(QStringLiteral("panel"), devicePixelIconSize(KIconLoader::global()->currentSize(KIconLoader::Panel))); > m_iconSizeHints->insert(QStringLiteral("desktop"), devicePixelIconSize(KIconLoader::global()->currentSize(KIconLoader::Desktop))); Note all these sizes are passed to devicePixelIconSize(), which is: https://github.com/KDE/plasma-framework/blob/b5f8095d96320bd14f724d6bb1da17e3d1dc8bbf/src/declarativeimports/core/units.cpp#L194 > int Units::devicePixelIconSize(const int size) const > { > /* in kiconloader.h > enum StdSizes { > SizeSmall=16, > SizeSmallMedium=22, > SizeMedium=32, > SizeLarge=48, > SizeHuge=64, > SizeEnormous=128 > }; > */ > // Scale the icon sizes up using the devicePixelRatio > // This function returns the next stepping icon size > // and multiplies the global settings with the dpi ratio. > const qreal ratio = devicePixelRatio(); > > return size * bestIconScaleForDevicePixelRatio(ratio); > } Note this function already multiplies its parameter by the zoom ratio. So roundToIconSize(QFontMetrics(QGuiApplication::font()).height()) shouldn't use the zoom ratio, otherwise the ratio is multiplied by twice. However, roundToIconSize is called in many other places, e.g. https://github.com/KDE/plasma-desktop/blob/48a0fc3719de3a87c394af905b16f8cedb9b6c08/applets/window-list/contents/ui/MenuButton.qml#L59 So I've no idea whether A. Units::roundToIconSize should not multiply by zoom ratio, which affects sizes all around Plasma. B. The line devicePixelIconSize(roundToIconSize(QFontMetrics(QGuiApplication::font()).height())) should be changed to roundToIconSize(QFontMetrics(QGuiApplication::font()).height()), which should only affect sizeForLabels. BTW, seems this bug should happen on any HiDPI display, not limited to 2.5x. This makes me wonder how many developers are using HiDPI + X11/Plasma Scaling daily... (In reply to Jin Liu from comment #6) > BTW, seems this bug should happen on any HiDPI display, not limited to 2.5x. > This makes me wonder how many developers are using HiDPI + X11/Plasma > Scaling daily... Perhaps not many as X11 HiDPI users tend to enable Qt scaling. It seems that using Plasma scaling actually fixes this. With Plasma scaling turned on on X11, I don't see the issue in Plasma or KRunner anymore (which respect the environment variable), but I do still see it in the search fields for KWin's Overview and Present Windows effects, because KWin doesn't respect that environment variable and always uses Plasma scaling on X11. Keeping the HI priority since X11 with Plasma scaling are the default settings for now. Git commit 26ae86dff198fbce05ba3824b89786224b3792a9 by Nate Graham. Committed on 23/06/2022 at 18:58. Pushed by ngraham into branch 'master'. Units: Fix sizeForLabels double-scaling icons with Plasma scaling The sizeForLabels unit internally uses roundToIconSize(), giving it a value and feeding that into devicePixelIconSize() to take into account required icons size differences when using Plasma scaling. But roundToIconSize() internally does that automatically! So as a result, the size is double-scaled and looks too big. To fix this, we simply have to stop using devicePixelIconSize() for this unit. FIXED-IN: 5.96 M +3 -2 src/declarativeimports/core/units.cpp https://invent.kde.org/frameworks/plasma-framework/commit/26ae86dff198fbce05ba3824b89786224b3792a9 |
Created attachment 149056 [details] search box in plasmashell vs. systemsettings SUMMARY The search icon in search box is too large, on HiDPI screen (240dpi, 3200x2000, X11). This only happens in plasmashell, e.g. Kickoff, Krunner, Klipper, NetworkManager. Search boxes in systemsettings and Discover are OK. SOFTWARE/OS VERSIONS Linux/KDE Plasma: archlinux (available in About System) KDE Plasma Version: 5.24.90 KDE Frameworks Version: 5.94.0 Qt Version: 5.15.4