don't re-deliver events to an item which already filtered

That is, if the filtering parent intercepts an event (returns true
from childMouseEventFilter), it does not also need direct delivery of
the same event.

Change-Id: I24003f72875b309fa10b2d316916c5f86702cb57
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>

3 files changed, 17 insertions, 15 deletions

diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp
index 023fa6f9f0..bd1a5076fd 100644
--- a/src/quick/items/qquickwindow.cpp
+++ b/src/quick/items/qquickwindow.cpp
@@ -2232,6 +2232,7 @@ void QQuickWindowPrivate::deliverPointerEvent(QQuickPointerEvent *event)
     // the usecase a bit evil, but we at least don't want to lose events.
     ++pointerEventRecursionGuard;
 
+    skipDelivery.clear();
     if (event->asPointerMouseEvent()) {
         deliverMouseEvent(event->asPointerMouseEvent());
     } else if (event->asPointerTouchEvent()) {
@@ -2426,7 +2427,6 @@ bool QQuickWindowPrivate::deliverPressOrReleaseEvent(QQuickPointerEvent *event,
         }
     }
 
-    QVarLengthArray<QQuickItem *> filteredItems;
     for (QQuickItem *item : targetItems) {
         if (!handlersOnly && sendFilteredPointerEvent(event, item)) {
             if (event->isAccepted()) {
@@ -2434,15 +2434,12 @@ bool QQuickWindowPrivate::deliverPressOrReleaseEvent(QQuickPointerEvent *event,
                     event->point(i)->setAccepted();
                 return true;
             }
-            filteredItems << item;
+            skipDelivery.append(item);
         }
 
-        // Do not deliverMatchingPointsTo any item for which the parent-filter already intercepted the event
-        if (filteredItems.contains(item))
-            continue;
-        // Do not deliverMatchingPointsTo any item which already had a chance to filter
-        // e.g. if Flickable has filtered events from one of its children, it does not need normal delivery
-        if (hasFiltered.contains(item))
+        // Do not deliverMatchingPointsTo any item for which the filtering parent already intercepted the event,
+        // nor to any item which already had a chance to filter.
+        if (skipDelivery.contains(item))
             continue;
         deliverMatchingPointsToItem(item, event, handlersOnly);
         if (event->allPointsAccepted())
@@ -2743,8 +2740,11 @@ bool QQuickWindowPrivate::sendFilteredPointerEventImpl(QQuickPointerEvent *event
             if (receiver->acceptedMouseButtons()) {
                 QPointF localPos = receiver->mapFromScene(pme->point(0)->scenePosition());
                 QMouseEvent *me = pme->asMouseEvent(localPos);
-                if (filteringParent->childMouseEventFilter(receiver, me))
+                if (filteringParent->childMouseEventFilter(receiver, me)) {
+                    qCDebug(DBG_MOUSE) << "mouse event intercepted by childMouseEventFilter of " << filteringParent;
+                    skipDelivery.append(filteringParent);
                     filtered = true;
+                }
             }
         } else if (QQuickPointerTouchEvent *pte = event->asPointerTouchEvent()) {
 #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
@@ -2760,6 +2760,7 @@ bool QQuickWindowPrivate::sendFilteredPointerEventImpl(QQuickPointerEvent *event
                     QVarLengthArray<QPair<QQuickPointerHandler *, QQuickEventPoint *>, 32> passiveGrabsToCancel;
                     if (filteringParent->childMouseEventFilter(receiver, filteringParentTouchEvent.data())) {
                         qCDebug(DBG_TOUCH) << "touch event intercepted by childMouseEventFilter of " << filteringParent;
+                        skipDelivery.append(filteringParent);
                         filteringParent->grabMouse();
                         for (auto point: qAsConst(filteringParentTouchEvent->touchPoints())) {
                             auto pointerEventPoint = pte->pointById(point.id());
@@ -2804,6 +2805,7 @@ bool QQuickWindowPrivate::sendFilteredPointerEventImpl(QQuickPointerEvent *event
                                 touchMouseDevice = event->device();
                                 if (filteringParent->childMouseEventFilter(receiver, mouseEvent.data())) {
                                     qCDebug(DBG_TOUCH) << "touch event intercepted as synth mouse event by childMouseEventFilter of " << filteringParent;
+                                    skipDelivery.append(filteringParent);
                                     if (t != QEvent::MouseButtonRelease) {
                                         qCDebug(DBG_TOUCH_TARGET) << "TP (mouse)" << hex << tp.id() << "->" << filteringParent;
                                         pointerEventInstance(touchMouseDevice)->pointById(tp.id())->setGrabberItem(filteringParent);
@@ -2851,8 +2853,10 @@ bool QQuickWindowPrivate::sendFilteredMouseEvent(QEvent *event, QQuickItem *rece
     bool filtered = false;
     if (filteringParentPrivate->filtersChildMouseEvents && !hasFiltered.contains(filteringParent)) {
         hasFiltered.append(filteringParent);
-        if (filteringParent->childMouseEventFilter(receiver, event))
+        if (filteringParent->childMouseEventFilter(receiver, event)) {
             filtered = true;
+            skipDelivery.append(filteringParent);
+        }
         qCDebug(DBG_MOUSE_TARGET) << "for" << receiver << filteringParent << "childMouseEventFilter ->" << filtered;
     }
 
diff --git a/src/quick/items/qquickwindow_p.h b/src/quick/items/qquickwindow_p.h
index 831c8c8d88..0399b26f62 100644
--- a/src/quick/items/qquickwindow_p.h
+++ b/src/quick/items/qquickwindow_p.h
@@ -227,7 +227,8 @@ public:
     QList<QSGNode *> cleanupNodeList;
 
     QVector<QQuickItem *> itemsToPolish;
-    QVector<QQuickItem *> hasFiltered; // during event delivery, the items for which childMouseEventFilter was already called
+    QVector<QQuickItem *> hasFiltered; // during event delivery to a single receiver, the filtering parents for which childMouseEventFilter was already called
+    QVector<QQuickItem *> skipDelivery; // during delivery of one event to all receivers, Items to which we know delivery is no longer necessary
 
     qreal devicePixelRatio;
     QMetaObject::Connection physicalDpiChangedConnection;
diff --git a/tests/auto/quick/qquickwindow/tst_qquickwindow.cpp b/tests/auto/quick/qquickwindow/tst_qquickwindow.cpp
index 0c3e580828..d4b505a74a 100644
--- a/tests/auto/quick/qquickwindow/tst_qquickwindow.cpp
+++ b/tests/auto/quick/qquickwindow/tst_qquickwindow.cpp
@@ -3139,8 +3139,6 @@ void tst_qquickwindow::testChildMouseEventFilter_data()
     QTest::addColumn<InputState>("inputState");
     QTest::addColumn<DeliveryRecordVector>("expectedDeliveryOrder");
 
-    /*
-    // (This is an intended behavior change in progress: I24003f72875b309fa10b2d316916c5f86702cb57)
     QTest::newRow("if filtered and rejected, do not deliver it to the item that filtered it")
         << QPoint(100, 100)
         << InputState({
@@ -3159,7 +3157,6 @@ void tst_qquickwindow::testChildMouseEventFilter_data()
             // DeliveryRecord("r2")         // r2 filtered it -> do not deliver
             << DeliveryRecord("r1")
             );
-    */
 
     QTest::newRow("no filtering, no accepting")
         << QPoint(100, 100)
@@ -3276,7 +3273,7 @@ void tst_qquickwindow::testChildMouseEventFilter_data()
             << DeliveryRecord("r0", "r2")
 //            << DeliveryRecord("r2"   // since it got filtered we don't deliver to r2
             << DeliveryRecord("r0", "r1")
-            << DeliveryRecord("r1")
+//            << DeliveryRecord("r1")  // since it acted as a filter and returned true, we don't deliver to r1
             << DeliveryRecord("r0")
             << DeliveryRecord("root")
             );