Keep track of grabbing inside QQuickEventPoint

With this we can get rid of "itemForTouchPointId" which kept track of
the individual grabbing of touch points. The new approach allows big
cleanups and makes understanding the code easier.

Change-Id: I19f7cda1b430e088ada707b2e75d97ca8dbadcc5
Reviewed-by: Frederik Gladhorn <frederik.gladhorn@qt.io>

src/quick/items/qquickevents.cpp

@@ -481,6 +481,11 @@ QQuickPointerDevice *QQuickPointerDevice::touchDevice(QTouchDevice *d)
     return dev;
 }
 
+QList<QQuickPointerDevice*> QQuickPointerDevice::touchDevices()
+{
+    return g_touchDevices->values();
+}
+
 QQuickPointerDevice *QQuickPointerDevice::genericMouseDevice()
 {
     return g_genericMouseDevice;
@@ -551,13 +556,28 @@ QQuickPointerEvent *QQuickPointerTouchEvent::reset(QEvent *event) {
     m_pressedButtons = Qt::NoButton;
 
     const QList<QTouchEvent::TouchPoint> &tps = ev->touchPoints();
-    m_pointCount = tps.count();
+    int newPointCount = tps.count();
-    m_touchPoints.reserve(m_pointCount);
+    m_touchPoints.reserve(newPointCount);
-    for (int i = m_touchPoints.size(); i < m_pointCount; ++i)
+
+    for (int i = m_touchPoints.size(); i < newPointCount; ++i)
         m_touchPoints.insert(i, new QQuickEventTouchPoint(this));
 
-    for (int i = 0; i < m_pointCount; ++i)
+    // Make sure the grabbers are right from one event to the next
+    QVector<QQuickItem*> grabbers;
+    // Copy all grabbers, because the order of points might have changed in the event.
+    // The ID is all that we can rely on (release might remove the first point etc).
+    for (int i = 0; i < newPointCount; ++i) {
+        QQuickItem *grabber = nullptr;
+        if (auto point = pointById(tps.at(i).id()))
+            grabber = point->grabber();
+        grabbers.append(grabber);
+    }
+
+    for (int i = 0; i < newPointCount; ++i) {
         m_touchPoints.at(i)->reset(tps.at(i), ev->timestamp());
+        m_touchPoints.at(i)->setGrabber(grabbers.at(i));
+    }
+    m_pointCount = newPointCount;
     return this;
 }
 
@@ -602,6 +622,10 @@ QVector<QQuickItem *> QQuickPointerMouseEvent::grabbers() const
     return result;
 }
 
