xcb: fix various bugs with _NET_WM_MOVERESIZE

1) After a37785ec7638e7485112b87dd7e767881fecc114 it become
apparent that we don't get mouse release event from X server
when system move/resize ends (because WM is grabbing the pointer).
The old code (before a37785ec) would wrongly deduce mouse move
as mouse release, which is why the issue was not seen before.
The solution is to subscribe to slave device events.

2) This patch also amends 2488f34ecfd68702b5508c50cca3fb8e967ac8ea as
that patch was solving the issue only for 1/3 of the supported DEs.
It worked with KWin, but not with Unity and Gnome. Its worth
noting that it also worked with two other WMs that I tested -
openbox and awesomewm. The way forward is to detect when system
move/resize was started as a result of touch event and let the
QSizeGrip do the move/resize instead of WMs that are known to
have bugs.

With this patch we also need to adjust the event compression algorithm
to not treat all XI_TouchUpdate events equally. For XI_Motion we don't
care if the event that we process comes from a master or a slave device,
so we can process them as equal.

Task-number: QTBUG-68501
Task-number: QTBUG-51385
Task-number: QTBUG-32476
Change-Id: Iab4e79a289d7bc0fe26f7ae2cff7c562f51a3334
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

5 files changed, 129 insertions, 9 deletions

diff --git a/src/plugins/platforms/xcb/qxcbconnection.cpp b/src/plugins/platforms/xcb/qxcbconnection.cpp
index 20414d0525..dbc53abeca 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.cpp
+++ b/src/plugins/platforms/xcb/qxcbconnection.cpp
@@ -586,6 +586,8 @@ QXcbConnection::QXcbConnection(QXcbNativeInterface *nativeInterface, bool canGra
 
     m_setup = xcb_get_setup(xcb_connection());
 
+    m_xdgCurrentDesktop = qgetenv("XDG_CURRENT_DESKTOP").toLower();
+
     initializeAllAtoms();
 
     if (!qEnvironmentVariableIsSet("QT_XCB_NO_MITSHM"))
@@ -1716,7 +1718,8 @@ bool QXcbConnection::compressEvent(xcb_generic_event_t *event, int currentIndex,
                     continue;
                 if (isXIType(next, m_xiOpCode, XI_TouchUpdate)) {
                     xXIDeviceEvent *xiDeviceNextEvent = reinterpret_cast<xXIDeviceEvent *>(next);
-                    if (id == xiDeviceNextEvent->detail % INT_MAX)
+                    if (id == xiDeviceNextEvent->detail % INT_MAX &&
+                        xiDeviceNextEvent->deviceid == xiDeviceEvent->deviceid)
                         return true;
                 }
             }
diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h
index 583659ea81..98f08e6b2c 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.h
+++ b/src/plugins/platforms/xcb/qxcbconnection.h
@@ -514,6 +514,9 @@ public:
     void grabServer();
     void ungrabServer();
 
+    bool isUnity() const { return m_xdgCurrentDesktop == "unity"; }
+    bool isGnome() const { return m_xdgCurrentDesktop == "gnome"; }
+
     QXcbNativeInterface *nativeInterface() const { return m_nativeInterface; }
 
     QXcbSystemTrayTracker *systemTrayTracker() const;
@@ -534,6 +537,7 @@ public:
 #endif
 #ifdef XCB_USE_XINPUT22
     bool startSystemMoveResizeForTouchBegin(xcb_window_t window, const QPoint &point, int corner);
+    void abortSystemMoveResizeForTouch();
     bool isTouchScreen(int id);
 #endif
 #endif
@@ -578,6 +582,7 @@ private:
 
     bool m_xi2Enabled = false;
 #if QT_CONFIG(xinput2)
+    QVector<int> m_floatingSlaveDevices;
     int m_xi2Minor = -1;
     void initializeXInput2();
     void xi2SetupDevice(void *info, bool removeExisting = true);
@@ -736,6 +741,8 @@ private:
     bool m_peekerIndexCacheDirty = false;
     QHash<qint32, qint32> m_peekerToCachedIndex;
     friend class QXcbEventReader;
+
+    QByteArray m_xdgCurrentDesktop;
 };
 #if QT_CONFIG(xinput2)
 #if QT_CONFIG(tabletevent)
diff --git a/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp b/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp
index e6e0ee988c..9a250b875f 100644
--- a/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp
+++ b/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp
@@ -127,7 +127,7 @@ void QXcbConnection::xi2SelectDeviceEvents(xcb_window_t window)
     XIEventMask mask;
     mask.mask_len = sizeof(bitMask);
     mask.mask = xiBitMask;
-    mask.deviceid = XIAllMasterDevices;
+    mask.deviceid = XIAllDevices;
     Display *dpy = static_cast<Display *>(m_xlib_display);
     Status result = XISelectEvents(dpy, window, &mask, 1);
     if (result == Success)
