From f1b07ea1ee9e839a5ca38c4c330511ab6cd0b56c Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Tue, 21 Nov 2017 10:13:43 +0100 Subject: [PATCH 1/9] Revert "Ref count buffer usage" When a buffer is committed multiple times, not all compositors (i.e. Weston) send a matching number of release events. This caused clients to freeze on some occasions on those compositors because they were waiting for a release event that never came. This reverts commit 1fbffb9dfb011c3b7c7aaa41b7be3a8167ce0c8a. Task-number: QTBUG-64631 Change-Id: I818d9bd71e5d9ce7a351a2010914b7219b1975bc Reviewed-by: David Edmundson Reviewed-by: Paul Olav Tvete --- 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 076a0d57d74..a0fcc532f5b 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--; + static_cast(data)->mBusy = 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 b3513d1515e..9e8cba2e490 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++; } - bool busy() const { return mBusy > 0; } + void setBusy() { mBusy = true; } + bool busy() const { return mBusy; } protected: struct wl_buffer *mBuffer; private: - int mBusy; + bool mBusy; static void release(void *data, wl_buffer *); static const wl_buffer_listener listener; From 0f29ae002458fda0c68c39bdc41dd35221e1b8a3 Mon Sep 17 00:00:00 2001 From: Aleix Pol Date: Fri, 13 Oct 2017 15:44:09 +0200 Subject: [PATCH 2/9] Fix crash when opening a window with a hidden parent We have a transient parent but it doesn't have a shell surface. We need to make sure that it exists before setting the transient parent's shell surface as the parent to the window. Change-Id: I918b2f14074217638529ba73530f0102f7438079 Reviewed-by: David Edmundson Reviewed-by: Pier Luigi Fiorini --- src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp b/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp index cd81778c79c..d9302ed57bb 100644 --- a/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp +++ b/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp @@ -169,7 +169,8 @@ void QWaylandXdgSurfaceV6::setType(Qt::WindowType type, QWaylandWindow *transien setToplevel(); if (transientParent) { auto parentXdgSurface = static_cast(transientParent->shellSurface()); - m_toplevel->set_parent(parentXdgSurface->m_toplevel->object()); + if (parentXdgSurface) + m_toplevel->set_parent(parentXdgSurface->m_toplevel->object()); } } } From d7f4f6ba10e1b981d891b082e51ec5ce56e5d7b0 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Fri, 22 Dec 2017 11:26:41 +0100 Subject: [PATCH 3/9] Client tests: Fix wrong position being sent when mocking touch events We were sending ints when we should have been sending wl_fixed_ts. Change-Id: I9f074334cb3ea8a3d61789ff641c2d022a5989b7 Reviewed-by: Pier Luigi Fiorini --- tests/auto/wayland/client/mockinput.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/auto/wayland/client/mockinput.cpp b/tests/auto/wayland/client/mockinput.cpp index 99acdd43a75..b2bcdf2e6b6 100644 --- a/tests/auto/wayland/client/mockinput.cpp +++ b/tests/auto/wayland/client/mockinput.cpp @@ -365,7 +365,9 @@ void Touch::sendDown(Surface *surface, const QPoint &position, int id) Q_ASSERT(surface); Resource *resource = resourceMap().value(surface->resource()->client()); Q_ASSERT(resource); - wl_touch_send_down(resource->handle, serial, time, surface->resource()->handle, id, position.x(), position.y()); + auto x = wl_fixed_from_int(position.x()); + auto y = wl_fixed_from_int(position.y()); + wl_touch_send_down(resource->handle, serial, time, surface->resource()->handle, id, x, y); } void Touch::sendUp(Surface *surface, int id) @@ -378,7 +380,9 @@ void Touch::sendMotion(Surface *surface, const QPoint &position, int id) { Resource *resource = resourceMap().value(surface->resource()->client()); uint32_t time = m_compositor->time(); - wl_touch_send_motion(resource->handle, time, id, position.x(), position.y()); + auto x = wl_fixed_from_int(position.x()); + auto y = wl_fixed_from_int(position.y()); + wl_touch_send_motion(resource->handle, time, id, x, y); } void Touch::sendFrame(Surface *surface) From 2a82da8787a177691641b17337c317ef3c699b9e Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Tue, 21 Nov 2017 10:13:43 +0100 Subject: [PATCH 4/9] Revert "Ref count buffer usage" When a buffer is committed multiple times, not all compositors (i.e. Weston) send a matching number of release events. This caused clients to freeze on some occasions on those compositors because they were waiting for a release event that never came. This reverts commit 1fbffb9dfb011c3b7c7aaa41b7be3a8167ce0c8a. Backport from 5.10.0 to 5.9.4 Task-number: QTBUG-64631 Reviewed-by: David Edmundson Reviewed-by: Paul Olav Tvete (cherry picked from commit f1b07ea1ee9e839a5ca38c4c330511ab6cd0b56c) Change-Id: I2acf5574a1b792d3bbf928fb0bc328aa8ee8ffe0 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 076a0d57d74..a0fcc532f5b 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--; + static_cast(data)->mBusy = 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 b3513d1515e..9e8cba2e490 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++; } - bool busy() const { return mBusy > 0; } + void setBusy() { mBusy = true; } + bool busy() const { return mBusy; } protected: struct wl_buffer *mBuffer; private: - int mBusy; + bool mBusy; static void release(void *data, wl_buffer *); static const wl_buffer_listener listener; From 0483742ad908d3408cdbf29330ee797972b3cf52 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Fri, 22 Dec 2017 11:21:38 +0100 Subject: [PATCH 5/9] Client tests: Support testing with window decorations enabled Previously tests would hang because eglInitialize (which was called on window decoration creation) would wait for a Wayland roundtrip while our compositor thread was waiting for more commands. Work around this by prematurely causing the clientBufferIntegration to be initialized before applicationInitialized (when the compositors switches to handling requests synchronously). Change-Id: I793c70a8f3a764cb3a70f00ddcab76cd4044b442 Reviewed-by: Pier Luigi Fiorini --- tests/auto/wayland/client/client.pro | 2 +- tests/auto/wayland/client/tst_client.cpp | 37 ++++++++++++++---------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/tests/auto/wayland/client/client.pro b/tests/auto/wayland/client/client.pro index 34fc67474db..e6e607c3f60 100644 --- a/tests/auto/wayland/client/client.pro +++ b/tests/auto/wayland/client/client.pro @@ -2,7 +2,7 @@ CONFIG += testcase link_pkgconfig TARGET = tst_client QT += testlib -QT += core-private gui-private +QT += core-private gui-private waylandclient-private QMAKE_USE += wayland-client wayland-server diff --git a/tests/auto/wayland/client/tst_client.cpp b/tests/auto/wayland/client/tst_client.cpp index 1eee90f49db..3897bd3b129 100644 --- a/tests/auto/wayland/client/tst_client.cpp +++ b/tests/auto/wayland/client/tst_client.cpp @@ -37,6 +37,8 @@ #include #include +#include +#include static const QSize screenSize(1600, 1200); @@ -96,6 +98,8 @@ public: ++touchEventCount; } + QPoint frameOffset() const { return QPoint(frameMargins().left(), frameMargins().top()); } + int focusInEventCount; int focusOutEventCount; int keyPressEventCount; @@ -201,7 +205,7 @@ void tst_WaylandClient::events() QPoint mousePressPos(16, 16); QCOMPARE(window.mousePressEventCount, 0); - compositor->sendMousePress(surface, mousePressPos); + compositor->sendMousePress(surface, window.frameOffset() + mousePressPos); QTRY_COMPARE(window.mousePressEventCount, 1); QTRY_COMPARE(window.mousePressPos, mousePressPos); @@ -210,7 +214,7 @@ void tst_WaylandClient::events() QTRY_COMPARE(window.mouseReleaseEventCount, 1); const int touchId = 0; - compositor->sendTouchDown(surface, QPoint(10, 10), touchId); + compositor->sendTouchDown(surface, window.frameOffset() + QPoint(10, 10), touchId); compositor->sendTouchFrame(surface); QTRY_COMPARE(window.touchEventCount, 1); @@ -269,6 +273,7 @@ public: m_dragIcon = QPixmap::fromImage(cursorImage); } ~DndWindow(){} + QPoint frameOffset() const { return QPoint(frameMargins().left(), frameMargins().top()); } bool dragStarted; protected: @@ -302,14 +307,14 @@ void tst_WaylandClient::touchDrag() QTRY_COMPARE(QGuiApplication::focusWindow(), &window); const int id = 0; - compositor->sendTouchDown(surface, QPoint(10, 10), id); + compositor->sendTouchDown(surface, window.frameOffset() + QPoint(10, 10), id); compositor->sendTouchFrame(surface); - compositor->sendTouchMotion(surface, QPoint(20, 20), id); + compositor->sendTouchMotion(surface, window.frameOffset() + QPoint(20, 20), id); compositor->sendTouchFrame(surface); compositor->waitForStartDrag(); compositor->sendDataDeviceDataOffer(surface); - compositor->sendDataDeviceEnter(surface, QPoint(20, 20)); - compositor->sendDataDeviceMotion( QPoint(21, 21)); + compositor->sendDataDeviceEnter(surface, window.frameOffset() + QPoint(20, 20)); + compositor->sendDataDeviceMotion(window.frameOffset() + QPoint(21, 21)); compositor->sendDataDeviceDrop(surface); compositor->sendDataDeviceLeave(surface); QTRY_VERIFY(window.dragStarted); @@ -326,10 +331,10 @@ void tst_WaylandClient::mouseDrag() compositor->setKeyboardFocus(surface); QTRY_COMPARE(QGuiApplication::focusWindow(), &window); - compositor->sendMousePress(surface, QPoint(10, 10)); + compositor->sendMousePress(surface, window.frameOffset() + QPoint(10, 10)); compositor->sendDataDeviceDataOffer(surface); - compositor->sendDataDeviceEnter(surface, QPoint(20, 20)); - compositor->sendDataDeviceMotion( QPoint(21, 21)); + compositor->sendDataDeviceEnter(surface, window.frameOffset() + QPoint(20, 20)); + compositor->sendDataDeviceMotion(window.frameOffset() + QPoint(21, 21)); compositor->waitForStartDrag(); compositor->sendDataDeviceDrop(surface); compositor->sendDataDeviceLeave(surface); @@ -390,7 +395,7 @@ void tst_WaylandClient::hiddenPopupParent() QTRY_VERIFY(surface = compositor->surface()); QPoint mousePressPos(16, 16); QCOMPARE(toplevel.mousePressEventCount, 0); - compositor->sendMousePress(surface, mousePressPos); + compositor->sendMousePress(surface, toplevel.frameOffset() + mousePressPos); QTRY_COMPARE(toplevel.mousePressEventCount, 1); QWindow popup; @@ -409,15 +414,17 @@ int main(int argc, char **argv) setenv("XDG_RUNTIME_DIR", ".", 1); setenv("QT_QPA_PLATFORM", "wayland", 1); // force QGuiApplication to use wayland plugin - // wayland-egl hangs in the test setup when we try to initialize. Until it gets - // figured out, avoid clientBufferIntegration() from being called in - // QWaylandWindow::createDecorations(). - setenv("QT_WAYLAND_DISABLE_WINDOWDECORATION", "1", 1); - MockCompositor compositor; compositor.setOutputGeometry(QRect(QPoint(), screenSize)); QGuiApplication app(argc, argv); + + // Initializing some client buffer integrations (i.e. eglInitialize) may block while waiting + // for a wayland sync. So we call clientBufferIntegration prior to applicationInitialized + // (while the compositor processes events without waiting) in order to avoid hanging later. + auto *waylandIntegration = static_cast(QGuiApplicationPrivate::platformIntegration()); + waylandIntegration->clientBufferIntegration(); + compositor.applicationInitialized(); tst_WaylandClient tc(&compositor); From f94c0f2cb6cb1ffa7a03e93228729802bfb5af93 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Fri, 5 Jan 2018 12:44:30 +0100 Subject: [PATCH 6/9] Fix protocol error when destroying xdg surfaces (v6) Destroy role object for toplevels when destroying the xdg surface. Task-number: QTBUG-65568 Change-Id: Ibe027c3eef8160f9fd2cfb05971c92ceb155f95b Reviewed-by: David Edmundson Reviewed-by: Pier Luigi Fiorini --- src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp b/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp index cd81778c79c..7473174d1bf 100644 --- a/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp +++ b/src/plugins/platforms/wayland/qwaylandxdgshellv6.cpp @@ -125,6 +125,8 @@ QWaylandXdgSurfaceV6::QWaylandXdgSurfaceV6(QWaylandXdgShellV6 *shell, ::zxdg_sur QWaylandXdgSurfaceV6::~QWaylandXdgSurfaceV6() { + if (m_toplevel) + zxdg_toplevel_v6_destroy(m_toplevel->object()); if (m_popup) zxdg_popup_v6_destroy(m_popup->object()); destroy(); From 6ec8b1093a3d2f6d62846091abe35098c55eec10 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Tue, 16 Jan 2018 12:02:30 +0100 Subject: [PATCH 7/9] Fix static builds with libwayland-egl There was a naming conflict between the client buffer integration, wayland-egl, and the system library libwayland-egl.so Rename the plugin binary to qt-plugin-wayland-egl to avoid the issue. Task-number: QTBUG-65652 Change-Id: Ib074c25e269a5e11b087fb4c3ddb15fef7d4a7ee Reviewed-by: Paul Olav Tvete --- .../plugins/hardwareintegration/wayland-egl/wayland-egl.pro | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/platforms/wayland/plugins/hardwareintegration/wayland-egl/wayland-egl.pro b/src/plugins/platforms/wayland/plugins/hardwareintegration/wayland-egl/wayland-egl.pro index d85c5efaaa1..409cd37d7c7 100644 --- a/src/plugins/platforms/wayland/plugins/hardwareintegration/wayland-egl/wayland-egl.pro +++ b/src/plugins/platforms/wayland/plugins/hardwareintegration/wayland-egl/wayland-egl.pro @@ -7,6 +7,8 @@ OTHER_FILES += \ SOURCES += main.cpp +TARGET = qt-plugin-wayland-egl + PLUGIN_TYPE = wayland-graphics-integration-client PLUGIN_CLASS_NAME = QWaylandEglClientBufferPlugin load(qt_plugin) From dd2018f0e54469dbb83930563a2ee603d1197e52 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Wed, 3 Jan 2018 19:18:42 +0000 Subject: [PATCH 8/9] Don't recreate hidden egl surfaces QWaylandEglWindow deletes surfaces when a window changes from hidden to visible, presumably as a result of us not having a valid wl_surface object. By extension it doesn't make sense to create a surface whilst a window is still hidden. This fixes a crash where a QQuickWindow hides and then is destroyed. In QQuickWindow destruction we have to create a valid context in order to delete any textures/assets owned by the scene graph; as the wl_surface has gone this causes an error in the EGL libs when we create an EGL surface. Task-number: QTBUG-65553 Change-Id: I9b37a86326bf2cd7737c4e839c1aa8c74cf08116 Reviewed-by: Johan Helsing --- tests/auto/wayland/client/tst_client.cpp | 37 ++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/auto/wayland/client/tst_client.cpp b/tests/auto/wayland/client/tst_client.cpp index 3897bd3b129..aed601d8a75 100644 --- a/tests/auto/wayland/client/tst_client.cpp +++ b/tests/auto/wayland/client/tst_client.cpp @@ -35,6 +35,8 @@ #include #include #include +#include +#include #include #include @@ -112,6 +114,25 @@ public: QPoint mousePressPos; }; +class TestGlWindow : public QOpenGLWindow +{ + Q_OBJECT + +public: + TestGlWindow(); + +protected: + void paintGL() override; +}; + +TestGlWindow::TestGlWindow() +{} + +void TestGlWindow::paintGL() +{ + glClear(GL_COLOR_BUFFER_BIT); +} + class tst_WaylandClient : public QObject { Q_OBJECT @@ -149,6 +170,7 @@ private slots: void dontCrashOnMultipleCommits(); void hiddenTransientParent(); void hiddenPopupParent(); + void glWindow(); private: MockCompositor *compositor; @@ -409,6 +431,21 @@ void tst_WaylandClient::hiddenPopupParent() QTRY_VERIFY(compositor->surface()); } +void tst_WaylandClient::glWindow() +{ + QSKIP("Skipping GL tests, as not supported by all CI systems: See https://bugreports.qt.io/browse/QTBUG-65802"); + + QScopedPointer testWindow(new TestGlWindow); + testWindow->show(); + QSharedPointer surface; + QTRY_VERIFY(surface = compositor->surface()); + + //confirm we don't crash when we delete an already hidden GL window + //QTBUG-65553 + testWindow->setVisible(false); + QTRY_VERIFY(!compositor->surface()); +} + int main(int argc, char **argv) { setenv("XDG_RUNTIME_DIR", ".", 1); From d4dc5f357353ca30e1a109983fa81d1231cf42e3 Mon Sep 17 00:00:00 2001 From: Paul Olav Tvete Date: Mon, 8 Jan 2018 13:30:51 +0100 Subject: [PATCH 9/9] Fix requestUpdate() After commit 027a7131543b0194a7d9e98a6b53928e30bc080f, mFrameCallback is set to null when the next buffer is attached, not when the callback arrives. This means that a requestUpdate() after the frame callback would never be delivered. The solution is to test mWaitingForFrameSync instead. There is still a small race condition, but the failure case is that the update will arrive after 5 ms instead of exactly at frame sync. Change-Id: I413ed2b76c8527f825e501077bab712146b6705f Reviewed-by: Johan Helsing --- src/plugins/platforms/wayland/qwaylandwindow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 6d7c0885c80..d9870810049 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -1029,7 +1029,7 @@ QVariant QWaylandWindow::property(const QString &name, const QVariant &defaultVa void QWaylandWindow::requestUpdate() { - if (!mFrameCallback) + if (!mWaitingForFrameSync) QPlatformWindow::requestUpdate(); else mUpdateRequested = true;