From d727d2e477bb94f7b0f045aa6b00cc86847521d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Tue, 29 Mar 2016 15:54:18 +0200 Subject: [PATCH] [server] Workaround for QtWayland bug https://bugreports.qt.io/browse/QTBUG-52192 Summary: QtWayland doesn't map the parent sub-surfaces in a sub-surface tree. According to the spec this would mean also the child sub-surface is not mapped. But being strict according to the spec will make applications like SystemSettings fail badly. Embedded child windows will not be rendered and QtWayland is going to hard freeze. This is not acceptable, thus we need to workaround this QtWayland bug till it's fixed. It's worth mentioning that Weston as the reference compositor also doesn't handle this situation according to spec and renders the sub-surface. See https://bugs.freedesktop.org/show_bug.cgi?id=94735 The difficult part for the workaround is to determine whether a surface should be considered unmapped. E.g. when the parent gets unmapped we need to really unmap it. But what's the difference between an unmapped parent surface which should be considered mapped and an unmapped parent surface which should be considered unmapped? The implementation goes with considering a new sub-surface always as mapped - independently of whether it ever got a buffer attached. As soon as it had a buffer attached and it gets unmapped again, it will go back to a standard conform way. The behavior now is not standard conform, thus the autotest is adjusted to have QEXPECT_FAIL for the now no longer standard conform areas. Reviewers: #plasma Subscribers: plasma-devel Projects: #plasma Differential Revision: https://phabricator.kde.org/D1250 --- src/wayland/autotests/client/test_wayland_subsurface.cpp | 6 ++++++ src/wayland/surface_interface.cpp | 4 +++- src/wayland/surface_interface_p.h | 6 ++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/wayland/autotests/client/test_wayland_subsurface.cpp b/src/wayland/autotests/client/test_wayland_subsurface.cpp index cd2c0c4311..40bbad59b4 100644 --- a/src/wayland/autotests/client/test_wayland_subsurface.cpp +++ b/src/wayland/autotests/client/test_wayland_subsurface.cpp @@ -869,8 +869,11 @@ void TestSubSurface::testMappingOfSurfaceTree() QVERIFY(parentSpy.wait()); QVERIFY(parentServerSurface->isMapped()); // children should not yet be mapped + QEXPECT_FAIL("", "Workaround for QtWayland bug https://bugreports.qt.io/browse/QTBUG-52192", Continue); QVERIFY(!child->surface()->isMapped()); + QEXPECT_FAIL("", "Workaround for QtWayland bug https://bugreports.qt.io/browse/QTBUG-52192", Continue); QVERIFY(!child2->surface()->isMapped()); + QEXPECT_FAIL("", "Workaround for QtWayland bug https://bugreports.qt.io/browse/QTBUG-52192", Continue); QVERIFY(!child3->surface()->isMapped()); // next level @@ -882,8 +885,11 @@ void TestSubSurface::testMappingOfSurfaceTree() QVERIFY(child2DamageSpy.wait()); QVERIFY(parentServerSurface->isMapped()); // children should not yet be mapped + QEXPECT_FAIL("", "Workaround for QtWayland bug https://bugreports.qt.io/browse/QTBUG-52192", Continue); QVERIFY(!child->surface()->isMapped()); + QEXPECT_FAIL("", "Workaround for QtWayland bug https://bugreports.qt.io/browse/QTBUG-52192", Continue); QVERIFY(!child2->surface()->isMapped()); + QEXPECT_FAIL("", "Workaround for QtWayland bug https://bugreports.qt.io/browse/QTBUG-52192", Continue); QVERIFY(!child3->surface()->isMapped()); // last but not least the first child level, which should map all our subsurfaces diff --git a/src/wayland/surface_interface.cpp b/src/wayland/surface_interface.cpp index 56fc18fe72..8791f6f5e7 100644 --- a/src/wayland/surface_interface.cpp +++ b/src/wayland/surface_interface.cpp @@ -343,6 +343,7 @@ void SurfaceInterface::Private::swapStates(State *source, State *target, bool em if (!windowRegion.isEmpty()) { target->damage = windowRegion.intersected(target->damage); if (emitChanged) { + subSurfaceIsMapped = true; emit q->damaged(target->damage); // workaround for https://bugreports.qt.io/browse/QTBUG-52092 // if the surface is a sub-surface, but the main surface is not yet mapped, fake frame rendered @@ -352,6 +353,7 @@ void SurfaceInterface::Private::swapStates(State *source, State *target, bool em } } } else if (!target->buffer && emitChanged) { + subSurfaceIsMapped = false; emit q->unmapped(); } } @@ -672,7 +674,7 @@ bool SurfaceInterface::isMapped() const if (d->subSurface) { // from spec: // "A sub-surface becomes mapped, when a non-NULL wl_buffer is applied and the parent surface is mapped." - return d->current.buffer && !d->subSurface->parentSurface().isNull() && d->subSurface->parentSurface()->isMapped(); + return d->subSurfaceIsMapped && !d->subSurface->parentSurface().isNull() && d->subSurface->parentSurface()->isMapped(); } return d->current.buffer != nullptr; } diff --git a/src/wayland/surface_interface_p.h b/src/wayland/surface_interface_p.h index 301248ab94..86e8186bf9 100644 --- a/src/wayland/surface_interface_p.h +++ b/src/wayland/surface_interface_p.h @@ -82,6 +82,12 @@ public: State subSurfacePending; QPointer subSurface; + // workaround for https://bugreports.qt.io/browse/QTBUG-52192 + // A subsurface needs to be considered mapped even if it doesn't have a buffer attached + // Otherwise Qt's sub-surfaces will never be visible and the client will freeze due to + // waiting on the frame callback of the never visible surface + bool subSurfaceIsMapped = true; + private: SurfaceInterface *q_func() { return reinterpret_cast(q);