From face687a6204a8f41b0bbe26de1e42c9dc7cb7e0 Mon Sep 17 00:00:00 2001 From: Giulio Camuffo Date: Thu, 29 Oct 2015 16:35:27 +0200 Subject: [PATCH 1/6] Fix deadlock when starting a drag With commit c55a36cb9015cf1eebd49eaa5b1b4f4ec9b28451 in qtbase the QSimpleDrag code changed in a way that caused a deadlock due to drawing a window without a role. However, thanks to that we can now remove that start/cancel hack and simplify the code. Change-Id: Icba6e7c9c4927855e48fb21632db1a10332c4ffb Reviewed-by: Laszlo Agocs --- src/plugins/platforms/wayland/qwaylanddnd.cpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylanddnd.cpp b/src/plugins/platforms/wayland/qwaylanddnd.cpp index 59f91411f03..e195d193a41 100644 --- a/src/plugins/platforms/wayland/qwaylanddnd.cpp +++ b/src/plugins/platforms/wayland/qwaylanddnd.cpp @@ -65,21 +65,9 @@ QMimeData * QWaylandDrag::platformDropData() void QWaylandDrag::startDrag() { - bool cancel = false; - if (!shapedPixmapWindow()) { - QBasicDrag::startDrag(); - // Don't call cancel() here, since that will hide 'shapedPixmapWindow()', and - // QWaylandWindow::setVisible(false) will flush the window system queue, - // ending up trying to render the window, which doesn't have a role yet, - // and so blocking waiting for a frame callback. - cancel = true; - } - + QBasicDrag::startDrag(); QWaylandWindow *icon = static_cast(shapedPixmapWindow()->handle()); m_display->currentInputDevice()->dataDevice()->startDrag(drag()->mimeData(), icon); - if (cancel) - QBasicDrag::cancel(); - QBasicDrag::startDrag(); } void QWaylandDrag::cancel() From eace7fb3ad7417fb285cc8c6672856b419f6e46a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Wed, 28 Oct 2015 13:14:35 +0100 Subject: [PATCH 2/6] Add wl_shell_surface as nativeResourceForWindow Allows to get the wl_shell_surface for a QWindow, if it exists. Change-Id: I16b1c578a1c605e58c96e94ae55a3331ecfa353d Reviewed-by: Giulio Camuffo --- .../platforms/wayland/qwaylandnativeinterface.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/plugins/platforms/wayland/qwaylandnativeinterface.cpp b/src/plugins/platforms/wayland/qwaylandnativeinterface.cpp index 0d20075f05d..98e1a73668b 100644 --- a/src/plugins/platforms/wayland/qwaylandnativeinterface.cpp +++ b/src/plugins/platforms/wayland/qwaylandnativeinterface.cpp @@ -39,6 +39,7 @@ #include "qwaylanddisplay_p.h" #include "qwaylandwindowmanagerintegration_p.h" #include "qwaylandscreen_p.h" +#include "qwaylandwlshellsurface_p.h" #include #include #include @@ -80,6 +81,15 @@ void *QWaylandNativeInterface::nativeResourceForWindow(const QByteArray &resourc if (lowerCaseResource == "surface") { return ((QWaylandWindow *) window->handle())->object(); } + if (lowerCaseResource == "wl_shell_surface") { + QWaylandWindow *w = (QWaylandWindow *) window->handle(); + if (!w) + return NULL; + QWaylandWlShellSurface *s = qobject_cast(w->shellSurface()); + if (!s) + return NULL; + return s->object(); + } if (lowerCaseResource == "egldisplay" && m_integration->clientBufferIntegration()) return m_integration->clientBufferIntegration()->nativeResource(QWaylandClientBufferIntegration::EglDisplay); From 0520481077abde68f5b02a53f88982d7abb8482f Mon Sep 17 00:00:00 2001 From: Kevin Funk Date: Mon, 2 Nov 2015 10:43:06 +0100 Subject: [PATCH 3/6] CMake: Create CMake files for WaylandClient Also add unit tests Change-Id: I66de887607f73b318884e4a35f18510b90cf0315 Task-number: QTBUG-47357 Reviewed-by: Jan Arne Petersen Reviewed-by: Giulio Camuffo --- src/plugins/platforms/wayland/client.pro | 1 - tests/auto/cmake/test_waylandclient/CMakeLists.txt | 12 ++++++++++++ tests/auto/cmake/test_waylandclient/main.cpp | 7 +++++++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/auto/cmake/test_waylandclient/CMakeLists.txt create mode 100644 tests/auto/cmake/test_waylandclient/main.cpp diff --git a/src/plugins/platforms/wayland/client.pro b/src/plugins/platforms/wayland/client.pro index ba17b21c57c..7182f3203d8 100644 --- a/src/plugins/platforms/wayland/client.pro +++ b/src/plugins/platforms/wayland/client.pro @@ -16,7 +16,6 @@ load(qt_module) QMAKE_CXXFLAGS_WARN_ON -= -Wcast-qual CONFIG -= precompile_header -CONFIG -= create_cmake CONFIG += link_pkgconfig qpa/genericunixfontdatabase wayland-scanner !equals(QT_WAYLAND_GL_CONFIG, nogl) { diff --git a/tests/auto/cmake/test_waylandclient/CMakeLists.txt b/tests/auto/cmake/test_waylandclient/CMakeLists.txt new file mode 100644 index 00000000000..3788a4927bd --- /dev/null +++ b/tests/auto/cmake/test_waylandclient/CMakeLists.txt @@ -0,0 +1,12 @@ +project(test_plugins) + +cmake_minimum_required(VERSION 2.8) +cmake_policy(SET CMP0056 NEW) + +find_package(Qt5WaylandClient REQUIRED) + +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${Qt5Core_EXECUTABLE_COMPILE_FLAGS}") + +include_directories(${Qt5WaylandClient_PRIVATE_INCLUDE_DIRS}) +add_executable(test_waylandclient_exe main.cpp) +target_link_libraries(test_waylandclient_exe Qt5::WaylandClient) diff --git a/tests/auto/cmake/test_waylandclient/main.cpp b/tests/auto/cmake/test_waylandclient/main.cpp new file mode 100644 index 00000000000..f0ccdef4b76 --- /dev/null +++ b/tests/auto/cmake/test_waylandclient/main.cpp @@ -0,0 +1,7 @@ +#include + +int main() +{ + // use symbol + QtWaylandClient::QWaylandCursor cursor(Q_NULLPTR); +} From d49cd790d8525071f6613066caf5e8dd2f5ca980 Mon Sep 17 00:00:00 2001 From: Giulio Camuffo Date: Fri, 23 Oct 2015 13:29:41 +0300 Subject: [PATCH 4/6] client: Remove the event thread If the compositor sends events to us while the main thread is blocked the socket notifier in the events thread would keep sending out the activated() signal, but no events would actually be read until the main thread starts to run again. That causes the event thread to keep queueing new events, and so allocating memory, potentially forever. This patch fixes the issue in maybe a bit radical way, that is by removing the event thread. The socket notifier now runs in the main thread so it will block if the events are not being read. Nowadays there is no real reason to keep the event thread around, as every thread that needs to receive wayland events can dispatch them on its own, we don't need a central dispatcher thread anymore. Change-Id: Ib7885e4b038b82719d78d193f465618a72cbe6af Reviewed-by: Pier Luigi Fiorini --- src/plugins/platforms/wayland/client.pro | 2 - .../platforms/wayland/qwaylanddisplay.cpp | 42 ++++--- .../platforms/wayland/qwaylanddisplay_p.h | 3 +- .../platforms/wayland/qwaylandeventthread.cpp | 112 ------------------ .../platforms/wayland/qwaylandeventthread_p.h | 97 --------------- .../platforms/wayland/qwaylandintegration.cpp | 5 + 6 files changed, 29 insertions(+), 232 deletions(-) delete mode 100644 src/plugins/platforms/wayland/qwaylandeventthread.cpp delete mode 100644 src/plugins/platforms/wayland/qwaylandeventthread_p.h diff --git a/src/plugins/platforms/wayland/client.pro b/src/plugins/platforms/wayland/client.pro index 7182f3203d8..8bd9e1dc83a 100644 --- a/src/plugins/platforms/wayland/client.pro +++ b/src/plugins/platforms/wayland/client.pro @@ -78,7 +78,6 @@ SOURCES += qwaylandintegration.cpp \ qwaylandabstractdecoration.cpp \ qwaylanddecorationfactory.cpp \ qwaylanddecorationplugin.cpp \ - qwaylandeventthread.cpp\ qwaylandwindowmanagerintegration.cpp \ qwaylandinputcontext.cpp \ qwaylanddatadevice.cpp \ @@ -111,7 +110,6 @@ HEADERS += qwaylandintegration_p.h \ qwaylandabstractdecoration_p.h \ qwaylanddecorationfactory_p.h \ qwaylanddecorationplugin_p.h \ - qwaylandeventthread_p.h \ qwaylandwindowmanagerintegration_p.h \ qwaylandinputcontext_p.h \ qwaylanddatadevice_p.h \ diff --git a/src/plugins/platforms/wayland/qwaylanddisplay.cpp b/src/plugins/platforms/wayland/qwaylanddisplay.cpp index dbb8e12dd07..a6f52bc2432 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay.cpp +++ b/src/plugins/platforms/wayland/qwaylanddisplay.cpp @@ -33,7 +33,6 @@ #include "qwaylanddisplay_p.h" -#include "qwaylandeventthread_p.h" #include "qwaylandintegration_p.h" #include "qwaylandwindow_p.h" #include "qwaylandscreen_p.h" @@ -145,21 +144,15 @@ QWaylandDisplay::QWaylandDisplay(QWaylandIntegration *waylandIntegration) { qRegisterMetaType("uint32_t"); - mEventThreadObject = new QWaylandEventThread(0); - mEventThread = new QThread(this); - mEventThread->setObjectName(QStringLiteral("QtWayland")); - mEventThreadObject->moveToThread(mEventThread); - mEventThread->start(); - - mEventThreadObject->displayConnect(); - mDisplay = mEventThreadObject->display(); //blocks until display is available + mDisplay = wl_display_connect(NULL); + if (mDisplay == NULL) { + qErrnoWarning(errno, "Failed to create display"); + ::exit(1); + } struct ::wl_registry *registry = wl_display_get_registry(mDisplay); init(registry); - connect(mEventThreadObject, SIGNAL(newEventsRead()), this, SLOT(flushRequests())); - connect(mEventThreadObject, &QWaylandEventThread::fatalError, this, &QWaylandDisplay::exitWithError); - mWindowManagerIntegration.reset(new QWaylandWindowManagerIntegration(this)); forceRoundTrip(); @@ -175,15 +168,28 @@ QWaylandDisplay::~QWaylandDisplay(void) } mScreens.clear(); delete mDndSelectionHandler.take(); - mEventThread->quit(); - mEventThread->wait(); - delete mEventThreadObject; + wl_display_disconnect(mDisplay); +} + +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?"); + } else { + qErrnoWarning(ecode, "The Wayland connection experienced a fatal error"); + } } void QWaylandDisplay::flushRequests() { + if (wl_display_prepare_read(mDisplay) == 0) { + wl_display_read_events(mDisplay); + } + if (wl_display_dispatch_pending(mDisplay) < 0) { - mEventThreadObject->checkError(); + checkError(); exitWithError(); } @@ -194,15 +200,13 @@ void QWaylandDisplay::flushRequests() void QWaylandDisplay::blockingReadEvents() { if (wl_display_dispatch(mDisplay) < 0) { - mEventThreadObject->checkError(); + checkError(); exitWithError(); } } void QWaylandDisplay::exitWithError() { - mEventThread->quit(); - mEventThread->wait(); ::exit(1); } diff --git a/src/plugins/platforms/wayland/qwaylanddisplay_p.h b/src/plugins/platforms/wayland/qwaylanddisplay_p.h index bf48c212304..8262e72be46 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay_p.h +++ b/src/plugins/platforms/wayland/qwaylanddisplay_p.h @@ -177,6 +177,7 @@ public slots: private: void waitForScreens(); void exitWithError(); + void checkError() const; struct Listener { RegistryListener listener; @@ -186,8 +187,6 @@ private: struct wl_display *mDisplay; QtWayland::wl_compositor mCompositor; struct wl_shm *mShm; - QThread *mEventThread; - QWaylandEventThread *mEventThreadObject; QScopedPointer mShell; QScopedPointer mShellXdg; QList mScreens; diff --git a/src/plugins/platforms/wayland/qwaylandeventthread.cpp b/src/plugins/platforms/wayland/qwaylandeventthread.cpp deleted file mode 100644 index e0a3edcb81f..00000000000 --- a/src/plugins/platforms/wayland/qwaylandeventthread.cpp +++ /dev/null @@ -1,112 +0,0 @@ -/**************************************************************************** -** -** Copyright (C) 2015 The Qt Company Ltd. -** Contact: http://www.qt.io/licensing/ -** -** This file is part of the plugins of the Qt Toolkit. -** -** $QT_BEGIN_LICENSE:LGPL21$ -** Commercial License Usage -** Licensees holding valid commercial Qt licenses may use this file in -** accordance with the commercial license agreement provided with the -** Software or, alternatively, in accordance with the terms contained in -** a written agreement between you and The Qt Company. For licensing terms -** and conditions see http://www.qt.io/terms-conditions. For further -** information use the contact form at http://www.qt.io/contact-us. -** -** GNU Lesser General Public License Usage -** Alternatively, this file may be used under the terms of the GNU Lesser -** General Public License version 2.1 or version 3 as published by the Free -** Software Foundation and appearing in the file LICENSE.LGPLv21 and -** LICENSE.LGPLv3 included in the packaging of this file. Please review the -** following information to ensure the GNU Lesser General Public License -** requirements will be met: https://www.gnu.org/licenses/lgpl.html and -** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. -** -** As a special exception, The Qt Company gives you certain additional -** rights. These rights are described in The Qt Company LGPL Exception -** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. -** -** $QT_END_LICENSE$ -** -****************************************************************************/ - -#include "qwaylandeventthread_p.h" -#include -#include - -#include -#include -#include -#include - -QT_BEGIN_NAMESPACE - -namespace QtWaylandClient { - -QWaylandEventThread::QWaylandEventThread(QObject *parent) - : QObject(parent) - , m_display(0) - , m_fileDescriptor(-1) - , m_readNotifier(0) - , m_displayLock(new QMutex) -{ -} - -QWaylandEventThread::~QWaylandEventThread() -{ - delete m_displayLock; - wl_display_disconnect(m_display); -} - -void QWaylandEventThread::displayConnect() -{ - m_displayLock->lock(); - QMetaObject::invokeMethod(this, "waylandDisplayConnect", Qt::QueuedConnection); -} - -// ### be careful what you do, this function may also be called from other -// threads to clean up & exit. -void QWaylandEventThread::checkError() const -{ - int ecode = wl_display_get_error(m_display); - if ((ecode == EPIPE || ecode == ECONNRESET)) { - // special case this to provide a nicer error - qWarning("The Wayland connection broke. Did the Wayland compositor die?"); - } else { - qErrnoWarning(ecode, "The Wayland connection experienced a fatal error"); - } -} - -void QWaylandEventThread::readWaylandEvents() -{ - if (wl_display_prepare_read(m_display) == 0) { - wl_display_read_events(m_display); - } - emit newEventsRead(); -} - -void QWaylandEventThread::waylandDisplayConnect() -{ - m_display = wl_display_connect(NULL); - if (m_display == NULL) { - qErrnoWarning(errno, "Failed to create display"); - ::exit(1); - } - m_displayLock->unlock(); - - m_fileDescriptor = wl_display_get_fd(m_display); - - m_readNotifier = new QSocketNotifier(m_fileDescriptor, QSocketNotifier::Read, this); - connect(m_readNotifier, SIGNAL(activated(int)), this, SLOT(readWaylandEvents())); -} - -wl_display *QWaylandEventThread::display() const -{ - QMutexLocker displayLock(m_displayLock); - return m_display; -} - -} - -QT_END_NAMESPACE diff --git a/src/plugins/platforms/wayland/qwaylandeventthread_p.h b/src/plugins/platforms/wayland/qwaylandeventthread_p.h deleted file mode 100644 index 0920d403e42..00000000000 --- a/src/plugins/platforms/wayland/qwaylandeventthread_p.h +++ /dev/null @@ -1,97 +0,0 @@ -/**************************************************************************** -** -** Copyright (C) 2015 The Qt Company Ltd. -** Contact: http://www.qt.io/licensing/ -** -** This file is part of the plugins of the Qt Toolkit. -** -** $QT_BEGIN_LICENSE:LGPL21$ -** Commercial License Usage -** Licensees holding valid commercial Qt licenses may use this file in -** accordance with the commercial license agreement provided with the -** Software or, alternatively, in accordance with the terms contained in -** a written agreement between you and The Qt Company. For licensing terms -** and conditions see http://www.qt.io/terms-conditions. For further -** information use the contact form at http://www.qt.io/contact-us. -** -** GNU Lesser General Public License Usage -** Alternatively, this file may be used under the terms of the GNU Lesser -** General Public License version 2.1 or version 3 as published by the Free -** Software Foundation and appearing in the file LICENSE.LGPLv21 and -** LICENSE.LGPLv3 included in the packaging of this file. Please review the -** following information to ensure the GNU Lesser General Public License -** requirements will be met: https://www.gnu.org/licenses/lgpl.html and -** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. -** -** As a special exception, The Qt Company gives you certain additional -** rights. These rights are described in The Qt Company LGPL Exception -** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. -** -** $QT_END_LICENSE$ -** -****************************************************************************/ - -#ifndef QWAYLANDEVENTTHREAD_H -#define QWAYLANDEVENTTHREAD_H - -// -// W A R N I N G -// ------------- -// -// This file is not part of the Qt API. It exists purely as an -// implementation detail. This header file may change from version to -// version without notice, or even be removed. -// -// We mean it. -// - -#include -#include -#include - -#include - -QT_BEGIN_NAMESPACE - -class QSocketNotifier; - -namespace QtWaylandClient { - -class Q_WAYLAND_CLIENT_EXPORT QWaylandEventThread : public QObject -{ - Q_OBJECT -public: - explicit QWaylandEventThread(QObject *parent = 0); - ~QWaylandEventThread(); - - void displayConnect(); - - wl_display *display() const; - - void checkError() const; - -private slots: - void readWaylandEvents(); - - void waylandDisplayConnect(); - -signals: - void newEventsRead(); - void fatalError(); - -private: - - struct wl_display *m_display; - int m_fileDescriptor; - - QSocketNotifier *m_readNotifier; - - QMutex *m_displayLock; - -}; - -} - -QT_END_NAMESPACE - -#endif // QWAYLANDEVENTTHREAD_H diff --git a/src/plugins/platforms/wayland/qwaylandintegration.cpp b/src/plugins/platforms/wayland/qwaylandintegration.cpp index 82df8a30a41..39fff533d32 100644 --- a/src/plugins/platforms/wayland/qwaylandintegration.cpp +++ b/src/plugins/platforms/wayland/qwaylandintegration.cpp @@ -54,6 +54,7 @@ #include #include #include +#include #include #include @@ -201,6 +202,10 @@ void QWaylandIntegration::initialize() QAbstractEventDispatcher *dispatcher = QGuiApplicationPrivate::eventDispatcher; QObject::connect(dispatcher, SIGNAL(aboutToBlock()), mDisplay, SLOT(flushRequests())); QObject::connect(dispatcher, SIGNAL(awake()), mDisplay, SLOT(flushRequests())); + + int fd = wl_display_get_fd(mDisplay->wl_display()); + QSocketNotifier *sn = new QSocketNotifier(fd, QSocketNotifier::Read, mDisplay); + QObject::connect(sn, SIGNAL(activated(int)), mDisplay, SLOT(flushRequests())); } QPlatformFontDatabase *QWaylandIntegration::fontDatabase() const From 8c457f7183d56c41cfb3789b88dcc9b2673045d7 Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Thu, 5 Nov 2015 13:51:25 +0100 Subject: [PATCH 5/6] Fix compiler warnings. qwaylandinputcontext.cpp:57:16: warning: unused parameter 'sym' [-Wunused-parameter] qwaylanddatadevice.cpp:173:135: warning: 'dragData' may be used uninitialized in this function [-Wmaybe-uninitialized] Change-Id: Id654360fd9b7fdb572565ad39b664af3355b5e79 Reviewed-by: Giulio Camuffo --- src/plugins/platforms/wayland/qwaylanddatadevice.cpp | 2 +- src/plugins/platforms/wayland/qwaylandinputcontext.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/platforms/wayland/qwaylanddatadevice.cpp b/src/plugins/platforms/wayland/qwaylanddatadevice.cpp index a3e084a0c10..3276348e9be 100644 --- a/src/plugins/platforms/wayland/qwaylanddatadevice.cpp +++ b/src/plugins/platforms/wayland/qwaylanddatadevice.cpp @@ -157,7 +157,7 @@ void QWaylandDataDevice::data_device_enter(uint32_t serial, wl_surface *surface, QDrag *drag = static_cast(QGuiApplicationPrivate::platformIntegration()->drag())->currentDrag(); - QMimeData *dragData; + QMimeData *dragData = Q_NULLPTR; Qt::DropActions supportedActions; if (drag) { dragData = drag->mimeData(); diff --git a/src/plugins/platforms/wayland/qwaylandinputcontext.cpp b/src/plugins/platforms/wayland/qwaylandinputcontext.cpp index 119a1528787..6a729509525 100644 --- a/src/plugins/platforms/wayland/qwaylandinputcontext.cpp +++ b/src/plugins/platforms/wayland/qwaylandinputcontext.cpp @@ -74,6 +74,7 @@ static Qt::Key toQtKey(uint32_t sym) return Qt::Key_unknown; } #else + Q_UNUSED(sym) return Qt::Key_unknown; #endif } From 5a4487e16b398ca805d7f8ad5e45e40aa86f6b1a Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Sun, 8 Nov 2015 00:18:21 +0100 Subject: [PATCH 6/6] Get rid of the egl config test and use what qtbase provides contains(QT_CONFIG, egl) and CONFIG += egl is the only sane way to test for and pull in EGL headers and libs. This is particularly important when trying to be robust and guard against half-broken sysroots on embedded where a naive PKGCONFIG += egl breaks. Also add an EGL_WAYLAND_BUFFER_WL define to keep wayland-egl compiling. We are not testing for that in any config tests may cause a failure in sysroots that have parts of Mesa thrown in but pick ip an older EGL header from the vendor's driver. Change-Id: I7b7e6a7a91e78dbda5b6954ad08761298c538efc Reviewed-by: Giulio Camuffo --- .../platforms/wayland/plugins/hardwareintegration/client.pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/platforms/wayland/plugins/hardwareintegration/client.pro b/src/plugins/platforms/wayland/plugins/hardwareintegration/client.pro index b7a9b46ee1d..37a90ab0d44 100644 --- a/src/plugins/platforms/wayland/plugins/hardwareintegration/client.pro +++ b/src/plugins/platforms/wayland/plugins/hardwareintegration/client.pro @@ -7,7 +7,7 @@ config_brcm_egl: \ SUBDIRS += brcm-egl config_xcomposite { - config_egl: \ + contains(QT_CONFIG, egl): \ SUBDIRS += xcomposite-egl !contains(QT_CONFIG, opengles2):config_glx: \