Investigate if KIconThemes is needed as a framework
Open, Needs TriagePublic

Description

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

Yes, please! The only thing I'm aware of that isn't available by other means is obtaining the file path of the icon, which is e.g. needed for use in Grantlee/HTML.

As disccused with @davidedmundson : review where icon file path access is used, might be replaceable with an icon "image provider" thingy for QTextBrowser/QWebEngine.

The KPixmapSequence stuff seems to be mostly used for "busy indiators" for which we now have a lovely KBusyIndicatorWidget

There's still many users of the icon loading and icon sizes, might make sense to port that first to QIcon, and then assess what's left. Right now it's hard to identify the problematic bits.

cfeck added a subscriber: cfeck.Sep 12 2019, 12:04 PM

KIconThemes also offers the icon chooser (button and dialog).

KIconThemes also offers the icon chooser (button and dialog).

Right, for which we need listing all icons in the theme, which QIcon doesn't have either I think. This is however used far less often, so dependency-wise it's the smaller problem I think.

dfaure added a subscriber: dfaure.Sep 12 2019, 12:26 PM

Then this task has the wrong title. It's not about killing the framework, but about not using it for simple icon loading use cases. Right?

I'd say that depends on what actually remains. The button and dialog could also fit in something like widget addons, unless forced to be separate by dependencies. Having this separate I think was mainly needed due to this being used all over the place.

KWidgetAddons is Tier 1. KIconThemes was separated, because it needs KConfig (and other dependencies).

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?

There is not. You should use this.

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:

  • PM_ToolBarIconSize
  • PM_SmallIconSize
  • PM_LargeIconSize
  • PM_IconViewIconSize
  • PM_ListViewIconSize
  • PM_TabBarIconSize
  • PM_MessageBoxIconSize
  • PM_ButtonIconSize
  • PM_TitleBarButtonIconSize

KIconThemes has the follow "semantic" sizes:

  • Desktop
  • Toolbar
  • MainToolbar
  • Small
  • Panel
  • Dialog

as well as the following "syntactic" sizes (ie. merely constants for fixed numeric values, without configuration):

  • SizeSmall
  • SizeSmallMedium
  • SizeMedium
  • SizeLarge
  • SizeHuge
  • SizeEnormous

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:

  • Toolbar, MainToolbar -> PM_ToolBarIconSize
  • Small -> PM_SmallIconSize
  • Dialog -> PM_MessageBoxIconSize

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.

cullmann added a comment.EditedMon, Oct 28, 6:02 PM

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.

I have here some patch that moves this icon loader "hacks" to breeze-icons themself.

https://phabricator.kde.org/D25119

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.

ndavis added a subscriber: ndavis.Sun, Nov 10, 8:19 AM

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