From d0f4c452ff3c65077b27ab9c040c20c31385cbf7 Mon Sep 17 00:00:00 2001 From: Eike Hein Date: Fri, 17 Jun 2016 06:33:49 +0900 Subject: [PATCH] Add a LauncherUrlWithoutIcon data role as a speed optimization. Summary: Makes the API more complex, but it's worth it as it allows knowledgable users to opt into avoding costly work and/or data copies in very common use cases. Significantly speeds up a lot of work in the models in the presence of X apps that can't be identified and fall back to the window icon. Based on profiling by David. Reviewers: dfaure, davidedmundson Subscribers: plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D1920 --- libtaskmanager/abstracttasksmodel.h | 1 + libtaskmanager/launchertasksmodel.cpp | 11 ++++++++ libtaskmanager/startuptasksmodel.cpp | 2 +- libtaskmanager/tasksmodel.cpp | 36 +++++++++++++-------------- libtaskmanager/tasktools.cpp | 19 +++++--------- libtaskmanager/tasktools.h | 14 +++++++---- libtaskmanager/waylandtasksmodel.cpp | 14 ++++++++--- libtaskmanager/xwindowtasksmodel.cpp | 8 +++--- 8 files changed, 60 insertions(+), 45 deletions(-) diff --git a/libtaskmanager/abstracttasksmodel.h b/libtaskmanager/abstracttasksmodel.h index 9d359d84b..8472388ef 100644 --- a/libtaskmanager/abstracttasksmodel.h +++ b/libtaskmanager/abstracttasksmodel.h @@ -53,6 +53,7 @@ public: AppName, /**< Application name. */ GenericName, /**< Generic application name. */ LauncherUrl, /**< URL that can be used to launch this application (.desktop or executable). */ + LauncherUrlWithoutIcon, /**< Special path to get a launcher URL while skipping fallback icon encoding. Used as speed optimization. */ LegacyWinIdList, /**< X11 window ids. Stopgap until we have something better. */ MimeType, /**< MIME type for this task (window, window group), needed for DND. */ MimeData, /**< Data for MimeType. */ diff --git a/libtaskmanager/launchertasksmodel.cpp b/libtaskmanager/launchertasksmodel.cpp index 6f4d177c1..cdb8640bf 100644 --- a/libtaskmanager/launchertasksmodel.cpp +++ b/libtaskmanager/launchertasksmodel.cpp @@ -135,6 +135,17 @@ QVariant LauncherTasksModel::data(const QModelIndex &index, int role) const } else if (role == LauncherUrl) { // Take resolved URL from cache. return data.url; + } else if (role == LauncherUrlWithoutIcon) { + // Take resolved URL from cache. + QUrl url = data.url; + + if (url.hasQuery()) { + QUrlQuery query(url); + query.removeQueryItem(QLatin1String("iconData")); + url.setQuery(query); + } + + return url; } else if (role == IsLauncher) { return true; } else if (role == IsOnAllVirtualDesktops) { diff --git a/libtaskmanager/startuptasksmodel.cpp b/libtaskmanager/startuptasksmodel.cpp index 1b5350d35..3fda2db54 100644 --- a/libtaskmanager/startuptasksmodel.cpp +++ b/libtaskmanager/startuptasksmodel.cpp @@ -246,7 +246,7 @@ QVariant StartupTasksModel::data(const QModelIndex &index, int role) const return idFromPath; } else if (role == AppName) { return data.findName(); - } else if (role == LauncherUrl) { + } else if (role == LauncherUrl || role == LauncherUrlWithoutIcon) { return d->launcherUrls.value(id); } else if (role == IsStartup) { return true; diff --git a/libtaskmanager/tasksmodel.cpp b/libtaskmanager/tasksmodel.cpp index 298a22a3d..9d462dd6a 100644 --- a/libtaskmanager/tasksmodel.cpp +++ b/libtaskmanager/tasksmodel.cpp @@ -346,15 +346,14 @@ void TasksModel::Private::initModels() // When we get a window or startup we have a launcher for, cause the launcher to be re-filtered. if (sourceIndex.data(AbstractTasksModel::IsWindow).toBool() || sourceIndex.data(AbstractTasksModel::IsStartup).toBool()) { - const QUrl &launcherUrl = sourceIndex.data(AbstractTasksModel::LauncherUrl).toUrl(); + const QUrl &launcherUrl = sourceIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl(); for (int i = 0; i < launcherTasksModel->rowCount(); ++i) { QModelIndex launcherIndex = launcherTasksModel->index(i, 0); const QString &launcherAppId = launcherIndex.data(AbstractTasksModel::AppId).toString(); - if ((!appId.isEmpty() && appId == launcherAppId) - || (launcherUrl.isValid() && launcherUrlsMatch(launcherUrl, - launcherIndex.data(AbstractTasksModel::LauncherUrl).toUrl(), IgnoreQueryItems))) { + if ((!appId.isEmpty() && appId == launcherAppId) || (launcherUrl.isValid() + && launcherUrl == launcherIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl())) { launcherTasksModel->dataChanged(launcherIndex, launcherIndex); } } @@ -382,7 +381,7 @@ void TasksModel::Private::initModels() continue; } - const QUrl &launcherUrl = sourceIndex.data(AbstractTasksModel::LauncherUrl).toUrl(); + const QUrl &launcherUrl = sourceIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl(); if (!launcherUrl.isEmpty() && launcherUrl.isValid()) { const int pos = launcherTasksModel->launcherPosition(launcherUrl); @@ -516,7 +515,7 @@ void TasksModel::Private::updateManualSortMap() const QModelIndex &concatProxyIndex = concatProxyModel->index(sortedPreFilterRows.at(i), 0); if (concatProxyIndex.data(AbstractTasksModel::IsLauncher).toBool() - || launcherTasksModel->launcherPosition(concatProxyIndex.data(AbstractTasksModel::LauncherUrl).toUrl()) != -1) { + || launcherTasksModel->launcherPosition(concatProxyIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl()) != -1) { insertPos = i + 1; } else { break; @@ -534,8 +533,7 @@ void TasksModel::Private::updateManualSortMap() const QModelIndex &concatProxyIndex = concatProxyModel->index(sortedPreFilterRows.at(i), 0); if (!concatProxyIndex.data(AbstractTasksModel::IsLauncher).toBool() - && launcherUrlsMatch(idx.data(AbstractTasksModel::LauncherUrl).toUrl(), - concatProxyIndex.data(AbstractTasksModel::LauncherUrl).toUrl(), IgnoreQueryItems)) { + && idx.data(AbstractTasksModel::LauncherUrlWithoutIcon) == concatProxyIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon)) { sortedPreFilterRows.move(i, insertPos); if (insertPos > i) { @@ -832,14 +830,15 @@ void TasksModel::updateLauncherCount() if (!filterIndex.data(AbstractTasksModel::IsLauncher).toBool()) { // TODO: It would be much faster if we didn't ask for a URL with serialized PNG // data in it, just to discard it a few lines below. - const QUrl &launcherUrl = filterIndex.data(AbstractTasksModel::LauncherUrl).toUrl(); + const QUrl &launcherUrl = filterIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl(); QMutableListIterator it(launchers); while(it.hasNext()) { it.next(); - if (launcherUrlsMatch(launcherUrl, it.value(), IgnoreQueryItems)) { + // RemoveQuery to strip possible fallback icon from stored launcher. + if (launcherUrl == it.value().adjusted(QUrl::RemoveQuery)) { it.remove(); } } @@ -1365,7 +1364,7 @@ bool TasksModel::move(int row, int newPos) if (!d->separateLaunchers && isLauncherMove) { const QModelIndex &idx = d->concatProxyModel->index(d->sortedPreFilterRows.at(newPos), 0); - const QUrl &launcherUrl = idx.data(AbstractTasksModel::LauncherUrl).toUrl(); + const QUrl &launcherUrl = idx.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl(); // Move launcher for launcher-backed task along with task if launchers // are not being kept separate. @@ -1382,8 +1381,7 @@ bool TasksModel::move(int row, int newPos) for (int i = (d->sortedPreFilterRows.count() - 1); i >= 0; --i) { const QModelIndex &concatProxyIndex = d->concatProxyModel->index(d->sortedPreFilterRows.at(i), 0); - if (launcherUrlsMatch(launcherUrl, - concatProxyIndex.data(AbstractTasksModel::LauncherUrl).toUrl(), IgnoreQueryItems)) { + if (launcherUrl == concatProxyIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl()) { d->sortedPreFilterRows.move(i, newPos); if (newPos > i) { @@ -1415,9 +1413,10 @@ void TasksModel::syncLaunchers() int row = -1; for (int i = 0; i < rowCount(); ++i) { - const QUrl &rowLauncherUrl = index(i, 0).data(AbstractTasksModel::LauncherUrl).toUrl(); + const QUrl &rowLauncherUrl = index(i, 0).data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl(); - if (launcherUrlsMatch(launcherUrl, rowLauncherUrl, IgnoreQueryItems)) { + // RemoveQuery to strip possible fallback icon from stored launcher. + if (launcherUrl.adjusted(QUrl::RemoveQuery) == rowLauncherUrl) { row = i; break; } @@ -1531,7 +1530,7 @@ bool TasksModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent // Filter launcher tasks we already have a startup or window task for (that // got through filtering). if (sourceIndex.data(AbstractTasksModel::IsLauncher).toBool()) { - const QUrl &launcherUrl = sourceIndex.data(AbstractTasksModel::LauncherUrl).toUrl(); + const QUrl &launcherUrl = sourceIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl(); for (int i = 0; i < d->filterProxyModel->rowCount(); ++i) { const QModelIndex &filteredIndex = d->filterProxyModel->index(i, 0); @@ -1543,9 +1542,8 @@ bool TasksModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent const QString &filteredAppId = filteredIndex.data(AbstractTasksModel::AppId).toString(); - if ((!appId.isEmpty() && appId == filteredAppId) - || (launcherUrl.isValid() && launcherUrlsMatch(launcherUrl, - filteredIndex.data(AbstractTasksModel::LauncherUrl).toUrl(), IgnoreQueryItems))) { + if ((!appId.isEmpty() && appId == filteredAppId) || (launcherUrl.isValid() + && launcherUrl == filteredIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl())) { return false; } } diff --git a/libtaskmanager/tasktools.cpp b/libtaskmanager/tasktools.cpp index 932a70afa..a072c274a 100644 --- a/libtaskmanager/tasktools.cpp +++ b/libtaskmanager/tasktools.cpp @@ -216,19 +216,13 @@ QString defaultApplication(const QUrl &url) return QString(""); } -bool launcherUrlsMatch(const QUrl &_a, const QUrl &_b, UrlComparisonMode mode) +bool launcherUrlsMatch(const QUrl &a, const QUrl &b, UrlComparisonMode mode) { if (mode == IgnoreQueryItems) { - QUrl a(_a); - a.setQuery(QUrlQuery()); - - QUrl b(_b); - b.setQuery(QUrlQuery()); - - return (a == b); + return (a.adjusted(QUrl::RemoveQuery) == b.adjusted(QUrl::RemoveQuery)); } - return (_a == _b); + return (a == b); } bool appsMatch(const QModelIndex &a, const QModelIndex &b) @@ -240,11 +234,10 @@ bool appsMatch(const QModelIndex &a, const QModelIndex &b) return true; } - const QUrl &aLauncherUrl = a.data(AbstractTasksModel::LauncherUrl).toUrl(); - const QUrl &bLauncherUrl = b.data(AbstractTasksModel::LauncherUrl).toUrl(); + const QUrl &aUrl = a.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl(); + const QUrl &bUrl = b.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl(); - if (aLauncherUrl.isValid() && launcherUrlsMatch(aLauncherUrl, bLauncherUrl, - IgnoreQueryItems)) { + if (aUrl.isValid() && aUrl == bUrl) { return true; } diff --git a/libtaskmanager/tasktools.h b/libtaskmanager/tasktools.h index fcc76c70b..122e23201 100644 --- a/libtaskmanager/tasktools.h +++ b/libtaskmanager/tasktools.h @@ -82,11 +82,8 @@ TASKMANAGER_EXPORT AppData appDataFromUrl(const QUrl &url, const QIcon &fallback TASKMANAGER_EXPORT QString defaultApplication(const QUrl &url); /** - * Compares two launcher URLs either strictly or ignoring the query string. - * - * In launcher URLs, the query string is used to hold metadata such as an - * icon. When comparing tasks by launcher URL, this metadata should usually - * be ignored. This function serves this need. + * Convenience function to compare two launcher URLs either strictly + * or ignoring their query strings. * * @see LauncherTasksModel * @param a The first launcher URL. @@ -96,6 +93,13 @@ TASKMANAGER_EXPORT QString defaultApplication(const QUrl &url); **/ TASKMANAGER_EXPORT bool launcherUrlsMatch(const QUrl &a, const QUrl &b, UrlComparisonMode mode = Strict); +/** + * Determines whether tasks model entries belong to the same app. + * + * @param a The first model index. + * @param b The second model index. + * @returns @c true if the model entries belong to the same app. + **/ TASKMANAGER_EXPORT bool appsMatch(const QModelIndex &a, const QModelIndex &b); } diff --git a/libtaskmanager/waylandtasksmodel.cpp b/libtaskmanager/waylandtasksmodel.cpp index 8e362306e..2cbdf188d 100644 --- a/libtaskmanager/waylandtasksmodel.cpp +++ b/libtaskmanager/waylandtasksmodel.cpp @@ -49,6 +49,7 @@ public: void initWayland(); void addWindow(KWayland::Client::PlasmaWindow *window); void dataChanged(KWayland::Client::PlasmaWindow *window, int role); + void dataChanged(KWayland::Client::PlasmaWindow *window, const QVector &roles); private: WaylandTasksModel *q; @@ -151,7 +152,8 @@ void WaylandTasksModel::Private::addWindow(KWayland::Client::PlasmaWindow *windo serviceCache.remove(window); } - dataChanged(window, AppId); + dataChanged(window, QVector{AppId, AppName, GenericName, + LauncherUrl, LauncherUrlWithoutIcon}); } ); @@ -234,6 +236,12 @@ void WaylandTasksModel::Private::dataChanged(KWayland::Client::PlasmaWindow *win emit q->dataChanged(idx, idx, QVector{role}); } +void WaylandTasksModel::Private::dataChanged(KWayland::Client::PlasmaWindow *window, const QVector &roles) +{ + QModelIndex idx = q->index(windows.indexOf(window)); + emit q->dataChanged(idx, idx, roles); +} + WaylandTasksModel::WaylandTasksModel(QObject *parent) : AbstractTasksModel(parent) , d(new Private(this)) @@ -267,11 +275,9 @@ QVariant WaylandTasksModel::data(const QModelIndex &index, int role) const if (d->serviceCache.contains(window)) { return d->serviceCache.value(window)->genericName(); } - } else if (role == LauncherUrl) { + } else if (role == LauncherUrl || role == LauncherUrlWithoutIcon) { if (d->serviceCache.contains(window)) { return QUrl::fromLocalFile(d->serviceCache.value(window)->entryPath()); - } else { - return window->title(); } } else if (role == IsWindow) { return true; diff --git a/libtaskmanager/xwindowtasksmodel.cpp b/libtaskmanager/xwindowtasksmodel.cpp index 33670c094..3c9943d8c 100644 --- a/libtaskmanager/xwindowtasksmodel.cpp +++ b/libtaskmanager/xwindowtasksmodel.cpp @@ -87,7 +87,7 @@ public: QIcon icon(WId window); QString mimeType() const; QUrl windowUrl(WId window); - QUrl launcherUrl(WId window); + QUrl launcherUrl(WId window, bool encodeFallbackIcon = true); QUrl serviceUrl(int pid, const QString &type, const QStringList &cmdRemovals); KService::List servicesFromPid(int pid); int screen(WId window); @@ -649,11 +649,11 @@ QUrl XWindowTasksModel::Private::windowUrl(WId window) return url; } -QUrl XWindowTasksModel::Private::launcherUrl(WId window) +QUrl XWindowTasksModel::Private::launcherUrl(WId window, bool encodeFallbackIcon) { const AppData &data = appData(window); - if (!data.icon.name().isEmpty()) { + if (!encodeFallbackIcon || !data.icon.name().isEmpty()) { return data.url; } @@ -844,6 +844,8 @@ QVariant XWindowTasksModel::data(const QModelIndex &index, int role) const return d->appData(window).genericName; } else if (role == LauncherUrl) { return d->launcherUrl(window); + } else if (role == LauncherUrlWithoutIcon) { + return d->launcherUrl(window, false /* encodeFallbackIcon */); } else if (role == LegacyWinIdList) { return QVariantList() << window; } else if (role == MimeType) {