xcb: cleanup updating of scrolling device valuators

This includes:

 - Better sharing of common code.
 - Less argument passing.
 - Improved code readability:

   It is somewhat confusing to read when QXcbWindow::handleEnterNotifyEvent
   calls connection()->handleEnterEvent().

 - Better organizing of ifdefs.

Change-Id: I4405390cada13b51db78f9fd30e26bb1793395ff
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

src/plugins/platforms/xcb/qxcbconnection.h

@@ -500,7 +500,7 @@ public:
     static bool xEmbedSystemTrayVisualHasAlphaChannel();
 
 #ifdef XCB_USE_XINPUT21
-    void handleEnterEvent();
+    void xi2UpdateScrollingDevices();
 #endif
 
 #ifdef XCB_USE_XINPUT22
@@ -616,9 +616,12 @@ private:
         Qt::Orientations legacyOrientations = 0;
         QPointF lastScrollPosition;
     };
-    void updateScrollingDevice(ScrollingDevice& scrollingDevice, int num_classes, void *classes);
-    void xi2HandleScrollEvent(void *event, ScrollingDevice &scrollingDevice);
     QHash<int, ScrollingDevice> m_scrollingDevices;
+#ifdef XCB_USE_XINPUT21
+    void xi2HandleScrollEvent(void *event, ScrollingDevice &scrollingDevice);
+    void xi2UpdateScrollingDevice(ScrollingDevice &scrollingDevice);
+    ScrollingDevice *scrollingDeviceForId(int id);
+#endif
 
     static bool xi2GetValuatorValueIfSet(const void *event, int valuatorNum, double *value);
     static void xi2PrepareXIGenericDeviceEvent(xcb_ge_event_t *event);

src/plugins/platforms/xcb/qxcbconnection_xi2.cpp

@@ -526,9 +526,8 @@ void QXcbConnection::xi2HandleEvent(xcb_ge_event_t *event)
 #endif // QT_CONFIG(tabletevent)
 
 #ifdef XCB_USE_XINPUT21
-    QHash<int, ScrollingDevice>::iterator device = m_scrollingDevices.find(sourceDeviceId);
+    if (ScrollingDevice *device = scrollingDeviceForId(sourceDeviceId))
-    if (device != m_scrollingDevices.end())
+        xi2HandleScrollEvent(xiEvent, *device);
-        xi2HandleScrollEvent(xiEvent, device.value());
 #endif // XCB_USE_XINPUT21
 
 #ifdef XCB_USE_XINPUT22
@@ -828,42 +827,39 @@ void QXcbConnection::xi2HandleHierachyEvent(void *event)
 void QXcbConnection::xi2HandleDeviceChangedEvent(void *event)
 {
     xXIDeviceChangedEvent *xiEvent = reinterpret_cast<xXIDeviceChangedEvent *>(event);
-
+    switch (xiEvent->reason) {
-    // ### If a slave device changes (XIDeviceChange), we should probably run setup on it again.
+    case XIDeviceChange:
-    if (xiEvent->reason != XISlaveSwitch)
+        break;
-        return;
+    case XISlaveSwitch: {
-
 #ifdef XCB_USE_XINPUT21
-    // This code handles broken scrolling device drivers that reset absolute positions
+        if (ScrollingDevice *scrollingDevice = scrollingDeviceForId(xiEvent->sourceid))
-    // when they are made active. Whenever a new slave device is made active the
+            xi2UpdateScrollingDevice(*scrollingDevice);
-    // primary pointer sends a DeviceChanged event with XISlaveSwitch, and the new
-    // active slave in sourceid.
-
-    QHash<int, ScrollingDevice>::iterator device = m_scrollingDevices.find(xiEvent->sourceid);
-    if (device == m_scrollingDevices.end())
-        return;
-
-    int nrDevices = 0;
-    XIDeviceInfo* xiDeviceInfo = XIQueryDevice(static_cast<Display *>(m_xlib_display), xiEvent->sourceid, &nrDevices);
-    if (nrDevices <= 0) {
-        qCDebug(lcQpaXInputDevices, "scrolling device %d no longer present", xiEvent->sourceid);
-        return;
-    }
-    updateScrollingDevice(*device, xiDeviceInfo->num_classes, xiDeviceInfo->classes);
-    XIFreeDeviceInfo(xiDeviceInfo);
 #endif
+        break;
+    }
+    default:
+        qCDebug(lcQpaXInputEvents, "unknown device-changed-event (device %d)", xiEvent->sourceid);
+        break;
+    }
 }
 