+void QQuickPointerMouseEvent::clearGrabbers() const {
+    m_mousePoint->setGrabber(nullptr);
+}
+
 bool QQuickPointerTouchEvent::allPointsAccepted() const {
     for (int i = 0; i < m_pointCount; ++i) {
         if (!m_touchPoints.at(i)->isAccepted())
@@ -623,6 +647,11 @@ QVector<QQuickItem *> QQuickPointerTouchEvent::grabbers() const
     return result;
 }
 
+void QQuickPointerTouchEvent::clearGrabbers() const {
+    for (int i = 0; i < pointCount(); ++i)
+        point(i)->setGrabber(nullptr);
+}
+
 /*!
     \internal
     Populate the reusable synth-mouse event from one touchpoint.

src/quick/items/qquickevents_p_p.h

@@ -365,6 +365,7 @@ public: // helpers for C++ only (during event delivery)
     virtual QQuickEventPoint *point(int i) const = 0;
     virtual QQuickEventPoint *pointById(quint64 pointId) const = 0;
     virtual QVector<QQuickItem *> grabbers() const = 0;
+    virtual void clearGrabbers() const = 0;
 
 protected:
 
@@ -393,6 +394,7 @@ public:
     QQuickEventPoint *pointById(quint64 pointId) const override;
     bool allPointsAccepted() const override;
     QVector<QQuickItem *> grabbers() const override;
+    void clearGrabbers() const override;
 
     QMouseEvent *asMouseEvent() const;
 
@@ -416,6 +418,7 @@ public:
     const QTouchEvent::TouchPoint *touchPointById(int pointId) const;
     bool allPointsAccepted() const override;
     QVector<QQuickItem *> grabbers() const override;
+    void clearGrabbers() const override;
 
     QMouseEvent *syntheticMouseEvent(int pointID, QQuickItem *relativeTo) const;
     QTouchEvent *touchEventForItem(const QList<const QQuickEventPoint *> &newPoints, QQuickItem *relativeTo) const;
@@ -507,6 +510,7 @@ public:
     QQuickPointerEvent *pointerEvent() const { return m_event; }
 
     static QQuickPointerDevice *touchDevice(QTouchDevice *d);
+    static QList<QQuickPointerDevice *> touchDevices();
     static QQuickPointerDevice *genericMouseDevice();
     static QQuickPointerDevice *tabletDevice(qint64);
 

src/quick/items/qquickwindow.cpp

@@ -624,6 +624,7 @@ bool QQuickWindowPrivate::checkIfDoubleClicked(ulong newPressEventTimestamp)
 bool QQuickWindowPrivate::translateTouchToMouse(QQuickItem *item, QTouchEvent *event)
 {
     Q_Q(QQuickWindow);
+    auto device = QQuickPointerDevice::touchDevice(event->device());
 
     // For each point, check if it is accepted, if not, try the next point.
     // Any of the fingers can become the mouse one.
@@ -642,7 +643,10 @@ bool QQuickWindowPrivate::translateTouchToMouse(QQuickItem *item, QTouchEvent *e
             // accepted. Cannot defer setting the new value because otherwise if the event
             // handler spins the event loop all subsequent moves and releases get lost.
             touchMouseId = p.id();
-            itemForTouchPointId[touchMouseId] = item;
+
+            // FIXME: this is a bit backwards, should just have the pointer event passed into the function
+            auto pointerEventPoint = device->pointerEvent()->pointById(touchMouseId);
+            pointerEventPoint->setGrabber(item);
             qCDebug(DBG_TOUCH_TARGET) << "TP (mouse)" << p.id() << "->" << item;
             QScopedPointer<QMouseEvent> mousePress(touchToMouseEvent(QEvent::MouseButtonPress, p, event, item, false));
 
@@ -655,9 +659,9 @@ bool QQuickWindowPrivate::translateTouchToMouse(QQuickItem *item, QTouchEvent *e
             event->setAccepted(mousePress->isAccepted());
             if (!mousePress->isAccepted()) {
                 touchMouseId = -1;
-                if (itemForTouchPointId.value(p.id()) == item) {
+                if (pointerEventPoint->grabber() == item) {
                     qCDebug(DBG_TOUCH_TARGET) << "TP (mouse)" << p.id() << "disassociated";
-                    itemForTouchPointId.remove(p.id());
+                    pointerEventPoint->setGrabber(nullptr);
                 }
 
                 if (q->mouseGrabberItem() == item)
@@ -694,7 +698,8 @@ bool QQuickWindowPrivate::translateTouchToMouse(QQuickItem *item, QTouchEvent *e
                     event->setAccepted(me->isAccepted());
                     if (me->isAccepted()) {
                         qCDebug(DBG_TOUCH_TARGET) << "TP (mouse)" << p.id() << "->" << mouseGrabberItem;
-                        itemForTouchPointId[p.id()] = q->mouseGrabberItem(); // N.B. the mouseGrabberItem may be different after returning from sendEvent()
+                        auto pointerEventPoint = device->pointerEvent()->pointById(p.id());
+                        pointerEventPoint->setGrabber(q->mouseGrabberItem()); // N.B. the mouseGrabberItem may be different after returning from sendEvent()
                         return true;
                     }
                 } else {
@@ -758,7 +763,12 @@ void QQuickWindowPrivate::setMouseGrabber(QQuickItem *grabber)
     if (grabber && touchMouseId != -1) {
         // update the touch item for mouse touch id to the new grabber
         qCDebug(DBG_TOUCH_TARGET) << "TP (mouse)" << touchMouseId << "->" << q->mouseGrabberItem();
-        itemForTouchPointId[touchMouseId] = grabber;
+        // FIXME: it is unclear which touch device the mouse grab is for
+        for (QQuickPointerDevice *touchDevice: QQuickPointerDevice::touchDevices()) {
+            auto point = touchDevice->pointerEvent()->pointById(touchMouseId);
+            if (point)
+                point->setGrabber(grabber);
+        }
     }
 
     if (oldGrabber) {
@@ -772,13 +782,19 @@ void QQuickWindowPrivate::grabTouchPoints(QQuickItem *grabber, const QVector<int
     Q_Q(QQuickWindow);
     QSet<QQuickItem*> ungrab;
     for (int i = 0; i < ids.count(); ++i) {
-        QQuickItem *oldGrabber = itemForTouchPointId.value(ids.at(i));
+        // FIXME: deprecate this function, we need a device
-        if (oldGrabber == grabber)
+        for (auto device: QQuickPointerDevice::touchDevices()) {
-            continue;
+            auto point = device->pointerEvent()->pointById(ids.at(i));
+            if (!point)
+                continue;
+            QQuickItem *oldGrabber = point->grabber();
+            if (oldGrabber == grabber)
+                continue;
 
-        itemForTouchPointId[ids.at(i)] = grabber;
+            point->setGrabber(grabber);
-        if (oldGrabber)
+            if (oldGrabber)
-            ungrab.insert(oldGrabber);
+                ungrab.insert(oldGrabber);
+        }
 
         QQuickItem *mouseGrabberItem = q->mouseGrabberItem();
         if (touchMouseId == ids.at(i) && mouseGrabberItem && mouseGrabberItem != grabber) {
@@ -794,11 +810,14 @@ void QQuickWindowPrivate::removeGrabber(QQuickItem *grabber, bool mouse, bool to
 {
     Q_Q(QQuickWindow);
     if (Q_LIKELY(touch)) {
-        QMutableHashIterator<int, QQuickItem *> itemTouchMapIt(itemForTouchPointId);
+        for (auto device: QQuickPointerDevice::touchDevices()) {
-        while (itemTouchMapIt.hasNext()) {
+            auto pointerEvent = device->pointerEvent();
-            if (itemTouchMapIt.next().value() == grabber) {
+            for (int i = 0; i < pointerEvent->pointCount(); ++i) {
-                itemTouchMapIt.remove();
+                if (pointerEvent->point(i)->grabber() == grabber) {
-                grabber->touchUngrabEvent();
+                    pointerEvent->point(i)->setGrabber(nullptr);
+                    // FIXME send ungrab event only once
+                    grabber->touchUngrabEvent();
+                }
             }
         }
     }
@@ -1864,20 +1883,21 @@ bool QQuickWindowPrivate::deliverTouchCancelEvent(QTouchEvent *event)
 {
     qCDebug(DBG_TOUCH) << event;
     Q_Q(QQuickWindow);
+
     // A TouchCancel event will typically not contain any points.
     // Deliver it to all items that have active touches.
-    QSet<QQuickItem *> cancelDelivered;
+    QQuickPointerEvent *pointerEvent = QQuickPointerDevice::touchDevice(event->device())->pointerEvent();
-    foreach (QQuickItem *item, itemForTouchPointId) {
+    QVector<QQuickItem *> grabbers = pointerEvent->grabbers();
-        if (cancelDelivered.contains(item))
+
-            continue;
+    for (QQuickItem *grabber: qAsConst(grabbers)) {
-        cancelDelivered.insert(item);
+        q->sendEvent(grabber, event);
-        q->sendEvent(item, event);
     }
     touchMouseId = -1;
     if (q->mouseGrabberItem())
         q->mouseGrabberItem()->ungrabMouse();
+
     // The next touch event can only be a TouchBegin so clean up.
-    itemForTouchPointId.clear();
+    pointerEvent->clearGrabbers();
     return true;
 }
 
@@ -2194,11 +2214,8 @@ void QQuickWindowPrivate::deliverTouchEvent(QQuickPointerEvent *event)
             // are also receiving touch points with some other state.
             // But we have not yet decided which points go to which item,
             // so for now we must include all non-new points in updatedPoints.
-            if (itemForTouchPointId.contains(point->pointId())) {
+            if (QQuickItem *grabber = point->grabber())
-                QQuickItem *item = itemForTouchPointId.value(point->pointId());
+                updatedPoints[grabber].append(point);
-                if (item)
-                    updatedPoints[item].append(point);
-            }
         }
     }
 
@@ -2213,24 +2230,23 @@ void QQuickWindowPrivate::deliverTouchEvent(QQuickPointerEvent *event)
     // Remove released points from itemForTouchPointId
     bool allReleased = true;
     for (int i = 0; i < pointCount; ++i) {
-        const QQuickEventPoint *point = event->point(i);
+        QQuickEventPoint *point = event->point(i);
-        int id = point->pointId();
         if (point->state() == Qt::TouchPointReleased) {
+            int id = point->pointId();
             qCDebug(DBG_TOUCH_TARGET) << "TP" << id << "released";
-            itemForTouchPointId.remove(id);
+            point->setGrabber(nullptr);
-            if (id == touchMouseId)
+            if (id == touchMouseId) {
                 touchMouseId = -1;
+            }
             touchMouseIdCandidates.remove(id);
         } else {
             allReleased = false;
         }
     }
 
-    if (allReleased && !itemForTouchPointId.isEmpty()) {
+    if (allReleased && !event->grabbers().isEmpty()) {
-        qWarning() << "No release received for" << itemForTouchPointId.size()
+        qWarning() << "No release received for some grabbers" << event->grabbers();
-            << "touch points over" << itemForTouchPointId.begin().value()
+        event->clearGrabbers();
-            << "on touch end.";
-        itemForTouchPointId.clear();
     }
 }
 
@@ -2346,7 +2362,7 @@ bool QQuickWindowPrivate::deliverMatchingPointsToItem(QQuickItem *item, const QQ
     // Since it can change in sendEvent, update itemForTouchPointId now
     foreach (int id, matchingNewPoints) {
         qCDebug(DBG_TOUCH_TARGET) << "TP" << id << "->" << item;
-        itemForTouchPointId[id] = item;
+        event->pointById(id)->setGrabber(item);
     }
 
     // Deliver the touch event to the given item
@@ -2371,9 +2387,9 @@ bool QQuickWindowPrivate::deliverMatchingPointsToItem(QQuickItem *item, const QQ
         // But if the event was not accepted then we know this item
         // will not be interested in further updates for those touchpoint IDs either.
         foreach (int id, matchingNewPoints)
-            if (itemForTouchPointId[id] == item) {
+            if (event->pointById(id)->grabber() == item) {
                 qCDebug(DBG_TOUCH_TARGET) << "TP" << id << "disassociated";
-                itemForTouchPointId.remove(id);
+                event->pointById(id)->setGrabber(nullptr);
             }
     }
 
@@ -2675,8 +2691,9 @@ bool QQuickWindowPrivate::sendFilteredTouchEvent(QQuickItem *target, QQuickItem
                         qCDebug(DBG_TOUCH) << " - second chance intercepted on childMouseEventFilter by " << target;
                         if (t != QEvent::MouseButtonRelease) {
                             qCDebug(DBG_TOUCH_TARGET) << "TP" << tp.id() << "->" << target;
-                            itemForTouchPointId[tp.id()] = target;
                             touchMouseId = tp.id();
+                            auto touchMouseDevice = QQuickPointerDevice::touchDevice(event->device());
+                            touchMouseDevice->pointerEvent()->pointById(tp.id())->setGrabber(target);
                             target->grabMouse();
                         }
                         touchMouseIdCandidates.clear();

src/quick/items/qquickwindow_p.h

@@ -266,8 +266,6 @@ public:
 
     QOpenGLVertexArrayObjectHelper *vaoHelper;
 
-    // Keeps track of which touch point (int) was last accepted by which item
-    QHash<int, QQuickItem *> itemForTouchPointId;
     QSet<int> touchMouseIdCandidates;
 
     mutable QQuickWindowIncubationController *incubationController;

tests/auto/quick/touchmouse/tst_touchmouse.cpp

@@ -33,12 +33,12 @@
 
 #include <QtQuick/qquickview.h>
 #include <QtQuick/qquickitem.h>
+#include <QtQuick/private/qquickevents_p_p.h>
 #include <QtQuick/private/qquickmousearea_p.h>
 #include <QtQuick/private/qquickmultipointtoucharea_p.h>
 #include <QtQuick/private/qquickpincharea_p.h>
 #include <QtQuick/private/qquickflickable_p.h>
-
+#include <QtQuick/private/qquickwindow_p.h>
-#include <private/qquickwindow_p.h>
 
 #include <QtQml/qqmlengine.h>
 #include <QtQml/qqmlproperty.h>
@@ -572,7 +572,8 @@ void tst_TouchMouse::buttonOnFlickable()
 
     QQuickWindowPrivate *windowPriv = QQuickWindowPrivate::get(window);
     QCOMPARE(windowPriv->touchMouseId, 0);
-    QCOMPARE(windowPriv->itemForTouchPointId[0], eventItem1);
+    auto pointerEvent = QQuickPointerDevice::touchDevices().at(0)->pointerEvent();
+    QCOMPARE(pointerEvent->point(0)->grabber(), eventItem1);
     QCOMPARE(window->mouseGrabberItem(), eventItem1);
 
     p1 += QPoint(0, -10);
@@ -593,7 +594,7 @@ void tst_TouchMouse::buttonOnFlickable()
 
     QCOMPARE(window->mouseGrabberItem(), flickable);
     QCOMPARE(windowPriv->touchMouseId, 0);
-    QCOMPARE(windowPriv->itemForTouchPointId[0], flickable);
+    QCOMPARE(pointerEvent->point(0)->grabber(), flickable);
     QVERIFY(flickable->isMovingVertically());
 
     QTest::touchEvent(window, device).release(0, p3, window);
@@ -653,11 +654,12 @@ void tst_TouchMouse::buttonOnDelayedPressFlickable()
     QCOMPARE(eventItem1->eventList.at(0).type, QEvent::MouseButtonPress);
     QCOMPARE(filteredEventList.count(), 1);
 
-    // eventItem1 should have the mouse grab, and have moved the itemForTouchPointId
+    // eventItem1 should have the mouse grab, and have moved the grab
     // for the touchMouseId to the new grabber.
     QQuickWindowPrivate *windowPriv = QQuickWindowPrivate::get(window);
     QCOMPARE(windowPriv->touchMouseId, 0);
-    QCOMPARE(windowPriv->itemForTouchPointId[0], eventItem1);
+    auto pointerEvent = QQuickPointerDevice::touchDevices().at(0)->pointerEvent();
+    QCOMPARE(pointerEvent->point(0)->grabber(), eventItem1);
     QCOMPARE(window->mouseGrabberItem(), eventItem1);
 
     p1 += QPoint(0, -10);
@@ -676,7 +678,7 @@ void tst_TouchMouse::buttonOnDelayedPressFlickable()
     // for the touchMouseId to the new grabber.
     QCOMPARE(window->mouseGrabberItem(), flickable);
     QCOMPARE(windowPriv->touchMouseId, 0);
-    QCOMPARE(windowPriv->itemForTouchPointId[0], flickable);
+    QCOMPARE(pointerEvent->point(0)->grabber(), flickable);
 
     QTest::touchEvent(window, device).release(0, p3, window);
     QQuickTouchUtils::flush(window);
@@ -1090,8 +1092,6 @@ void tst_TouchMouse::mouseOnFlickableOnPinch()
     pinchSequence.move(0, p, window).commit();
     QQuickTouchUtils::flush(window);
 
-    QQuickWindowPrivate *windowPriv = QQuickWindowPrivate::get(window);
-    qDebug() << "Mouse Grabber: " << window->mouseGrabberItem() << " itemForTouchPointId: " << windowPriv->itemForTouchPointId;
     QCOMPARE(window->mouseGrabberItem(), flickable);
 
     // Add a second finger, this should lead to stealing