xcb: Compress mouse motion and touch update events

The current version of the mouse motion event compression
algorithm does not work with certain configurations -
situations where we get one XCB_GE_GENERIC event between
every XCB_MOTION_NOTIFY event.

The new implementation tries to be less fragile. The
previous approach checked "is *the next* event the same
type as the current event", the new check asks "have
we buffered more events of the same type as the current
event". We buffer events of the same type only when the
main thread is unresponsive.

This patch adds event compression for XI_TouchUpdate in
addition to the fix for motion even compression.

Change-Id: Ie215eb5969e2060e463ebe48e3d3007390a30deb
Task-number: QTBUG-40889
Task-number: QTBUG-47069
Reviewed-by: Laszlo Agocs <laszlo.agocs@theqtcompany.com>

3 files changed, 208 insertions, 134 deletions

diff --git a/src/plugins/platforms/xcb/qxcbconnection.cpp b/src/plugins/platforms/xcb/qxcbconnection.cpp
index 5fd0df54d8..1cc71b4e1e 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.cpp
+++ b/src/plugins/platforms/xcb/qxcbconnection.cpp
@@ -107,6 +107,24 @@ Q_LOGGING_CATEGORY(lcQpaScreen, "qt.qpa.screen")
 #define XCB_GE_GENERIC 35
 #endif
 