-void QXcbConnection::updateScrollingDevice(ScrollingDevice &scrollingDevice, int num_classes, void *classInfo)
-{
 #ifdef XCB_USE_XINPUT21
-    XIAnyClassInfo **classes = reinterpret_cast<XIAnyClassInfo**>(classInfo);
+void QXcbConnection::xi2UpdateScrollingDevice(ScrollingDevice &scrollingDevice)
+{
+    int nrDevices = 0;
+    Display *dpy = static_cast<Display *>(m_xlib_display);
+    XIDeviceInfo* deviceInfo = XIQueryDevice(dpy, scrollingDevice.deviceId, &nrDevices);
+    if (nrDevices <= 0) {
+        qCDebug(lcQpaXInputDevices, "scrolling device %d no longer present", scrollingDevice.deviceId);
+        return;
+    }
     QPointF lastScrollPosition;
     if (lcQpaXInput().isDebugEnabled())
         lastScrollPosition = scrollingDevice.lastScrollPosition;
-    for (int c = 0; c < num_classes; ++c) {
+    for (int c = 0; c < deviceInfo->num_classes; ++c) {
-        if (classes[c]->type == XIValuatorClass) {
+        XIAnyClassInfo *classInfo = deviceInfo->classes[c];
-            XIValuatorClassInfo *vci = reinterpret_cast<XIValuatorClassInfo *>(classes[c]);
+        if (classInfo->type == XIValuatorClass) {
+            XIValuatorClassInfo *vci = reinterpret_cast<XIValuatorClassInfo *>(classInfo);
             const int valuatorAtom = qatom(vci->label);
             if (valuatorAtom == QXcbAtom::RelHorizScroll || valuatorAtom == QXcbAtom::RelHorizWheel)
                 scrollingDevice.lastScrollPosition.setX(vci->value);
@@ -876,37 +872,30 @@ void QXcbConnection::updateScrollingDevice(ScrollingDevice &scrollingDevice, int
                 lastScrollPosition.x(), lastScrollPosition.y(),
                 scrollingDevice.lastScrollPosition.x(),
                 scrollingDevice.lastScrollPosition.y());
-#else
+
-    Q_UNUSED(scrollingDevice);
+    XIFreeDeviceInfo(deviceInfo);
-    Q_UNUSED(num_classes);
-    Q_UNUSED(classInfo);
-#endif
 }
 
-#ifdef XCB_USE_XINPUT21
+void QXcbConnection::xi2UpdateScrollingDevices()
-void QXcbConnection::handleEnterEvent()
 {
     QHash<int, ScrollingDevice>::iterator it = m_scrollingDevices.begin();
     const QHash<int, ScrollingDevice>::iterator end = m_scrollingDevices.end();
     while (it != end) {
-        ScrollingDevice& scrollingDevice = it.value();
+        xi2UpdateScrollingDevice(it.value());
-        int nrDevices = 0;
-        XIDeviceInfo* xiDeviceInfo = XIQueryDevice(static_cast<Display *>(m_xlib_display), scrollingDevice.deviceId, &nrDevices);
-        if (nrDevices <= 0) {
-            qCDebug(lcQpaXInputDevices, "scrolling device %d no longer present", scrollingDevice.deviceId);
-            it = m_scrollingDevices.erase(it);
-            continue;
-        }
-        updateScrollingDevice(scrollingDevice, xiDeviceInfo->num_classes, xiDeviceInfo->classes);
-        XIFreeDeviceInfo(xiDeviceInfo);
         ++it;
     }
 }
-#endif
+
+QXcbConnection::ScrollingDevice *QXcbConnection::scrollingDeviceForId(int id)
+{
+    ScrollingDevice *dev = nullptr;
+    if (m_scrollingDevices.contains(id))
+        dev = &m_scrollingDevices[id];
+    return dev;
+}
 
 void QXcbConnection::xi2HandleScrollEvent(void *event, ScrollingDevice &scrollingDevice)
 {
-#ifdef XCB_USE_XINPUT21
     xXIGenericDeviceEvent *xiEvent = reinterpret_cast<xXIGenericDeviceEvent *>(event);
 
     if (xiEvent->evtype == XI_Motion && scrollingDevice.orientations) {
@@ -976,11 +965,8 @@ void QXcbConnection::xi2HandleScrollEvent(void *event, ScrollingDevice &scrollin
             }
         }
     }
-#else
-    Q_UNUSED(event);
-    Q_UNUSED(scrollingDevice);
-#endif // XCB_USE_XINPUT21
 }
+#endif // XCB_USE_XINPUT21
 
 static int xi2ValuatorOffset(const unsigned char *maskPtr, int maskLen, int number)
 {

src/plugins/platforms/xcb/qxcbwindow.cpp

@@ -2288,7 +2288,8 @@ void QXcbWindow::handleEnterNotifyEvent(int event_x, int event_y, int root_x, in
 {
     connection()->setTime(timestamp);
 #ifdef XCB_USE_XINPUT21
-    connection()->handleEnterEvent();
+    // Updates scroll valuators, as user might have done some scrolling outside our X client.
+    connection()->xi2UpdateScrollingDevices();
 #endif
 
     const QPoint global = QPoint(root_x, root_y);