From: "Michael Palimaka" <kensington@gentoo.org>
To: gentoo-commits@lists.gentoo.org
Subject: [gentoo-commits] proj/kde:master commit in: kde-plasma/plasma-workspace/files/, kde-plasma/plasma-workspace/
Date: Sat, 12 Nov 2016 08:34:26 +0000 (UTC) [thread overview]
Message-ID: <1478939656.3783c67d83ac1267dc35cf6163dc20e7aa13664c.kensington@gentoo> (raw)
commit: 3783c67d83ac1267dc35cf6163dc20e7aa13664c
Author: Michael Palimaka <kensington <AT> gentoo <DOT> org>
AuthorDate: Sat Nov 12 08:34:09 2016 +0000
Commit: Michael Palimaka <kensington <AT> gentoo <DOT> org>
CommitDate: Sat Nov 12 08:34:16 2016 +0000
URL: https://gitweb.gentoo.org/proj/kde.git/commit/?id=3783c67d
kde-plasma/plasma-workspace: backport patch from master
Package-Manager: portage-2.3.2
.../plasma-workspace-5.8.3-systray-cpuload.patch | 177 +++++++++++++++++++++
.../plasma-workspace-5.8.49.9999.ebuild | 2 +
2 files changed, 179 insertions(+)
diff --git a/kde-plasma/plasma-workspace/files/plasma-workspace-5.8.3-systray-cpuload.patch b/kde-plasma/plasma-workspace/files/plasma-workspace-5.8.3-systray-cpuload.patch
new file mode 100644
index 0000000..fada327
--- /dev/null
+++ b/kde-plasma/plasma-workspace/files/plasma-workspace-5.8.3-systray-cpuload.patch
@@ -0,0 +1,177 @@
+From: Lindsay Roberts <m@lindsayr.com>
+Date: Mon, 10 Oct 2016 16:55:49 +0000
+Subject: Systray: Move all icon resolution to dataengine
+X-Git-Url: http://quickgit.kde.org/?p=plasma-workspace.git&a=commitdiff&h=749f60b89f4a166833fb64a5b593a801f63f9615
+---
+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
+---
+
+
+--- a/applets/systemtray/package/contents/ui/ConfigEntries.qml
++++ b/applets/systemtray/package/contents/ui/ConfigEntries.qml
+@@ -75,7 +75,7 @@
+ "index": i,
+ "taskId": item.Id,
+ "name": item.Title,
+- "iconName": plasmoid.nativeInterface.resolveIcon(item.IconName, item.IconThemePath),
++ "iconName": item.IconName,
+ "icon": item.Icon
+ });
+ }
+
+--- a/applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
++++ b/applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
+@@ -28,7 +28,7 @@
+ text: Title
+ mainText: ToolTipTitle != "" ? ToolTipTitle : Title
+ subText: ToolTipSubTitle
+- icon: ToolTipIcon != "" ? ToolTipIcon : plasmoid.nativeInterface.resolveIcon(IconName != "" ? IconName : Icon, IconThemePath)
++ icon: ToolTipIcon != "" ? ToolTipIcon : Icon ? Icon : IconName
+ textFormat: Text.AutoText
+ category: Category
+
+@@ -48,7 +48,7 @@
+
+ PlasmaCore.IconItem {
+ id: iconItem
+- source: plasmoid.nativeInterface.resolveIcon(IconName != "" ? IconName : Icon, IconThemePath)
++ source: Icon ? Icon : IconName
+ width: Math.min(parent.width, parent.height)
+ height: width
+ active: taskIcon.containsMouse
+
+--- a/applets/systemtray/systemtray.cpp
++++ b/applets/systemtray/systemtray.cpp
+@@ -37,36 +37,10 @@
+ #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
+ {
+@@ -167,32 +141,6 @@
+ emit appletDeleted(applet);
+ }
+ }
+-}
+-
+-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)
+
+--- a/applets/systemtray/systemtray.h
++++ b/applets/systemtray/systemtray.h
+@@ -59,12 +59,6 @@
+ void cleanupTask(const QString &task);
+
+ //Invokable utilities
+- /**
+- * returns either a simple icon name or a custom path if the app is
+- * using a custom theme
+- */
+- Q_INVOKABLE QVariant resolveIcon(const QVariant &variant, const QString &iconThemePath);
+-
+ /**
+ * Given an AppletInterface pointer, shows a proper context menu for it
+ */
+
+--- a/dataengines/statusnotifieritem/statusnotifieritemsource.cpp
++++ b/dataengines/statusnotifieritem/statusnotifieritemsource.cpp
+@@ -240,14 +240,19 @@
+ if (!m_customIconLoader) {
+ m_customIconLoader = new KIconLoader(QString(), QStringList(), this);
+ }
++ // FIXME: If last part of path is not "icons", this won't work!
++ QString appName;
++ auto tokens = path.splitRef('/', QString::SkipEmptyParts);
++ if (tokens.length() >= 3 && tokens.takeLast() == QLatin1String("icons"))
++ appName = tokens.takeLast().toString();
+
+ //icons may be either in the root directory of the passed path or in a appdir format
+ //i.e hicolor/32x32/iconname.png
+
+- m_customIconLoader->reconfigure(QString(), QStringList(path));
++ m_customIconLoader->reconfigure(appName, QStringList(path));
+
+ //add app dir requires an app name, though this is completely unused in this context
+- m_customIconLoader->addAppDir(QStringLiteral("unused"), path);
++ m_customIconLoader->addAppDir(appName.size() ? appName : QStringLiteral("unused"), path);
+ }
+ setData(QStringLiteral("IconThemePath"), path);
+
+
diff --git a/kde-plasma/plasma-workspace/plasma-workspace-5.8.49.9999.ebuild b/kde-plasma/plasma-workspace/plasma-workspace-5.8.49.9999.ebuild
index fa55905..b57fff0 100644
--- a/kde-plasma/plasma-workspace/plasma-workspace-5.8.49.9999.ebuild
+++ b/kde-plasma/plasma-workspace/plasma-workspace-5.8.49.9999.ebuild
@@ -121,7 +121,9 @@ DEPEND="${COMMON_DEPEND}
PATCHES=(
"${FILESDIR}/${PN}-5.4-startkde-script.patch"
+ # master
"${FILESDIR}/${PN}-5.7.90-baloo-optional.patch"
+ "${FILESDIR}/${PN}-5.8.3-systray-cpuload.patch"
)
RESTRICT+=" test"
next reply other threads:[~2016-11-12 8:34 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-12 8:34 Michael Palimaka [this message]
-- strict thread matches above, loose matches on Subject: below --
2022-02-21 11:47 [gentoo-commits] proj/kde:master commit in: kde-plasma/plasma-workspace/files/, kde-plasma/plasma-workspace/ Andreas Sturmlechner
2021-11-09 13:04 Andreas Sturmlechner
2021-05-05 3:00 Andreas Sturmlechner
2021-05-04 21:12 Andreas Sturmlechner
2019-10-15 17:10 Andreas Sturmlechner
2019-05-14 6:28 Andreas Sturmlechner
2018-10-22 18:14 Andreas Sturmlechner
2016-10-08 15:51 Michael Palimaka
2016-03-20 14:47 Michael Palimaka
2015-12-28 0:33 Marc Schiffbauer
2015-11-14 16:01 Michael Palimaka
2015-11-10 12:41 Michael Palimaka
2015-08-16 20:36 Johannes Huber
2015-07-04 14:48 Johannes Huber
2015-06-01 18:18 Michael Palimaka
2015-04-29 15:03 Michael Palimaka
2015-01-27 20:07 Johannes Huber
2015-01-21 20:48 Johannes Huber
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1478939656.3783c67d83ac1267dc35cf6163dc20e7aa13664c.kensington@gentoo \
--to=kensington@gentoo.org \
--cc=gentoo-commits@lists.gentoo.org \
--cc=gentoo-dev@lists.gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox