From 21f976f4f0f79b1c4c77a402ebed88d8afb3d9e1 Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Sat, 4 Aug 2018 00:47:14 +0200 Subject: xcb: rewrite auto-repeat key detection logic It's unclear what the original code was doing. It relied on 'm_release' which could never be 'false' (ref. QTBUG-69679). It was subtracting event times and comparing with arbitrary '10'. On X11 auto-repeat keys can be detected by checking time and keycode of the current release event and the next event in the queue. If an event is an auto-repeat, then next event in the queue will be a key press with matching time and keycode. Verified that auto-repeat was unreliable in Qt 4 as well. With this patch auto-repeat works as expected. Added support for Xlib's XPeekEvent in our XCB implementation QXcbConnection::checkEvent(): "The XPeekEvent() function returns the first event from the event queue, but it does not remove the event from the queue." Sneaking in one variable renaming: "string" -> "text", to match the QKeyEvent::text(). Task-number: QTBUG-57335 Task-number: QTBUG-69679 Change-Id: I0a23f138287f57eaaecf1a009bd939e7e0e23269 Reviewed-by: Shawn Rutledge --- src/plugins/platforms/xcb/qxcbconnection.h | 7 +- src/plugins/platforms/xcb/qxcbkeyboard.cpp | 108 ++++++----------------------- src/plugins/platforms/xcb/qxcbkeyboard.h | 3 +- 3 files changed, 26 insertions(+), 92 deletions(-) (limited to 'src') diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h index 24719a6c31..697b509bf0 100644 --- a/src/plugins/platforms/xcb/qxcbconnection.h +++ b/src/plugins/platforms/xcb/qxcbconnection.h @@ -441,7 +441,7 @@ public: QXcbWindow *platformWindowFromId(xcb_window_t id); template - inline xcb_generic_event_t *checkEvent(Functor &&filter); + inline xcb_generic_event_t *checkEvent(Functor &&filter, bool removeFromQueue = true); typedef bool (*PeekFunc)(QXcbConnection *, xcb_generic_event_t *); void addPeekFunc(PeekFunc f); @@ -735,14 +735,15 @@ Q_DECLARE_TYPEINFO(QXcbConnection::TabletData, Q_MOVABLE_TYPE); #endif template -xcb_generic_event_t *QXcbConnection::checkEvent(Functor &&filter) +xcb_generic_event_t *QXcbConnection::checkEvent(Functor &&filter, bool removeFromQueue) { QXcbEventArray *eventqueue = m_reader->lock(); for (int i = 0; i < eventqueue->size(); ++i) { xcb_generic_event_t *event = eventqueue->at(i); if (event && filter(event, event->response_type & ~0x80)) { - (*eventqueue)[i] = nullptr; + if (removeFromQueue) + (*eventqueue)[i] = nullptr; m_reader->unlock(); return event; } diff --git a/src/plugins/platforms/xcb/qxcbkeyboard.cpp b/src/plugins/platforms/xcb/qxcbkeyboard.cpp index 37c8def6cc..40100c3a62 100644 --- a/src/plugins/platforms/xcb/qxcbkeyboard.cpp +++ b/src/plugins/platforms/xcb/qxcbkeyboard.cpp @@ -1472,60 +1472,6 @@ void QXcbKeyboard::resolveMaskConflicts() } } -class KeyChecker -{ -public: - KeyChecker(xcb_window_t window, xcb_keycode_t code, xcb_timestamp_t time, quint16 state) - : m_window(window) - , m_code(code) - , m_time(time) - , m_state(state) - , m_error(false) - , m_release(true) - { - } - - bool operator() (xcb_generic_event_t *ev, int type) - { - if (m_error || !ev) - return false; - - if (type != XCB_KEY_PRESS && type != XCB_KEY_RELEASE) - return false; - - xcb_key_press_event_t *event = (xcb_key_press_event_t *)ev; - - if (event->event != m_window || event->detail != m_code || event->state != m_state) { - m_error = true; - return false; - } - - if (type == XCB_KEY_PRESS) { - m_error = !m_release || event->time - m_time > 10; - return !m_error; - } - - if (m_release) { - m_error = true; - return false; - } - - m_release = true; - m_time = event->time; - - return false; - } - -private: - xcb_window_t m_window; - xcb_keycode_t m_code; - xcb_timestamp_t m_time; - quint16 m_state; - - bool m_error; - bool m_release; -}; - void QXcbKeyboard::handleKeyEvent(xcb_window_t sourceWindow, QEvent::Type type, xcb_keycode_t code, quint16 state, xcb_timestamp_t time, bool fromSendEvent) { @@ -1539,7 +1485,6 @@ void QXcbKeyboard::handleKeyEvent(xcb_window_t sourceWindow, QEvent::Type type, if (type == QEvent::KeyPress) targetWindow->updateNetWmUserTime(time); - ScopedXKBState sendEventState; if (fromSendEvent) { // Have a temporary keyboard state filled in from state @@ -1557,7 +1502,7 @@ void QXcbKeyboard::handleKeyEvent(xcb_window_t sourceWindow, QEvent::Type type, struct xkb_state *xkbState = fromSendEvent ? sendEventState.get() : m_xkbState.get(); xcb_keysym_t sym = xkb_state_key_get_one_sym(xkbState, code); - QString string = lookupString(xkbState, code); + QString text = lookupString(xkbState, code); Qt::KeyboardModifiers modifiers = translateModifiers(state); if (sym >= XKB_KEY_KP_Space && sym <= XKB_KEY_KP_9) @@ -1582,55 +1527,42 @@ void QXcbKeyboard::handleKeyEvent(xcb_window_t sourceWindow, QEvent::Type type, int qtcode = keysymToQtKey(latinKeysym != XKB_KEY_NoSymbol ? latinKeysym : sym, modifiers, xkbState, code); - bool isAutoRepeat = false; if (type == QEvent::KeyPress) { - if (m_autorepeat_code == code) { - isAutoRepeat = true; - m_autorepeat_code = 0; - } + if (m_isAutoRepeat && m_autoRepeatCode != code) + // Some other key was pressed while we are auto-repeating on a different key. + m_isAutoRepeat = false; } else { - // look ahead for auto-repeat - KeyChecker checker(source->xcb_window(), code, time, state); - xcb_generic_event_t *event = connection()->checkEvent(checker); - if (event) { - isAutoRepeat = true; - free(event); - } - m_autorepeat_code = isAutoRepeat ? code : 0; + m_isAutoRepeat = false; + // Look at the next event in the queue to see if we are auto-repeating. + connection()->checkEvent([this, time, code](xcb_generic_event_t *event, int type) { + if (type == XCB_KEY_PRESS) { + auto keyPress = reinterpret_cast(event); + m_isAutoRepeat = keyPress->time == time && keyPress->detail == code; + if (m_isAutoRepeat) + m_autoRepeatCode = code; + } + return true; + }, false /* removeFromQueue */); } bool filtered = false; - QPlatformInputContext *inputContext = QGuiApplicationPrivate::platformIntegration()->inputContext(); - if (inputContext) { - QKeyEvent event(type, qtcode, modifiers, code, sym, state, string, isAutoRepeat, string.length()); + if (auto inputContext = QGuiApplicationPrivate::platformIntegration()->inputContext()) { + QKeyEvent event(type, qtcode, modifiers, code, sym, state, text, m_isAutoRepeat, text.size()); event.setTimestamp(time); filtered = inputContext->filterEvent(&event); } - QWindow *window = targetWindow->window(); if (!filtered) { + QWindow *window = targetWindow->window(); #ifndef QT_NO_CONTEXTMENU if (type == QEvent::KeyPress && qtcode == Qt::Key_Menu) { const QPoint globalPos = window->screen()->handle()->cursor()->pos(); const QPoint pos = window->mapFromGlobal(globalPos); QWindowSystemInterface::handleContextMenuEvent(window, false, pos, globalPos, modifiers); } -#endif // QT_NO_CONTEXTMENU +#endif QWindowSystemInterface::handleExtendedKeyEvent(window, time, type, qtcode, modifiers, - code, sym, state, string, isAutoRepeat); - } - - if (isAutoRepeat && type == QEvent::KeyRelease) { - // since we removed it from the event queue using checkEvent we need to send the key press here - filtered = false; - if (inputContext) { - QKeyEvent event(QEvent::KeyPress, qtcode, modifiers, code, sym, state, string, isAutoRepeat, string.length()); - event.setTimestamp(time); - filtered = inputContext->filterEvent(&event); - } - if (!filtered) - QWindowSystemInterface::handleExtendedKeyEvent(window, time, QEvent::KeyPress, qtcode, modifiers, - code, sym, state, string, isAutoRepeat); + code, sym, state, text, m_isAutoRepeat); } } diff --git a/src/plugins/platforms/xcb/qxcbkeyboard.h b/src/plugins/platforms/xcb/qxcbkeyboard.h index ab926eab84..95915fb2e6 100644 --- a/src/plugins/platforms/xcb/qxcbkeyboard.h +++ b/src/plugins/platforms/xcb/qxcbkeyboard.h @@ -109,7 +109,8 @@ protected: private: bool m_config = false; - xcb_keycode_t m_autorepeat_code = 0; + bool m_isAutoRepeat = false; + xcb_keycode_t m_autoRepeatCode = 0; struct _mod_masks { uint alt; -- cgit v1.2.3