backport candidate systray-icon cpu fix from master (kde#356479)

8 years ago · b90b0a5fd5
parent 6108a2250d
commit b90b0a5fd5
3 changed files with 220 additions and 1 deletions
--- a/0001-Port-to-new-plasma-framework-API.patch
+++ b/0001-Port-to-new-plasma-framework-API.patch
@ -0,0 +1,100 @@
 From d1708e3011969823e67869b8a1c639f15b6fd73a Mon Sep 17 00:00:00 2001
 From: Aleix Pol <aleixpol@kde.org>
 Date: Mon, 10 Oct 2016 16:30:24 +0200
 Subject: [PATCH 1/2] Port to new plasma-framework API
 Reduces unnecessary castings.
 Ports away the WindowedWidgets runner from KService
 REVIEW: 129101
 ---
 applets/systemtray/systemtray.cpp | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)
 diff --git a/applets/systemtray/systemtray.cpp b/applets/systemtray/systemtray.cpp
 index e1cd610..ecc23a4 100644
 --- a/applets/systemtray/systemtray.cpp
 +++ b/applets/systemtray/systemtray.cpp
@@ -99,19 +99,19 @@ void SystemTray::init()
 {
     Containment::init();
 -    for (const auto &info: Plasma::PluginLoader::self()->listAppletInfo(QString())) {
 -        if (!info.isValid() || info.property(QStringLiteral("X-Plasma-NotificationArea")) != "true") {
 +    for (const auto &info: Plasma::PluginLoader::self()->listAppletMetaData(QString())) {
 +        if (!info.isValid() || info.value(QStringLiteral("X-Plasma-NotificationArea")) != "true") {
             continue;
         }
 -        m_systrayApplets[info.pluginName()] = info;
 +        m_systrayApplets[info.pluginId()] = KPluginInfo(info);
 -        if (info.isPluginEnabledByDefault()) {
 -            m_defaultPlasmoids += info.pluginName();
 +        if (info.isEnabledByDefault()) {
 +            m_defaultPlasmoids += info.pluginId();
         }
 -        const QString dbusactivation = info.property(QStringLiteral("X-Plasma-DBusActivationService")).toString();
 +        const QString dbusactivation = info.value(QStringLiteral("X-Plasma-DBusActivationService"));
         if (!dbusactivation.isEmpty()) {
 -            qCDebug(SYSTEM_TRAY) << "ST Found DBus-able Applet: " << info.pluginName() << dbusactivation;
 -            m_dbusActivatableTasks[info.pluginName()] = dbusactivation;
 +            qCDebug(SYSTEM_TRAY) << "ST Found DBus-able Applet: " << info.pluginId() << dbusactivation;
 +            m_dbusActivatableTasks[info.pluginId()] = dbusactivation;
         }
     }
 }
@@ -119,12 +119,12 @@ void SystemTray::init()
 void SystemTray::newTask(const QString &task)
 {
     foreach (Plasma::Applet *applet, applets()) {
 -        if (!applet->pluginInfo().isValid()) {
 +        if (!applet->pluginMetaData().isValid()) {
             continue;
         }
         //only allow one instance per applet
 -        if (task == applet->pluginInfo().pluginName()) {
 +        if (task == applet->pluginMetaData().pluginId()) {
             //Applet::destroy doesn't delete the applet from Containment::applets in the same event
             //potentially a dbus activated service being restarted can be added in this time.
             if (!applet->destroyed()) {
@@ -156,7 +156,7 @@ void SystemTray::newTask(const QString &task)
 void SystemTray::cleanupTask(const QString &task)
 {
     foreach (Plasma::Applet *applet, applets()) {
 -        if (!applet->pluginInfo().isValid() || task == applet->pluginInfo().pluginName()) {
 +        if (!applet->pluginMetaData().isValid() || task == applet->pluginMetaData().pluginId()) {
             //we are *not* cleaning the config here, because since is one
             //of those automatically loaded/unloaded by dbus, we want to recycle
             //the config the next time it's loaded, in case the user configured something here
@@ -255,11 +255,11 @@ Q_INVOKABLE QString SystemTray::plasmoidCategory(QQuickItem *appletInterface) co
     }
     Plasma::Applet *applet = appletInterface->property("_plasma_applet").value<Plasma::Applet*>();
 -    if (!applet || !applet->pluginInfo().isValid()) {
 +    if (!applet || !applet->pluginMetaData().isValid()) {
         return "UnknownCategory";
     }
 -    const QString cat = applet->pluginInfo().property(QStringLiteral("X-Plasma-NotificationAreaCategory")).toString();
 +    const QString cat = applet->pluginMetaData().value(QStringLiteral("X-Plasma-NotificationAreaCategory"));
     if (cat.isEmpty()) {
         return "UnknownCategory";
@@ -385,11 +385,11 @@ void SystemTray::restorePlasmoids()
     foreach (Plasma::Applet *applet, applets()) {
         //Here it should always be valid.
         //for some reason it not always is.
 -        if (!applet->pluginInfo().isValid()) {
 +        if (!applet->pluginMetaData().isValid()) {
             applet->config().parent().deleteGroup();
             applet->deleteLater();
         } else {
 -            const QString task = applet->pluginInfo().pluginName();
 +            const QString task = applet->pluginMetaData().pluginId();
             if (!m_allowedPlasmoids.contains(task)) {
                 //in those cases we do delete the applet config completely
                 //as they were explicitly disabled by the user
 -- 
 2.7.4
--- a/0002-Systray-Move-all-icon-resolution-to-dataengine.patch
+++ b/0002-Systray-Move-all-icon-resolution-to-dataengine.patch
@ -0,0 +1,107 @@
 From 749f60b89f4a166833fb64a5b593a801f63f9615 Mon Sep 17 00:00:00 2001
 From: Lindsay Roberts <m@lindsayr.com>
 Date: Mon, 10 Oct 2016 19:55:49 +0300
 Subject: [PATCH 2/2] Systray: Move all icon resolution to dataengine
 Summary:
 Changes triggered by investigation into a long-running high CPU usage bug with system tray animations. The systray itself had icon name to icon resolution code, which was being triggered (twice) for every icon, every time any icon in the systray was updated. This code was spinning up a KIconLoader on each of these instances, and throwing it directly away. Each one triggered a large quantity of memory allocations and disk scans.
 This patch moves the extra bit of "appName" logic from the native part of the system tray to the statusnotifieritem datasource, which already had a stored 'customIconLoader' to handle icon theme paths, and removes the special lookup from the sytemtray applet completely. It also prefers icons provided by the dataengine to doing another lookup (contentious?). This removes all the extra CPU usage outside of the QML scene graph and graphics drivers locally.
 This is very much a looking for feedback item - there are things about the icon loading paths I almost certainly haven't appreciated yet, and perhaps preferring loading by icon name in the applet has a another purpose.
 BUG: 356479
 Test Plan: Have tested locally with kgpg and steam, the two apps I have that trigger the old code path. In neither case, however, did the appName logic produce a different result to the code with just the icon search path in statusnotifieritem.
 Reviewers: #plasma, davidedmundson, mart
 Reviewed By: #plasma, davidedmundson, mart
 Subscribers: davidedmundson, plasma-devel
 Tags: #plasma
 Differential Revision: https://phabricator.kde.org/D2986
 ---
 applets/systemtray/systemtray.cpp | 52 ---------------------------------------
 1 file changed, 52 deletions(-)
 diff --git a/applets/systemtray/systemtray.cpp b/applets/systemtray/systemtray.cpp
 index ecc23a4..3234dd1 100644
 --- a/applets/systemtray/systemtray.cpp
 +++ b/applets/systemtray/systemtray.cpp
@@ -37,37 +37,11 @@
 #include <Plasma/PluginLoader>
 #include <Plasma/ServiceJob>
 -#include <KIconLoader>
 -#include <KIconEngine>
 #include <KActionCollection>
 #include <KLocalizedString>
 #include <plasma_version.h>
 -/*
 - * An app may also load icons from their own directories, so we need a new iconloader that takes this into account
 - * This is wrapped into a subclass of iconengine so the iconloader lifespan matches the icon object
 - */
 -class AppIconEngine : public KIconEngine
 -{
 -public:
 -    AppIconEngine(const QString &variant, const QString &path, const QString &appName);
 -    ~AppIconEngine();
 -private:
 -    KIconLoader* m_loader;
 -};
 -
 -AppIconEngine::AppIconEngine(const QString &variant, const QString &path, const QString &appName) :
 -    KIconEngine(variant, m_loader = new KIconLoader(appName, QStringList()))
 -{
 -    m_loader->addAppDir(appName, path);
 -}
 -
 -AppIconEngine::~AppIconEngine()
 -{
 -    delete m_loader;
 -}
 -
 class PlasmoidModel: public QStandardItemModel
 {
 public:
@@ -169,32 +143,6 @@ void SystemTray::cleanupTask(const QString &task)
     }
 }
 -QVariant SystemTray::resolveIcon(const QVariant &variant, const QString &iconThemePath)
 -{
 -    if (variant.canConvert<QString>()) {
 -        if (!iconThemePath.isEmpty()) {
 -            const QString path = iconThemePath;
 -            if (!path.isEmpty()) {
 -                // FIXME: If last part of path is not "icons", this won't work!
 -                auto tokens = path.splitRef('/', QString::SkipEmptyParts);
 -                if (tokens.length() >= 3 && tokens.takeLast() == QLatin1String("icons")) {
 -                    const QString appName = tokens.takeLast().toString();
 -
 -                    return QVariant(QIcon(new AppIconEngine(variant.toString(), path, appName)));
 -                } else {
 -                    qCWarning(SYSTEM_TRAY) << "Wrong IconThemePath" << path << ": too short or does not end with 'icons'";
 -                }
 -            }
 -
 -            //return just the string hoping that IconItem will know how to interpret it anyways as either a normal icon or a SVG from the theme
 -            return variant;
 -        }
 -    }
 -
 -    // Most importantly QIcons. Nothing to do for those.
 -    return variant;
 -}
 -
 void SystemTray::showPlasmoidMenu(QQuickItem *appletInterface, int x, int y)
 {
     if (!appletInterface) {
 -- 
 2.7.4
--- a/plasma-workspace.spec
+++ b/plasma-workspace.spec
@ -7,7 +7,7 @@
 Name:    plasma-workspace
 Summary: Plasma workspace, applications and applets
 Version: 5.8.2
-Release: 1%{?dist}
+Release: 2%{?dist}
 License: GPLv2+
 URL:     https://quickgit.kde.org/?p=%{name}.git
@ -53,6 +53,11 @@ Patch52:        plasma-workspace-5.6.4-installdbgsymbols.patch
 ## upstream Patches
 ## upstream Patches (master branch)
 # systray-related fixes
 Patch21: 0001-Port-to-new-plasma-framework-API.patch
 Patch22: 0002-Systray-Move-all-icon-resolution-to-dataengine.patch
 # udev
 BuildRequires:  zlib-devel
 BuildRequires:  dbusmenu-qt5-devel
@ -424,6 +429,10 @@ BuildArch: noarch
 ## upstream patches
 ## not strictly needed atm, we care more about patch22 -- rex
 #patch21 -p1 -b .0001
 %patch22 -p1 -b .0002
 %if 0%{?fedora} > 23
 # dnf debuginfo-install
 %patch52 -p1 -b .installdgbsymbols
@ -698,6 +707,9 @@ fi
 %changelog
 * Fri Oct 21 2016 Rex Dieter <rdieter@fedoraproject.org> - 5.8.2-2
 - backport candidate systray-icon cpu fix from master (kde#356479)
 * Tue Oct 18 2016 Rex Dieter <rdieter@fedoraproject.org> - 5.8.2-1
 - 5.8.2