From 77d6aef4063a9bf18074609fc32d3ffeed88617c Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Tue, 22 Jul 2014 11:31:04 +1000 Subject: Fix touch/mouse propagation bugs Filtered mouse release was not delivered if another touch started after a touchMouseId was activated. This meant that any filters expecting a release event would not receive it if another touch was made before release of the touchMouseId. We prevented a touch becoming the touchMouseId in the child mouse filters if there were any existing touches. The normal event delivery, however, does not require a single touch. Further to the previous, a touch could become the touchMouseId, even if the initial press happened when there was an existing touchMouseId. This meant that a touch could turn into a mouse when the existing mouse event was released, resulting in a new touchMouseId which hadn't been through child mouse filters. Flickable delayed press should be sent via normal event processing, as other touch/mouse events are now delivered in this way. We often called childMouseEventFilter() multiple times for each event. This is bad because the gesture handling relies on claiming a gesture in one event, then stealing it in the next. Instead of sending touch to mouse candidate points already determined to be within the item bounds and already transformed, we sent all of the points to the mouse recipient. PinchArea did not store the starting position at the original touch points, so other items could pass the dragThreshold before PinchArea and steal a gesture meant for PinchArea. Task-number: QTBUG-40330 Change-Id: Ic0009c176d3d1cb7cff0b5eda076a2c3ca864136 Reviewed-by: Robin Burchell --- src/quick/items/qquickflickable.cpp | 2 +- src/quick/items/qquickpincharea.cpp | 10 +-- src/quick/items/qquickwindow.cpp | 122 +++++++++++++++++++++++------------- src/quick/items/qquickwindow_p.h | 9 +-- 4 files changed, 89 insertions(+), 54 deletions(-) (limited to 'src') diff --git a/src/quick/items/qquickflickable.cpp b/src/quick/items/qquickflickable.cpp index ee71ea8a76..3f072359d2 100644 --- a/src/quick/items/qquickflickable.cpp +++ b/src/quick/items/qquickflickable.cpp @@ -1380,7 +1380,7 @@ void QQuickFlickablePrivate::replayDelayedPress() // Use the event handler that will take care of finding the proper item to propagate the event replayingPressEvent = true; - QQuickWindowPrivate::get(w)->deliverMouseEvent(mouseEvent.data()); + QCoreApplication::sendEvent(w, mouseEvent.data()); replayingPressEvent = false; } } diff --git a/src/quick/items/qquickpincharea.cpp b/src/quick/items/qquickpincharea.cpp index 234f78c380..d8478806c6 100644 --- a/src/quick/items/qquickpincharea.cpp +++ b/src/quick/items/qquickpincharea.cpp @@ -268,8 +268,6 @@ QQuickPinchArea::QQuickPinchArea(QQuickItem *parent) { Q_D(QQuickPinchArea); d->init(); - setAcceptedMouseButtons(Qt::LeftButton); - setFiltersChildMouseEvents(true); #ifdef Q_OS_OSX setAcceptHoverEvents(true); // needed to enable touch events on mouse hover. #endif @@ -413,6 +411,12 @@ void QQuickPinchArea::updatePinch() QTouchEvent::TouchPoint touchPoint1 = d->touchPoints.at(0); QTouchEvent::TouchPoint touchPoint2 = d->touchPoints.at(d->touchPoints. count() >= 2 ? 1 : 0); + if (touchPoint1.state() == Qt::TouchPointPressed) + d->sceneStartPoint1 = touchPoint1.scenePos(); + + if (touchPoint2.state() == Qt::TouchPointPressed) + d->sceneStartPoint2 = touchPoint2.scenePos(); + QRectF bounds = clipRect(); // Pinch is not started unless there are exactly two touch points // AND one or more of the points has just now been pressed (wasn't pressed already) @@ -421,8 +425,6 @@ void QQuickPinchArea::updatePinch() && (touchPoint1.state() & Qt::TouchPointPressed || touchPoint2.state() & Qt::TouchPointPressed) && bounds.contains(touchPoint1.pos()) && bounds.contains(touchPoint2.pos())) { d->id1 = touchPoint1.id(); - d->sceneStartPoint1 = touchPoint1.scenePos(); - d->sceneStartPoint2 = touchPoint2.scenePos(); d->pinchActivated = true; d->initPinch = true; diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index 0233e93443..12be33d790 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -556,7 +556,7 @@ bool QQuickWindowPrivate::translateTouchToMouse(QQuickItem *item, QTouchEvent *e // handler spins the event loop all subsequent moves and releases get lost. touchMouseId = p.id(); itemForTouchPointId[touchMouseId] = item; - QScopedPointer mousePress(touchToMouseEvent(QEvent::MouseButtonPress, p, event, item)); + QScopedPointer mousePress(touchToMouseEvent(QEvent::MouseButtonPress, p, event, item, false)); // Send a single press and see if that's accepted if (!mouseGrabberItem) @@ -575,18 +575,21 @@ bool QQuickWindowPrivate::translateTouchToMouse(QQuickItem *item, QTouchEvent *e } if (mousePress->isAccepted() && checkIfDoubleClicked(event->timestamp())) { - QScopedPointer mouseDoubleClick(touchToMouseEvent(QEvent::MouseButtonDblClick, p, event, item)); + QScopedPointer mouseDoubleClick(touchToMouseEvent(QEvent::MouseButtonDblClick, p, event, item, false)); QCoreApplication::sendEvent(item, mouseDoubleClick.data()); event->setAccepted(mouseDoubleClick->isAccepted()); if (mouseDoubleClick->isAccepted()) { + touchMouseIdCandidates.clear(); return true; } else { touchMouseId = -1; } } // The event was accepted, we are done. - if (mousePress->isAccepted()) + if (mousePress->isAccepted()) { + touchMouseIdCandidates.clear(); return true; + } // The event was not accepted but touchMouseId was set. if (touchMouseId != -1) return false; @@ -596,7 +599,7 @@ bool QQuickWindowPrivate::translateTouchToMouse(QQuickItem *item, QTouchEvent *e } else if (p.id() == touchMouseId) { if (p.state() & Qt::TouchPointMoved) { if (mouseGrabberItem) { - QScopedPointer me(touchToMouseEvent(QEvent::MouseMove, p, event, mouseGrabberItem)); + QScopedPointer me(touchToMouseEvent(QEvent::MouseMove, p, event, mouseGrabberItem, false)); QCoreApplication::sendEvent(item, me.data()); event->setAccepted(me->isAccepted()); if (me->isAccepted()) { @@ -607,7 +610,7 @@ bool QQuickWindowPrivate::translateTouchToMouse(QQuickItem *item, QTouchEvent *e // no grabber, check if we care about mouse hover // FIXME: this should only happen once, not recursively... I'll ignore it just ignore hover now. // hover for touch??? - QScopedPointer me(touchToMouseEvent(QEvent::MouseMove, p, event, item)); + QScopedPointer me(touchToMouseEvent(QEvent::MouseMove, p, event, item, false)); if (lastMousePosition.isNull()) lastMousePosition = me->windowPos(); QPointF last = lastMousePosition; @@ -626,7 +629,7 @@ bool QQuickWindowPrivate::translateTouchToMouse(QQuickItem *item, QTouchEvent *e // currently handled point was released touchMouseId = -1; if (mouseGrabberItem) { - QScopedPointer me(touchToMouseEvent(QEvent::MouseButtonRelease, p, event, mouseGrabberItem)); + QScopedPointer me(touchToMouseEvent(QEvent::MouseButtonRelease, p, event, mouseGrabberItem, false)); QCoreApplication::sendEvent(item, me.data()); if (mouseGrabberItem) // might have ungrabbed due to event mouseGrabberItem->ungrabMouse(); @@ -1887,7 +1890,8 @@ void QQuickWindowPrivate::reallyDeliverTouchEvent(QTouchEvent *event) // or some item accepted a point and should receive an update if (newPoints.count() > 0 || updatedPoints.count() > 0) { QSet acceptedNewPoints; - event->setAccepted(deliverTouchPoints(contentItem, event, newPoints, &acceptedNewPoints, &updatedPoints)); + QSet hasFiltered; + event->setAccepted(deliverTouchPoints(contentItem, event, newPoints, &acceptedNewPoints, &updatedPoints, &hasFiltered)); } else event->ignore(); @@ -1898,6 +1902,7 @@ void QQuickWindowPrivate::reallyDeliverTouchEvent(QTouchEvent *event) itemForTouchPointId.remove(touchPoints[i].id()); if (touchPoints[i].id() == touchMouseId) touchMouseId = -1; + touchMouseIdCandidates.remove(touchPoints[i].id()); } } } @@ -1910,7 +1915,9 @@ void QQuickWindowPrivate::reallyDeliverTouchEvent(QTouchEvent *event) } // This function recurses and sends the events to the individual items -bool QQuickWindowPrivate::deliverTouchPoints(QQuickItem *item, QTouchEvent *event, const QList &newPoints, QSet *acceptedNewPoints, QHash > *updatedPoints) +bool QQuickWindowPrivate::deliverTouchPoints(QQuickItem *item, QTouchEvent *event, const QList &newPoints, + QSet *acceptedNewPoints, QHash > *updatedPoints, QSet *hasFiltered) { QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); @@ -1929,7 +1936,7 @@ bool QQuickWindowPrivate::deliverTouchPoints(QQuickItem *item, QTouchEvent *even QQuickItem *child = children.at(ii); if (!child->isEnabled() || !child->isVisible() || QQuickItemPrivate::get(child)->culled) continue; - if (deliverTouchPoints(child, event, newPoints, acceptedNewPoints, updatedPoints)) + if (deliverTouchPoints(child, event, newPoints, acceptedNewPoints, updatedPoints, hasFiltered)) return true; } @@ -1979,7 +1986,7 @@ bool QQuickWindowPrivate::deliverTouchPoints(QQuickItem *item, QTouchEvent *even // with only the subset of TouchPoints which are relevant to that item: that's matchingPoints. QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); transformTouchPoints(matchingPoints, itemPrivate->windowToItemTransform()); - deliverMatchingPointsToItem(item, event, acceptedNewPoints, matchingNewPoints, matchingPoints); + deliverMatchingPointsToItem(item, event, acceptedNewPoints, matchingNewPoints, matchingPoints, hasFiltered); } // record the fact that this item has been visited already @@ -1993,7 +2000,7 @@ bool QQuickWindowPrivate::deliverTouchPoints(QQuickItem *item, QTouchEvent *even // only the points that are relevant for this item. Thus the need for // matchingPoints to already be that set of interesting points. // They are all pre-transformed, too. -bool QQuickWindowPrivate::deliverMatchingPointsToItem(QQuickItem *item, QTouchEvent *event, QSet *acceptedNewPoints, const QSet &matchingNewPoints, const QList &matchingPoints) +bool QQuickWindowPrivate::deliverMatchingPointsToItem(QQuickItem *item, QTouchEvent *event, QSet *acceptedNewPoints, const QSet &matchingNewPoints, const QList &matchingPoints, QSet *hasFiltered) { QScopedPointer touchEvent(touchEventWithPoints(*event, matchingPoints)); touchEvent.data()->setTarget(item); @@ -2001,7 +2008,7 @@ bool QQuickWindowPrivate::deliverMatchingPointsToItem(QQuickItem *item, QTouchEv // First check whether the parent wants to be a filter, // and if the parent accepts the event we are done. - if (sendFilteredTouchEvent(item->parentItem(), item, event)) { + if (sendFilteredTouchEvent(item->parentItem(), item, event, hasFiltered)) { // If the touch was accepted (regardless by whom or in what form), // update acceptedNewPoints foreach (int id, matchingNewPoints) @@ -2021,7 +2028,7 @@ bool QQuickWindowPrivate::deliverMatchingPointsToItem(QQuickItem *item, QTouchEv QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); if (!touchEventAccepted && (itemPrivate->acceptedMouseButtons() & Qt::LeftButton)) { // send mouse event - event->setAccepted(translateTouchToMouse(item, event)); + event->setAccepted(translateTouchToMouse(item, touchEvent.data())); if (event->isAccepted()) { touchEventAccepted = true; } @@ -2271,7 +2278,7 @@ QQuickItem *QQuickWindowPrivate::findCursorItem(QQuickItem *item, const QPointF } #endif -bool QQuickWindowPrivate::sendFilteredTouchEvent(QQuickItem *target, QQuickItem *item, QTouchEvent *event) +bool QQuickWindowPrivate::sendFilteredTouchEvent(QQuickItem *target, QQuickItem *item, QTouchEvent *event, QSet *hasFiltered) { if (!target) return false; @@ -2279,13 +2286,14 @@ bool QQuickWindowPrivate::sendFilteredTouchEvent(QQuickItem *target, QQuickItem bool filtered = false; QQuickItemPrivate *targetPrivate = QQuickItemPrivate::get(target); - if (targetPrivate->filtersChildMouseEvents) { + if (targetPrivate->filtersChildMouseEvents && !hasFiltered->contains(target)) { + hasFiltered->insert(target); QScopedPointer targetEvent(touchEventForItemBounds(target, *event)); if (!targetEvent->touchPoints().isEmpty()) { - QVector touchIds; - for (int i = 0; i < event->touchPoints().size(); ++i) - touchIds.append(event->touchPoints().at(i).id()); if (target->childMouseEventFilter(item, targetEvent.data())) { + QVector touchIds; + for (int i = 0; i < targetEvent->touchPoints().size(); ++i) + touchIds.append(targetEvent->touchPoints().at(i).id()); target->grabTouchPoints(touchIds); if (mouseGrabberItem) { mouseGrabberItem->ungrabMouse(); @@ -2294,38 +2302,54 @@ bool QQuickWindowPrivate::sendFilteredTouchEvent(QQuickItem *target, QQuickItem filtered = true; } - // Only offer a mouse event to the filter if we have one point - if (targetEvent->touchPoints().count() == 1) { + for (int i = 0; i < targetEvent->touchPoints().size(); ++i) { + const QTouchEvent::TouchPoint &tp = targetEvent->touchPoints().at(i); + QEvent::Type t; - const QTouchEvent::TouchPoint &tp = targetEvent->touchPoints().first(); switch (tp.state()) { case Qt::TouchPointPressed: t = QEvent::MouseButtonPress; + if (touchMouseId == -1) { + // We don't want to later filter touches as a mouse event if they were pressed + // while a touchMouseId was already active. + // Remember this touch as a potential to become the touchMouseId. + touchMouseIdCandidates.insert(tp.id()); + } break; case Qt::TouchPointReleased: t = QEvent::MouseButtonRelease; break; + case Qt::TouchPointStationary: + continue; default: - // move or stationary t = QEvent::MouseMove; break; } - // targetEvent is already transformed wrt local position, velocity, etc. - QScopedPointer mouseEvent(touchToMouseEvent(t, targetEvent->touchPoints().first(), event, item, false)); - if (target->childMouseEventFilter(item, mouseEvent.data())) { - itemForTouchPointId[tp.id()] = target; - touchMouseId = tp.id(); - target->grabMouse(); - filtered = true; + // Only deliver mouse event if it is the touchMouseId or it could become the touchMouseId + if ((touchMouseIdCandidates.contains(tp.id()) && touchMouseId == -1) || touchMouseId == tp.id()) { + // targetEvent is already transformed wrt local position, velocity, etc. + QScopedPointer mouseEvent(touchToMouseEvent(t, tp, event, item, false)); + if (target->childMouseEventFilter(item, mouseEvent.data())) { + if (t != QEvent::MouseButtonRelease) { + itemForTouchPointId[tp.id()] = target; + touchMouseId = tp.id(); + target->grabMouse(); + } + touchMouseIdCandidates.clear(); + filtered = true; + } + // Only one event can be filtered as a mouse event. + break; } } } } - return sendFilteredTouchEvent(target->parentItem(), item, event) || filtered; + + return sendFilteredTouchEvent(target->parentItem(), item, event, hasFiltered) || filtered; } -bool QQuickWindowPrivate::sendFilteredMouseEvent(QQuickItem *target, QQuickItem *item, QEvent *event) +bool QQuickWindowPrivate::sendFilteredMouseEvent(QQuickItem *target, QQuickItem *item, QEvent *event, QSet *hasFiltered) { if (!target) return false; @@ -2333,11 +2357,13 @@ bool QQuickWindowPrivate::sendFilteredMouseEvent(QQuickItem *target, QQuickItem bool filtered = false; QQuickItemPrivate *targetPrivate = QQuickItemPrivate::get(target); - if (targetPrivate->filtersChildMouseEvents) + if (targetPrivate->filtersChildMouseEvents && !hasFiltered->contains(target)) { + hasFiltered->insert(target); if (target->childMouseEventFilter(item, event)) filtered = true; + } - return sendFilteredMouseEvent(target->parentItem(), item, event) || filtered; + return sendFilteredMouseEvent(target->parentItem(), item, event, hasFiltered) || filtered; } bool QQuickWindowPrivate::dragOverThreshold(qreal d, Qt::Axis axis, QMouseEvent *event, int startDragThreshold) @@ -2478,18 +2504,22 @@ bool QQuickWindow::sendEvent(QQuickItem *item, QEvent *e) case QEvent::MouseButtonPress: case QEvent::MouseButtonRelease: case QEvent::MouseButtonDblClick: - case QEvent::MouseMove: - // XXX todo - should sendEvent be doing this? how does it relate to forwarded events? - if (!d->sendFilteredMouseEvent(item->parentItem(), item, e)) { - // accept because qml items by default accept and have to explicitly opt out of accepting - e->accept(); - QCoreApplication::sendEvent(item, e); + case QEvent::MouseMove: { + // XXX todo - should sendEvent be doing this? how does it relate to forwarded events? + QSet hasFiltered; + if (!d->sendFilteredMouseEvent(item->parentItem(), item, e, &hasFiltered)) { + // accept because qml items by default accept and have to explicitly opt out of accepting + e->accept(); + QCoreApplication::sendEvent(item, e); + } } break; - case QEvent::UngrabMouse: - if (!d->sendFilteredMouseEvent(item->parentItem(), item, e)) { - e->accept(); - item->mouseUngrabEvent(); + case QEvent::UngrabMouse: { + QSet hasFiltered; + if (!d->sendFilteredMouseEvent(item->parentItem(), item, e, &hasFiltered)) { + e->accept(); + item->mouseUngrabEvent(); + } } break; #ifndef QT_NO_WHEELEVENT @@ -2511,8 +2541,10 @@ bool QQuickWindow::sendEvent(QQuickItem *item, QEvent *e) break; case QEvent::TouchBegin: case QEvent::TouchUpdate: - case QEvent::TouchEnd: - d->sendFilteredTouchEvent(item->parentItem(), item, static_cast(e)); + case QEvent::TouchEnd: { + QSet hasFiltered; + d->sendFilteredTouchEvent(item->parentItem(), item, static_cast(e), &hasFiltered); + } break; default: break; diff --git a/src/quick/items/qquickwindow_p.h b/src/quick/items/qquickwindow_p.h index 8f0696914e..93ceefe5af 100644 --- a/src/quick/items/qquickwindow_p.h +++ b/src/quick/items/qquickwindow_p.h @@ -137,21 +137,21 @@ public: static QMouseEvent *cloneMouseEvent(QMouseEvent *event, QPointF *transformedLocalPos = 0); bool deliverInitialMousePressEvent(QQuickItem *, QMouseEvent *); bool deliverMouseEvent(QMouseEvent *); - bool sendFilteredMouseEvent(QQuickItem *, QQuickItem *, QEvent *); + bool sendFilteredMouseEvent(QQuickItem *, QQuickItem *, QEvent *, QSet *); #ifndef QT_NO_WHEELEVENT bool deliverWheelEvent(QQuickItem *, QWheelEvent *); #endif bool deliverTouchPoints(QQuickItem *, QTouchEvent *, const QList &, QSet *, - QHash > *); + QHash > *, QSet *filtered); void deliverTouchEvent(QTouchEvent *); void reallyDeliverTouchEvent(QTouchEvent *); bool deliverTouchCancelEvent(QTouchEvent *); void flushDelayedTouchEvent(); bool deliverHoverEvent(QQuickItem *, const QPointF &scenePos, const QPointF &lastScenePos, Qt::KeyboardModifiers modifiers, bool &accepted); - bool deliverMatchingPointsToItem(QQuickItem *item, QTouchEvent *event, QSet *acceptedNewPoints, const QSet &matchingNewPoints, const QList &matchingPoints); + bool deliverMatchingPointsToItem(QQuickItem *item, QTouchEvent *event, QSet *acceptedNewPoints, const QSet &matchingNewPoints, const QList &matchingPoints, QSet *filtered); QTouchEvent *touchEventForItemBounds(QQuickItem *target, const QTouchEvent &originalEvent); QTouchEvent *touchEventWithPoints(const QTouchEvent &event, const QList &newPoints); - bool sendFilteredTouchEvent(QQuickItem *target, QQuickItem *item, QTouchEvent *event); + bool sendFilteredTouchEvent(QQuickItem *target, QQuickItem *item, QTouchEvent *event, QSet *filtered); bool sendHoverEvent(QEvent::Type, QQuickItem *, const QPointF &scenePos, const QPointF &lastScenePos, Qt::KeyboardModifiers modifiers, bool accepted); bool clearHover(); @@ -239,6 +239,7 @@ public: // Keeps track of which touch point (int) was last accepted by which item QHash itemForTouchPointId; + QSet touchMouseIdCandidates; mutable QQuickWindowIncubationController *incubationController; -- cgit v1.2.3