From b7bd5f9a09cb3874532269838b53f03c800d8a44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Thu, 27 Oct 2016 15:59:08 +0200 Subject: [PATCH] Add support for desktopFileName provided by NETWinInfo Summary: KWindowSystem provides a KDE specific property for the desktop file name. This allows KWin to take the icon from the desktop file. The advantage from the desktop file is that KWin normally gets higher resolution icons than provided through the xproperty based icons used previously. If the desktop file does not provide an icon name, KWin falls back to the previous implementation. As on Wayland the icon is taken from the desktop file name already the code for X11 and Wayland is merged in AbstractClient. Also to the PlasmaWindowInterface the appId is taken from the new desktop file instead of the resourceName. Due to that for Xwayland windows where KWin knows the desktop file name it can be passed to PlasmaWindowInterface. This allows e.g. the task manager to better map the windows to applications and provide better icons. Also it means that icons do not need to be passed as bitmap data to the clients. Test Plan: Verified that icon is taking from desktop file if provided and from X property if not provided and that Wayland windows still have icon. Reviewers: #kwin, #plasma_on_wayland, hein Subscribers: plasma-devel, kwin Tags: #plasma_on_wayland, #kwin Differential Revision: https://phabricator.kde.org/D3177 --- abstract_client.cpp | 36 +++++++++++++++++---- abstract_client.h | 21 ++++++++++++ autotests/integration/data/example.desktop | 3 ++ autotests/integration/shell_client_test.cpp | 33 +++++++++++++++++++ client.cpp | 5 +++ events.cpp | 3 ++ manage.cpp | 6 +++- shell_client.cpp | 22 +++++-------- 8 files changed, 108 insertions(+), 21 deletions(-) create mode 100644 autotests/integration/data/example.desktop diff --git a/abstract_client.cpp b/abstract_client.cpp index 41e75c5503..0ba96360e6 100644 --- a/abstract_client.cpp +++ b/abstract_client.cpp @@ -39,6 +39,8 @@ along with this program. If not, see . #include +#include + #include #include @@ -674,7 +676,10 @@ void AbstractClient::setupWindowManagementInterface() w->setMinimizeable(isMinimizable()); w->setFullscreenable(isFullScreenable()); w->setIcon(icon()); - w->setAppId(QString::fromUtf8(resourceName())); + auto updateAppId = [this, w] { + w->setAppId(QString::fromUtf8(m_desktopFileName.isEmpty() ? resourceName() : m_desktopFileName)); + }; + updateAppId(); w->setSkipTaskbar(skipTaskbar()); w->setShadeable(isShadeable()); w->setShaded(isShade()); @@ -716,11 +721,8 @@ void AbstractClient::setupWindowManagementInterface() w->setIcon(icon()); } ); - connect(this, &AbstractClient::windowClassChanged, w, - [w, this] { - w->setAppId(QString::fromUtf8(resourceName())); - } - ); + connect(this, &AbstractClient::windowClassChanged, w, updateAppId); + connect(this, &AbstractClient::desktopFileNameChanged, w, updateAppId); connect(this, &AbstractClient::shadeChanged, w, [w, this] { w->setShaded(isShade()); }); connect(this, &AbstractClient::transientChanged, w, [w, this] { @@ -1639,4 +1641,26 @@ bool AbstractClient::dockWantsInput() const return false; } +void AbstractClient::setDesktopFileName(const QByteArray &name) +{ + if (name == m_desktopFileName) { + return; + } + m_desktopFileName = name; + emit desktopFileNameChanged(); +} + +QString AbstractClient::iconFromDesktopFile() const +{ + if (m_desktopFileName.isEmpty()) { + return QString(); + } + QString desktopFile = QString::fromUtf8(m_desktopFileName); + if (!desktopFile.endsWith(QLatin1String(".desktop"))) { + desktopFile.append(QLatin1String(".desktop")); + } + KDesktopFile df(desktopFile); + return df.readIcon(); +} + } diff --git a/abstract_client.h b/abstract_client.h index b78f84ca61..058571e5cc 100644 --- a/abstract_client.h +++ b/abstract_client.h @@ -244,6 +244,17 @@ class KWIN_EXPORT AbstractClient : public Toplevel * Because of that there is no notify signal. **/ Q_PROPERTY(bool resizeable READ isResizable) + + /** + * The desktop file name of the application this AbstractClient belongs to. + * + * This is either the base name without full path and without file extension of the + * desktop file for the window's application (e.g. "org.kde.foo"). + * + * The application's desktop file name can also be the full path to the desktop file + * (e.g. "/opt/kde/share/org.kde.foo.desktop") in case it's not in a standard location. + **/ + Q_PROPERTY(QByteArray desktopFileName READ desktopFileName NOTIFY desktopFileNameChanged) public: virtual ~AbstractClient(); @@ -599,6 +610,10 @@ public: **/ virtual void showOnScreenEdge() = 0; + QByteArray desktopFileName() const { + return m_desktopFileName; + } + // TODO: remove boolean trap static bool belongToSameApplication(const AbstractClient* c1, const AbstractClient* c2, bool active_hack = false); @@ -640,6 +655,7 @@ Q_SIGNALS: void minimizeableChanged(bool); void shadeableChanged(bool); void maximizeableChanged(bool); + void desktopFileNameChanged(); protected: AbstractClient(); @@ -918,6 +934,9 @@ protected: void startDecorationDoubleClickTimer(); void invalidateDecorationDoubleClickTimer(); + void setDesktopFileName(const QByteArray &name); + QString iconFromDesktopFile() const; + private: void handlePaletteChange(); QSharedPointer m_tabBoxClient; @@ -985,6 +1004,8 @@ private: QElapsedTimer doubleClickTimer; } m_decoration; + QByteArray m_desktopFileName; + static bool s_haveResizeEffect; }; diff --git a/autotests/integration/data/example.desktop b/autotests/integration/data/example.desktop new file mode 100644 index 0000000000..739f5d3087 --- /dev/null +++ b/autotests/integration/data/example.desktop @@ -0,0 +1,3 @@ +[Desktop Entry] +Name=An example application +Icon=kwin diff --git a/autotests/integration/shell_client_test.cpp b/autotests/integration/shell_client_test.cpp index 28b65f80a8..45c9ddc880 100644 --- a/autotests/integration/shell_client_test.cpp +++ b/autotests/integration/shell_client_test.cpp @@ -62,6 +62,7 @@ private Q_SLOTS: void testWindowOpensLargerThanScreen(); void testHidden_data(); void testHidden(); + void testDesktopFileName(); }; void TestShellClient::initTestCase() @@ -583,5 +584,37 @@ void TestShellClient::testHidden() //QCOMPARE(workspace()->activeClient(), c); } +void TestShellClient::testDesktopFileName() +{ + // this test verifies that desktop file name is passed correctly to the window + QScopedPointer surface(Test::createSurface()); + // only xdg-shell as ShellSurface misses the setter + QScopedPointer shellSurface(qobject_cast(Test::createShellSurface(Test::ShellSurfaceType::XdgShellV5, surface.data()))); + shellSurface->setAppId(QByteArrayLiteral("org.kde.foo")); + auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue); + QVERIFY(c); + QCOMPARE(c->desktopFileName(), QByteArrayLiteral("org.kde.foo")); + // the desktop file does not exist, so icon should be generic Wayland + QCOMPARE(c->icon().name(), QStringLiteral("wayland")); + + QSignalSpy desktopFileNameChangedSpy(c, &AbstractClient::desktopFileNameChanged); + QVERIFY(desktopFileNameChangedSpy.isValid()); + QSignalSpy iconChangedSpy(c, &ShellClient::iconChanged); + QVERIFY(iconChangedSpy.isValid()); + shellSurface->setAppId(QByteArrayLiteral("org.kde.bar")); + QVERIFY(desktopFileNameChangedSpy.wait()); + QCOMPARE(c->desktopFileName(), QByteArrayLiteral("org.kde.bar")); + // icon should still be wayland + QCOMPARE(c->icon().name(), QStringLiteral("wayland")); + QVERIFY(iconChangedSpy.isEmpty()); + + const QString dfPath = QFINDTESTDATA("data/example.desktop"); + shellSurface->setAppId(dfPath.toUtf8()); + QVERIFY(desktopFileNameChangedSpy.wait()); + QCOMPARE(iconChangedSpy.count(), 1); + QCOMPARE(QString::fromUtf8(c->desktopFileName()), dfPath); + QCOMPARE(c->icon().name(), QStringLiteral("kwin")); +} + WAYLANDTEST_MAIN(TestShellClient) #include "shell_client_test.moc" diff --git a/client.cpp b/client.cpp index 8ae4d9e172..4cdfbe4a03 100644 --- a/client.cpp +++ b/client.cpp @@ -1683,6 +1683,11 @@ void Client::getMotifHints() void Client::getIcons() { // First read icons from the window itself + const QString themedIconName = iconFromDesktopFile(); + if (!themedIconName.isEmpty()) { + setIcon(QIcon::fromTheme(themedIconName)); + return; + } QIcon icon; auto readIcon = [this, &icon](int size, bool scale = true) { const QPixmap pix = KWindowSystem::icon(window(), size, size, scale, KWindowSystem::NETWM | KWindowSystem::WMHints, info); diff --git a/events.cpp b/events.cpp index 4976369eca..d8da5a7c0a 100644 --- a/events.cpp +++ b/events.cpp @@ -651,6 +651,9 @@ bool Client::windowEvent(xcb_generic_event_t *e) if (dirtyProperties2 & NET::WM2OpaqueRegion) { getWmOpaqueRegion(); } + if (dirtyProperties2 & NET::WM2DesktopFileName) { + setDesktopFileName(QByteArray(info->desktopFileName())); + } } const uint8_t eventType = e->response_type & ~0x80; diff --git a/manage.cpp b/manage.cpp index 67abd95c79..bf372a4d75 100644 --- a/manage.cpp +++ b/manage.cpp @@ -93,7 +93,8 @@ bool Client::manage(xcb_window_t w, bool isMapped) NET::WM2Protocols | NET::WM2InitialMappingState | NET::WM2IconPixmap | - NET::WM2OpaqueRegion; + NET::WM2OpaqueRegion | + NET::WM2DesktopFileName; auto wmClientLeaderCookie = fetchWmClientLeader(); auto skipCloseAnimationCookie = fetchSkipCloseAnimation(); @@ -142,7 +143,10 @@ bool Client::manage(xcb_window_t w, bool isMapped) setModal((info->state() & NET::Modal) != 0); // Needs to be valid before handling groups readTransientProperty(transientCookie); + setDesktopFileName(QByteArray(info->desktopFileName())); getIcons(); + connect(this, &Client::desktopFileNameChanged, this, &Client::getIcons); + m_geometryHints.read(); getMotifHints(); getWmOpaqueRegion(); diff --git a/shell_client.cpp b/shell_client.cpp index dbf0b4da55..5edf298c24 100644 --- a/shell_client.cpp +++ b/shell_client.cpp @@ -108,12 +108,13 @@ void ShellClient::initSurface(T *shellSurface) performMouseCommand(Options::MouseMove, Cursor::pos()); } ); - connect(shellSurface, &T::windowClassChanged, this, &ShellClient::updateIcon); setResourceClass(shellSurface->windowClass()); + setDesktopFileName(shellSurface->windowClass()); connect(shellSurface, &T::windowClassChanged, this, [this] (const QByteArray &windowClass) { setResourceClass(windowClass); + setDesktopFileName(windowClass); } ); connect(shellSurface, &T::resizeRequested, this, @@ -168,6 +169,7 @@ void ShellClient::initSurface(T *shellSurface) void ShellClient::init() { + connect(this, &ShellClient::desktopFileNameChanged, this, &ShellClient::updateIcon); findInternalWindow(); createWindowId(); setupCompositing(); @@ -233,7 +235,6 @@ void ShellClient::init() } else if (m_xdgShellPopup) { connect(m_xdgShellPopup, &XdgShellPopupInterface::destroyed, this, &ShellClient::destroyClient); } - updateIcon(); // setup shadow integration getShadow(); @@ -1184,19 +1185,12 @@ bool ShellClient::hasStrut() const void ShellClient::updateIcon() { const QString waylandIconName = QStringLiteral("wayland"); - QString desktopFile; - if (m_shellSurface) { - desktopFile = QString::fromUtf8(m_shellSurface->windowClass()); + const QString dfIconName = iconFromDesktopFile(); + const QString iconName = dfIconName.isEmpty() ? waylandIconName : dfIconName; + if (iconName == icon().name()) { + return; } - if (desktopFile.isEmpty()) { - setIcon(QIcon::fromTheme(waylandIconName)); - } - if (!desktopFile.endsWith(QLatin1String(".desktop"))) { - desktopFile.append(QLatin1String(".desktop")); - } - KDesktopFile df(desktopFile); - const QString iconName = df.readIcon(); - setIcon(QIcon::fromTheme(iconName.isEmpty() ? waylandIconName : iconName)); + setIcon(QIcon::fromTheme(iconName)); } bool ShellClient::isTransient() const