With Qt having theme support and having "QIcon::fromTheme", is there any point to having KIconThemes be a lib and not just some code in our QPT?
There's some public APIs for overlays, which maybe could be upstreamed / moved into KGuiAddons.
But little else
| Status | Assigned | Task | ||
|---|---|---|---|---|
| Open | cullmann | T11637 KIconThemes Cleanups | ||
| Open | None | T11654 Deprecate SmallIcon/DesktopIcon/BarIcon/UserIcon/MainBarIcon and KDE::icon in favor of KIconLoader or QIcon API | ||
| Open | None | T12306 Port ksystemlog away from deprecated KIconLoader API | ||
| Open | None | T11865 Deprecate KIconTheme::assignIconsToContextMenu | ||
| Open | None | T14296 Split KIconThemes into GUI/Widgets libs | ||
| Open | cullmann | T14297 Upstream Icon-Recoloring to Qt (perhaps XDG, too) | ||
| Resolved | ngraham | T14340 Deprecate Panel icon size and port away from it |
The part of this that seems to cause the most wide-spread use is the size constants; overlays, icon dialogs, icon listing, icon file paths, etc are much much less common. So if we want to address something regarding dependencies, the sizes is the thing to go after.
Is there an alternative API for listing installed icon themes? I added functionality to Cuttlefish that uses this and the KCM may use it?
For icon sizes, would that be something worth adding to QStyleHints in Qt? There's also already some icon size stuff in QStyle. Our QPT could then read those values from kdeglobals like normal.
QStyle currently has:
KIconThemes has the follow "semantic" sizes:
as well as the following "syntactic" sizes (ie. merely constants for fixed numeric values, without configuration):
I guess the main question would be how to map the KIconThemes semantic sizes to the QStyle values.
From an application point of view that seems possible:
Panel and Desktop should only be relevant for parts of Plasma based on their description, but are harder to map to QStyle.
Plasma abstracts icon sizes through another layer. PlasmaFramework -> units.cpp we could hardcode equivalent in there.
Though AFAIK we don't actually use the panel size hint
Panel icon size is used as *maximum* size a widget may grow in the panel so they don't get insanely huge when you have a thick panel.
I think one important part for non-unices is the ability that iconthemes will auto-load qrc files with icons for e.g. Windows.
(I intend to extend that to support the loading of multiple themes, as at the moment this falls flat for supporting e.g. normal + dark mode)
Looking at internal dependencies, it seems the only reason KIconThemes is a tier 3+ framework with a dependency on KAuth etc is KColorScheme, used in two place. With T11587 KIconThemes would therefore automatically become a tier 2 framework.
I think one important part for non-unices is the ability that iconthemes will auto-load qrc files with icons for e.g. Windows.
Please can you expand on this.
How do the non-Plasma end up using KIconLoader? On windows or OSX you'll use their icon loader implementation. Unless you're explicitly forcing it somewhere.
You don't need to use the loader, but KIconThemes will setup QIcon themeName in a usable way.
See the code at top of
kicontheme.cpp
Without that, each and every application that e.g. wants to use breeze icons (aka all KDE applications) needs to do that stuff manually.
One thing I learned in https://phabricator.kde.org/D25119 is that KIconThemes provides the means to adjust the SVGs out of e.g. Breeze to match the palette (e.g. for Dark Mode).
This would be nice to have.
But at the moment this only works for non-resource icons on Linux with the platform integration plasma-integration.
The only place where we use KIconThemes in KDE Connect is using KIconloader to get the file path from an icon name.
https://invent.kde.org/kde/kdeconnect-kde/blob/master/plugins/sendnotifications/notificationslistener.cpp#L165
KStyle currently though does not map those, instead it does:
int KStyle::pixelMetric(PixelMetric metric, const QStyleOption *option, const QWidget *widget) const
{
switch (metric) {
case PM_SmallIconSize:
case PM_ButtonIconSize:
return KIconLoader::global()->currentSize(KIconLoader::Small);
case PM_ToolBarIconSize:
return KIconLoader::global()->currentSize(KIconLoader::Toolbar);
case PM_LargeIconSize:
return KIconLoader::global()->currentSize(KIconLoader::Dialog);
case PM_MessageBoxIconSize:
// TODO return KIconLoader::global()->currentSize(KIconLoader::MessageBox);
return KIconLoader::SizeHuge;
default:
break;
}
return QCommonStyle::pixelMetric(metric, option, widget);
}see https://phabricator.kde.org/source/frameworkintegration/browse/master/src/kstyle/kstyle.cpp$439
And given that KIconThemes API dox defines "Dialog" to be for
"Icons for use in dialog titles, page lists, etc"
this indeed might not be a match then, as this seems more about some header-inlined icon, not the "picture" symbol icon in the left dialog column. Also confirmed by
Looking back, KMessageBox also used KIconLoader::SizeHuge (64, in older version SizeLarge 48) to create the pixmap:
https://phabricator.kde.org/source/kdelibs/browse/KDE%252F4.14/kdeui/dialogs/kmessagebox.cpp$102
Also is the default value for "Dialog" "32" (https://phabricator.kde.org/source/kiconthemes/browse/master/src/kicontheme.cpp$374), which is not the size one expects for the message box side picture icon.
So, KIconLoader::Dialog -> PM_LargeIconSize like done by KStyle seems the better mapping.
You are right (and that matches how we have been porting this when moving away from IconSize() so far). It's actually rather KIconLoader::Desktop that maps to PM_MessageBoxIconSize, when used in application code.
From KDevelop I can report those usages of KIconThemes:
Semi related: I'm trying to standardise the way SVG icons are colourised: https://gitlab.gnome.org/GNOME/gtk/issues/1762
If this does become a standard we can /maybe/ move it into Qt?
One thing I thought about today. KIconLoader/Theme has MatchExact. Using this I can for example get an icon for "start-here-kde-plasma" without falling back to "start-here" which is a foot in Adwaita. I think this is not possible with QIcon
I don't have a particular use case right now I thought of it during thinking about https://bugs.kde.org/show_bug.cgi?id=415173 . We surely don't want to display a gnome icon in systemsettings. But that can calso be solved by shipping the icon in the repo.
I learned the hard way in https://invent.kde.org/frameworks/kiconthemes/-/merge_requests/26 that our current KIconEngine is no proper 1:1 replacement for the QSvgIconEngine.
On the other hand, without our own engine, breeze icons are not recolored and therefor useless in dark mode settings, see https://kate-editor.org/post/2021/2021-03-07-cross-platform-light-dark-themes-and-icons/
Perhaps a "proper" solution would be to have the recoloring in Qt.
But not sure how feasible that is.
I think that's what @davidedmundson was hinting at in https://phabricator.kde.org/T11637#214021
Yeah, but that went nowhere, true, I somehow did forget about it ;=)
That would be cool to have, at the moment I still need to patch the icon themes engine "on" for Windows/macOS and in e.g. GNOME/.... Kate will just break (like any other KDE application that relies on Breeze icons for some actions).
Short dump of notes done during sprint:
Notes room 2: KIconThemes
icon coloring => upstream? https://gitlab.gnome.org/GNOME/gtk/-/issues/1762
Get rid of KIconLoader public API in favor of QIcon API
Keep auxiliary API like KIconButton, possibly move to something like KWidgetsAddons
icon size: Querying icon sizes via pixelmetrics is ugly, better API in Qt? style hints? be able to override via platform theme? Integration with Kirigami.Theme
porting issues: which size to use?
One remaining thing not solved by moving the 2 classes to WidgetAddons and re-coloring upstreaming is the auto-loading of the RCC stuff.
KIconThemes also defines standard icon sizes which would need to be moved elsewhere if we remove it.