From 794c81c1eab54cb58f2143ab3efc90e9c4bf9092 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Samuel=20R=C3=B8dal?= Date: Thu, 25 Aug 2011 13:39:07 +0200 Subject: [PATCH] Move XCB event reading to a separate thread. Work-around for bug in XCB which causes a xcb_wait_for_reply to block if xcb_poll_for_events() is called simultaneously from a different thread. If the XCB version is too old this work-around causes even more problems, so we kill two birds with one stone by only using the work-around if the XCB version has the recent xcb_poll_for_queue_event() function, which we also need to read events from a separate thread with reasonable efficiency. Change-Id: I8a899dad6ded381ce42cba0112e77da3c8aa6887 Reviewed-on: http://codereview.qt.nokia.com/3612 Reviewed-by: Lars Knoll --- .../xcb-poll-for-queued-event.cpp | 59 ++++++++++++ .../xcb-poll-for-queued-event.pro | 5 + configure | 12 ++- src/plugins/platforms/xcb/qxcbconnection.cpp | 105 ++++++++++++++++++--- src/plugins/platforms/xcb/qxcbconnection.h | 48 ++++++++-- src/plugins/platforms/xcb/xcb.pro | 3 + 6 files changed, 212 insertions(+), 20 deletions(-) create mode 100644 config.tests/qpa/xcb-poll-for-queued-event/xcb-poll-for-queued-event.cpp create mode 100644 config.tests/qpa/xcb-poll-for-queued-event/xcb-poll-for-queued-event.pro diff --git a/config.tests/qpa/xcb-poll-for-queued-event/xcb-poll-for-queued-event.cpp b/config.tests/qpa/xcb-poll-for-queued-event/xcb-poll-for-queued-event.cpp new file mode 100644 index 0000000..d15e554 --- /dev/null +++ b/config.tests/qpa/xcb-poll-for-queued-event/xcb-poll-for-queued-event.cpp @@ -0,0 +1,59 @@ +/**************************************************************************** +** +** Copyright (C) 2011 Nokia Corporation and/or its subsidiary(-ies). +** All rights reserved. +** Contact: Nokia Corporation (qt-info@nokia.com) +** +** This file is part of the config.tests of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** GNU Lesser General Public License Usage +** This file may be used under the terms of the GNU Lesser General Public +** License version 2.1 as published by the Free Software Foundation and +** appearing in the file LICENSE.LGPL included in the packaging of this +** file. Please review the following information to ensure the GNU Lesser +** General Public License version 2.1 requirements will be met: +** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Nokia gives you certain additional +** rights. These rights are described in the Nokia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU General +** Public License version 3.0 as published by the Free Software Foundation +** and appearing in the file LICENSE.GPL included in the packaging of this +** file. Please review the following information to ensure the GNU General +** Public License version 3.0 requirements will be met: +** http://www.gnu.org/copyleft/gpl.html. +** +** Other Usage +** Alternatively, this file may be used in accordance with the terms and +** conditions contained in a signed written agreement between you and Nokia. +** +** +** +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#include +#include +#include +#include +#include +#include +#include + +int main(int, char **) +{ + int primaryScreen = 0; + + xcb_connection_t *connection = xcb_connect("", &primaryScreen); + + xcb_generic_event_t *event = xcb_poll_for_queued_event(connection); + + return 0; +} diff --git a/config.tests/qpa/xcb-poll-for-queued-event/xcb-poll-for-queued-event.pro b/config.tests/qpa/xcb-poll-for-queued-event/xcb-poll-for-queued-event.pro new file mode 100644 index 0000000..6075c55 --- /dev/null +++ b/config.tests/qpa/xcb-poll-for-queued-event/xcb-poll-for-queued-event.pro @@ -0,0 +1,5 @@ +SOURCES = xcb-poll-for-queued-event.cpp +CONFIG -= qt + +LIBS += -lxcb -lxcb-image -lxcb-keysyms -lxcb-icccm -lxcb-sync -lxcb-xfixes + diff --git a/configure b/configure index 987242e..7f8491e 100755 --- a/configure +++ b/configure @@ -750,6 +750,7 @@ CFG_DECORATION_PLUGIN= CFG_XINPUT=runtime CFG_XKB=auto CFG_XCB=auto +CFG_XCB_LIMITED=yes CFG_NIS=auto CFG_CUPS=auto CFG_ICONV=auto @@ -6248,6 +6249,11 @@ if [ "$PLATFORM_QPA" = "yes" ]; then QT_CONFIG="$QT_CONFIG xcb-render" fi + if "$unixtests/compile.test" "$XQMAKESPEC" "$QMAKE_CONFIG" $OPT_VERBOSE "$relpath" "$outpath" config.tests/qpa/xcb-poll-for-queued-event "xcb-poll-for-queued-event" $L_FLAGS $I_FLAGS $l_FLAGS; then + CFG_XCB_LIMITED=no + QT_CONFIG="$QT_CONFIG xcb-poll-for-queued-event" + fi + if "$unixtests/compile.test" "$XQMAKESPEC" "$QMAKE_CONFIG" $OPT_VERBOSE "$relpath" "$outpath" config.tests/qpa/xcb-xlib "xcb-xlib" $L_FLAGS $I_FLAGS $l_FLAGS; then QT_CONFIG="$QT_CONFIG xcb-xlib" fi @@ -8760,7 +8766,11 @@ if [ "$PLATFORM_MAC" = "yes" ]; then fi echo "ICD support ............ $CFG_ICD" echo "libICU support ......... $CFG_ICU" -echo "Xcb support ............ $CFG_XCB" +if [ "$CFG_XCB_LIMITED" = "yes" ] && [ "$CFG_XCB" = "yes" ]; then + echo "Xcb support ............ limited (old version)" +else + echo "Xcb support ............ $CFG_XCB" +fi echo [ "$CFG_PTMALLOC" != "no" ] && echo "Use ptmalloc ........... $CFG_PTMALLOC" diff --git a/src/plugins/platforms/xcb/qxcbconnection.cpp b/src/plugins/platforms/xcb/qxcbconnection.cpp index 0727af4..a8ffc58 100644 --- a/src/plugins/platforms/xcb/qxcbconnection.cpp +++ b/src/plugins/platforms/xcb/qxcbconnection.cpp @@ -53,6 +53,7 @@ #include #include #include +#include #include #include @@ -114,12 +115,18 @@ QXcbConnection::QXcbConnection(const char *displayName) if (m_connection) printf("Successfully connected to display %s\n", m_displayName.constData()); + m_reader = new QXcbEventReader(m_connection); +#ifdef XCB_POLL_FOR_QUEUED_EVENT + connect(m_reader, SIGNAL(eventPending()), this, SLOT(processXcbEvents()), Qt::QueuedConnection); + m_reader->start(); +#else QSocketNotifier *notifier = new QSocketNotifier(xcb_get_file_descriptor(xcb_connection()), QSocketNotifier::Read, this); connect(notifier, SIGNAL(activated(int)), this, SLOT(processXcbEvents())); QAbstractEventDispatcher *dispatcher = QGuiApplicationPrivate::eventDispatcher; connect(dispatcher, SIGNAL(aboutToBlock()), this, SLOT(processXcbEvents())); connect(dispatcher, SIGNAL(awake()), this, SLOT(processXcbEvents())); +#endif xcb_prefetch_extension_data (m_connection, &xcb_xfixes_id); @@ -137,6 +144,12 @@ QXcbConnection::QXcbConnection(const char *displayName) xcb_screen_next(&it); } + m_connectionEventListener = xcb_generate_id(m_connection); + xcb_create_window(m_connection, XCB_COPY_FROM_PARENT, + m_connectionEventListener, m_screens.at(0)->root(), + 0, 0, 1, 1, 0, XCB_WINDOW_CLASS_INPUT_ONLY, + m_screens.at(0)->screen()->root_visual, 0, 0); + initializeXFixes(); initializeXRender(); @@ -157,6 +170,12 @@ QXcbConnection::~QXcbConnection() qDeleteAll(m_screens); +#ifdef XCB_POLL_FOR_QUEUED_EVENT + sendConnectionEvent(QXcbAtom::_QT_CLOSE_CONNECTION); + m_reader->wait(); +#endif + delete m_reader; + #ifdef XCB_USE_XLIB XCloseDisplay((Display *)m_xlib_display); #else @@ -531,16 +550,72 @@ void QXcbConnection::addPeekFunc(PeekFunc f) m_peekFuncs.append(f); } +#ifdef XCB_POLL_FOR_QUEUED_EVENT +void QXcbEventReader::run() +{ + xcb_generic_event_t *event; + while (m_connection && (event = xcb_wait_for_event(m_connection))) { + m_mutex.lock(); + addEvent(event); + while (m_connection && (event = xcb_poll_for_queued_event(m_connection))) + addEvent(event); + m_mutex.unlock(); + emit eventPending(); + } + + for (int i = 0; i < m_events.size(); ++i) + free(m_events.at(i)); +} +#endif + +void QXcbEventReader::addEvent(xcb_generic_event_t *event) +{ + if ((event->response_type & ~0x80) == XCB_CLIENT_MESSAGE + && ((xcb_client_message_event_t *)event)->type == QXcbAtom::_QT_CLOSE_CONNECTION) + m_connection = 0; + m_events << event; +} + +QList *QXcbEventReader::lock() +{ + m_mutex.lock(); +#ifndef XCB_POLL_FOR_QUEUED_EVENT + while (xcb_generic_event_t *event = xcb_poll_for_event(m_connection)) + m_events << event; +#endif + return &m_events; +} + +void QXcbEventReader::unlock() +{ + m_mutex.unlock(); +} + +void QXcbConnection::sendConnectionEvent(QXcbAtom::Atom atom, uint id) +{ + xcb_client_message_event_t event; + memset(&event, 0, sizeof(event)); + + event.response_type = XCB_CLIENT_MESSAGE; + event.format = 32; + event.sequence = 0; + event.window = m_connectionEventListener; + event.type = atom; + event.data.data32[0] = id; + + Q_XCB_CALL(xcb_send_event(xcb_connection(), false, m_connectionEventListener, XCB_EVENT_MASK_NO_EVENT, (const char *)&event)); + xcb_flush(xcb_connection()); +} + void QXcbConnection::processXcbEvents() { - while (xcb_generic_event_t *event = xcb_poll_for_event(xcb_connection())) - eventqueue.append(event); + QList *eventqueue = m_reader->lock(); - for(int i = 0; i < eventqueue.size(); ++i) { - xcb_generic_event_t *event = eventqueue.at(i); + for(int i = 0; i < eventqueue->size(); ++i) { + xcb_generic_event_t *event = eventqueue->at(i); if (!event) continue; - eventqueue[i] = 0; + (*eventqueue)[i] = 0; uint response_type = event->response_type & ~0x80; @@ -562,7 +637,9 @@ void QXcbConnection::processXcbEvents() free(event); } - eventqueue.clear(); + eventqueue->clear(); + + m_reader->unlock(); // Indicate with a null event that the event the callbacks are waiting for // is not in the queue currently. @@ -591,19 +668,21 @@ void QXcbConnection::handleClientMessageEvent(const xcb_client_message_event_t * window->handleClientMessageEvent(event); } - xcb_generic_event_t *QXcbConnection::checkEvent(int type) { - while (xcb_generic_event_t *event = xcb_poll_for_event(xcb_connection())) - eventqueue.append(event); + QList *eventqueue = m_reader->lock(); - for (int i = 0; i < eventqueue.size(); ++i) { - xcb_generic_event_t *event = eventqueue.at(i); + for (int i = 0; i < eventqueue->size(); ++i) { + xcb_generic_event_t *event = eventqueue->at(i); if (event->response_type == type) { - eventqueue[i] = 0; + (*eventqueue)[i] = 0; + m_reader->unlock(); return event; } } + + m_reader->unlock(); + return 0; } @@ -646,6 +725,8 @@ static const char * xcb_atomnames = { "_QT_SCROLL_DONE\0" "_QT_INPUT_ENCODING\0" + "_QT_CLOSE_CONNECTION\0" + "_MOTIF_WM_HINTS\0" "DTWM_IS_RUNNING\0" diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h index 6a80f57..8c05fe2 100644 --- a/src/plugins/platforms/xcb/qxcbconnection.h +++ b/src/plugins/platforms/xcb/qxcbconnection.h @@ -46,7 +46,9 @@ #include #include +#include #include +#include #include #define Q_XCB_DEBUG @@ -100,6 +102,9 @@ namespace QXcbAtom { _QT_SCROLL_DONE, _QT_INPUT_ENCODING, + // Qt/XCB specific + _QT_CLOSE_CONNECTION, + _MOTIF_WM_HINTS, DTWM_IS_RUNNING, @@ -222,6 +227,33 @@ namespace QXcbAtom { }; } +class QXcbEventReader : public QThread +{ + Q_OBJECT +public: + QXcbEventReader(xcb_connection_t *connection) + : m_connection(connection) + { + } + +#ifdef XCB_POLL_FOR_QUEUED_EVENT + void run(); +#endif + + QList *lock(); + void unlock(); + +signals: + void eventPending(); + +private: + void addEvent(xcb_generic_event_t *event); + + QMutex m_mutex; + QList m_events; + xcb_connection_t *m_connection; +}; + class QAbstractEventDispatcher; class QXcbConnection : public QObject { @@ -314,6 +346,8 @@ private: QByteArray m_displayName; + xcb_window_t m_connectionEventListener; + QXcbKeyboard *m_keyboard; QXcbClipboard *m_clipboard; QXcbDrag *m_drag; @@ -322,7 +356,7 @@ private: #if defined(XCB_USE_XLIB) void *m_xlib_display; #endif - + QXcbEventReader *m_reader; #ifdef XCB_USE_DRI2 uint32_t m_dri2_major; uint32_t m_dri2_minor; @@ -345,7 +379,6 @@ private: template friend cookie_t q_xcb_call_template(const cookie_t &cookie, QXcbConnection *connection, const char *file, int line); #endif - QVector eventqueue; WindowMapper m_mapper; @@ -359,16 +392,17 @@ private: template xcb_generic_event_t *QXcbConnection::checkEvent(const T &checker) { - while (xcb_generic_event_t *event = xcb_poll_for_event(xcb_connection())) - eventqueue.append(event); + QList *eventqueue = m_reader->lock(); - for (int i = 0; i < eventqueue.size(); ++i) { - xcb_generic_event_t *event = eventqueue.at(i); + for (int i = 0; i < eventqueue->size(); ++i) { + xcb_generic_event_t *event = eventqueue->at(i); if (checker.check(event)) { - eventqueue[i] = 0; + (*eventqueue)[i] = 0; + m_reader->unlock(); return event; } } + m_reader->unlock(); return 0; } diff --git a/src/plugins/platforms/xcb/xcb.pro b/src/plugins/platforms/xcb/xcb.pro index cf03f8b..4f16c3b 100644 --- a/src/plugins/platforms/xcb/xcb.pro +++ b/src/plugins/platforms/xcb/xcb.pro @@ -37,6 +37,9 @@ HEADERS = \ qxcbcursor.h \ qxcbimage.h +contains(QT_CONFIG, xcb-poll-for-queued-event) { + DEFINES += XCB_POLL_FOR_QUEUED_EVENT +} # needed by GLX, Xcursor, XLookupString, ... contains(QT_CONFIG, xcb-xlib) { -- 2.7.4