From fe63900dc9891dd355ca1f10d6c7e5fd1516f5d5 Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Mon, 5 Nov 2018 14:26:24 +0100 Subject: [PATCH] xcb: rework focus-in peeker so we can drop PeekFunc API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API for registering temporary peek function was added ~7 years ago by 78264f333eb7c262380714ed6517562266f11a03. It was never been used for anything else. The solution from 78264f333 also did not work very well on KDE desktop, quoting Martin Flöser: "In case the keyboard gets grabbed by another process and immediately ungrabbed the active Qt application window receives a FocusOut and a FocusIn event. FocusOut on the grab of keyboard, FocusIn on the ungrab. Qt registers a peek function for checking the FocusIn event, but the timespan is too short: the new event is not yet queued. This causes a QEvent::WindowDeactivate being emitted, followed directly by a QEvent::WindowActivate. This has quite some side effects, for example rendering flickering in the GUI (switching to inactive/active in short time frame), hooks on WindowDeactivate being run, etc. Real world examples for such short keyboard grabs are global shortcut listener applications like kglobalaccel5. It has e.g. a passive key grab on the mute key, which is then turned into an active grab when the key is grabbed. Kglobalaccel5 immediately ungrabs the keyboard and flushes the connection if it gets a key event, but it of course causes the sequence of FocusOut and FocusIn events in the active Qt window." Reworked the code to use QTimer instead, which is more elegant solution, because it does not rely on race-conditions, but uses a concreate time to wait instead. Also the need to write focusInPeeker() caused us to duplicate event handlers that were present already elsewhere. Change-Id: I647e52fb2634fdf55a640e19b13265c356f96c95 Reviewed-by: Mikhail Svetkin Reviewed-by: Gatis Paeglis --- src/plugins/platforms/xcb/qxcbconnection.cpp | 26 ++++----------- src/plugins/platforms/xcb/qxcbconnection.h | 10 +++--- src/plugins/platforms/xcb/qxcbwindow.cpp | 33 +++----------------- 3 files changed, 15 insertions(+), 54 deletions(-) diff --git a/src/plugins/platforms/xcb/qxcbconnection.cpp b/src/plugins/platforms/xcb/qxcbconnection.cpp index 37ee980924..15ffaaf2e2 100644 --- a/src/plugins/platforms/xcb/qxcbconnection.cpp +++ b/src/plugins/platforms/xcb/qxcbconnection.cpp @@ -131,6 +131,12 @@ QXcbConnection::QXcbConnection(QXcbNativeInterface *nativeInterface, bool canGra if (!m_startupId.isNull()) qunsetenv("DESKTOP_STARTUP_ID"); + m_focusInTimer.setSingleShot(true); + m_focusInTimer.callOnTimeout([]() { + // No FocusIn events for us, proceed with FocusOut normally. + QWindowSystemInterface::handleWindowActivated(nullptr, Qt::ActiveWindowFocusReason); + }); + sync(); } @@ -731,11 +737,6 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event) m_glIntegration->handleXcbEvent(event, response_type); } -void QXcbConnection::addPeekFunc(PeekFunc f) -{ - m_peekFuncs.append(f); -} - void QXcbConnection::setFocusWindow(QWindow *w) { m_focusWindow = w ? static_cast(w->handle()) : nullptr; @@ -1015,15 +1016,6 @@ void QXcbConnection::processXcbEvents(QEventLoop::ProcessEventsFlags flags) if (compressEvent(event)) continue; - auto isWaitingFor = [=](PeekFunc peekFunc) { - // These callbacks return true if the event is what they were - // waiting for, remove them from the list in that case. - return peekFunc(this, event); - }; - m_peekFuncs.erase(std::remove_if(m_peekFuncs.begin(), m_peekFuncs.end(), - isWaitingFor), - m_peekFuncs.end()); - handleXcbEvent(event); // The lock-based solution used to free the lock inside this loop, @@ -1032,12 +1024,6 @@ void QXcbConnection::processXcbEvents(QEventLoop::ProcessEventsFlags flags) m_eventQueue->flushBufferedEvents(); } - // Indicate with a null event that the event the callbacks are waiting for - // is not in the queue currently. - for (PeekFunc f : qAsConst(m_peekFuncs)) - f(this, nullptr); - m_peekFuncs.clear(); - xcb_flush(xcb_connection()); } diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h index 47036ca257..d63888b48f 100644 --- a/src/plugins/platforms/xcb/qxcbconnection.h +++ b/src/plugins/platforms/xcb/qxcbconnection.h @@ -43,6 +43,7 @@ #include #include +#include #include #include "qxcbexport.h" #include @@ -183,9 +184,6 @@ public: QXcbWindowEventListener *windowEventListenerFromId(xcb_window_t id); QXcbWindow *platformWindowFromId(xcb_window_t id); - typedef bool (*PeekFunc)(QXcbConnection *, xcb_generic_event_t *); - void addPeekFunc(PeekFunc f); - inline xcb_timestamp_t time() const { return m_time; } inline void setTime(xcb_timestamp_t t) { if (t > m_time) m_time = t; } @@ -247,6 +245,8 @@ public: void flush() { xcb_flush(xcb_connection()); } void processXcbEvents(QEventLoop::ProcessEventsFlags flags); + QTimer &focusInTimer() { return m_focusInTimer; }; + protected: bool event(QEvent *e) override; @@ -364,8 +364,6 @@ private: WindowMapper m_mapper; - QVector m_peekFuncs; - Qt::MouseButtons m_buttonState = 0; Qt::MouseButton m_button = Qt::NoButton; @@ -386,6 +384,8 @@ private: friend class QXcbEventQueue; QByteArray m_xdgCurrentDesktop; + QTimer m_focusInTimer; + }; #if QT_CONFIG(xcb_xinput) #if QT_CONFIG(tabletevent) diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp index 3bfcbf2adb..9382488b74 100644 --- a/src/plugins/platforms/xcb/qxcbwindow.cpp +++ b/src/plugins/platforms/xcb/qxcbwindow.cpp @@ -843,40 +843,12 @@ void QXcbWindow::doFocusIn() QWindowSystemInterface::handleWindowActivated(w, Qt::ActiveWindowFocusReason); } -static bool focusInPeeker(QXcbConnection *connection, xcb_generic_event_t *event) -{ - if (!event) { - // FocusIn event is not in the queue, proceed with FocusOut normally. - QWindowSystemInterface::handleWindowActivated(nullptr, Qt::ActiveWindowFocusReason); - return true; - } - uint response_type = event->response_type & ~0x80; - if (response_type == XCB_FOCUS_IN) { - // Ignore focus events that are being sent only because the pointer is over - // our window, even if the input focus is in a different window. - xcb_focus_in_event_t *e = (xcb_focus_in_event_t *) event; - if (e->detail != XCB_NOTIFY_DETAIL_POINTER) - return true; - } - - /* We are also interested in XEMBED_FOCUS_IN events */ - if (response_type == XCB_CLIENT_MESSAGE) { - xcb_client_message_event_t *cme = (xcb_client_message_event_t *)event; - if (cme->type == connection->atom(QXcbAtom::_XEMBED) - && cme->data.data32[1] == XEMBED_FOCUS_IN) - return true; - } - - return false; -} - void QXcbWindow::doFocusOut() { connection()->setFocusWindow(nullptr); relayFocusToModalWindow(); // Do not set the active window to nullptr if there is a FocusIn coming. - // The FocusIn handler will update QXcbConnection::setFocusWindow() accordingly. - connection()->addPeekFunc(focusInPeeker); + connection()->focusInTimer().start(400); } struct QtMotifWmHints { @@ -2264,6 +2236,8 @@ void QXcbWindow::handleFocusInEvent(const xcb_focus_in_event_t *event) // our window, even if the input focus is in a different window. if (event->detail == XCB_NOTIFY_DETAIL_POINTER) return; + + connection()->focusInTimer().stop(); doFocusIn(); } @@ -2491,6 +2465,7 @@ void QXcbWindow::handleXEmbedMessage(const xcb_client_message_event_t *event) xcbScreen()->windowShown(this); break; case XEMBED_FOCUS_IN: + connection()->focusInTimer().stop(); Qt::FocusReason reason; switch (event->data.data32[2]) { case XEMBED_FOCUS_FIRST: