From 610fbc672f043389c37ff2f13e4b3cc470a3303e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Mon, 21 Mar 2016 15:53:13 +0100 Subject: [PATCH] [server] Don't double buffer adding/removing of sub-surfaces QtWayland doesn't commit the parent surface when creating a sub-surface. This results in a QtWayland application to freeze as it renders to the surface and waits for the frame rendered, which it will never get as the Compositor waits for the commit on the parent prior to mapping the sub-surface. To work around this behavior, we apply the adding/removing directly. The behavior around this is actually not fully documented, so QtWayland is not wrong per se. See: https://lists.freedesktop.org/archives/wayland-devel/2016-March/027540.html Once this is properly clarified and implemented in the Client, we should revert this change. Differential Revision: https://phabricator.kde.org/D1191 --- .../client/test_wayland_subsurface.cpp | 8 +++++++- src/wayland/surface_interface.cpp | 20 ++++++++----------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/wayland/autotests/client/test_wayland_subsurface.cpp b/src/wayland/autotests/client/test_wayland_subsurface.cpp index 7e5d20f5f5..938e986d05 100644 --- a/src/wayland/autotests/client/test_wayland_subsurface.cpp +++ b/src/wayland/autotests/client/test_wayland_subsurface.cpp @@ -207,6 +207,7 @@ void TestSubSurface::testCreate() QCOMPARE(serverSubSurface->surface().data(), serverSurface); QCOMPARE(serverSurface->subSurface().data(), serverSubSurface); // children are only added after committing the surface + QEXPECT_FAIL("", "Incorrect adding of child windows to workaround QtWayland behavior", Continue); QCOMPARE(serverParentSurface->childSubSurfaces().count(), 0); // so let's commit the surface, to apply the stacking change parent->commit(Surface::CommitFlag::None); @@ -222,9 +223,12 @@ void TestSubSurface::testCreate() QVERIFY(destroyedSpy.wait()); QCOMPARE(serverSurface->subSurface(), QPointer()); // only applied after next commit + QEXPECT_FAIL("", "Incorrect removing of child windows to workaround QtWayland behavior", Continue); QCOMPARE(serverParentSurface->childSubSurfaces().count(), 1); // but the surface should be invalid - QVERIFY(serverParentSurface->childSubSurfaces().first().isNull()); + if (!serverParentSurface->childSubSurfaces().isEmpty()) { + QVERIFY(serverParentSurface->childSubSurfaces().first().isNull()); + } // committing the state should solve it parent->commit(Surface::CommitFlag::None); wl_display_flush(m_connection->display()); @@ -364,6 +368,7 @@ void TestSubSurface::testPlaceAbove() subSurfaceCreatedSpy.clear(); // so far the stacking order should still be empty + QEXPECT_FAIL("", "Incorrect adding of child windows to workaround QtWayland behavior", Continue); QVERIFY(serverSubSurface1->parentSurface()->childSubSurfaces().isEmpty()); // committing the parent should create the stacking order @@ -464,6 +469,7 @@ void TestSubSurface::testPlaceBelow() subSurfaceCreatedSpy.clear(); // so far the stacking order should still be empty + QEXPECT_FAIL("", "Incorrect adding of child windows to workaround QtWayland behavior", Continue); QVERIFY(serverSubSurface1->parentSurface()->childSubSurfaces().isEmpty()); // committing the parent should create the stacking order diff --git a/src/wayland/surface_interface.cpp b/src/wayland/surface_interface.cpp index 777b2cdf00..688da3f2f5 100644 --- a/src/wayland/surface_interface.cpp +++ b/src/wayland/surface_interface.cpp @@ -47,12 +47,10 @@ SurfaceInterface::Private::~Private() void SurfaceInterface::Private::addChild(QPointer< SubSurfaceInterface > child) { - // protocol is not precise on how to handle the addition of new sub surfaces, this follows Weston's behavior - if (subSurface.isNull() || !subSurface->isSynchronized()) { - pending.children.append(child); - } else { - subSurfacePending.children.append(child); - } + // protocol is not precise on how to handle the addition of new sub surfaces + pending.children.append(child); + subSurfacePending.children.append(child); + current.children.append(child); Q_Q(SurfaceInterface); emit q->subSurfaceTreeChanged(); QObject::connect(child.data(), &SubSurfaceInterface::positionChanged, q, &SurfaceInterface::subSurfaceTreeChanged); @@ -63,12 +61,10 @@ void SurfaceInterface::Private::addChild(QPointer< SubSurfaceInterface > child) void SurfaceInterface::Private::removeChild(QPointer< SubSurfaceInterface > child) { - // protocol is not precise on how to handle the addition of new sub surfaces, this follows Weston's behavior - if (subSurface.isNull() || !subSurface->isSynchronized()) { - pending.children.removeAll(child); - } else { - subSurfacePending.children.removeAll(child); - } + // protocol is not precise on how to handle the addition of new sub surfaces + pending.children.removeAll(child); + subSurfacePending.children.removeAll(child); + current.children.removeAll(child); Q_Q(SurfaceInterface); emit q->subSurfaceTreeChanged(); QObject::disconnect(child.data(), &SubSurfaceInterface::positionChanged, q, &SurfaceInterface::subSurfaceTreeChanged);