From 62e23e1ae6542b5ab837a2481d9257c81e484013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Arve=20S=C3=A6ther?= Date: Wed, 1 Nov 2017 15:49:47 +0100 Subject: Avoid bogus "cancelGrab: no grabber" warning Should not alter any behavior except that the warning is gone. Change-Id: I684532ece7eddaeafd7ff26daa23a1c9968243f3 Task-number: QTBUG-62424 Reviewed-by: Shawn Rutledge --- src/quick/items/qquickwindow.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src/quick/items/qquickwindow.cpp') diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index 45c14f10a1..4210ba36ad 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -762,8 +762,11 @@ void QQuickWindowPrivate::setMouseGrabber(QQuickItem *grabber) auto point = pointerEventInstance(touchMouseDevice)->pointById(touchMouseId); if (point) { auto originalEvent = pointerEventInstance(point->pointerEvent()->device()); - for (int i = 0; i < originalEvent->pointCount(); ++i) - originalEvent->point(i)->cancelExclusiveGrab(); + for (int i = 0; i < originalEvent->pointCount(); ++i) { + QQuickEventPoint *pt = originalEvent->point(i); + if (pt->exclusiveGrabber()) + pt->cancelExclusiveGrab(); + } point->setGrabberItem(grabber); for (auto handler : point->passiveGrabbers()) point->cancelPassiveGrab(handler); -- cgit v1.2.3 From 0821180dc833376a738742e33f728983b9ca6f84 Mon Sep 17 00:00:00 2001 From: Daniel d'Andrada Date: Mon, 9 Oct 2017 17:39:44 +0200 Subject: Fix bug preventing ungrabMouse() on TouchCancel The order matters. There won't be a mouseGrabberItem() after the cancelExclusiveGrabImpl() call. So ungrab the mouse before calling it, not after. Task-number: QTBUG-63680 Change-Id: I81e03e079362c865e13792feb8c3af8cb3abedc8 Reviewed-by: Qt CI Bot Reviewed-by: Shawn Rutledge --- src/quick/items/qquickwindow.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'src/quick/items/qquickwindow.cpp') diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index 4210ba36ad..143c76cb1b 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -1976,15 +1976,16 @@ bool QQuickWindowPrivate::deliverTouchCancelEvent(QTouchEvent *event) qCDebug(DBG_TOUCH) << event; Q_Q(QQuickWindow); + if (q->mouseGrabberItem()) + q->mouseGrabberItem()->ungrabMouse(); + touchMouseId = -1; + touchMouseDevice = nullptr; + // A TouchCancel event will typically not contain any points. // Deliver it to all items and handlers that have active touches. QQuickPointerEvent *pointerEvent = pointerEventInstance(QQuickPointerDevice::touchDevice(event->device())); for (int i = 0; i < pointerEvent->pointCount(); ++i) pointerEvent->point(i)->cancelExclusiveGrabImpl(event); - touchMouseId = -1; - touchMouseDevice = nullptr; - if (q->mouseGrabberItem()) - q->mouseGrabberItem()->ungrabMouse(); // The next touch event can only be a TouchBegin, so clean up. pointerEvent->clearGrabbers(); -- cgit v1.2.3 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 From 28893b5ab2ee0984d6caa813ac1bbecf6d9e583c Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Thu, 16 Nov 2017 11:30:43 +0100 Subject: never skip updating passive grabbers, or ungrab, on mouse release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If you drag a ListView with the right mouse button, it was reacting, but not rebounding after release. And if a PointHandler is used to show feedback about mouse position, it also did not get the release. This was due to returning early from deliverMouseEvent. Change-Id: I24b39e4769d6824d3bd1f400dbf1f973bb29fbb6 Reviewed-by: Jan Arve Sæther --- src/quick/items/qquickwindow.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'src/quick/items/qquickwindow.cpp') diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index e865a609a6..2dc3caf430 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -1712,24 +1712,27 @@ void QQuickWindowPrivate::deliverMouseEvent(QQuickPointerMouseEvent *pointerEven if (point->exclusiveGrabber()) { if (auto grabber = point->grabberItem()) { + bool handled = false; if (sendFilteredPointerEvent(pointerEvent, grabber)) - return; + handled = true; // if the grabber is an Item: // if the update consists of changing button state, don't accept it unless // the button is one in which the grabber is interested Qt::MouseButtons acceptedButtons = grabber->acceptedMouseButtons(); - if (pointerEvent->button() != Qt::NoButton && acceptedButtons + if (!handled && pointerEvent->button() != Qt::NoButton && acceptedButtons && !(acceptedButtons & pointerEvent->button())) { pointerEvent->setAccepted(false); - return; + handled = true; } // send update - QPointF localPos = grabber->mapFromScene(lastMousePosition); - auto me = pointerEvent->asMouseEvent(localPos); - me->accept(); - QCoreApplication::sendEvent(grabber, me); - point->setAccepted(me->isAccepted()); + if (!handled) { + QPointF localPos = grabber->mapFromScene(lastMousePosition); + auto me = pointerEvent->asMouseEvent(localPos); + me->accept(); + QCoreApplication::sendEvent(grabber, me); + point->setAccepted(me->isAccepted()); + } // release event: ungrab if no buttons are pressed anymore if (mouseIsReleased) -- cgit v1.2.3 From 52f7ab28172cea3710a16775b7a512fce821fc77 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Mon, 13 Nov 2017 14:43:54 +0100 Subject: fix bugs in native gesture event delivery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1) always use Item-localized coordinates when testing Item.contains() 2) QQuickPinchHandler::wantsPointerEvent() was returning true for all native gestures, so in the manual test with two PinchHandlers, it was possible to pinch them both at the same time. Task-number: QTBUG-64848 Change-Id: Ia146aaf03f9982696ae2986249f8d4650a7bf727 Reviewed-by: Jan Arve Sæther --- src/quick/items/qquickwindow.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/quick/items/qquickwindow.cpp') diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index 2dc3caf430..530d165850 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -1938,7 +1938,8 @@ bool QQuickWindowPrivate::deliverNativeGestureEvent(QQuickItem *item, QNativeGes { QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); - if ((itemPrivate->flags & QQuickItem::ItemClipsChildrenToShape) && !item->contains(event->localPos())) + QPointF p = item->mapFromScene(event->windowPos()); + if ((itemPrivate->flags & QQuickItem::ItemClipsChildrenToShape) && !item->contains(p)) return false; QList children = itemPrivate->paintOrderChildItems(); @@ -1961,7 +1962,6 @@ bool QQuickWindowPrivate::deliverNativeGestureEvent(QQuickItem *item, QNativeGes } // If still not accepted, try direct delivery to the item - QPointF p = item->mapFromScene(event->localPos()); if (item->contains(p)) { QNativeGestureEvent copy(event->gestureType(), event->device(), p, event->windowPos(), event->screenPos(), event->value(), 0L, 0L); // TODO can't copy things I can't access -- cgit v1.2.3