From 019427b9ae963c172a05defb21d7034db1ac76a5 Mon Sep 17 00:00:00 2001 From: Paul Olav Tvete Date: Tue, 19 Jun 2018 14:51:08 +0200 Subject: [PATCH 1/2] Don't commit same buffer multiple times In Qt we call flush() when we think the window might need to be updated. It is also possible to trigger a flush while painting. Two fixes: 1) If there are attempted flushes between beginPaint() and endPaint, queue them up, and do them in endPaint(). 2) Make sure we only commit the buffer once: after that the compositor owns the buffer, and it can repaint on its own. Change-Id: Ibf61068fa95760eb67dbc0b1d0534854114ea528 Reviewed-by: Johan Helsing --- .../platforms/wayland/qwaylandbuffer.cpp | 4 +++- .../platforms/wayland/qwaylandbuffer_p.h | 4 ++++ .../wayland/qwaylandshmbackingstore.cpp | 17 ++++++++++++----- .../wayland/qwaylandshmbackingstore_p.h | 2 ++ .../platforms/wayland/qwaylandwindow.cpp | 9 +++++++++ .../platforms/wayland/qwaylandwindow_p.h | 3 +++ 6 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandbuffer.cpp b/src/plugins/platforms/wayland/qwaylandbuffer.cpp index 9792cdd6136..12df9cc4fb2 100644 --- a/src/plugins/platforms/wayland/qwaylandbuffer.cpp +++ b/src/plugins/platforms/wayland/qwaylandbuffer.cpp @@ -64,7 +64,9 @@ void QWaylandBuffer::init(wl_buffer *buf) void QWaylandBuffer::release(void *data, wl_buffer *) { - static_cast(data)->mBusy = false; + QWaylandBuffer *self = static_cast(data); + self->mBusy = false; + self->mCommitted = false; } 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 156ea9530b4..eea090f35cd 100644 --- a/src/plugins/platforms/wayland/qwaylandbuffer_p.h +++ b/src/plugins/platforms/wayland/qwaylandbuffer_p.h @@ -76,11 +76,15 @@ public: void setBusy() { mBusy = true; } bool busy() const { return mBusy; } + void setCommitted() { mCommitted = true; } + bool committed() const { return mCommitted; } + protected: struct wl_buffer *mBuffer = nullptr; private: bool mBusy = false; + bool mCommitted = false; static void release(void *data, wl_buffer *); static const wl_buffer_listener listener; diff --git a/src/plugins/platforms/wayland/qwaylandshmbackingstore.cpp b/src/plugins/platforms/wayland/qwaylandshmbackingstore.cpp index dde01357b76..ecb03c0d602 100644 --- a/src/plugins/platforms/wayland/qwaylandshmbackingstore.cpp +++ b/src/plugins/platforms/wayland/qwaylandshmbackingstore.cpp @@ -48,7 +48,6 @@ #include #include #include -#include #include #include @@ -68,10 +67,6 @@ QT_BEGIN_NAMESPACE namespace QtWaylandClient { -Q_DECLARE_LOGGING_CATEGORY(lcWaylandBackingstore) - -Q_LOGGING_CATEGORY(lcWaylandBackingstore, "qt.qpa.wayland.backingstore") - QWaylandShmBuffer::QWaylandShmBuffer(QWaylandDisplay *display, const QSize &size, QImage::Format format, int scale) { @@ -199,6 +194,8 @@ void QWaylandShmBackingStore::beginPaint(const QRegion ®ion) void QWaylandShmBackingStore::endPaint() { mPainting = false; + if (mPendingFlush) + flush(window(), mPendingRegion, QPoint()); waylandWindow()->setCanResize(true); } @@ -218,9 +215,19 @@ void QWaylandShmBackingStore::flush(QWindow *window, const QRegion ®ion, cons // called instead. The default implementation from QPlatformBackingStore is sufficient // however so no need to reimplement that. + Q_UNUSED(window); Q_UNUSED(offset); + if (mPainting) { + mPendingRegion |= region; + mPendingFlush = true; + return; + } + + mPendingFlush = false; + mPendingRegion = QRegion(); + if (windowDecoration() && windowDecoration()->isDirty()) updateDecorations(); diff --git a/src/plugins/platforms/wayland/qwaylandshmbackingstore_p.h b/src/plugins/platforms/wayland/qwaylandshmbackingstore_p.h index cb66288fcd9..88ecfc5ece1 100644 --- a/src/plugins/platforms/wayland/qwaylandshmbackingstore_p.h +++ b/src/plugins/platforms/wayland/qwaylandshmbackingstore_p.h @@ -120,6 +120,8 @@ private: QWaylandShmBuffer *mFrontBuffer = nullptr; QWaylandShmBuffer *mBackBuffer = nullptr; bool mPainting = false; + bool mPendingFlush = false; + QRegion mPendingRegion; QMutex mMutex; QSize mRequestedSize; diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 5d658f675d6..b88541ad850 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -75,6 +75,8 @@ QT_BEGIN_NAMESPACE namespace QtWaylandClient { +Q_LOGGING_CATEGORY(lcWaylandBackingstore, "qt.qpa.wayland.backingstore") + QWaylandWindow *QWaylandWindow::mMouseGrab = nullptr; QWaylandWindow::QWaylandWindow(QWindow *window) @@ -556,6 +558,7 @@ void QWaylandWindow::handleScreenRemoved(QScreen *qScreen) void QWaylandWindow::attach(QWaylandBuffer *buffer, int x, int y) { + Q_ASSERT(!buffer->committed()); if (mFrameCallback) { wl_callback_destroy(mFrameCallback); mFrameCallback = nullptr; @@ -586,12 +589,18 @@ void QWaylandWindow::damage(const QRect &rect) void QWaylandWindow::commit(QWaylandBuffer *buffer, const QRegion &damage) { + if (buffer->committed()) { + qCDebug(lcWaylandBackingstore) << "Buffer already committed, ignoring."; + return; + } if (!isInitialized()) return; attachOffset(buffer); for (const QRect &rect: damage) wl_surface::damage(rect.x(), rect.y(), rect.width(), rect.height()); + Q_ASSERT(!buffer->committed()); + buffer->setCommitted(); wl_surface::commit(); } diff --git a/src/plugins/platforms/wayland/qwaylandwindow_p.h b/src/plugins/platforms/wayland/qwaylandwindow_p.h index 3324bf70056..366548635a2 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow_p.h +++ b/src/plugins/platforms/wayland/qwaylandwindow_p.h @@ -55,6 +55,7 @@ #include #include #include +#include #include @@ -67,6 +68,8 @@ QT_BEGIN_NAMESPACE namespace QtWaylandClient { +Q_DECLARE_LOGGING_CATEGORY(lcWaylandBackingstore) + class QWaylandDisplay; class QWaylandBuffer; class QWaylandShellSurface; From fc8cf4426399e320ac64ae117406ab00025e8c0f Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Fri, 3 Aug 2018 11:42:19 +0200 Subject: [PATCH 2/2] Client: Don't leak toplevels for xdg-shell-unstable-v6 Change-Id: Ifd6d4956eeed663e45219b428dfe562e7a82e626 Reviewed-by: David Edmundson Reviewed-by: Paul Olav Tvete --- src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp b/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp index 5bcd3b25496..2c910d116f1 100644 --- a/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp +++ b/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp @@ -136,8 +136,10 @@ QWaylandXdgSurfaceV6::QWaylandXdgSurfaceV6(QWaylandXdgShellV6 *shell, ::zxdg_sur QWaylandXdgSurfaceV6::~QWaylandXdgSurfaceV6() { - if (m_toplevel) - zxdg_toplevel_v6_destroy(m_toplevel->object()); + if (m_toplevel) { + delete m_toplevel; + m_toplevel = nullptr; + } if (m_popup) { delete m_popup; m_popup = nullptr;