From b3621cfe7c42873f4f8c4735250d90043f6b2d39 Mon Sep 17 00:00:00 2001 From: Christophe Chapuis Date: Mon, 21 Aug 2017 13:29:47 +0000 Subject: [PATCH 1/4] QWaylandWindow: reset window should reset mask When QWaylandWindow::reset() is called, the window's mask is not changed. It means that when the window will be initialized again, it will not re-send the mask description to the server side through Wayland. Task-number: QTBUG-62638 Change-Id: I07d561f466836bbd90ae58521c0768ed85554256 Reviewed-by: Johan Helsing Reviewed-by: Christophe Chapuis --- src/plugins/platforms/wayland/qwaylandwindow.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index e5edd0e5e5c..f7f296d0aaa 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -254,6 +254,8 @@ void QWaylandWindow::reset() wl_callback_destroy(mFrameCallback); mFrameCallback = nullptr; } + + mMask = QRegion(); } QWaylandWindow *QWaylandWindow::fromWlSurface(::wl_surface *surface) From 6cf44bf690cf0a84325b78d871a96c009b2513ec Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Wed, 18 Oct 2017 11:05:48 +0200 Subject: [PATCH 2/4] Fix crash when wl-shell setType is called with a hidden parent Fall back to creating a toplevel instead Change-Id: If7db27d08b79e4f9f8c82fa8f9bf73abdb2585d9 Reviewed-by: David Edmundson Reviewed-by: Paul Olav Tvete --- .../wayland/qwaylandwlshellsurface.cpp | 6 ++- tests/auto/wayland/client/tst_client.cpp | 44 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandwlshellsurface.cpp b/src/plugins/platforms/wayland/qwaylandwlshellsurface.cpp index 77434e98b6c..92223f45ee6 100644 --- a/src/plugins/platforms/wayland/qwaylandwlshellsurface.cpp +++ b/src/plugins/platforms/wayland/qwaylandwlshellsurface.cpp @@ -185,6 +185,7 @@ void QWaylandWlShellSurface::updateTransientParent(QWindow *parent) || testShowWithoutActivating(m_window->window())) flags |= WL_SHELL_SURFACE_TRANSIENT_INACTIVE; + Q_ASSERT(parent_wayland_window->object()); set_transient(parent_wayland_window->object(), transientPos.x(), transientPos.y(), @@ -211,15 +212,16 @@ void QWaylandWlShellSurface::setPopup(QWaylandWindow *parent, QWaylandInputDevic transientPos.setY(transientPos.y() + parent_wayland_window->decoration()->margins().top()); } + Q_ASSERT(parent_wayland_window->object()); set_popup(device->wl_seat(), serial, parent_wayland_window->object(), transientPos.x(), transientPos.y(), 0); } void QWaylandWlShellSurface::setType(Qt::WindowType type, QWaylandWindow *transientParent) { - if (type == Qt::Popup && transientParent) + if (type == Qt::Popup && transientParent && transientParent->object()) setPopup(transientParent, m_window->display()->lastInputDevice(), m_window->display()->lastInputSerial()); - else if (transientParent) + else if (transientParent && transientParent->object()) updateTransientParent(transientParent->window()); else setTopLevel(); diff --git a/tests/auto/wayland/client/tst_client.cpp b/tests/auto/wayland/client/tst_client.cpp index 8acddfbe740..1eee90f49db 100644 --- a/tests/auto/wayland/client/tst_client.cpp +++ b/tests/auto/wayland/client/tst_client.cpp @@ -143,6 +143,8 @@ private slots: void touchDrag(); void mouseDrag(); void dontCrashOnMultipleCommits(); + void hiddenTransientParent(); + void hiddenPopupParent(); private: MockCompositor *compositor; @@ -360,6 +362,48 @@ void tst_WaylandClient::dontCrashOnMultipleCommits() QTRY_VERIFY(!compositor->surface()); } +void tst_WaylandClient::hiddenTransientParent() +{ + QWindow parent; + QWindow transient; + + transient.setTransientParent(&parent); + + parent.show(); + QTRY_VERIFY(compositor->surface()); + + parent.hide(); + QTRY_VERIFY(!compositor->surface()); + + transient.show(); + QTRY_VERIFY(compositor->surface()); +} + +void tst_WaylandClient::hiddenPopupParent() +{ + TestWindow toplevel; + toplevel.show(); + + // wl_shell relies on a mouse event in order to send a serial and seat + // with the set_popup request. + QSharedPointer surface; + QTRY_VERIFY(surface = compositor->surface()); + QPoint mousePressPos(16, 16); + QCOMPARE(toplevel.mousePressEventCount, 0); + compositor->sendMousePress(surface, mousePressPos); + QTRY_COMPARE(toplevel.mousePressEventCount, 1); + + QWindow popup; + popup.setTransientParent(&toplevel); + popup.setFlag(Qt::Popup, true); + + toplevel.hide(); + QTRY_VERIFY(!compositor->surface()); + + popup.show(); + QTRY_VERIFY(compositor->surface()); +} + int main(int argc, char **argv) { setenv("XDG_RUNTIME_DIR", ".", 1); From 1fbffb9dfb011c3b7c7aaa41b7be3a8167ce0c8a Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Wed, 18 Oct 2017 20:41:50 +0200 Subject: [PATCH 3/4] Ref count buffer usage The QPlatformBackingStore can get flushed multiple times between paints. Flush sets the front buffer, but it does not create a new backbuffer. We can't do so without doing an expensive pre-emptive copy. This means we send the same front buffer multiple times. This is somewhat questionable with regards to the Wayland specification, but seems to work. If we do send a buffer multiple times we can't consider it free until the last attached buffer is released; otherwise we end up painting into a buffer whilst the server is still using it, leading to flickering. Change-Id: I8235eed6a85f0d52b37544e7bcb623b16a9dd832 Reviewed-by: Marco Martin Reviewed-by: Johan Helsing --- src/plugins/platforms/wayland/qwaylandbuffer.cpp | 2 +- src/plugins/platforms/wayland/qwaylandbuffer_p.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandbuffer.cpp b/src/plugins/platforms/wayland/qwaylandbuffer.cpp index a0fcc532f5b..076a0d57d74 100644 --- a/src/plugins/platforms/wayland/qwaylandbuffer.cpp +++ b/src/plugins/platforms/wayland/qwaylandbuffer.cpp @@ -66,7 +66,7 @@ void QWaylandBuffer::init(wl_buffer *buf) void QWaylandBuffer::release(void *data, wl_buffer *) { - static_cast(data)->mBusy = false; + static_cast(data)->mBusy--; } const wl_buffer_listener QWaylandBuffer::listener = { diff --git a/src/plugins/platforms/wayland/qwaylandbuffer_p.h b/src/plugins/platforms/wayland/qwaylandbuffer_p.h index 9e8cba2e490..b3513d1515e 100644 --- a/src/plugins/platforms/wayland/qwaylandbuffer_p.h +++ b/src/plugins/platforms/wayland/qwaylandbuffer_p.h @@ -73,14 +73,14 @@ public: virtual QSize size() const = 0; virtual int scale() const { return 1; } - void setBusy() { mBusy = true; } - bool busy() const { return mBusy; } + void setBusy() { mBusy++; } + bool busy() const { return mBusy > 0; } protected: struct wl_buffer *mBuffer; private: - bool mBusy; + int mBusy; static void release(void *data, wl_buffer *); static const wl_buffer_listener listener; From 7f8d9b7f54f6d0764227cdf09e34f2b0231ae061 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Thu, 19 Oct 2017 12:13:01 +0200 Subject: [PATCH 4/4] Ensure QWaylandWindow::transientParent has a shell surface This may not be a perfect solution, but it's better than the current one, where the transient parent may not have a shell surface (because the window may be hidden or not yet initialized). Task-number: QTBUG-63840 Change-Id: Ia5f04376d4b6d12b41ceeab5ba13cdc1b63b4e3c Reviewed-by: Paul Olav Tvete --- .../platforms/wayland/qwaylandwindow.cpp | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index f7f296d0aaa..6d7c0885c80 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -785,22 +785,27 @@ QWaylandAbstractDecoration *QWaylandWindow::decoration() const return mWindowDecoration; } -static QWindow *topLevelWindow(QWindow *window) +static QWaylandWindow *closestShellSurfaceWindow(QWindow *window) { - while (QWindow *parent = window->parent()) - window = parent; - return window; + while (window) { + auto w = static_cast(window->handle()); + if (w->shellSurface()) + return w; + window = window->transientParent() ? window->transientParent() : window->parent(); + } + return nullptr; } QWaylandWindow *QWaylandWindow::transientParent() const { - // Take the top level window here, since the transient parent may be a QWidgetWindow - // or some other window without a shell surface, which is then not able to get mouse - // events. - if (auto transientParent = window()->transientParent()) - return static_cast(topLevelWindow(transientParent)->handle()); - else if (QGuiApplication::focusWindow() && (window()->type() == Qt::ToolTip || window()->type() == Qt::Popup)) - return static_cast(topLevelWindow(QGuiApplication::focusWindow())->handle()); + // Take the closest window with a shell surface, since the transient parent may be a + // QWidgetWindow or some other window without a shell surface, which is then not able to + // get mouse events. + if (auto transientParent = closestShellSurfaceWindow(window()->transientParent())) + return transientParent; + + if (QGuiApplication::focusWindow() && (window()->type() == Qt::ToolTip || window()->type() == Qt::Popup)) + return closestShellSurfaceWindow(QGuiApplication::focusWindow()); return nullptr; }