@@ -315,6 +315,7 @@ void QXcbConnection::xi2SetupDevices()
 #endif
     m_scrollingDevices.clear();
     m_touchDevices.clear();
+    m_floatingSlaveDevices.clear();
 
     Display *xDisplay = static_cast<Display *>(m_xlib_display);
     int deviceCount = 0;
@@ -322,6 +323,10 @@ void QXcbConnection::xi2SetupDevices()
     m_xiMasterPointerIds.clear();
     for (int i = 0; i < deviceCount; ++i) {
         XIDeviceInfo deviceInfo = devices[i];
+        if (deviceInfo.use == XIFloatingSlave) {
+            m_floatingSlaveDevices.append(deviceInfo.deviceid);
+            continue;
+        }
         if (deviceInfo.use == XIMasterPointer) {
             m_xiMasterPointerIds.append(deviceInfo.deviceid);
             continue;
@@ -550,6 +555,72 @@ static inline qreal fixed1616ToReal(FP1616 val)
 }
 #endif // defined(XCB_USE_XINPUT21) || QT_CONFIG(tabletevent)
 
+namespace {
+
+/*! \internal
+
+    Qt listens for XIAllDevices to avoid losing mouse events. This function
+    ensures that we don't process the same event twice: from a slave device and
+    then again from a master device.
+
+    In a normal use case (e.g. mouse press and release inside a window), we will
+    drop events from master devices as duplicates. Other advantage of processing
+    events from slave devices is that they don't share button state. All buttons
+    on a master device share the state.
+
+    Examples of special cases:
+
+    - During system move/resize, window manager (_NET_WM_MOVERESIZE) grabs the
+      master pointer, in this case we process the matching release from the slave
+      device. A master device event is not sent by the server, hence no duplicate
+      event to drop. If we listened for XIAllMasterDevices instead, we would never
+      see a release event in this case.
+
+    - If we dismiss a context menu by clicking somewhere outside a Qt application,
+      we will process the mouse press from the master pointer as that is the
+      device we are grabbing. We are not grabbing slave devices (grabbing on the
+      slave device is buggy according to 19d289ab1b5bde3e136765e5432b5c7d004df3a4).
+      And since the event occurs outside our window, the slave device event is
+      not sent to us by the server, hence no duplicate event to drop.
+*/
+bool isDuplicateEvent(xcb_ge_event_t *event)
+{
+    struct qXIEvent {
+        bool isValid = false;
+        uint16_t sourceid;
+        uint8_t evtype;
+        uint32_t detail;
+        int32_t root_x;
+        int32_t root_y;
+    };
+    static qXIEvent lastSeenEvent;
+
+    bool isDuplicate = false;
+    auto xiDeviceEvent = reinterpret_cast<xXIDeviceEvent *>(event);
+    if (lastSeenEvent.isValid) {
+        isDuplicate = lastSeenEvent.sourceid == xiDeviceEvent->sourceid &&
+                      lastSeenEvent.evtype == xiDeviceEvent->evtype &&
+                      lastSeenEvent.detail == xiDeviceEvent->detail &&
+                      lastSeenEvent.root_x == xiDeviceEvent->root_x &&
+                      lastSeenEvent.root_y == xiDeviceEvent->root_y;
+    } else {
+        lastSeenEvent.isValid = true;
+    }
+    lastSeenEvent.sourceid = xiDeviceEvent->sourceid;
+    lastSeenEvent.evtype = xiDeviceEvent->evtype;
+    lastSeenEvent.detail = xiDeviceEvent->detail;
+    lastSeenEvent.root_x = xiDeviceEvent->root_x;
+    lastSeenEvent.root_y = xiDeviceEvent->root_y;
+
+    if (isDuplicate)
+        // This sanity check ensures that special cases like QTBUG-59277 keep working.
+        lastSeenEvent.isValid = false; // An event can be a duplicate only once.
+
+    return isDuplicate;
+}
+
+} // namespace
+
 void QXcbConnection::xi2HandleEvent(xcb_ge_event_t *event)
 {
     xi2PrepareXIGenericDeviceEvent(event);
@@ -559,10 +630,14 @@ void QXcbConnection::xi2HandleEvent(xcb_ge_event_t *event)
     xXIEnterEvent *xiEnterEvent = 0;
     QXcbWindowEventListener *eventListener = 0;
 
+    bool isTouchEvent = true;
     switch (xiEvent->evtype) {
     case XI_ButtonPress:
     case XI_ButtonRelease:
     case XI_Motion:
+        isTouchEvent = false;
+        if (!xi2MouseEventsDisabled() && isDuplicateEvent(event))
+            return;
 #ifdef XCB_USE_XINPUT22
     case XI_TouchBegin:
     case XI_TouchUpdate:
@@ -570,6 +645,18 @@ void QXcbConnection::xi2HandleEvent(xcb_ge_event_t *event)
 #endif
     {
         xiDeviceEvent = reinterpret_cast<xXIDeviceEvent *>(event);
+
+        if (m_floatingSlaveDevices.contains(xiDeviceEvent->sourceid))
+            return; // Not interested in floating slave device events, only in attached slaves.
+
+        bool isSlaveEvent = xiDeviceEvent->deviceid == xiDeviceEvent->sourceid;
+        if (!xi2MouseEventsDisabled() && isTouchEvent && isSlaveEvent) {
+            // For touch events we want events only from master devices, at least
+            // currently there is no apparent reason why we would need to consider
+            // events from slave devices.
+            return;
+        }
+
         eventListener = windowEventListenerFromId(xiDeviceEvent->event);
         sourceDeviceId = xiDeviceEvent->sourceid; // use the actual device id instead of the master
         break;
@@ -845,6 +932,11 @@ bool QXcbConnection::startSystemMoveResizeForTouchBegin(xcb_window_t window, con
     }
     return false;
 }
+
+void QXcbConnection::abortSystemMoveResizeForTouch()
+{
+    m_startSystemMoveResizeInfo.window = XCB_NONE;
+}
 #endif // XCB_USE_XINPUT22
 
 bool QXcbConnection::xi2SetMouseGrabEnabled(xcb_window_t w, bool grab)
diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp
index 50711e75c3..8c096ddc74 100644
--- a/src/plugins/platforms/xcb/qxcbwindow.cpp
+++ b/src/plugins/platforms/xcb/qxcbwindow.cpp
@@ -2635,15 +2635,34 @@ bool QXcbWindow::startSystemMoveResize(const QPoint &pos, int corner)
     const xcb_atom_t moveResize = connection()->atom(QXcbAtom::_NET_WM_MOVERESIZE);
     if (!connection()->wmSupport()->isSupportedByWM(moveResize))
         return false;
+
     const QPoint globalPos = QHighDpi::toNativePixels(window()->mapToGlobal(pos), window()->screen());
+    bool startedByTouch = false;
 #ifdef XCB_USE_XINPUT22
-    if (connection()->startSystemMoveResizeForTouchBegin(m_window, globalPos, corner))
-        return true;
+    // ### FIXME QTBUG-53389
+    startedByTouch = connection()->startSystemMoveResizeForTouchBegin(m_window, globalPos, corner);
 #endif
-    return doStartSystemMoveResize(globalPos, corner);
-}
+    if (startedByTouch) {
+        if (connection()->isUnity() || connection()->isGnome()) {
+            // These desktops fail to move/resize via _NET_WM_MOVERESIZE (WM bug?).
+            connection()->abortSystemMoveResizeForTouch();
+            return false;
+        }
+        // KWin, Openbox, AwesomeWM have been tested to work with _NET_WM_MOVERESIZE.
+    } else { // Started by mouse press.
+        if (!connection()->hasXInput2() || connection()->xi2MouseEventsDisabled()) {
+            // Without XI2 we can't get button press/move/release events.
+            return false;
+        }
+        if (connection()->isUnity())
+            return false; // _NET_WM_MOVERESIZE on this WM is bouncy (WM bug?).
+
+        doStartSystemMoveResize(globalPos, corner);
+    }
 
-bool QXcbWindow::doStartSystemMoveResize(const QPoint &globalPos, int corner)
+    return true;
+}
+void QXcbWindow::doStartSystemMoveResize(const QPoint &globalPos, int corner)
 {
     const xcb_atom_t moveResize = connection()->atom(QXcbAtom::_NET_WM_MOVERESIZE);
     xcb_client_message_event_t xev;
@@ -2670,7 +2689,6 @@ bool QXcbWindow::doStartSystemMoveResize(const QPoint &globalPos, int corner)
     xcb_send_event(connection()->xcb_connection(), false, xcbScreen()->root(),
                    XCB_EVENT_MASK_SUBSTRUCTURE_REDIRECT | XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY,
                    (const char *)&xev);
-    return true;
 }
 
 // Sends an XEmbed message.
diff --git a/src/plugins/platforms/xcb/qxcbwindow.h b/src/plugins/platforms/xcb/qxcbwindow.h
index cb8af9e666..c4f49c593c 100644
--- a/src/plugins/platforms/xcb/qxcbwindow.h
+++ b/src/plugins/platforms/xcb/qxcbwindow.h
@@ -177,7 +177,7 @@ public:
     QXcbScreen *xcbScreen() const;
 
     bool startSystemMoveResize(const QPoint &pos, int corner);
-    bool doStartSystemMoveResize(const QPoint &globalPos, int corner);
+    void doStartSystemMoveResize(const QPoint &globalPos, int corner);
 
     virtual void create();
     virtual void destroy();