Client: Refactor touch handling and fix various bugs

Rename mTouchPoints to mPendingTouchPoints, to clarify that they're the
accumulated state so far, which will be applied with the wl_touch.frame
event. QWaylandInputDevice::Touch::mPrevTouchPoints is no longer needed
and has been removed.

Fixes the following issues with the old approach:

- touchPointsReleased() only checked mTouchPoints, which was cleared on
touch_frame and populated again on touch_motion and touch_down, which
meant that it could return true even though there were still touch
points left. Leading to the workaround for missing wl_touch.frame events
on Weston being triggered to often.
- Touch focus was cleared on any wl_touch.up event, not just the last
one.
- The order of the touch events was not stable and relied on the order
of the events (QTBUG-77014).

Fixes: QTBUG-77014
Change-Id: Ic3ecdc87e77b0e0276afefd127ad2b965142cbd4
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

src/plugins/platforms/wayland/qwaylandinputdevice.cpp

@@ -874,15 +874,20 @@ void QWaylandInputDevice::Touch::touch_up(uint32_t serial, uint32_t time, int32_
 {
     Q_UNUSED(serial);
     Q_UNUSED(time);
-    mFocus = nullptr;
     mParent->handleTouchPoint(id, 0, 0, Qt::TouchPointReleased);
 
-    // As of Weston 1.5.90 there is no touch_frame after the last touch_up
+    if (allTouchPointsReleased()) {
-    // (i.e. when the last finger is released). To accommodate for this, issue a
+        mFocus = nullptr;
-    // touch_frame. This cannot hurt since it is safe to call the touch_frame
+
-    // handler multiple times when there are no points left.
+        // As of Weston 7.0.0 there is no touch_frame after the last touch_up
-    if (allTouchPointsReleased())
+        // (i.e. when the last finger is released). To accommodate for this, issue a
+        // touch_frame. This cannot hurt since it is safe to call the touch_frame
+        // handler multiple times when there are no points left.
+        // See: https://gitlab.freedesktop.org/wayland/weston/issues/44
+        // TODO: change logging category to lcQpaWaylandInput in newer versions.
+        qCDebug(lcQpaWayland, "Generating fake frame event to work around Weston bug");
         touch_frame();
+    }
 }
 
 void QWaylandInputDevice::Touch::touch_motion(uint32_t time, int32_t id, wl_fixed_t x, wl_fixed_t y)
@@ -893,8 +898,7 @@ void QWaylandInputDevice::Touch::touch_motion(uint32_t time, int32_t id, wl_fixe
 
 void QWaylandInputDevice::Touch::touch_cancel()
 {
-    mPrevTouchPoints.clear();
+    mPendingTouchPoints.clear();
-    mTouchPoints.clear();
 
     QWaylandTouchExtension *touchExt = mParent->mQDisplay->touchExtension();
     if (touchExt)
@@ -905,19 +909,16 @@ void QWaylandInputDevice::Touch::touch_cancel()
 
 void QWaylandInputDevice::handleTouchPoint(int id, double x, double y, Qt::TouchPointState state)
 {
-    QWindowSystemInterface::TouchPoint tp;
+    auto end = mTouch->mPendingTouchPoints.end();
+    auto it = std::find_if(mTouch->mPendingTouchPoints.begin(), end, [id](auto tp){ return tp.id == id; });
+    if (it == end) {
+        it = mTouch->mPendingTouchPoints.insert(end, QWindowSystemInterface::TouchPoint());
+        it->id = id;
+    }
+    QWindowSystemInterface::TouchPoint &tp = *it;
 
-    // Find out the coordinates for Released events.
+    // Only moved and pressed needs to update/set position
-    bool coordsOk = false;
+    if (state == Qt::TouchPointMoved || state == Qt::TouchPointPressed) {
-    if (state == Qt::TouchPointReleased)
-        for (int i = 0; i < mTouch->mPrevTouchPoints.count(); ++i)
-            if (mTouch->mPrevTouchPoints.at(i).id == id) {
-                tp.area = mTouch->mPrevTouchPoints.at(i).area;
-                coordsOk = true;
-                break;
-            }
-
-    if (!coordsOk) {
         // x and y are surface relative.
         // We need a global (screen) position.
         QWaylandWindow *win = mTouch->mFocus;
@@ -936,59 +937,37 @@ void QWaylandInputDevice::handleTouchPoint(int id, double x, double y, Qt::Touch
     }
 
     tp.state = state;
-    tp.id = id;
     tp.pressure = tp.state == Qt::TouchPointReleased ? 0 : 1;
-    mTouch->mTouchPoints.append(tp);
 }
 
 bool QWaylandInputDevice::Touch::allTouchPointsReleased()
 {
-    for (int i = 0; i < mTouchPoints.count(); ++i)
+    for (const auto &tp : qAsConst(mPendingTouchPoints)) {
-        if (mTouchPoints.at(i).state != Qt::TouchPointReleased)
+        if (tp.state != Qt::TouchPointReleased)
             return false;
-
+    }
     return true;
 }
 
 void QWaylandInputDevice::Touch::releasePoints()
 {
-    Q_FOREACH (const QWindowSystemInterface::TouchPoint &previousPoint, mPrevTouchPoints) {
+    if (mPendingTouchPoints.empty())
-        QWindowSystemInterface::TouchPoint tp = previousPoint;
+        return;
+
+    for (QWindowSystemInterface::TouchPoint &tp : mPendingTouchPoints)
         tp.state = Qt::TouchPointReleased;
-        mTouchPoints.append(tp);
+
-    }
     touch_frame();
 }
 
 void QWaylandInputDevice::Touch::touch_frame()
 {
-    // Copy all points, that are in the previous but not in the current list, as stationary.
+    // TODO: early return if no events?
-    for (int i = 0; i < mPrevTouchPoints.count(); ++i) {
-        const QWindowSystemInterface::TouchPoint &prevPoint(mPrevTouchPoints.at(i));
-        if (prevPoint.state == Qt::TouchPointReleased)
-            continue;
-        bool found = false;
-        for (int j = 0; j < mTouchPoints.count(); ++j)
-            if (mTouchPoints.at(j).id == prevPoint.id) {
-                found = true;
-                break;
-            }
-        if (!found) {
-            QWindowSystemInterface::TouchPoint p = prevPoint;
-            p.state = Qt::TouchPointStationary;
-            mTouchPoints.append(p);
-        }
-    }
-
-    if (mTouchPoints.isEmpty()) {
-        mPrevTouchPoints.clear();
-        return;
-    }
 
     QWindow *window = mFocus ? mFocus->window() : nullptr;
 
     if (mFocus) {
-        const QWindowSystemInterface::TouchPoint &tp = mTouchPoints.last();
+        const QWindowSystemInterface::TouchPoint &tp = mPendingTouchPoints.last();
         // When the touch event is received, the global pos is calculated with the margins
         // in mind. Now we need to adjust again to get the correct local pos back.
         QMargins margins = window->frameMargins();
@@ -997,14 +976,21 @@ void QWaylandInputDevice::Touch::touch_frame()
         if (mFocus->touchDragDecoration(mParent, localPos, tp.area.center(), tp.state, mParent->modifiers()))
             return;
     }
-    QWindowSystemInterface::handleTouchEvent(window, mParent->mTouchDevice, mTouchPoints);
 
-    if (allTouchPointsReleased())
+    QWindowSystemInterface::handleTouchEvent(window, mParent->mTouchDevice, mPendingTouchPoints);
-        mPrevTouchPoints.clear();
+
-    else
+    // Prepare state for next frame
-        mPrevTouchPoints = mTouchPoints;
+    const auto prevTouchPoints = mPendingTouchPoints;
+    mPendingTouchPoints.clear();
+    for (const auto &prevPoint: prevTouchPoints) {
+        // All non-released touch points should be part of the next touch event
+        if (prevPoint.state != Qt::TouchPointReleased) {
+            QWindowSystemInterface::TouchPoint tp = prevPoint;
+            tp.state = Qt::TouchPointStationary; // ... as stationary (unless proven otherwise)
+            mPendingTouchPoints.append(tp);
+        }
+    }
 
-    mTouchPoints.clear();
 }
 
 }

src/plugins/platforms/wayland/qwaylandinputdevice_p.h

@@ -304,8 +304,7 @@ public:
 
     QWaylandInputDevice *mParent = nullptr;
     QPointer<QWaylandWindow> mFocus;
-    QList<QWindowSystemInterface::TouchPoint> mTouchPoints;
+    QList<QWindowSystemInterface::TouchPoint> mPendingTouchPoints;
-    QList<QWindowSystemInterface::TouchPoint> mPrevTouchPoints;
 };
 
 class QWaylandPointerEvent