From 75cc5fdc219c42ae26ec8497647eda5440c324f2 Mon Sep 17 00:00:00 2001 From: Jan Arve Saether Date: Mon, 6 Nov 2017 14:20:59 +0100 Subject: Never create pointer events from mouseGrabberItem() Normally, this was not a problem, but it is problematic during QQuickWindow destruction: The list of pointer event instances are destroyed, but later the grabber is removed (through call to removeGrabber()). This queries the mouseGrabberItem(), which would create a new pointer event instance. It also has the benefit that d->pointerEventInstances are now only populated due to actual incoming events. Task-number: QTBUG-61434 Change-Id: I4e7b6f5643f3b971138a1f7c7237ee734d29783c Reviewed-by: Shawn Rutledge --- src/quick/items/qquickwindow.cpp | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) (limited to 'src/quick/items/qquickwindow.cpp') diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index b58caa061a..112a338920 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -823,12 +823,13 @@ void QQuickWindowPrivate::removeGrabber(QQuickItem *grabber, bool mouse, bool to if (Q_LIKELY(touch)) { const auto touchDevices = QQuickPointerDevice::touchDevices(); for (auto device : touchDevices) { - auto pointerEvent = pointerEventInstance(device); - for (int i = 0; i < pointerEvent->pointCount(); ++i) { - if (pointerEvent->point(i)->grabber() == grabber) { - pointerEvent->point(i)->setGrabber(nullptr); - // FIXME send ungrab event only once - grabber->touchUngrabEvent(); + if (auto pointerEvent = queryPointerEventInstance(device)) { + for (int i = 0; i < pointerEvent->pointCount(); ++i) { + if (pointerEvent->point(i)->grabber() == grabber) { + pointerEvent->point(i)->setGrabber(nullptr); + // FIXME send ungrab event only once + grabber->touchUngrabEvent(); + } } } } @@ -1492,14 +1493,15 @@ QQuickItem *QQuickWindow::mouseGrabberItem() const Q_D(const QQuickWindow); if (d->touchMouseId != -1 && d->touchMouseDevice) { - QQuickPointerEvent *event = d->pointerEventInstance(d->touchMouseDevice); - auto point = event->pointById(d->touchMouseId); - return point ? point->grabber() : nullptr; + if (QQuickPointerEvent *event = d->queryPointerEventInstance(d->touchMouseDevice)) { + auto point = event->pointById(d->touchMouseId); + return point ? point->grabber() : nullptr; + } + } else if (QQuickPointerEvent *event = d->queryPointerEventInstance(QQuickPointerDevice::genericMouseDevice())) { + Q_ASSERT(event->pointCount()); + return event->point(0)->grabber(); } - - QQuickPointerEvent *event = d->pointerEventInstance(QQuickPointerDevice::genericMouseDevice()); - Q_ASSERT(event->pointCount()); - return event->point(0)->grabber(); + return nullptr; } @@ -2111,15 +2113,21 @@ void QQuickWindowPrivate::flushFrameSynchronousEvents() } } -QQuickPointerEvent *QQuickWindowPrivate::pointerEventInstance(QQuickPointerDevice *device) const +QQuickPointerEvent *QQuickWindowPrivate::queryPointerEventInstance(QQuickPointerDevice *device) const { // the list of devices should be very small so a linear search should be ok for (QQuickPointerEvent *e: pointerEventInstances) { if (e->device() == device) return e; } + return nullptr; +} - QQuickPointerEvent *ev = nullptr; +QQuickPointerEvent *QQuickWindowPrivate::pointerEventInstance(QQuickPointerDevice *device) const +{ + QQuickPointerEvent *ev = queryPointerEventInstance(device); + if (ev) + return ev; QQuickWindow *q = const_cast(q_func()); switch (device->type()) { case QQuickPointerDevice::Mouse: -- cgit v1.2.3 From 4331ccd4b735d9d721a384193a3d42ee2ce6c805 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 14 Jul 2017 16:37:22 +0200 Subject: QQuickWindow: cleanup pointer event instances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pointer event instances are QObject-children of QQuickWindow, meaning that they remain alive after ~QQuickWindow(), until execution reaches ~QObject(). Make sure the pointer event instances are cleaned up in ~QQuickWindow() to avoid accessing them later during the destruction phase. Task-number: QTBUG-61434 Change-Id: Icd4576e7581524773a3eb33864fdd64df821e0e8 Reviewed-by: Jan Arve Sæther Reviewed-by: Shawn Rutledge --- src/quick/items/qquickwindow.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/quick/items/qquickwindow.cpp') diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index 112a338920..70c8590ae7 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -1292,6 +1292,8 @@ QQuickWindow::~QQuickWindow() delete d->dragGrabber; d->dragGrabber = 0; #endif delete d->contentItem; d->contentItem = 0; + qDeleteAll(d->pointerEventInstances); + d->pointerEventInstances.clear(); d->renderJobMutex.lock(); qDeleteAll(d->beforeSynchronizingJobs); -- cgit v1.2.3 From a2e2c8a329768e783b205564e44b2f486b777d74 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Thu, 18 May 2017 11:23:25 +0200 Subject: Let passive-grabbing PointerHandlers see all point updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit even if all points are accepted or grabbed. A passive grab isn't much good if there are cases where the handler is prevented from monitoring. This enables e.g. the PinchHandler to steal the grab when the right number of touchpoints are present and have moved past the drag threshold, and enables completion of a couple of autotests. Change-Id: I78dc6fc585f80bfb3c13e0c6e757ef815fb94afe Reviewed-by: Jan Arve Sæther --- src/quick/items/qquickwindow.cpp | 90 +++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 53 deletions(-) (limited to 'src/quick/items/qquickwindow.cpp') diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index 73a594b281..4bee6dcd9a 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -768,16 +768,12 @@ void QQuickWindowPrivate::setMouseGrabber(QQuickItem *grabber) pt->cancelExclusiveGrab(); } point->setGrabberItem(grabber); - for (auto handler : point->passiveGrabbers()) - point->cancelPassiveGrab(handler); } } else { QQuickPointerEvent *event = pointerEventInstance(QQuickPointerDevice::genericMouseDevice()); Q_ASSERT(event->pointCount() == 1); auto point = event->point(0); point->setGrabberItem(grabber); - for (auto handler : point->passiveGrabbers()) - point->cancelPassiveGrab(handler); } @@ -1745,6 +1741,7 @@ void QQuickWindowPrivate::deliverMouseEvent(QQuickPointerMouseEvent *pointerEven if (mouseIsReleased) point->setGrabberPointerHandler(nullptr, true); } + deliverToPassiveGrabbers(point->passiveGrabbers(), pointerEvent); } else { bool delivered = false; if (pointerEvent->isPressEvent()) { @@ -2410,6 +2407,7 @@ void QQuickWindowPrivate::deliverTouchEvent(QQuickPointerTouchEvent *event) // Deliver touch points to existing grabbers void QQuickWindowPrivate::deliverUpdatedTouchPoints(QQuickPointerTouchEvent *event) { + bool done = false; const auto grabbers = event->exclusiveGrabbers(); for (auto grabber : grabbers) { // The grabber is guaranteed to be either an item or a handler. @@ -2419,52 +2417,52 @@ void QQuickWindowPrivate::deliverUpdatedTouchPoints(QQuickPointerTouchEvent *eve QQuickPointerHandler *handler = static_cast(grabber); receiver = static_cast(grabber)->parentItem(); if (sendFilteredPointerEvent(event, receiver)) - return; + done = true; event->localize(receiver); handler->handlePointerEvent(event); if (event->allPointsAccepted()) - return; + done = true; } + if (done) + break; // If the grabber is an item or the grabbing handler didn't handle it, // then deliver the event to the item (which may have multiple handlers). deliverMatchingPointsToItem(receiver, event); } - // If some points weren't grabbed, deliver only to non-grabber PointerHandlers - if (!event->allPointsGrabbed()) { - int pointCount = event->pointCount(); + // Deliver to each eventpoint's passive grabbers (but don't visit any handler more than once) + int pointCount = event->pointCount(); + for (int i = 0; i < pointCount; ++i) { + QQuickEventPoint *point = event->point(i); + deliverToPassiveGrabbers(point->passiveGrabbers(), event); + } + + if (done) + return; - // Deliver to each eventpoint's passive grabbers (but don't visit any handler more than once) + // If some points weren't grabbed, deliver only to non-grabber PointerHandlers in reverse paint order + if (!event->allPointsGrabbed()) { + QVector targetItems; for (int i = 0; i < pointCount; ++i) { QQuickEventPoint *point = event->point(i); - deliverToPassiveGrabbers(point->passiveGrabbers(), event); - } - - // If some points weren't grabbed, deliver to non-grabber PointerHandlers in reverse paint order - if (!event->allPointsGrabbed()) { - QVector targetItems; - for (int i = 0; i < pointCount; ++i) { - QQuickEventPoint *point = event->point(i); - if (point->state() == QQuickEventPoint::Pressed) - continue; // presses were delivered earlier; not the responsibility of deliverUpdatedTouchPoints - QVector targetItemsForPoint = pointerTargets(contentItem, point->scenePosition(), false, false); - if (targetItems.count()) { - targetItems = mergePointerTargets(targetItems, targetItemsForPoint); - } else { - targetItems = targetItemsForPoint; - } - } - - for (QQuickItem *item: targetItems) { - if (grabbers.contains(item)) - continue; - QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); - event->localize(item); - itemPrivate->handlePointerEvent(event, true); // avoid re-delivering to grabbers - if (event->allPointsGrabbed()) - break; + if (point->state() == QQuickEventPoint::Pressed) + continue; // presses were delivered earlier; not the responsibility of deliverUpdatedTouchPoints + QVector targetItemsForPoint = pointerTargets(contentItem, point->scenePosition(), false, false); + if (targetItems.count()) { + targetItems = mergePointerTargets(targetItems, targetItemsForPoint); + } else { + targetItems = targetItemsForPoint; } } + for (QQuickItem *item : targetItems) { + if (grabbers.contains(item)) + continue; + QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); + event->localize(item); + itemPrivate->handlePointerEvent(event, true); // avoid re-delivering to grabbers + if (event->allPointsGrabbed()) + break; + } } } @@ -2503,7 +2501,7 @@ bool QQuickWindowPrivate::deliverPressOrReleaseEvent(QQuickPointerEvent *event, continue; deliverMatchingPointsToItem(item, event, handlersOnly); if (event->allPointsAccepted()) - break; + handlersOnly = true; } return event->allPointsAccepted(); @@ -2518,8 +2516,9 @@ void QQuickWindowPrivate::deliverMatchingPointsToItem(QQuickItem *item, QQuickPo // Let the Item's handlers (if any) have the event first. // However, double click should never be delivered to handlers. if (!pointerEvent->isDoubleClickEvent()) { + bool wasAccepted = pointerEvent->allPointsAccepted(); itemPrivate->handlePointerEvent(pointerEvent); - allowDoubleClick = !(pointerEvent->asPointerMouseEvent() && pointerEvent->isPressEvent() && pointerEvent->allPointsAccepted()); + allowDoubleClick = wasAccepted || !(pointerEvent->asPointerMouseEvent() && pointerEvent->isPressEvent() && pointerEvent->allPointsAccepted()); } if (handlersOnly) return; @@ -2831,17 +2830,10 @@ bool QQuickWindowPrivate::sendFilteredPointerEventImpl(QQuickPointerEvent *event // get a touch event customized for delivery to filteringParent QScopedPointer filteringParentTouchEvent(pte->touchEventForItem(receiver, true)); if (filteringParentTouchEvent) { - QVarLengthArray, 32> passiveGrabsToCancel; if (filteringParent->childMouseEventFilter(receiver, filteringParentTouchEvent.data())) { qCDebug(DBG_TOUCH) << "touch event intercepted by childMouseEventFilter of " << filteringParent; skipDelivery.append(filteringParent); for (auto point: qAsConst(filteringParentTouchEvent->touchPoints())) { - auto pointerEventPoint = pte->pointById(point.id()); - for (auto handler : pointerEventPoint->passiveGrabbers()) { - QPair grab(handler, pointerEventPoint); - if (!passiveGrabsToCancel.contains(grab)) - passiveGrabsToCancel.append(grab); - } QQuickEventPoint *pt = event->pointById(point.id()); pt->setAccepted(); pt->setGrabberItem(filteringParent); @@ -2887,12 +2879,6 @@ bool QQuickWindowPrivate::sendFilteredPointerEventImpl(QQuickPointerEvent *event touchMouseUnset = false; // We want to leave touchMouseId and touchMouseDevice set if (mouseEvent->isAccepted()) filteringParent->grabMouse(); - auto pointerEventPoint = pte->pointById(tp.id()); - for (auto handler : pointerEventPoint->passiveGrabbers()) { - QPair grab(handler, pointerEventPoint); - if (!passiveGrabsToCancel.contains(grab)) - passiveGrabsToCancel.append(grab); - } } filtered = true; } @@ -2908,8 +2894,6 @@ bool QQuickWindowPrivate::sendFilteredPointerEventImpl(QQuickPointerEvent *event } } } - for (auto grab : passiveGrabsToCancel) - grab.second->cancelPassiveGrab(grab.first); } } } -- cgit v1.2.3 From 5d0785d38adfbc8b6af3f4db5c201b67f5f811e9 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Tue, 14 Nov 2017 15:32:20 +0100 Subject: QQuickWindowPrivate::deliverToPassiveGrabbers: localize the event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Always localize the pointer event to the handler's parent's coordinate system before sending it to a handler. Change-Id: I3006329a07cc9439b472a475444bcd4a0336ad0c Reviewed-by: Jan Arve Sæther --- src/quick/items/qquickwindow.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/quick/items/qquickwindow.cpp') diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index 4bee6dcd9a..e865a609a6 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -1694,8 +1694,10 @@ void QQuickWindowPrivate::deliverToPassiveGrabbers(const QVector(par, alreadyFiltered); } - if (!alreadyFiltered) + if (!alreadyFiltered) { + pointerEvent->localize(handler->parentItem()); handler->handlePointerEvent(pointerEvent); + } } } } -- cgit v1.2.3