diff --git a/autotests/integration/effects/maximize_animation_test.cpp b/autotests/integration/effects/maximize_animation_test.cpp --- a/autotests/integration/effects/maximize_animation_test.cpp +++ b/autotests/integration/effects/maximize_animation_test.cpp @@ -116,12 +116,14 @@ QVERIFY(!surface.isNull()); QFETCH(Test::ShellSurfaceType, type); - QScopedPointer shellSurface(qobject_cast( - Test::createShellSurface(type, surface.data()))); + QScopedPointer shellSurface(createXdgShellSurface(type, surface.data(), nullptr, Test::CreationSetup::CreateOnly)); // Wait for the initial configure event. XdgShellSurface::States states; QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested); + + surface->commit(Surface::CommitFlag::None); + QVERIFY(configureRequestedSpy.isValid()); QVERIFY(configureRequestedSpy.wait()); QCOMPARE(configureRequestedSpy.count(), 1); diff --git a/autotests/integration/kwin_wayland_test.h b/autotests/integration/kwin_wayland_test.h --- a/autotests/integration/kwin_wayland_test.h +++ b/autotests/integration/kwin_wayland_test.h @@ -142,12 +142,48 @@ XdgShellV6, XdgShellStable }; + +enum class CreationSetup { + CreateOnly, + CreateAndConfigure, /// commit and wait for the configure event, making this surface ready to commit buffers +}; + +/** + * Creates either a ShellSurface * or XdgShellSurface * as defined by @arg type + * For XDG top levels this method will block for a configure event, make this surface ready to commit buffers + */ QObject *createShellSurface(ShellSurfaceType type, KWayland::Client::Surface *surface, QObject *parent = nullptr); -KWayland::Client::ShellSurface *createShellSurface(KWayland::Client::Surface *surface, QObject *parent = nullptr); -KWayland::Client::XdgShellSurface *createXdgShellV5Surface(KWayland::Client::Surface *surface, QObject *parent = nullptr); -KWayland::Client::XdgShellSurface *createXdgShellV6Surface(KWayland::Client::Surface *surface, QObject *parent = nullptr); -KWayland::Client::XdgShellSurface *createXdgShellStableSurface(KWayland::Client::Surface *surface, QObject *parent = nullptr); -KWayland::Client::XdgShellPopup *createXdgShellStablePopup(KWayland::Client::Surface *surface, KWayland::Client::XdgShellSurface *parentSurface, const KWayland::Client::XdgPositioner &positioner, QObject *parent = nullptr); + +KWayland::Client::XdgShellSurface *createXdgShellSurface(ShellSurfaceType type, + KWayland::Client::Surface *surface, + QObject *parent = nullptr, + CreationSetup creationSetup = CreationSetup::CreateAndConfigure); + +KWayland::Client::ShellSurface *createShellSurface(KWayland::Client::Surface *surface, + QObject *parent = nullptr); +KWayland::Client::XdgShellSurface *createXdgShellV5Surface(KWayland::Client::Surface *surface, + QObject *parent = nullptr, + CreationSetup = CreationSetup::CreateAndConfigure); +KWayland::Client::XdgShellSurface *createXdgShellV6Surface(KWayland::Client::Surface *surface, + QObject *parent = nullptr, + CreationSetup = CreationSetup::CreateAndConfigure); +KWayland::Client::XdgShellSurface *createXdgShellStableSurface(KWayland::Client::Surface *surface, + QObject *parent = nullptr, + CreationSetup = CreationSetup::CreateAndConfigure); +KWayland::Client::XdgShellPopup *createXdgShellStablePopup(KWayland::Client::Surface *surface, + KWayland::Client::XdgShellSurface *parentSurface, + const KWayland::Client::XdgPositioner &positioner, + QObject *parent = nullptr, + CreationSetup = CreationSetup::CreateAndConfigure); + + +/** + * Commits the XdgShellSurface to the given surface, and waits for the configure event from the compositor + */ +void initXdgShellSurface(KWayland::Client::Surface *surface, KWayland::Client::XdgShellSurface *shellSurface); +void initXdgShellPopup(KWayland::Client::Surface *surface, KWayland::Client::XdgShellPopup *popup); + + /** * Creates a shared memory buffer of @p size in @p color and attaches it to the @p surface. diff --git a/autotests/integration/quick_tiling_test.cpp b/autotests/integration/quick_tiling_test.cpp --- a/autotests/integration/quick_tiling_test.cpp +++ b/autotests/integration/quick_tiling_test.cpp @@ -464,18 +464,28 @@ QScopedPointer surface(Test::createSurface()); QVERIFY(!surface.isNull()); - QScopedPointer shellSurface(Test::createXdgShellV6Surface(surface.data())); + QScopedPointer shellSurface(Test::createXdgShellV6Surface( + surface.data(), surface.data(), Test::CreationSetup::CreateOnly)); QVERIFY(!shellSurface.isNull()); + + // wait for the initial configure event QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested); QVERIFY(configureRequestedSpy.isValid()); + surface->commit(Surface::CommitFlag::None); + QVERIFY(configureRequestedSpy.wait()); + QCOMPARE(configureRequestedSpy.count(), 1); + // let's render + shellSurface->ackConfigure(configureRequestedSpy.last().at(2).value()); auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue); QVERIFY(c); QCOMPARE(workspace()->activeClient(), c); QCOMPARE(c->geometry(), QRect(0, 0, 100, 50)); QCOMPARE(c->quickTileMode(), QuickTileMode(QuickTileFlag::None)); QCOMPARE(c->maximizeMode(), MaximizeRestore); + + // we have to receive a configure event when the client becomes active QVERIFY(configureRequestedSpy.wait()); QTRY_COMPARE(configureRequestedSpy.count(), 2); @@ -526,11 +536,19 @@ QVERIFY(!surface.isNull()); QScopedPointer deco(Test::waylandServerSideDecoration()->create(surface.data())); - QScopedPointer shellSurface(Test::createXdgShellV6Surface(surface.data())); + QScopedPointer shellSurface(Test::createXdgShellV6Surface( + surface.data(), surface.data(), Test::CreationSetup::CreateOnly)); QVERIFY(!shellSurface.isNull()); + + // wait for the initial configure event QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested); QVERIFY(configureRequestedSpy.isValid()); + surface->commit(Surface::CommitFlag::None); + QVERIFY(configureRequestedSpy.wait()); + QCOMPARE(configureRequestedSpy.count(), 1); + // let's render + shellSurface->ackConfigure(configureRequestedSpy.last().at(2).value()); auto c = Test::renderAndWaitForShown(surface.data(), QSize(1000, 50), Qt::blue); QVERIFY(c); @@ -542,6 +560,8 @@ 50 + decoration->borderTop() + decoration->borderBottom())); QCOMPARE(c->quickTileMode(), QuickTileMode(QuickTileFlag::None)); QCOMPARE(c->maximizeMode(), MaximizeRestore); + + // we have to receive a configure event when the client becomes active QVERIFY(configureRequestedSpy.wait()); QTRY_COMPARE(configureRequestedSpy.count(), 2); diff --git a/autotests/integration/shell_client_test.cpp b/autotests/integration/shell_client_test.cpp --- a/autotests/integration/shell_client_test.cpp +++ b/autotests/integration/shell_client_test.cpp @@ -103,6 +103,10 @@ void testMinimizeWindowWithTransients(); void testXdgDecoration_data(); void testXdgDecoration(); + void testXdgNeverCommitted(); + void testXdgInitialState(); + void testXdgInitiallyMaximised(); + void testXdgInitiallyMinimized(); }; void TestShellClient::initTestCase() @@ -540,34 +544,45 @@ // this test verifies that windows created fullscreen can be later properly restored QScopedPointer surface(Test::createSurface()); QFETCH(Test::ShellSurfaceType, type); - QScopedPointer shellSurface(Test::createShellSurface(type, surface.data())); + XdgShellSurface *xdgShellSurface = Test::createXdgShellSurface(type, surface.data(), surface.data(), Test::CreationSetup::CreateOnly); + QSignalSpy configureRequestedSpy(xdgShellSurface, SIGNAL(configureRequested(QSize, KWayland::Client::XdgShellSurface::States, quint32))); - XdgShellSurface *xdgShellSurface = nullptr; // fullscreen the window - xdgShellSurface = qobject_cast(shellSurface.data()); xdgShellSurface->setFullscreen(true); + surface->commit(Surface::CommitFlag::None); - auto c = Test::renderAndWaitForShown(surface.data(), QSize(screens()->size(0)), Qt::blue); + configureRequestedSpy.wait(); + QCOMPARE(configureRequestedSpy.count(), 1); + + const auto size = configureRequestedSpy.first()[0].value(); + const auto state = configureRequestedSpy.first()[1].value(); + + QCOMPARE(size, screens()->size(0)); + QVERIFY(state & KWayland::Client::XdgShellSurface::State::Fullscreen); + xdgShellSurface->ackConfigure(configureRequestedSpy.first()[2].toUInt()); + + auto c = Test::renderAndWaitForShown(surface.data(), size, Qt::blue); QVERIFY(c); QVERIFY(c->isFullScreen()); + configureRequestedSpy.wait(100); + QSignalSpy fullscreenChangedSpy(c, &ShellClient::fullScreenChanged); QVERIFY(fullscreenChangedSpy.isValid()); QSignalSpy geometryChangedSpy(c, &ShellClient::geometryChanged); QVERIFY(geometryChangedSpy.isValid()); - QSignalSpy configureRequestedSpy(shellSurface.data(), SIGNAL(configureRequested(QSize, KWayland::Client::XdgShellSurface::States, quint32))); - QVERIFY(configureRequestedSpy.isValid()); // swap back to normal + configureRequestedSpy.clear(); xdgShellSurface->setFullscreen(false); QVERIFY(fullscreenChangedSpy.wait()); QVERIFY(configureRequestedSpy.wait()); QCOMPARE(configureRequestedSpy.last().first().toSize(), QSize(0, 0)); QVERIFY(!c->isFullScreen()); for (const auto &it: configureRequestedSpy) { - xdgShellSurface->ackConfigure(it[2].toInt()); + xdgShellSurface->ackConfigure(it[2].toUInt()); } Test::render(surface.data(), QSize(100, 50), Qt::red); @@ -630,17 +645,25 @@ { QScopedPointer surface(Test::createSurface()); QFETCH(Test::ShellSurfaceType, type); - QScopedPointer shellSurface(dynamic_cast(Test::createShellSurface(type, surface.data()))); + QScopedPointer shellSurface(Test::createXdgShellSurface( + type, surface.data(), surface.data(), Test::CreationSetup::CreateOnly)); QVERIFY(!shellSurface.isNull()); + + // wait for the initial configure event QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested); QVERIFY(configureRequestedSpy.isValid()); + surface->commit(Surface::CommitFlag::None); + QVERIFY(configureRequestedSpy.wait()); + QCOMPARE(configureRequestedSpy.count(), 1); + + shellSurface->ackConfigure(configureRequestedSpy.last().at(2).value()); auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue); QVERIFY(c); QVERIFY(c->isActive()); QVERIFY(!c->isFullScreen()); - // two, one for initial sync, second as it becomes active - QTRY_COMPARE(configureRequestedSpy.count(), 2); + // The client gets activated, which gets another configure event. Though that's not relevant to the test + configureRequestedSpy.wait(10); QSignalSpy fullscreenChangedSpy(c, &AbstractClient::fullScreenChanged); QVERIFY(fullscreenChangedSpy.isValid()); @@ -1354,5 +1377,87 @@ QCOMPARE(c->isDecorated(), expectedMode == XdgDecoration::Mode::ServerSide); } +void TestShellClient::testXdgNeverCommitted() +{ + //check we don't crash if we create a shell object but delete the ShellClient before committing it + QScopedPointer surface(Test::createSurface()); + QScopedPointer shellSurface(Test::createXdgShellStableSurface(surface.data(), nullptr, Test::CreationSetup::CreateOnly)); +} + +void TestShellClient::testXdgInitialState() +{ + QScopedPointer surface(Test::createSurface()); + QScopedPointer shellSurface(Test::createXdgShellStableSurface(surface.data(), nullptr, Test::CreationSetup::CreateOnly)); + QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested); + surface->commit(Surface::CommitFlag::None); + + configureRequestedSpy.wait(); + + QCOMPARE(configureRequestedSpy.count(), 1); + + const auto size = configureRequestedSpy.first()[0].value(); + + QCOMPARE(size, QSize(0, 0)); //client should chose it's preferred size + + shellSurface->ackConfigure(configureRequestedSpy.first()[2].toUInt()); + + auto c = Test::renderAndWaitForShown(surface.data(), QSize(200,100), Qt::blue); + QCOMPARE(c->size(), QSize(200, 100)); +} + +void TestShellClient::testXdgInitiallyMaximised() +{ + QScopedPointer surface(Test::createSurface()); + QScopedPointer shellSurface(Test::createXdgShellStableSurface(surface.data(), nullptr, Test::CreationSetup::CreateOnly)); + QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested); + + shellSurface->setMaximized(true); + surface->commit(Surface::CommitFlag::None); + + configureRequestedSpy.wait(); + + QCOMPARE(configureRequestedSpy.count(), 1); + + const auto size = configureRequestedSpy.first()[0].value(); + const auto state = configureRequestedSpy.first()[1].value(); + + QCOMPARE(size, QSize(1280, 1024)); + QVERIFY(state & KWayland::Client::XdgShellSurface::State::Maximized); + + shellSurface->ackConfigure(configureRequestedSpy.first()[2].toUInt()); + + auto c = Test::renderAndWaitForShown(surface.data(), size, Qt::blue); + QCOMPARE(c->maximizeMode(), MaximizeFull); + QCOMPARE(c->size(), QSize(1280, 1024)); +} + +void TestShellClient::testXdgInitiallyMinimized() +{ + QScopedPointer surface(Test::createSurface()); + QScopedPointer shellSurface(Test::createXdgShellStableSurface(surface.data(), nullptr, Test::CreationSetup::CreateOnly)); + QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested); + + shellSurface->requestMinimize(); + surface->commit(Surface::CommitFlag::None); + + configureRequestedSpy.wait(); + + QCOMPARE(configureRequestedSpy.count(), 1); + + const auto size = configureRequestedSpy.first()[0].value(); + const auto state = configureRequestedSpy.first()[1].value(); + + QCOMPARE(size, QSize(0, 0)); + QCOMPARE(state, 0); + + shellSurface->ackConfigure(configureRequestedSpy.first()[2].toUInt()); + + QEXPECT_FAIL("", "Client created in a minimised state is not exposed to kwin bug 404838", Abort); + auto c = Test::renderAndWaitForShown(surface.data(), size, Qt::blue, QImage::Format_ARGB32, 10); + QVERIFY(c); + QVERIFY(c->isMinimized()); +} + + WAYLANDTEST_MAIN(TestShellClient) #include "shell_client_test.moc" diff --git a/autotests/integration/test_helpers.cpp b/autotests/integration/test_helpers.cpp --- a/autotests/integration/test_helpers.cpp +++ b/autotests/integration/test_helpers.cpp @@ -484,7 +484,7 @@ return s; } -XdgShellSurface *createXdgShellV5Surface(Surface *surface, QObject *parent) +XdgShellSurface *createXdgShellV5Surface(Surface *surface, QObject *parent, CreationSetup creationSetup) { if (!s_waylandConnection.xdgShellV5) { return nullptr; @@ -494,10 +494,13 @@ delete s; return nullptr; } + if (creationSetup == CreationSetup::CreateAndConfigure) { + initXdgShellSurface(surface, s); + } return s; } -XdgShellSurface *createXdgShellV6Surface(Surface *surface, QObject *parent) +XdgShellSurface *createXdgShellV6Surface(Surface *surface, QObject *parent, CreationSetup creationSetup) { if (!s_waylandConnection.xdgShellV6) { return nullptr; @@ -507,10 +510,13 @@ delete s; return nullptr; } + if (creationSetup == CreationSetup::CreateAndConfigure) { + initXdgShellSurface(surface, s); + } return s; } -XdgShellSurface *createXdgShellStableSurface(Surface *surface, QObject *parent) +XdgShellSurface *createXdgShellStableSurface(Surface *surface, QObject *parent, CreationSetup creationSetup) { if (!s_waylandConnection.xdgShellStable) { return nullptr; @@ -520,10 +526,13 @@ delete s; return nullptr; } + if (creationSetup == CreationSetup::CreateAndConfigure) { + initXdgShellSurface(surface, s); + } return s; } -XdgShellPopup *createXdgShellStablePopup(Surface *surface, XdgShellSurface *parentSurface, const XdgPositioner &positioner, QObject *parent) +XdgShellPopup *createXdgShellStablePopup(Surface *surface, XdgShellSurface *parentSurface, const XdgPositioner &positioner, QObject *parent, CreationSetup creationSetup) { if (!s_waylandConnection.xdgShellStable) { return nullptr; @@ -533,26 +542,64 @@ delete s; return nullptr; } + if (creationSetup == CreationSetup::CreateAndConfigure) { + initXdgShellPopup(surface, s); + } return s; } +void initXdgShellSurface(KWayland::Client::Surface *surface, KWayland::Client::XdgShellSurface *shellSurface) +{ + //wait for configure + QSignalSpy configureRequestedSpy(shellSurface, &KWayland::Client::XdgShellSurface::configureRequested); + QVERIFY(configureRequestedSpy.isValid()); + surface->commit(Surface::CommitFlag::None); + QVERIFY(configureRequestedSpy.wait()); + shellSurface->ackConfigure(configureRequestedSpy.last()[2].toInt()); +} + +void initXdgShellPopup(KWayland::Client::Surface *surface, KWayland::Client::XdgShellPopup *shellPopup) +{ + //wait for configure + QSignalSpy configureRequestedSpy(shellPopup, &KWayland::Client::XdgShellPopup::configureRequested); + QVERIFY(configureRequestedSpy.isValid()); + surface->commit(Surface::CommitFlag::None); + QVERIFY(configureRequestedSpy.wait()); + shellPopup->ackConfigure(configureRequestedSpy.last()[1].toInt()); +} + + QObject *createShellSurface(ShellSurfaceType type, KWayland::Client::Surface *surface, QObject *parent) { switch (type) { case ShellSurfaceType::WlShell: return createShellSurface(surface, parent); case ShellSurfaceType::XdgShellV5: - return createXdgShellV5Surface(surface, parent); + return createXdgShellV5Surface(surface, parent, CreationSetup::CreateAndConfigure); case ShellSurfaceType::XdgShellV6: - return createXdgShellV6Surface(surface, parent); + return createXdgShellV6Surface(surface, parent, CreationSetup::CreateAndConfigure); case ShellSurfaceType::XdgShellStable: - return createXdgShellStableSurface(surface, parent); + return createXdgShellStableSurface(surface, parent, CreationSetup::CreateAndConfigure); default: Q_UNREACHABLE(); return nullptr; } } +KWayland::Client::XdgShellSurface *createXdgShellSurface(ShellSurfaceType type, KWayland::Client::Surface *surface, QObject *parent, CreationSetup creationSetup) +{ + switch (type) { + case ShellSurfaceType::XdgShellV5: + return createXdgShellV5Surface(surface, parent, creationSetup); + case ShellSurfaceType::XdgShellV6: + return createXdgShellV6Surface(surface, parent, creationSetup); + case ShellSurfaceType::XdgShellStable: + return createXdgShellStableSurface(surface, parent, creationSetup); + default: + return nullptr; + } +} + bool waitForWindowDestroyed(AbstractClient *client) { QSignalSpy destroyedSpy(client, &QObject::destroyed); diff --git a/shell_client.h b/shell_client.h --- a/shell_client.h +++ b/shell_client.h @@ -195,7 +195,15 @@ void clientFullScreenChanged(bool fullScreen); private: + /** + * Called when the shell is created. + **/ void init(); + /** + * Called for the XDG case when the shell surface is committed to the surface. + * At this point all initial properties should have been set by the client. + **/ + void finishInit(); template void initSurface(T *shellSurface); void createDecoration(const QRect &oldgeom); @@ -258,7 +266,7 @@ bool m_hasPopupGrab = false; qreal m_opacity = 1.0; - class RequestGeometryBlocker { + class RequestGeometryBlocker { //TODO rename ConfigureBlocker when this class is Xdg only public: RequestGeometryBlocker(ShellClient *client) : m_client(client) diff --git a/shell_client.cpp b/shell_client.cpp --- a/shell_client.cpp +++ b/shell_client.cpp @@ -88,7 +88,9 @@ , m_internal(surface->client() == waylandServer()->internalConnection()) { setSurface(surface->surface()); + m_requestGeometryBlockCounter++; init(); + connect(surface->surface(), &SurfaceInterface::committed, this, &ShellClient::finishInit); } ShellClient::ShellClient(XdgShellPopupInterface *surface) @@ -99,7 +101,9 @@ , m_internal(surface->client() == waylandServer()->internalConnection()) { setSurface(surface->surface()); + m_requestGeometryBlockCounter++; init(); + connect(surface->surface(), &SurfaceInterface::committed, this, &ShellClient::finishInit); } ShellClient::~ShellClient() = default; @@ -314,7 +318,6 @@ } m_xdgShellSurface->configure(xdgSurfaceStates(), m_requestedClientSize); }; - configure(); connect(this, &AbstractClient::activeChanged, this, configure); connect(this, &AbstractClient::clientStartUserMovedResized, this, configure); connect(this, &AbstractClient::clientFinishUserMovedResized, this, configure); @@ -330,9 +333,6 @@ m_lastAckedConfigureRequest = serial; }); - QRect position = QRect(m_xdgShellPopup->transientOffset(), m_xdgShellPopup->initialSize()); - m_xdgShellPopup->configure(position); - connect(m_xdgShellPopup, &XdgShellPopupInterface::destroyed, this, &ShellClient::destroyClient); } @@ -370,6 +370,21 @@ } } +void ShellClient::finishInit() { + SurfaceInterface *s = surface(); + disconnect(s, &SurfaceInterface::committed, this, &ShellClient::finishInit); + + if (m_xdgShellPopup) { + QRect area = workspace()->clientArea(PlacementArea, Screens::self()->current(), desktop()); + placeIn(area); + } + + m_requestGeometryBlockCounter--; + if (m_requestGeometryBlockCounter == 0) { + requestGeometry(m_blockedRequestGeometry); + } +} + void ShellClient::destroyClient() { m_closing = true;