Deprecate SmallIcon/DesktopIcon/BarIcon/UserIcon/MainBarIcon and KDE::icon in favor of KIconLoader or QIcon API
Open, Needs TriagePublic

Description

A lot of this is fixed-size pixmap based, which is problematic for high dpi support.

vkrause created this task.Sep 13 2019, 2:07 PM

UserIcon is now only used in Kolourpaint, uses in PIM were actually wrong and have been replaced.

I just ported Kolourpaint away from UserIcon.

As part of this task, KDE::icon should actually be deprecated in favor of QIcon::fromTheme(). Seems it's not yet deprecated.

As part of this task, KDE::icon should actually be deprecated in favor of QIcon::fromTheme(). Seems it's not yet deprecated.

KDE::icon optionally takes a list of overlays, something that QIcon::fromTheme() cannot do afaik

OK then we could deprecate KDE::icon(1 arg), and keep KDE::icon(2 args)

nicolasfella added a comment.EditedDec 2 2019, 12:13 AM

Another usage of KDE::icon that is not trivially replacable by QIcon::fromTheme:
https://cgit.kde.org/systemsettings.git/tree/sidebar/ToolTips/tooltipmanager.cpp#n224

It's using KIconLoader with a custom palette

For overlays there's an alternative with KIconUtils from KGuiAddons.
Besides that it might still make sense to move from KDE::Icon to the more canonical KIconLoader API, we use the KDE:: namespace nowhere else AFAIK.

DesktopIcon is now only used in kexi

Wow I had never seen KIconUtils. This is very nice stuff (because very independent from anything else).

I also agree that KDE:: is weird and that a KIconLoader method would be better unless KIconUtils can do the job.

ahmadsamir added a subscriber: ahmadsamir.EditedDec 16 2020, 2:45 PM

For overlays there's an alternative with KIconUtils from KGuiAddons.

KIconUtils::addOverlay/addOverlays() wouldn't work for replacing:

QIcon KDE::icon(const QString &iconName, const QStringList &overlays, KIconLoader *iconLoader)
{
    return QIcon(new KIconEngine(iconName, iconLoader ? iconLoader : KIconLoader::global(), overlays));
}

right? since this one takes a QStringList of "overlays" (to account for e.g. different device states, connected, disconnected... etc).

(KIconUtils::addOverlays() doesn't something different than the above KDE::icon() method.

Besides that it might still make sense to move from KDE::Icon to the more canonical KIconLoader API, we use the KDE:: namespace nowhere else AFAIK.

So, IIUC, we replace the above KDE::icon() with something like KIconLoader::iconWithOverLays().

EDIT: replace, as in introduce new method, deprecate old one of course.

I've created https://invent.kde.org/frameworks/kiconthemes/-/merge_requests/15 which adds a new method, KIconLoader::iconWithOverlays().

A replacement for KDE::icon() is now in place, KIconUtils::addOverlays() https://invent.kde.org/frameworks/kguiaddons/-/merge_requests/13

(And please see the comments I posted at https://invent.kde.org/frameworks/kiconthemes/-/merge_requests/16).

SmallIcon/DesktopIcon/BarIcon/UserIcon/MainBarIcon is done.

KDE::icon is still used in use, but it's ~fine, it uses QIcon instead of QPixmap

nicolasfella moved this task from Backlog to Done on the KF6 board.Jul 12 2023, 9:42 AM