+// Starting from the xcb version 1.9.3 struct xcb_ge_event_t has changed:
+// - "pad0" became "extension"
+// - "pad1" and "pad" became "pad0"
+// New and old version of this struct share the following fields:
+typedef struct qt_xcb_ge_event_t {
+    uint8_t  response_type;
+    uint8_t  extension;
+    uint16_t sequence;
+    uint32_t length;
+    uint16_t event_type;
+} qt_xcb_ge_event_t;
+
+static inline bool isXIEvent(xcb_generic_event_t *event, int opCode)
+{
+    qt_xcb_ge_event_t *e = (qt_xcb_ge_event_t *)event;
+    return e->extension == opCode;
+}
+
 #ifdef XCB_USE_XLIB
 static const char * const xcbConnectionErrors[] = {
     "No error", /* Error 0 */
@@ -1109,7 +1127,7 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
 #if defined(XCB_USE_XINPUT2)
         case XCB_GE_GENERIC:
             // Here the windowEventListener is invoked from xi2HandleEvent()
-            if (m_xi2Enabled)
+            if (m_xi2Enabled && isXIEvent(event, m_xiOpCode))
                 xi2HandleEvent(reinterpret_cast<xcb_ge_event_t *>(event));
             break;
 #endif
@@ -1431,6 +1449,105 @@ void *QXcbConnection::createVisualInfoForDefaultVisualId() const
 
 #endif
 
+#if defined(XCB_USE_XINPUT2)
+// it is safe to cast XI_* events here as long as we are only touching the first 32 bytes,
+// after that position event needs memmove, see xi2PrepareXIGenericDeviceEvent
+static inline bool isXIType(xcb_generic_event_t *event, int opCode, uint16_t type)
+{
+    if (!isXIEvent(event, opCode))
+        return false;
+
+    xXIGenericDeviceEvent *xiEvent = reinterpret_cast<xXIGenericDeviceEvent *>(event);
+    return xiEvent->evtype == type;
+}
+#endif
+static inline bool isValid(xcb_generic_event_t *event)
+{
+    return event && (event->response_type & ~0x80);
+}
+
+/*! \internal
+
+    Compresses events of the same type to avoid swamping the event queue.
+    If event compression is not desired there are several options what developers can do:
+
+    1) Write responsive applications. We drop events that have been buffered in the event
+       queue while waiting on unresponsive GUI thread.
+    2) Use QAbstractNativeEventFilter to get all events from X connection. This is not optimal
+       because it requires working with native event types.
+    3) Or add public API to Qt for disabling event compression QTBUG-44964
+
+*/
+bool QXcbConnection::compressEvent(xcb_generic_event_t *event, int currentIndex, QXcbEventArray *eventqueue) const
+{
+    uint responseType = event->response_type & ~0x80;
+    int nextIndex = currentIndex + 1;
+
+    if (responseType == XCB_MOTION_NOTIFY) {
+        // compress XCB_MOTION_NOTIFY notify events
+        for (int j = nextIndex; j < eventqueue->size(); ++j) {
+            xcb_generic_event_t *next = eventqueue->at(j);
+            if (!isValid(next))
+                continue;
+            if (next->response_type == XCB_MOTION_NOTIFY)
+                return true;
+        }
+        return false;
+    }
+#if defined(XCB_USE_XINPUT2)
+    // compress XI_* events
+    if (responseType == XCB_GE_GENERIC) {
+        if (!m_xi2Enabled)
+            return false;
+
+        // compress XI_Motion
+        if (isXIType(event, m_xiOpCode, XI_Motion)) {
+            for (int j = nextIndex; j < eventqueue->size(); ++j) {
+                xcb_generic_event_t *next = eventqueue->at(j);
+                if (!isValid(next))
+                    continue;
+                if (isXIType(next, m_xiOpCode, XI_Motion))
+                    return true;
+            }
+            return false;
+        }
+#ifdef XCB_USE_XINPUT22
+        // compress XI_TouchUpdate for the same touch point id
+        if (isXIType(event, m_xiOpCode, XI_TouchUpdate)) {
+            xXIDeviceEvent *xiDeviceEvent = reinterpret_cast<xXIDeviceEvent *>(event);
+            uint32_t id = xiDeviceEvent->detail % INT_MAX;
+            for (int j = nextIndex; j < eventqueue->size(); ++j) {
+                xcb_generic_event_t *next = eventqueue->at(j);
+                if (!isValid(next))
+                    continue;
+                if (isXIType(next, m_xiOpCode, XI_TouchUpdate)) {
+                    xXIDeviceEvent *xiDeviceNextEvent = reinterpret_cast<xXIDeviceEvent *>(next);
+                    if (id == xiDeviceNextEvent->detail % INT_MAX)
+                        return true;
+                }
+            }
+            return false;
+        }
+#endif
+        return false;
+    }
+#endif
+    if (responseType == XCB_CONFIGURE_NOTIFY) {
+        // compress multiple configure notify events for the same window
+        for (int j = nextIndex; j < eventqueue->size(); ++j) {
+            xcb_generic_event_t *next = eventqueue->at(j);
+            if (isValid(next) && next->response_type == XCB_CONFIGURE_NOTIFY
+                && ((xcb_configure_notify_event_t *)next)->event == ((xcb_configure_notify_event_t*)event)->event)
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    return false;
+}
+
 void QXcbConnection::processXcbEvents()
 {
     int connection_error = xcb_connection_has_error(xcb_connection());
@@ -1441,61 +1558,39 @@ void QXcbConnection::processXcbEvents()
 
     QXcbEventArray *eventqueue = m_reader->lock();
 
-    for(int i = 0; i < eventqueue->size(); ++i) {
+    for (int i = 0; i < eventqueue->size(); ++i) {
         xcb_generic_event_t *event = eventqueue->at(i);
         if (!event)
             continue;
         QScopedPointer<xcb_generic_event_t, QScopedPointerPodDeleter> eventGuard(event);
         (*eventqueue)[i] = 0;
 
-        uint response_type = event->response_type & ~0x80;
-
-        if (!response_type) {
+        if (!(event->response_type & ~0x80)) {
             handleXcbError((xcb_generic_error_t *)event);
-        } else {
-            if (response_type == XCB_MOTION_NOTIFY) {
-                // compress multiple motion notify events in a row
-                // to avoid swamping the event queue
-                xcb_generic_event_t *next = eventqueue->value(i+1, 0);
-                if (next && (next->response_type & ~0x80) == XCB_MOTION_NOTIFY)
-                    continue;
-            }
+            continue;
+        }
 
-            if (response_type == XCB_CONFIGURE_NOTIFY) {
-                // compress multiple configure notify events for the same window
-                bool found = false;
-                for (int j = i; j < eventqueue->size(); ++j) {
-                    xcb_generic_event_t *other = eventqueue->at(j);
-                    if (other && (other->response_type & ~0x80) == XCB_CONFIGURE_NOTIFY
-                        && ((xcb_configure_notify_event_t *)other)->event == ((xcb_configure_notify_event_t *)event)->event)
-                    {
-                        found = true;
-                        break;
-                    }
-                }
-                if (found)
-                    continue;
-            }
+        if (compressEvent(event, i, eventqueue))
+            continue;
 
-            bool accepted = false;
-            if (clipboard()->processIncr())
-                clipboard()->incrTransactionPeeker(event, accepted);
-            if (accepted)
-                continue;
+        bool accepted = false;
+        if (clipboard()->processIncr())
+            clipboard()->incrTransactionPeeker(event, accepted);
+        if (accepted)
+            continue;
 
-            QVector<PeekFunc>::iterator it = m_peekFuncs.begin();
-            while (it != m_peekFuncs.end()) {
-                // These callbacks return true if the event is what they were
-                // waiting for, remove them from the list in that case.
-                if ((*it)(this, event))
-                    it = m_peekFuncs.erase(it);
-                else
-                    ++it;
-            }
-            m_reader->unlock();
-            handleXcbEvent(event);
-            m_reader->lock();
+        QVector<PeekFunc>::iterator it = m_peekFuncs.begin();
+        while (it != m_peekFuncs.end()) {
+            // These callbacks return true if the event is what they were
+            // waiting for, remove them from the list in that case.
+            if ((*it)(this, event))
+                it = m_peekFuncs.erase(it);
+            else
+                ++it;
         }
+        m_reader->unlock();
+        handleXcbEvent(event);
+        m_reader->lock();
     }
 
     eventqueue->clear();
@@ -2037,34 +2132,13 @@ bool QXcbConnection::xi2GetValuatorValueIfSet(void *event, int valuatorNum, doub
     return true;
 }
 
-// Starting from the xcb version 1.9.3 struct xcb_ge_event_t has changed:
-// - "pad0" became "extension"
-// - "pad1" and "pad" became "pad0"
-// New and old version of this struct share the following fields:
-// NOTE: API might change again in the next release of xcb in which case this comment will
-// need to be updated to reflect the reality.
-typedef struct qt_xcb_ge_event_t {
-    uint8_t  response_type;
-    uint8_t  extension;
-    uint16_t sequence;
-    uint32_t length;
-    uint16_t event_type;
-} qt_xcb_ge_event_t;
-
-bool QXcbConnection::xi2PrepareXIGenericDeviceEvent(xcb_ge_event_t *ev, int opCode)
-{
-    qt_xcb_ge_event_t *event = (qt_xcb_ge_event_t *)ev;
-    // xGenericEvent has "extension" on the second byte, the same is true for xcb_ge_event_t starting from
-    // the xcb version 1.9.3, prior to that it was called "pad0".
-    if (event->extension == opCode) {
-        // xcb event structs contain stuff that wasn't on the wire, the full_sequence field
-        // adds an extra 4 bytes and generic events cookie data is on the wire right after the standard 32 bytes.
-        // Move this data back to have the same layout in memory as it was on the wire
-        // and allow casting, overwriting the full_sequence field.
-        memmove((char*) event + 32, (char*) event + 36, event->length * 4);
-        return true;
-    }
-    return false;
+void QXcbConnection::xi2PrepareXIGenericDeviceEvent(xcb_ge_event_t *event)
+{
+    // xcb event structs contain stuff that wasn't on the wire, the full_sequence field
+    // adds an extra 4 bytes and generic events cookie data is on the wire right after the standard 32 bytes.
+    // Move this data back to have the same layout in memory as it was on the wire
+    // and allow casting, overwriting the full_sequence field.
+    memmove((char*) event + 32, (char*) event + 36, event->length * 4);
 }
 #endif // defined(XCB_USE_XINPUT2)
 
diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h
index 4a0348aa0c..2e5a2d7788 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.h
+++ b/src/plugins/platforms/xcb/qxcbconnection.h
@@ -522,6 +522,7 @@ private:
     bool checkOutputIsPrimary(xcb_window_t rootWindow, xcb_randr_output_t output);
     void initializeScreens();
     void updateScreens(const xcb_randr_notify_event_t *event);
+    bool compressEvent(xcb_generic_event_t *event, int currentIndex, QXcbEventArray *eventqueue) const;
 
     bool m_xi2Enabled;
     int m_xi2Minor;
@@ -574,7 +575,7 @@ private:
     QHash<int, ScrollingDevice> m_scrollingDevices;
 
     static bool xi2GetValuatorValueIfSet(void *event, int valuatorNum, double *value);
-    static bool xi2PrepareXIGenericDeviceEvent(xcb_ge_event_t *event, int opCode);
+    static void xi2PrepareXIGenericDeviceEvent(xcb_ge_event_t *event);
 #endif
 
     xcb_connection_t *m_connection;
diff --git a/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp b/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp
index 7a5480a8a5..e78ae8fa35 100644
--- a/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp
+++ b/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp
@@ -459,82 +459,81 @@ static inline qreal fixed1616ToReal(FP1616 val)
 
 void QXcbConnection::xi2HandleEvent(xcb_ge_event_t *event)
 {
-    if (xi2PrepareXIGenericDeviceEvent(event, m_xiOpCode)) {
-        xXIGenericDeviceEvent *xiEvent = reinterpret_cast<xXIGenericDeviceEvent *>(event);
-        int sourceDeviceId = xiEvent->deviceid; // may be the master id
-        xXIDeviceEvent *xiDeviceEvent = 0;
-        QXcbWindowEventListener *eventListener = 0;
+    xi2PrepareXIGenericDeviceEvent(event);
+    xXIGenericDeviceEvent *xiEvent = reinterpret_cast<xXIGenericDeviceEvent *>(event);
+    int sourceDeviceId = xiEvent->deviceid; // may be the master id
+    xXIDeviceEvent *xiDeviceEvent = 0;
+    QXcbWindowEventListener *eventListener = 0;
 
-        switch (xiEvent->evtype) {
-        case XI_ButtonPress:
-        case XI_ButtonRelease:
-        case XI_Motion:
+    switch (xiEvent->evtype) {
+    case XI_ButtonPress:
+    case XI_ButtonRelease:
+    case XI_Motion:
 #ifdef XCB_USE_XINPUT22
-        case XI_TouchBegin:
-        case XI_TouchUpdate:
-        case XI_TouchEnd:
+    case XI_TouchBegin:
+    case XI_TouchUpdate:
+    case XI_TouchEnd:
 #endif
-        {
-            xiDeviceEvent = reinterpret_cast<xXIDeviceEvent *>(event);
-            eventListener = windowEventListenerFromId(xiDeviceEvent->event);
-            if (eventListener) {
-                long result = 0;
-                if (eventListener->handleGenericEvent(reinterpret_cast<xcb_generic_event_t *>(event), &result))
-                    return;
-            }
-            sourceDeviceId = xiDeviceEvent->sourceid; // use the actual device id instead of the master
-            break;
-        }
-        case XI_HierarchyChanged:
-            xi2HandleHierachyEvent(xiEvent);
-            return;
-        case XI_DeviceChanged:
-            xi2HandleDeviceChangedEvent(xiEvent);
-            return;
-        default:
-            break;
+    {
+        xiDeviceEvent = reinterpret_cast<xXIDeviceEvent *>(event);
+        eventListener = windowEventListenerFromId(xiDeviceEvent->event);
+        if (eventListener) {
+            long result = 0;
+            if (eventListener->handleGenericEvent(reinterpret_cast<xcb_generic_event_t *>(event), &result))
+                return;
         }
+        sourceDeviceId = xiDeviceEvent->sourceid; // use the actual device id instead of the master
+        break;
+    }
+    case XI_HierarchyChanged:
+        xi2HandleHierachyEvent(xiEvent);
+        return;
+    case XI_DeviceChanged:
+        xi2HandleDeviceChangedEvent(xiEvent);
+        return;
+    default:
+        break;
+    }
 
 #ifndef QT_NO_TABLETEVENT
-        for (int i = 0; i < m_tabletData.count(); ++i) {
-            if (m_tabletData.at(i).deviceId == sourceDeviceId) {
-                if (xi2HandleTabletEvent(xiEvent, &m_tabletData[i], eventListener))
-                    return;
-            }
+    for (int i = 0; i < m_tabletData.count(); ++i) {
+        if (m_tabletData.at(i).deviceId == sourceDeviceId) {
+            if (xi2HandleTabletEvent(xiEvent, &m_tabletData[i], eventListener))
+                return;
         }
+    }
 #endif // QT_NO_TABLETEVENT
 
 #ifdef XCB_USE_XINPUT21
-        QHash<int, ScrollingDevice>::iterator device = m_scrollingDevices.find(sourceDeviceId);
-        if (device != m_scrollingDevices.end())
-            xi2HandleScrollEvent(xiEvent, device.value());
+    QHash<int, ScrollingDevice>::iterator device = m_scrollingDevices.find(sourceDeviceId);
+    if (device != m_scrollingDevices.end())
+        xi2HandleScrollEvent(xiEvent, device.value());
 #endif // XCB_USE_XINPUT21
 
 #ifdef XCB_USE_XINPUT22
-        if (xiDeviceEvent) {
-            switch (xiDeviceEvent->evtype) {
-            case XI_ButtonPress:
-            case XI_ButtonRelease:
-            case XI_Motion:
-                if (xi2MouseEvents() && eventListener && !(xiDeviceEvent->flags & XIPointerEmulated))
-                    eventListener->handleXIMouseEvent(event);
-                break;
+    if (xiDeviceEvent) {
+        switch (xiDeviceEvent->evtype) {
+        case XI_ButtonPress:
+        case XI_ButtonRelease:
+        case XI_Motion:
+            if (xi2MouseEvents() && eventListener && !(xiDeviceEvent->flags & XIPointerEmulated))
+                eventListener->handleXIMouseEvent(event);
+            break;
 
-            case XI_TouchBegin:
-            case XI_TouchUpdate:
-            case XI_TouchEnd:
-                if (Q_UNLIKELY(lcQpaXInput().isDebugEnabled()))
-                    qCDebug(lcQpaXInput, "XI2 touch event type %d seq %d detail %d pos %6.1f, %6.1f root pos %6.1f, %6.1f on window %x",
-                            event->event_type, xiDeviceEvent->sequenceNumber, xiDeviceEvent->detail,
-                            fixed1616ToReal(xiDeviceEvent->event_x), fixed1616ToReal(xiDeviceEvent->event_y),
-                            fixed1616ToReal(xiDeviceEvent->root_x), fixed1616ToReal(xiDeviceEvent->root_y),xiDeviceEvent->event);
-                if (QXcbWindow *platformWindow = platformWindowFromId(xiDeviceEvent->event))
-                    xi2ProcessTouch(xiDeviceEvent, platformWindow);
-                break;
-            }
+        case XI_TouchBegin:
+        case XI_TouchUpdate:
+        case XI_TouchEnd:
+            if (Q_UNLIKELY(lcQpaXInput().isDebugEnabled()))
+                qCDebug(lcQpaXInput, "XI2 touch event type %d seq %d detail %d pos %6.1f, %6.1f root pos %6.1f, %6.1f on window %x",
+                        event->event_type, xiDeviceEvent->sequenceNumber, xiDeviceEvent->detail,
+                        fixed1616ToReal(xiDeviceEvent->event_x), fixed1616ToReal(xiDeviceEvent->event_y),
+                        fixed1616ToReal(xiDeviceEvent->root_x), fixed1616ToReal(xiDeviceEvent->root_y),xiDeviceEvent->event);
+            if (QXcbWindow *platformWindow = platformWindowFromId(xiDeviceEvent->event))
+                xi2ProcessTouch(xiDeviceEvent, platformWindow);
+            break;
         }
-#endif // XCB_USE_XINPUT22
     }
+#endif // XCB_USE_XINPUT22
 }
 
 #ifdef XCB_USE_XINPUT22