From 0b803993e706c6abb7eb3c49e3fc3a1fd7e288dc Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Mon, 8 Apr 2019 13:51:28 +0200 Subject: [PATCH 1/9] Client: Add safer fromObject function to scanner Makes the scanner produce generated static functions such as QtWaylandClient::wl_surface *wl_surface::fromObject(struct ::wl_surface *object); Which casts from the wayland-scanner generated types, such as struct ::wl_surface *, to types types generated by qtwaylandscanner, but performs a check to see if the listener is set to the wrapper class first (at least for interfaces with events). This lets us easily fix crashes in a couple of places where we receive events with wayland objects that we didn't create. Also adds nullptr checks whenever we use the fromWlSurface() and fromWlOutput() functions to handle failed conversions. Task-number: QTBUG-73801 Fixes: QTBUG-74085 Change-Id: I9f33c31c7d1a939ccb3ebbbcb0eb67af10037237 Reviewed-by: Jaroslaw Kubik Reviewed-by: Paul Olav Tvete (cherry picked from commit c6476ae915b8ac64121d379274363227b1c300ff) Reviewed-by: Pier Luigi Fiorini --- .../platforms/wayland/qwaylanddatadevice.cpp | 8 ++++++-- .../platforms/wayland/qwaylandinputdevice.cpp | 13 ++++++++++++- src/plugins/platforms/wayland/qwaylandqtkey.cpp | 6 +++++- src/plugins/platforms/wayland/qwaylandscreen.cpp | 5 +++-- src/tools/qtwaylandscanner/qtwaylandscanner.cpp | 11 +++++++++++ 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylanddatadevice.cpp b/src/plugins/platforms/wayland/qwaylanddatadevice.cpp index 11984f9d394..6b2a408eb19 100644 --- a/src/plugins/platforms/wayland/qwaylanddatadevice.cpp +++ b/src/plugins/platforms/wayland/qwaylanddatadevice.cpp @@ -154,9 +154,13 @@ void QWaylandDataDevice::data_device_drop() void QWaylandDataDevice::data_device_enter(uint32_t serial, wl_surface *surface, wl_fixed_t x, wl_fixed_t y, wl_data_offer *id) { - m_enterSerial = serial; - m_dragWindow = QWaylandWindow::fromWlSurface(surface)->window(); + auto *dragWaylandWindow = QWaylandWindow::fromWlSurface(surface); + if (!dragWaylandWindow) + return; // Ignore foreign surfaces + + m_dragWindow = dragWaylandWindow->window(); m_dragPoint = calculateDragPosition(x, y, m_dragWindow); + m_enterSerial = serial; QMimeData *dragData = nullptr; Qt::DropActions supportedActions; diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp index 90e138a3dcc..495f258ded9 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp +++ b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp @@ -451,6 +451,9 @@ void QWaylandInputDevice::Pointer::pointer_enter(uint32_t serial, struct wl_surf return; QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface); + if (!window) + return; // Ignore foreign surfaces + mFocus = window; mSurfacePos = QPointF(wl_fixed_to_double(sx), wl_fixed_to_double(sy)); mGlobalPos = window->window()->mapToGlobal(mSurfacePos.toPoint()); @@ -477,6 +480,10 @@ void QWaylandInputDevice::Pointer::pointer_leave(uint32_t time, struct wl_surfac if (!surface) return; + auto *window = QWaylandWindow::fromWlSurface(surface); + if (!window) + return; // Ignore foreign surfaces + if (!QWaylandWindow::mouseGrab()) { QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface); window->handleMouseLeave(mParent); @@ -852,9 +859,13 @@ void QWaylandInputDevice::Touch::touch_down(uint32_t serial, if (!surface) return; + auto *window = QWaylandWindow::fromWlSurface(surface); + if (!window) + return; // Ignore foreign surfaces + mParent->mTime = time; mParent->mSerial = serial; - mFocus = QWaylandWindow::fromWlSurface(surface); + mFocus = window; mParent->mQDisplay->setLastInputDevice(mParent, serial, mFocus); mParent->handleTouchPoint(id, wl_fixed_to_double(x), wl_fixed_to_double(y), Qt::TouchPointPressed); } diff --git a/src/plugins/platforms/wayland/qwaylandqtkey.cpp b/src/plugins/platforms/wayland/qwaylandqtkey.cpp index a60185bd641..19261973853 100644 --- a/src/plugins/platforms/wayland/qwaylandqtkey.cpp +++ b/src/plugins/platforms/wayland/qwaylandqtkey.cpp @@ -70,7 +70,11 @@ void QWaylandQtKeyExtension::zqt_key_v1_key(struct wl_surface *surface, } QWaylandInputDevice *dev = inputDevices.first(); - QWaylandWindow *win = surface ? QWaylandWindow::fromWlSurface(surface) : dev->keyboardFocus(); + + auto *win = surface ? QWaylandWindow::fromWlSurface(surface) : nullptr; + + if (!win) + win = dev->keyboardFocus(); if (!win || !win->window()) { qWarning("qt_key_extension: handle_qtkey: No keyboard focus"); diff --git a/src/plugins/platforms/wayland/qwaylandscreen.cpp b/src/plugins/platforms/wayland/qwaylandscreen.cpp index 38d61f88c35..1fe0125e64b 100644 --- a/src/plugins/platforms/wayland/qwaylandscreen.cpp +++ b/src/plugins/platforms/wayland/qwaylandscreen.cpp @@ -199,8 +199,9 @@ QWaylandScreen * QWaylandScreen::waylandScreenFromWindow(QWindow *window) QWaylandScreen *QWaylandScreen::fromWlOutput(::wl_output *output) { - auto wlOutput = static_cast(wl_output_get_user_data(output)); - return static_cast(wlOutput); + if (auto *o = QtWayland::wl_output::fromObject(output)) + return static_cast(o); + return nullptr; } void QWaylandScreen::output_mode(uint32_t flags, int width, int height, int refresh) diff --git a/src/tools/qtwaylandscanner/qtwaylandscanner.cpp b/src/tools/qtwaylandscanner/qtwaylandscanner.cpp index 6f4a33b5f6b..c617369aee2 100644 --- a/src/tools/qtwaylandscanner/qtwaylandscanner.cpp +++ b/src/tools/qtwaylandscanner/qtwaylandscanner.cpp @@ -982,6 +982,7 @@ bool Scanner::process() printf("\n"); printf(" struct ::%s *object() { return m_%s; }\n", interfaceName, interfaceName); printf(" const struct ::%s *object() const { return m_%s; }\n", interfaceName, interfaceName); + printf(" static %s *fromObject(struct ::%s *object);\n", interfaceName, interfaceName); printf("\n"); printf(" bool isInitialized() const;\n"); printf("\n"); @@ -1110,6 +1111,16 @@ bool Scanner::process() printf(" }\n"); printf("\n"); + printf(" %s *%s::fromObject(struct ::%s *object)\n", interfaceName, interfaceName, interfaceName); + printf(" {\n"); + if (hasEvents) { + printf(" if (wl_proxy_get_listener((struct ::wl_proxy *)object) != (void *)&m_%s_listener)\n", interfaceName); + printf(" return nullptr;\n"); + } + printf(" return static_cast<%s *>(%s_get_user_data(object));\n", interfaceName, interfaceName); + printf(" }\n"); + printf("\n"); + printf(" bool %s::isInitialized() const\n", interfaceName); printf(" {\n"); printf(" return m_%s != nullptr;\n", interfaceName); From 7163c286e4b451c8436a95b94dc32b7f62b19ee2 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Fri, 16 Aug 2019 12:54:56 +0200 Subject: [PATCH 2/9] Fix incorrect conversion to straight alpha pixel formats Previously, QWaylandSharedMemoryFormatHelper::fromWaylandShmFormat(WL_SHM_FORMAT_ARGB8888) would return Format_ARGB32, i.e. the non-premultiplied version, while, according to the wayland-devel mailing list (https://lists.freedesktop.org/archives/wayland-devel/2017-August/034791.html), all Wayland RGB-based pixel formats with alpha should have premultiplied alpha. This patch makes sure we return the premultiplied variants for ARGB8888, as well as for ABGR8888. Using a switch instead of the array also allows us more freedom to choose the preferred format in other cases where multiple QImage formats map to the same wl_shm format. While being wrapped and exported as QtWaylandClient::QWaylandShm::fromFormat (private API), this conversion function doesn't seem to be used anywhere, so this patch shouldn't cause any changes in behavior for projects that only use public API. Change-Id: Ie09f9a339b4540dd0383a72b3c951eb8c93e3ab4 Reviewed-by: Pier Luigi Fiorini --- .../qwaylandsharedmemoryformathelper_p.h | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/plugins/platforms/wayland/shared/qwaylandsharedmemoryformathelper_p.h b/src/plugins/platforms/wayland/shared/qwaylandsharedmemoryformathelper_p.h index 85905c02fa3..72cc8401c26 100644 --- a/src/plugins/platforms/wayland/shared/qwaylandsharedmemoryformathelper_p.h +++ b/src/plugins/platforms/wayland/shared/qwaylandsharedmemoryformathelper_p.h @@ -51,7 +51,26 @@ class QWaylandSharedMemoryFormatHelper { public: static inline wl_shm_format fromQImageFormat(QImage::Format format); - static inline QImage::Format fromWaylandShmFormat(wl_shm_format format); + static inline QImage::Format fromWaylandShmFormat(wl_shm_format format) + { + switch (format) { + case WL_SHM_FORMAT_XRGB8888: return QImage::Format_RGB32; + case WL_SHM_FORMAT_ARGB8888: return QImage::Format_ARGB32_Premultiplied; + case WL_SHM_FORMAT_RGB565: return QImage::Format_RGB16; + case WL_SHM_FORMAT_XRGB1555: return QImage::Format_RGB555; + case WL_SHM_FORMAT_RGB888: return QImage::Format_RGB888; + case WL_SHM_FORMAT_XRGB4444: return QImage::Format_RGB444; + case WL_SHM_FORMAT_ARGB4444: return QImage::Format_ARGB4444_Premultiplied; + case WL_SHM_FORMAT_XBGR8888: return QImage::Format_RGBX8888; + case WL_SHM_FORMAT_ABGR8888: return QImage::Format_RGBA8888_Premultiplied; + case WL_SHM_FORMAT_XBGR2101010: return QImage::Format_BGR30; + case WL_SHM_FORMAT_ABGR2101010: return QImage::Format_A2BGR30_Premultiplied; + case WL_SHM_FORMAT_XRGB2101010: return QImage::Format_RGB30; + case WL_SHM_FORMAT_ARGB2101010: return QImage::Format_A2RGB30_Premultiplied; + case WL_SHM_FORMAT_C8: return QImage::Format_Alpha8; + default: return QImage::Format_Invalid; + } + } static inline QVector supportedWaylandFormats(); private: @@ -108,16 +127,6 @@ wl_shm_format QWaylandSharedMemoryFormatHelper::fromQImageFormat(QImage::Format return array.data[format]; } -QImage::Format QWaylandSharedMemoryFormatHelper::fromWaylandShmFormat(wl_shm_format format) -{ - Array array = getData(); - for (size_t i = 0; i < array.size; i++) { - if (array.data[i] == format) - return QImage::Format(i); - } - return QImage::Format_Invalid; -} - QVector QWaylandSharedMemoryFormatHelper::supportedWaylandFormats() { QVector retFormats; From cb68cead894f058315fe7a0736d6f9da24d4fcd1 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Tue, 23 Jul 2019 01:40:36 +0200 Subject: [PATCH 3/9] Client: Fix large clipboard pasting With the non-blocking file opening, when no data was available in the pipe, read could return 0 even though we were not at the end of the file. This resulted in truncated data when pasting the clipboard. This patch uses select to block until a file is available, removing the polling sleep. This means the file can be safely opened in blocking mode whilst keeping the timeout feature allowing read to work as intended. Change-Id: I936aa85e9f25197e7abe6fb3fa729b618c00924d Reviewed-by: David Faure Reviewed-by: Johan Helsing --- .../platforms/wayland/qwaylanddataoffer.cpp | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylanddataoffer.cpp b/src/plugins/platforms/wayland/qwaylanddataoffer.cpp index 0c732c02078..3da16ed0041 100644 --- a/src/plugins/platforms/wayland/qwaylanddataoffer.cpp +++ b/src/plugins/platforms/wayland/qwaylanddataoffer.cpp @@ -135,7 +135,7 @@ QVariant QWaylandMimeData::retrieveData_sys(const QString &mimeType, QVariant::T } int pipefd[2]; - if (qt_safe_pipe(pipefd, O_NONBLOCK) == -1) { + if (qt_safe_pipe(pipefd) == -1) { qWarning("QWaylandMimeData: pipe2() failed"); return QVariant(); } @@ -158,23 +158,32 @@ QVariant QWaylandMimeData::retrieveData_sys(const QString &mimeType, QVariant::T int QWaylandMimeData::readData(int fd, QByteArray &data) const { - char buf[4096]; - int retryCount = 0; - int n; - while (true) { - n = QT_READ(fd, buf, sizeof buf); - if (n == -1 && (errno == EAGAIN || errno == EWOULDBLOCK) && ++retryCount < 1000) - usleep(1000); - else - break; - } - if (retryCount >= 1000) + fd_set readset; + FD_ZERO(&readset); + FD_SET(fd, &readset); + struct timeval timeout; + timeout.tv_sec = 1; + timeout.tv_usec = 0; + + int ready = select(FD_SETSIZE, &readset, nullptr, nullptr, &timeout); + if (ready < 0) { + qWarning() << "QWaylandDataOffer: select() failed"; + return -1; + } else if (ready == 0) { qWarning("QWaylandDataOffer: timeout reading from pipe"); - if (n > 0) { - data.append(buf, n); - n = readData(fd, data); + return -1; + } else { + char buf[4096]; + int n = QT_READ(fd, buf, sizeof buf); + + if (n > 0) { + data.append(buf, n); + n = readData(fd, data); + } else if (n < 0) { + qWarning("QWaylandDataOffer: read() failed"); + } + return n; } - return n; } } From ec510cb38c00231406f77dfea771e3c96874412b Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Wed, 26 Jun 2019 09:47:03 +0200 Subject: [PATCH 4/9] Client: Emit wl_surface lifetime signals on QWaylandWindow Recent changes in QtBase means QtWayland will have to follow the convention of the rest of the QPA plugins and have QPlatformSurfaceEvent::SurfaceCreated and SurfaceAboutToBeDestroyed follow the QPlatformWindow (QWaylandWindow) lifetime and not the lifetime of wl_surface. Some users were depending on those events to get notified about wl_surface changes and used QPlatformNativeInterface in order to get the window's underlying wl_surfaces in responses to the events. The good news is that QPlatformNativeInterface is private (QPA) API, so we can provide an alternative by exposing new private API, which is what this patch does. The QWaylandWindow::wlSurfaceDestroyed signal already exists in the dev branch (introduced in 04be9cf380), so this is a backport of that signal as well as an addition of a new QWaylandWindow::wlSurfaceCreated signal. Task-number: QTBUG-76324 Change-Id: Ibc5748474cd52f5b9461fd1ad6cef973491174b1 Reviewed-by: Pier Luigi Fiorini --- src/plugins/platforms/wayland/qwaylandwindow.cpp | 6 +++++- src/plugins/platforms/wayland/qwaylandwindow_p.h | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 76d7715a852..abc54f58438 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -210,7 +210,9 @@ void QWaylandWindow::initWindow() void QWaylandWindow::initializeWlSurface() { + Q_ASSERT(!isInitialized()); init(mDisplay->createSurface(static_cast(this))); + emit wlSurfaceCreated(); } bool QWaylandWindow::shouldCreateShellSurface() const @@ -245,8 +247,10 @@ void QWaylandWindow::reset(bool sendDestroyEvent) mShellSurface = nullptr; delete mSubSurfaceWindow; mSubSurfaceWindow = nullptr; - if (isInitialized()) + if (isInitialized()) { + emit wlSurfaceDestroyed(); destroy(); + } mScreens.clear(); if (mFrameCallback) { diff --git a/src/plugins/platforms/wayland/qwaylandwindow_p.h b/src/plugins/platforms/wayland/qwaylandwindow_p.h index 23432e39873..ec9bd7d0288 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow_p.h +++ b/src/plugins/platforms/wayland/qwaylandwindow_p.h @@ -199,6 +199,10 @@ public: public slots: void applyConfigure(); +signals: + void wlSurfaceCreated(); + void wlSurfaceDestroyed(); + protected: void surface_enter(struct ::wl_output *output) override; void surface_leave(struct ::wl_output *output) override; From 8b50d2c6cc99e6da91ac50477ae8191e29b35f8c Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Mon, 8 Jul 2019 10:22:03 +0200 Subject: [PATCH 5/9] Fix race condition for client buffer integration initialization This race happened because QWaylandIntegration::clientBufferIntegration (which lazily initializes the integration) was called from numerous places including code that may run on different threads without any kind of syncrhonization. An example of this is Qt3D, which indirectly and simultaneously calls createPlatformWindow (from the GUI thread) and createPlatformOpenGLContext (from its render thread), both of which needs to use the client buffer integration. In this patch, we fix it by first checking if the integration is initialized. In that case, it's safe to use it. Otherwise we lock a mutex, re-check if initialization has happened on another thread in the meantime, and then finally we initialize it. This way we should avoid the expense of mutex locking after initialization is complete, while still staying race free during initialization. Fixes: QTBUG-76504 Change-Id: I327840ebf41e014882cb659bc3e4fafb7bdb7a98 Reviewed-by: David Edmundson Reviewed-by: Paul Olav Tvete --- .../platforms/wayland/qwaylandintegration.cpp | 31 ++++++++++++------- .../platforms/wayland/qwaylandintegration_p.h | 2 ++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandintegration.cpp b/src/plugins/platforms/wayland/qwaylandintegration.cpp index 97e0203cdc9..46bef2944fe 100644 --- a/src/plugins/platforms/wayland/qwaylandintegration.cpp +++ b/src/plugins/platforms/wayland/qwaylandintegration.cpp @@ -301,11 +301,14 @@ QPlatformTheme *QWaylandIntegration::createPlatformTheme(const QString &name) co return GenericWaylandTheme::createUnixTheme(name); } +// May be called from non-GUI threads QWaylandClientBufferIntegration *QWaylandIntegration::clientBufferIntegration() const { - if (!mClientBufferIntegrationInitialized) + // Do an inexpensive check first to avoid locking whenever possible + if (Q_UNLIKELY(!mClientBufferIntegrationInitialized)) const_cast(this)->initializeClientBufferIntegration(); + Q_ASSERT(mClientBufferIntegrationInitialized); return mClientBufferIntegration && mClientBufferIntegration->isValid() ? mClientBufferIntegration.data() : nullptr; } @@ -325,9 +328,12 @@ QWaylandShellIntegration *QWaylandIntegration::shellIntegration() const return mShellIntegration.data(); } +// May be called from non-GUI threads void QWaylandIntegration::initializeClientBufferIntegration() { - mClientBufferIntegrationInitialized = true; + QMutexLocker lock(&mClientBufferInitLock); + if (mClientBufferIntegrationInitialized) + return; QString targetKey; bool disableHardwareIntegration = qEnvironmentVariableIsSet("QT_WAYLAND_DISABLE_HW_INTEGRATION"); @@ -345,17 +351,20 @@ void QWaylandIntegration::initializeClientBufferIntegration() if (targetKey.isEmpty()) { qWarning("Failed to determine what client buffer integration to use"); - return; + } else { + QStringList keys = QWaylandClientBufferIntegrationFactory::keys(); + if (keys.contains(targetKey)) { + mClientBufferIntegration.reset(QWaylandClientBufferIntegrationFactory::create(targetKey, QStringList())); + } + if (mClientBufferIntegration) + mClientBufferIntegration->initialize(mDisplay.data()); + else + qWarning("Failed to load client buffer integration: %s\n", qPrintable(targetKey)); } - QStringList keys = QWaylandClientBufferIntegrationFactory::keys(); - if (keys.contains(targetKey)) { - mClientBufferIntegration.reset(QWaylandClientBufferIntegrationFactory::create(targetKey, QStringList())); - } - if (mClientBufferIntegration) - mClientBufferIntegration->initialize(mDisplay.data()); - else - qWarning("Failed to load client buffer integration: %s\n", qPrintable(targetKey)); + // This must be set last to make sure other threads don't use the + // integration before initialization is complete. + mClientBufferIntegrationInitialized = true; } void QWaylandIntegration::initializeServerBufferIntegration() diff --git a/src/plugins/platforms/wayland/qwaylandintegration_p.h b/src/plugins/platforms/wayland/qwaylandintegration_p.h index a5a3d7b6956..7c1cb978abe 100644 --- a/src/plugins/platforms/wayland/qwaylandintegration_p.h +++ b/src/plugins/platforms/wayland/qwaylandintegration_p.h @@ -54,6 +54,7 @@ #include #include #include +#include QT_BEGIN_NAMESPACE @@ -148,6 +149,7 @@ private: QScopedPointer mAccessibility; #endif bool mFailed = false; + QMutex mClientBufferInitLock; bool mClientBufferIntegrationInitialized = false; bool mServerBufferIntegrationInitialized = false; bool mShellIntegrationInitialized = false; From f168c11529709a4fc3c3ec180bbc5c2e47960b34 Mon Sep 17 00:00:00 2001 From: Andy Shaw Date: Tue, 9 Jul 2019 13:15:20 +0200 Subject: [PATCH 6/9] Handle Key_Return explicitly instead of depending on the text When Key_Return is sent from Qt VirtualKeyboard it will send it as \n and not \r which means it will be interpreted as an unknown key. So since we know it will be able to map it in this case, we explicitly account for it so it can be mapped to the right key. Change-Id: Id5d8d9653e78975203f80790b7a3d332f0e011fa Reviewed-by: Johan Helsing --- src/plugins/platforms/wayland/shared/qwaylandxkb.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/platforms/wayland/shared/qwaylandxkb.cpp b/src/plugins/platforms/wayland/shared/qwaylandxkb.cpp index 3cfc4b074e5..2dff8a5b16d 100644 --- a/src/plugins/platforms/wayland/shared/qwaylandxkb.cpp +++ b/src/plugins/platforms/wayland/shared/qwaylandxkb.cpp @@ -376,7 +376,7 @@ QVector QWaylandXkb::toKeysym(QKeyEvent *event) keysyms.append(XKB_KEY_KP_0 + (event->key() - Qt::Key_0)); else keysyms.append(toKeysymFromTable(event->key())); - } else if (!event->text().isEmpty()) { + } else if (!event->text().isEmpty() && event->key() != Qt::Key_Return) { // From libxkbcommon keysym-utf.c: // "We allow to represent any UCS character in the range U-00000000 to // U-00FFFFFF by a keysym value in the range 0x01000000 to 0x01ffffff." From d87b6d823153d855fe4b052ccd503eeb6cdbcd8d Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Tue, 20 Aug 2019 13:38:12 +0200 Subject: [PATCH 7/9] Client: Crash instead of exit when there's a wayland error Qt applications should not call exit. Task-number: QTBUG-75779 Change-Id: I91190b10f8c8e111996cd73283061e6ceaa6b1f6 Reviewed-by: Pier Luigi Fiorini --- .../platforms/wayland/qwaylanddisplay.cpp | 22 +++++-------------- .../platforms/wayland/qwaylanddisplay_p.h | 1 - 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylanddisplay.cpp b/src/plugins/platforms/wayland/qwaylanddisplay.cpp index 82003a308c6..47b107e807f 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay.cpp +++ b/src/plugins/platforms/wayland/qwaylanddisplay.cpp @@ -172,9 +172,9 @@ void QWaylandDisplay::checkError() const int ecode = wl_display_get_error(mDisplay); if ((ecode == EPIPE || ecode == ECONNRESET)) { // special case this to provide a nicer error - qWarning("The Wayland connection broke. Did the Wayland compositor die?"); + qFatal("The Wayland connection broke. Did the Wayland compositor die?"); } else { - qErrnoWarning(ecode, "The Wayland connection experienced a fatal error"); + qFatal("The Wayland connection experienced a fatal error: %s", strerror(ecode)); } } @@ -184,25 +184,16 @@ void QWaylandDisplay::flushRequests() wl_display_read_events(mDisplay); } - if (wl_display_dispatch_pending(mDisplay) < 0) { + if (wl_display_dispatch_pending(mDisplay) < 0) checkError(); - exitWithError(); - } wl_display_flush(mDisplay); } void QWaylandDisplay::blockingReadEvents() { - if (wl_display_dispatch(mDisplay) < 0) { + if (wl_display_dispatch(mDisplay) < 0) checkError(); - exitWithError(); - } -} - -void QWaylandDisplay::exitWithError() -{ - ::exit(1); } wl_event_queue *QWaylandDisplay::createEventQueue() @@ -231,10 +222,9 @@ void QWaylandDisplay::dispatchQueueWhile(wl_event_queue *queue, std::function Date: Wed, 28 Aug 2019 11:13:55 +0200 Subject: [PATCH 8/9] Client tests: Set XDG_CURRENT_DESKTOP to avoid platform themes If the client tests are run from within a desktop environment, the XDG_CURRENT_DESKTOP variable may be set, causing the client to try to load a platform theme for the user's desktop. The tests, however, are running against a mock Wayland compositor, so launching a platform theme that expects to connect to a certain compositor implementation is probably not a good idea. And furthermore, if the gtk3 platform theme is used, it will start binding to interfaces and create wayland objects, confusing our existing test code. Setting XDG_CURRENT_DESKTOP to qtwaylandtests will prevent detection of the (outer) desktop environment, making tests more predictable. Change-Id: I57fa76e51cecdd0cbe8be6bd075ce67e9d2892bc Reviewed-by: David Edmundson Reviewed-by: Paul Olav Tvete --- tests/auto/wayland/shared/mockcompositor.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/auto/wayland/shared/mockcompositor.h b/tests/auto/wayland/shared/mockcompositor.h index 75ef1eaea1d..05bf32c8d50 100644 --- a/tests/auto/wayland/shared/mockcompositor.h +++ b/tests/auto/wayland/shared/mockcompositor.h @@ -78,6 +78,7 @@ public: int main(int argc, char **argv) \ { \ setenv("XDG_RUNTIME_DIR", ".", 1); \ + setenv("XDG_CURRENT_DESKTOP", "qtwaylandtests", 1); \ setenv("QT_QPA_PLATFORM", "wayland", 1); \ test tc; \ QGuiApplication app(argc, argv); \ From f199375e5cee3e115aa90f3bdf4105d94cc25c13 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Mon, 2 Sep 2019 10:45:20 +0200 Subject: [PATCH 9/9] Client: Fix crash on wl_pointer.up after destroying a window When wl_pointer version 5 was implemented, we added a raw QWaylandWindow pointer in QWaylandPointerEvent. This is a problem, because the events are stored, and the window may be deleted in the meantime. This manifested itself as flakiness in tst_xdgshell::popup() which is now fixed. Fixes: QTBUG-77976 Change-Id: If34eee0286d5a63734535d67503378516d5768c3 Reviewed-by: Paul Olav Tvete --- src/plugins/platforms/wayland/qwaylandinputdevice.cpp | 11 +++++++++-- src/plugins/platforms/wayland/qwaylandinputdevice_p.h | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp index 8aa0239d079..f2dee75e63c 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp +++ b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp @@ -1045,8 +1045,15 @@ void QWaylandInputDevice::Pointer::flushScrollEvent() void QWaylandInputDevice::Pointer::flushFrameEvent() { - if (mFrameData.event) { - mFrameData.event->surface->handleMouse(mParent, *mFrameData.event); + if (auto *event = mFrameData.event) { + if (auto window = event->surface) { + window->handleMouse(mParent, *event); + } else if (mFrameData.event->type == QWaylandPointerEvent::Type::Release) { + // If the window has been destroyed, we still need to report an up event, but it can't + // be handled by the destroyed window (obviously), so send the event here instead. + QWindowSystemInterface::handleMouseEvent(nullptr, event->timestamp, event->local, + event->global, event->buttons, event->modifiers); + } delete mFrameData.event; mFrameData.event = nullptr; } diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice_p.h b/src/plugins/platforms/wayland/qwaylandinputdevice_p.h index 4ac1dca3525..7fbb5667f70 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice_p.h +++ b/src/plugins/platforms/wayland/qwaylandinputdevice_p.h @@ -446,7 +446,7 @@ public: QPoint pixelDelta; QPoint angleDelta; Qt::MouseEventSource source = Qt::MouseEventNotSynthesized; - QWaylandWindow *surface = nullptr; + QPointer surface; }; }