Fix a bug with TapHandler+DragHandler on top of Flickable

If an item that had a TapHandler and a DragHandler was placed inside a
Flickable it would call sendFilteredPointerEvent() for each of the
handlers, even if they were siblings.

This is not ideal, and because of a mechanism in flickable it even caused
the DragHandler to not drag the item as expected.
This is because of a mechanism in Flickable where the value returned is
actually derived from the previous call to filterMouseEvent() (stored in
member variable stealGrab).

Below are the relevant steps it went through when the mouse drag exceeded
the drag threshold (mouse pointer started on the item):

Precondition: QQuickFlickablePrivate::stealMouse == false
1. Mouse Drag (which exceeded drag threshold)

2. sendFilteredPointerEvent(tapHandler->parentItem()) returns false.
   (but since it moved beyond threshold, it would set stealGrab-> true).

3. tapHandler->handlePointerEvent() declined and ungrabbed (due to
   DragThreshold gesture policy).

4. sendFilteredPointerEvent(dragHandler->parentItem()) returns true
   (because the previous call to sendFilteredPointerEvent() set stealGrab
   to true).

As a consequence of (4), dragHandler->handlePointerEvent() was not called,
and the flickable would start to flick instead of the item starting to drag.

Change-Id: Ia99eae91cad0903ebbaedaaafcdf92a25705a922
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

1 files changed, 33 insertions, 16 deletions

diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp
index 65d69a3bcf..45c14f10a1 100644
--- a/src/quick/items/qquickwindow.cpp
+++ b/src/quick/items/qquickwindow.cpp
@@ -1669,6 +1669,36 @@ QMouseEvent *QQuickWindowPrivate::cloneMouseEvent(QMouseEvent *event, QPointF *t
     return me;
 }
 
+void QQuickWindowPrivate::deliverToPassiveGrabbers(const QVector<QPointer <QQuickPointerHandler> > &passiveGrabbers,
+                                                   QQuickPointerEvent *pointerEvent)
+{
+    const QVector<QQuickPointerHandler *> &eventDeliveryTargets = pointerEvent->device()->eventDeliveryTargets();
+    QVarLengthArray<QPair<QQuickItem *, bool>, 4> sendFilteredPointerEventResult;
+    for (auto handler : passiveGrabbers) {
+        // a null pointer in passiveGrabbers is unlikely, unless the grabbing handler was deleted dynamically
+        if (Q_LIKELY(handler) && !eventDeliveryTargets.contains(handler)) {
+            bool alreadyFiltered = false;
+            QQuickItem *par = handler->parentItem();
+
+            // see if we already have sent a filter event to the parent
+            auto it = std::find_if(sendFilteredPointerEventResult.begin(), sendFilteredPointerEventResult.end(),
+                                        [par](const QPair<QQuickItem *, bool> &pair) { return pair.first == par; });
+            if (it != sendFilteredPointerEventResult.end()) {
+                // Yes, the event was already filtered to that parent, do not call it again but use
+                // the result of the previous call to determine if we should call the handler.
+                alreadyFiltered = it->second;
+            } else {
+                alreadyFiltered = sendFilteredPointerEvent(pointerEvent, par);
+                sendFilteredPointerEventResult << qMakePair<QQuickItem*, bool>(par, alreadyFiltered);
+            }
+            if (!alreadyFiltered)
+                handler->handlePointerEvent(pointerEvent);
+        }
+    }
+}
+
+
+
 void QQuickWindowPrivate::deliverMouseEvent(QQuickPointerMouseEvent *pointerEvent)
 {
     auto point = pointerEvent->point(0);
@@ -1717,14 +1747,8 @@ void QQuickWindowPrivate::deliverMouseEvent(QQuickPointerMouseEvent *pointerEven
             // if this is an update or release from an actual mouse,
             // and the point wasn't grabbed, deliver only to PointerHandlers:
             // passive grabbers first, then the rest
-            const QVector<QQuickPointerHandler *> &eventDeliveryTargets = pointerEvent->device()->eventDeliveryTargets();
-            for (auto handler : point->passiveGrabbers()) {
-                // a null pointer in passiveGrabbers is unlikely, unless the grabbing handler was deleted dynamically
-                if (Q_LIKELY(handler) && !eventDeliveryTargets.contains(handler)) {
-                    if (!sendFilteredPointerEvent(pointerEvent, handler->parentItem()))
-                        handler->handlePointerEvent(pointerEvent);
-                }
-            }
+            deliverToPassiveGrabbers(point->passiveGrabbers(), pointerEvent);
+
             // If some points weren't grabbed, deliver to non-grabber PointerHandlers in reverse paint order
             if (!pointerEvent->allPointsGrabbed() && pointerEvent->buttons()) {
                 QVector<QQuickItem *> targetItems = pointerTargets(contentItem, point->scenePosition(), false, false);
@@ -2398,16 +2422,9 @@ void QQuickWindowPrivate::deliverUpdatedTouchPoints(QQuickPointerTouchEvent *eve
         int pointCount = event->pointCount();
 
         // Deliver to each eventpoint's passive grabbers (but don't visit any handler more than once)
-        const QVector<QQuickPointerHandler *> &eventDeliveryTargets = event->device()->eventDeliveryTargets();
         for (int i = 0; i < pointCount; ++i) {
             QQuickEventPoint *point = event->point(i);
-            for (auto handler : point->passiveGrabbers()) {
-                if (Q_LIKELY(handler) && !eventDeliveryTargets.contains(handler)) {
-                    if (sendFilteredPointerEvent(event, handler->parentItem()))
-                        return;
-                    handler->handlePointerEvent(event);
-                }
-            }
+            deliverToPassiveGrabbers(point->passiveGrabbers(), event);
         }
 
         // If some points weren't grabbed, deliver to non-grabber PointerHandlers in reverse paint order