From 47b3b1b10246cad6709c0cd99f02208dbaf6b7c0 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Mon, 11 Sep 2017 14:10:51 +0200 Subject: don't re-deliver events to an item which already filtered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/quick/items/qquickwindow.cpp | 24 +++++++++++++--------- src/quick/items/qquickwindow_p.h | 3 ++- tests/auto/quick/qquickwindow/tst_qquickwindow.cpp | 5 +---- 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 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, 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 cleanupNodeList; QVector itemsToPolish; - QVector hasFiltered; // during event delivery, the items for which childMouseEventFilter was already called + QVector hasFiltered; // during event delivery to a single receiver, the filtering parents for which childMouseEventFilter was already called + QVector 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"); QTest::addColumn("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") ); -- cgit v1.2.3