From 37a03693f22cba7a17a5e7bdc3c340e7646aa6ef Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Wed, 26 Jun 2019 09:47:03 +0200 Subject: 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 eb66211ea9), 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/client/qwaylandwindow.cpp | 6 +++++- src/client/qwaylandwindow_p.h | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp index 76d7715a8..abc54f584 100644 --- a/src/client/qwaylandwindow.cpp +++ b/src/client/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/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h index 23432e398..ec9bd7d02 100644 --- a/src/client/qwaylandwindow_p.h +++ b/src/client/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; -- cgit v1.2.3 From 28c852df3825be05bea9e3d17475c73c409ee534 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Mon, 8 Jul 2019 10:22:03 +0200 Subject: 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 --- src/client/qwaylandintegration.cpp | 31 ++++++++++++++++++++----------- src/client/qwaylandintegration_p.h | 2 ++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/client/qwaylandintegration.cpp b/src/client/qwaylandintegration.cpp index 97e0203cd..46bef2944 100644 --- a/src/client/qwaylandintegration.cpp +++ b/src/client/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/client/qwaylandintegration_p.h b/src/client/qwaylandintegration_p.h index a5a3d7b69..7c1cb978a 100644 --- a/src/client/qwaylandintegration_p.h +++ b/src/client/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; -- cgit v1.2.3 From 70473e7d3b7820ba171ee67bc0098906a2a59fe4 Mon Sep 17 00:00:00 2001 From: Andy Shaw Date: Tue, 9 Jul 2019 13:15:20 +0200 Subject: 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/shared/qwaylandxkb.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/qwaylandxkb.cpp b/src/shared/qwaylandxkb.cpp index 3cfc4b074..2dff8a5b1 100644 --- a/src/shared/qwaylandxkb.cpp +++ b/src/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." -- cgit v1.2.3 From f5a28afe4c2cb82540c94616e7a9e3e72e0e8327 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Tue, 20 Aug 2019 13:38:12 +0200 Subject: 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 --- src/client/qwaylanddisplay.cpp | 22 ++++++---------------- src/client/qwaylanddisplay_p.h | 1 - 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/client/qwaylanddisplay.cpp b/src/client/qwaylanddisplay.cpp index 82003a308..47b107e80 100644 --- a/src/client/qwaylanddisplay.cpp +++ b/src/client/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