diff --git a/autotests/integration/stacking_order_test.cpp b/autotests/integration/stacking_order_test.cpp --- a/autotests/integration/stacking_order_test.cpp +++ b/autotests/integration/stacking_order_test.cpp @@ -23,6 +23,7 @@ #include "abstract_client.h" #include "atoms.h" #include "client.h" +#include "deleted.h" #include "main.h" #include "platform.h" #include "shell_client.h" @@ -51,16 +52,19 @@ void testTransientIsAboveParent(); void testRaiseTransient(); + void testDeletedTransient(); void testGroupTransientIsAboveWindowGroup(); void testRaiseGroupTransient(); + void testDeletedGroupTransient(); void testDontKeepAboveNonModalDialogGroupTransients(); }; void StackingOrderTest::initTestCase() { qRegisterMetaType(); + qRegisterMetaType(); qRegisterMetaType(); QSignalSpy workspaceCreatedSpy(kwinApp(), &Application::workspaceCreated); @@ -202,6 +206,98 @@ QCOMPARE(workspace()->stackingOrder(), (ToplevelList{anotherClient, parent, transient})); } +struct WindowUnrefDeleter +{ + static inline void cleanup(Deleted *d) { + if (d != nullptr) { + d->unrefWindow(); + } + } +}; + +void StackingOrderTest::testDeletedTransient() +{ + // This test verifies that deleted transients are kept above their + // old parents. + + // Create the parent. + KWayland::Client::Surface *parentSurface = + Test::createSurface(Test::waylandCompositor()); + QVERIFY(parentSurface); + KWayland::Client::ShellSurface *parentShellSurface = + Test::createShellSurface(parentSurface, parentSurface); + QVERIFY(parentShellSurface); + ShellClient *parent = Test::renderAndWaitForShown(parentSurface, QSize(256, 256), Qt::blue); + QVERIFY(parent); + QVERIFY(parent->isActive()); + QVERIFY(!parent->isTransient()); + + QCOMPARE(workspace()->stackingOrder(), (ToplevelList{parent})); + + // Create the first transient. + KWayland::Client::Surface *transient1Surface = + Test::createSurface(Test::waylandCompositor()); + QVERIFY(transient1Surface); + KWayland::Client::ShellSurface *transient1ShellSurface = + Test::createShellSurface(transient1Surface, transient1Surface); + QVERIFY(transient1ShellSurface); + transient1ShellSurface->setTransient(parentSurface, QPoint(0, 0)); + ShellClient *transient1 = Test::renderAndWaitForShown( + transient1Surface, QSize(128, 128), Qt::red); + QVERIFY(transient1); + QTRY_VERIFY(transient1->isActive()); + QVERIFY(transient1->isTransient()); + QCOMPARE(transient1->transientFor(), parent); + + QCOMPARE(workspace()->stackingOrder(), (ToplevelList{parent, transient1})); + + // Create the second transient. + KWayland::Client::Surface *transient2Surface = + Test::createSurface(Test::waylandCompositor()); + QVERIFY(transient2Surface); + KWayland::Client::ShellSurface *transient2ShellSurface = + Test::createShellSurface(transient2Surface, transient2Surface); + QVERIFY(transient2ShellSurface); + transient2ShellSurface->setTransient(transient1Surface, QPoint(0, 0)); + ShellClient *transient2 = Test::renderAndWaitForShown( + transient2Surface, QSize(128, 128), Qt::red); + QVERIFY(transient2); + QTRY_VERIFY(transient2->isActive()); + QVERIFY(transient2->isTransient()); + QCOMPARE(transient2->transientFor(), transient1); + + QCOMPARE(workspace()->stackingOrder(), (ToplevelList{parent, transient1, transient2})); + + // Activate the parent, both transients have to be above it. + workspace()->activateClient(parent); + QTRY_VERIFY(parent->isActive()); + QTRY_VERIFY(!transient1->isActive()); + QTRY_VERIFY(!transient2->isActive()); + + // Close the top-most transient. + connect(transient2, &ShellClient::windowClosed, this, + [](Toplevel *toplevel, Deleted *deleted) { + Q_UNUSED(toplevel) + deleted->refWindow(); + } + ); + + QSignalSpy windowClosedSpy(transient2, &ShellClient::windowClosed); + QVERIFY(windowClosedSpy.isValid()); + delete transient2ShellSurface; + delete transient2Surface; + QVERIFY(windowClosedSpy.wait()); + + QScopedPointer deletedTransient( + windowClosedSpy.first().at(1).value()); + QVERIFY(deletedTransient.data()); + + // The deleted transient still has to be above its old parent (transient1). + QTRY_VERIFY(parent->isActive()); + QTRY_VERIFY(!transient1->isActive()); + QCOMPARE(workspace()->stackingOrder(), (ToplevelList{parent, transient1, deletedTransient.data()})); +} + static xcb_window_t createGroupWindow(xcb_connection_t *conn, const QRect &geometry, xcb_window_t leaderWid = XCB_WINDOW_NONE) @@ -501,6 +597,127 @@ QCOMPARE(workspace()->stackingOrder(), (ToplevelList{member1, leader, member2, anotherClient, transient})); } +void StackingOrderTest::testDeletedGroupTransient() +{ + // This test verifies that deleted group transients are kept above their + // old window groups. + + const QRect geometry = QRect(0, 0, 128, 128); + + QScopedPointer conn( + xcb_connect(nullptr, nullptr)); + + QSignalSpy windowCreatedSpy(workspace(), &Workspace::clientAdded); + QVERIFY(windowCreatedSpy.isValid()); + + // Create the group leader. + xcb_window_t leaderWid = createGroupWindow(conn.data(), geometry); + xcb_map_window(conn.data(), leaderWid); + xcb_flush(conn.data()); + + QVERIFY(windowCreatedSpy.wait()); + Client *leader = windowCreatedSpy.first().first().value(); + QVERIFY(leader); + QVERIFY(leader->isActive()); + QCOMPARE(leader->windowId(), leaderWid); + QVERIFY(!leader->isTransient()); + + QCOMPARE(workspace()->stackingOrder(), (ToplevelList{leader})); + + // Create another group member. + windowCreatedSpy.clear(); + xcb_window_t member1Wid = createGroupWindow(conn.data(), geometry, leaderWid); + xcb_map_window(conn.data(), member1Wid); + xcb_flush(conn.data()); + + QVERIFY(windowCreatedSpy.wait()); + Client *member1 = windowCreatedSpy.first().first().value(); + QVERIFY(member1); + QVERIFY(member1->isActive()); + QCOMPARE(member1->windowId(), member1Wid); + QCOMPARE(member1->group(), leader->group()); + QVERIFY(!member1->isTransient()); + + QCOMPARE(workspace()->stackingOrder(), (ToplevelList{leader, member1})); + + // Create yet another group member. + windowCreatedSpy.clear(); + xcb_window_t member2Wid = createGroupWindow(conn.data(), geometry, leaderWid); + xcb_map_window(conn.data(), member2Wid); + xcb_flush(conn.data()); + + QVERIFY(windowCreatedSpy.wait()); + Client *member2 = windowCreatedSpy.first().first().value(); + QVERIFY(member2); + QVERIFY(member2->isActive()); + QCOMPARE(member2->windowId(), member2Wid); + QCOMPARE(member2->group(), leader->group()); + QVERIFY(!member2->isTransient()); + + QCOMPARE(workspace()->stackingOrder(), (ToplevelList{leader, member1, member2})); + + // Create a group transient. + windowCreatedSpy.clear(); + xcb_window_t transientWid = createGroupWindow(conn.data(), geometry, leaderWid); + xcb_icccm_set_wm_transient_for(conn.data(), transientWid, rootWindow()); + + // Currently, we have some weird bug workaround: if a group transient + // is a non-modal dialog, then it won't be kept above its window group. + // We need to explicitly specify window type, otherwise the window type + // will be deduced to _NET_WM_WINDOW_TYPE_DIALOG because we set transient + // for before (the EWMH spec says to do that). + xcb_atom_t net_wm_window_type = Xcb::Atom( + QByteArrayLiteral("_NET_WM_WINDOW_TYPE"), false, conn.data()); + xcb_atom_t net_wm_window_type_normal = Xcb::Atom( + QByteArrayLiteral("_NET_WM_WINDOW_TYPE_NORMAL"), false, conn.data()); + xcb_change_property( + conn.data(), // c + XCB_PROP_MODE_REPLACE, // mode + transientWid, // window + net_wm_window_type, // property + XCB_ATOM_ATOM, // type + 32, // format + 1, // data_len + &net_wm_window_type_normal // data + ); + + xcb_map_window(conn.data(), transientWid); + xcb_flush(conn.data()); + + QVERIFY(windowCreatedSpy.wait()); + Client *transient = windowCreatedSpy.first().first().value(); + QVERIFY(transient); + QVERIFY(transient->isActive()); + QCOMPARE(transient->windowId(), transientWid); + QCOMPARE(transient->group(), leader->group()); + QVERIFY(transient->isTransient()); + QVERIFY(transient->groupTransient()); + QVERIFY(!transient->isDialog()); // See above why + + QCOMPARE(workspace()->stackingOrder(), (ToplevelList{leader, member1, member2, transient})); + + // Unmap the transient. + connect(transient, &Client::windowClosed, this, + [](Toplevel *toplevel, Deleted *deleted) { + Q_UNUSED(toplevel) + deleted->refWindow(); + } + ); + + QSignalSpy windowClosedSpy(transient, &Client::windowClosed); + QVERIFY(windowClosedSpy.isValid()); + xcb_unmap_window(conn.data(), transientWid); + xcb_flush(conn.data()); + QVERIFY(windowClosedSpy.wait()); + + QScopedPointer deletedTransient( + windowClosedSpy.first().at(1).value()); + QVERIFY(deletedTransient.data()); + + // The transient has to be above each member of the window group. + QCOMPARE(workspace()->stackingOrder(), (ToplevelList{leader, member1, member2, deletedTransient.data()})); +} + void StackingOrderTest::testDontKeepAboveNonModalDialogGroupTransients() { // Bug 76026 diff --git a/deleted.h b/deleted.h --- a/deleted.h +++ b/deleted.h @@ -104,14 +104,89 @@ QString caption() const { return m_caption; } + + /** + * Returns whether the client was active. + * + * @returns @c true if the client was active at the time when it was closed, + * @c false otherwise + **/ + bool wasActive() const { + return m_wasActive; + } + + /** + * Returns whether this was an X11 client. + * + * @returns @c true if it was an X11 client, @c false otherwise. + **/ + bool wasX11Client() const { + return m_wasX11Client; + } + + /** + * Returns whether this was a Wayland client. + * + * @returns @c true if it was a Wayland client, @c false otherwise. + **/ + bool wasWaylandClient() const { + return m_wasWaylandClient; + } + + /** + * Returns whether the client was a transient. + * + * @returns @c true if it was a transient, @c false otherwise. + **/ + bool wasTransient() const { + return !m_transientFor.isEmpty(); + } + + /** + * Returns whether the client was a group transient. + * + * @returns @c true if it was a group transient, @c false otherwise. + * @note This is relevant only for X11 clients. + **/ + bool wasGroupTransient() const { + return m_wasGroupTransient; + } + + /** + * Checks whether this client was a transient for given toplevel. + * + * @param toplevel Toplevel against which we are testing. + * @returns @c true if it was a transient for given toplevel, @c false otherwise. + **/ + bool wasTransientFor(const Toplevel *toplevel) const { + return m_transientFor.contains(const_cast(toplevel)); + } + + /** + * Returns the list of transients. + * + * Because the window is Deleted, it can have only Deleted child transients. + **/ + DeletedList transients() const { + return m_transients; + } + protected: virtual void debug(QDebug& stream) const; private Q_SLOTS: void mainClientClosed(KWin::Toplevel *client); + void transientForClosed(Toplevel *toplevel, Deleted *deleted); + private: Deleted(); // use create() void copyToDeleted(Toplevel* c); virtual ~Deleted(); // deleted only using unrefWindow() + + void addTransient(Deleted *transient); + void removeTransient(Deleted *transient); + void addTransientFor(AbstractClient *parent); + void removeTransientFor(Deleted *parent); + int delete_refcount; double window_opacity; int desk; @@ -140,6 +215,12 @@ bool m_keepAbove; bool m_keepBelow; QString m_caption; + bool m_wasActive; + bool m_wasX11Client; + bool m_wasWaylandClient; + bool m_wasGroupTransient; + ToplevelList m_transientFor; + DeletedList m_transients; }; inline void Deleted::refWindow() diff --git a/deleted.cpp b/deleted.cpp --- a/deleted.cpp +++ b/deleted.cpp @@ -22,8 +22,10 @@ #include "workspace.h" #include "client.h" +#include "group.h" #include "netinfo.h" #include "shadow.h" +#include "shell_client.h" #include "decorations/decoratedclient.h" #include "decorations/decorationrenderer.h" @@ -46,6 +48,10 @@ , m_fullscreen(false) , m_keepAbove(false) , m_keepBelow(false) + , m_wasActive(false) + , m_wasX11Client(false) + , m_wasWaylandClient(false) + , m_wasGroupTransient(false) { } @@ -57,6 +63,14 @@ if (workspace()) { workspace()->removeDeleted(this); } + for (Toplevel *toplevel : qAsConst(m_transientFor)) { + if (auto *deleted = qobject_cast(toplevel)) { + deleted->removeTransient(this); + } + } + for (Deleted *transient : qAsConst(m_transients)) { + transient->removeTransientFor(this); + } deleteEffectWindow(); } @@ -117,7 +131,29 @@ m_keepAbove = client->keepAbove(); m_keepBelow = client->keepBelow(); m_caption = client->caption(); + + m_wasActive = client->isActive(); + + const auto *x11Client = qobject_cast(client); + m_wasGroupTransient = x11Client && x11Client->groupTransient(); + + if (m_wasGroupTransient) { + const auto members = x11Client->group()->members(); + for (Client *member : members) { + if (member != client) { + addTransientFor(member); + } + } + } else { + AbstractClient *transientFor = client->transientFor(); + if (transientFor != nullptr) { + addTransientFor(transientFor); + } + } } + + m_wasWaylandClient = qobject_cast(c) != nullptr; + m_wasX11Client = !m_wasWaylandClient; } void Deleted::unrefWindow() @@ -192,6 +228,22 @@ m_mainClients.removeAll(c); } +void Deleted::transientForClosed(Toplevel *toplevel, Deleted *deleted) +{ + if (deleted == nullptr) { + m_transientFor.removeAll(toplevel); + return; + } + + const int index = m_transientFor.indexOf(toplevel); + if (index == -1) { + return; + } + + m_transientFor[index] = deleted; + deleted->addTransient(this); +} + xcb_window_t Deleted::frameId() const { return m_frame; @@ -207,5 +259,26 @@ return m_windowRole; } +void Deleted::addTransient(Deleted *transient) +{ + m_transients.append(transient); +} + +void Deleted::removeTransient(Deleted *transient) +{ + m_transients.removeAll(transient); +} + +void Deleted::addTransientFor(AbstractClient *parent) +{ + m_transientFor.append(parent); + connect(parent, &AbstractClient::windowClosed, this, &Deleted::transientForClosed); +} + +void Deleted::removeTransientFor(Deleted *parent) +{ + m_transientFor.removeAll(parent); +} + } // namespace diff --git a/layers.cpp b/layers.cpp --- a/layers.cpp +++ b/layers.cpp @@ -525,39 +525,86 @@ stacking += layer[ lay ]; // now keep transients above their mainwindows // TODO this could(?) use some optimization - for (int i = stacking.size() - 1; - i >= 0; - ) { - AbstractClient *current = qobject_cast(stacking[i]); - if (!current || !current->isTransient()) { - --i; - continue; - } + for (int i = stacking.size() - 1; i >= 0;) { + // Index of the main window for the current transient window. int i2 = -1; - // find topmost client this one is transient for - for (i2 = stacking.size() - 1; i2 >= 0; --i2) { - AbstractClient *c2 = qobject_cast(stacking[i2]); - if (!c2) { + + // If the current transient has "child" transients, we'd like to restart + // construction of the constrained stacking order from the position where + // the current transient will be moved. + bool hasTransients = false; + + // Find topmost client this one is transient for. + if (auto *client = qobject_cast(stacking[i])) { + if (!client->isTransient()) { + --i; continue; } - if (c2 == current) { - i2 = -1; // Don't reorder, already on top of its main window. - break; + for (i2 = stacking.size() - 1; i2 >= 0; --i2) { + auto *c2 = qobject_cast(stacking[i2]); + if (!c2) { + continue; + } + if (c2 == client) { + i2 = -1; // Don't reorder, already on top of its main window. + break; + } + if (c2->hasTransient(client, true) + && keepTransientAbove(c2, client)) { + break; + } } - if (c2->hasTransient(current, true) - && keepTransientAbove(c2, current)) { - break; + + hasTransients = !client->transients().isEmpty(); + + // If the current transient doesn't have any "alive" transients, check + // whether it has deleted transients that have to be raised. + const bool searchForDeletedTransients = !hasTransients + && !deletedList().isEmpty(); + if (searchForDeletedTransients) { + for (int j = i + 1; j < stacking.count(); ++j) { + auto *deleted = qobject_cast(stacking[j]); + if (!deleted) { + continue; + } + if (deleted->wasTransientFor(client)) { + hasTransients = true; + break; + } + } + } + } else if (auto *deleted = qobject_cast(stacking[i])) { + if (!deleted->wasTransient()) { + --i; + continue; + } + for (i2 = stacking.size() - 1; i2 >= 0; --i2) { + Toplevel *c2 = stacking[i2]; + if (c2 == deleted) { + i2 = -1; // Don't reorder, already on top of its main window. + break; + } + if (deleted->wasTransientFor(c2) + && keepDeletedTransientAbove(c2, deleted)) { + break; + } } + hasTransients = !deleted->transients().isEmpty(); } + if (i2 == -1) { --i; continue; } + + Toplevel *current = stacking[i]; + stacking.removeAt(i); --i; // move onto the next item (for next for () iteration) --i2; // adjust index of the mainwindow after the remove above - if (!current->transients().isEmpty()) // this one now can be possibly above its transients, + if (hasTransients) { // this one now can be possibly above its transients, i = i2; // so go again higher in the stack order and possibly move those transients again + } ++i2; // insert after (on top of) the mainwindow, it's ok if it2 is now stacking.end() stacking.insert(i2, current); } @@ -637,6 +684,40 @@ return true; } +bool Workspace::keepDeletedTransientAbove(const Toplevel *mainWindow, const Deleted *transient) const +{ + // #93832 - Don't keep splashscreens above dialogs. + if (transient->isSplash() && mainWindow->isDialog()) { + return false; + } + + if (transient->wasX11Client()) { + // If a group transient was active, we should keep it above no matter + // what, because at the time when the transient was closed, it was above + // the main window. + if (transient->wasGroupTransient() && transient->wasActive()) { + return true; + } + + // This is rather a hack for #76026. Don't keep non-modal dialogs above + // the mainwindow, but only if they're group transient (since only such + // dialogs have taskbar entry in Kicker). A proper way of doing this + // (both kwin and kicker) needs to be found. + if (transient->wasGroupTransient() && transient->isDialog() + && !transient->isModal()) { + return false; + } + + // #63223 - Don't keep transients above docks, because the dock is kept + // high, and e.g. dialogs for them would be too high too. + if (mainWindow->isDock()) { + return false; + } + } + + return true; +} + // Returns all windows in their stacking order on the root window. ToplevelList Workspace::xStackingOrder() const { diff --git a/workspace.h b/workspace.h --- a/workspace.h +++ b/workspace.h @@ -511,6 +511,7 @@ void lowerClientWithinApplication(AbstractClient* c); bool allowFullClientRaising(const AbstractClient* c, xcb_timestamp_t timestamp); bool keepTransientAbove(const AbstractClient* mainwindow, const AbstractClient* transient); + bool keepDeletedTransientAbove(const Toplevel *mainWindow, const Deleted *transient) const; void blockStackingUpdates(bool block); void updateToolWindows(bool also_hide); void fixPositionAfterCrash(xcb_window_t w, const xcb_get_geometry_reply_t *geom);