From a8729cf143a79b274c002166476c54dc152c6679 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Mon, 19 Nov 2018 16:22:45 +0100 Subject: TapHander: do not "want" an eventPoint that is outside parent bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So far it was checking parentContains() on press, release, or when the gesturePolicy is WithinBounds, but not for each movement when the policy is DragThreshold (the default). This might explain most of the remaining warning noise: "pointId is missing from current event, but was neither canceled nor released" because it was possible for TapHandler to remember wanting a point that it should not have wanted, but without taking any kind of grab, and then complaining when that point was no longer present. Since it did not grab, it did not get the release, unless the release was part of an event containing a point that it DID grab. Fixes: QTBUG-71887 Change-Id: I26ce62279574cf6b0150f24e486f224a604ac6b1 Reviewed-by: Jan Arve Sæther --- src/quick/handlers/qquicktaphandler.cpp | 2 +- src/quick/items/qquickevents.cpp | 2 +- .../pointerhandlers/flickableinterop/tst_flickableinterop.cpp | 10 +++++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/quick/handlers/qquicktaphandler.cpp b/src/quick/handlers/qquicktaphandler.cpp index c1658ec5d2..9386676b7a 100644 --- a/src/quick/handlers/qquicktaphandler.cpp +++ b/src/quick/handlers/qquicktaphandler.cpp @@ -129,7 +129,7 @@ bool QQuickTapHandler::wantsEventPoint(QQuickEventPoint *point) case QQuickEventPoint::Updated: switch (m_gesturePolicy) { case DragThreshold: - ret = !overThreshold; + ret = !overThreshold && parentContains(point); break; case WithinBounds: ret = parentContains(point); diff --git a/src/quick/items/qquickevents.cpp b/src/quick/items/qquickevents.cpp index 31c56b7cb7..2eaab164a0 100644 --- a/src/quick/items/qquickevents.cpp +++ b/src/quick/items/qquickevents.cpp @@ -851,7 +851,7 @@ void QQuickEventPoint::setGrabberItem(QQuickItem *grabber) if (oldGrabberHandler && !oldGrabberHandler->approveGrabTransition(this, grabber)) return; if (Q_UNLIKELY(lcPointerGrab().isDebugEnabled())) { - qCDebug(lcPointerGrab) << pointDeviceName(this) << "point" << hex << m_pointId << pointStateString(this) + qCDebug(lcPointerGrab) << pointDeviceName(this) << "point" << hex << m_pointId << pointStateString(this) << "@" << m_scenePos << ": grab" << m_exclusiveGrabber << "->" << grabber; } QQuickItem *oldGrabberItem = grabberItem(); diff --git a/tests/auto/quick/pointerhandlers/flickableinterop/tst_flickableinterop.cpp b/tests/auto/quick/pointerhandlers/flickableinterop/tst_flickableinterop.cpp index e73588bdef..cf2ac4a830 100644 --- a/tests/auto/quick/pointerhandlers/flickableinterop/tst_flickableinterop.cpp +++ b/tests/auto/quick/pointerhandlers/flickableinterop/tst_flickableinterop.cpp @@ -301,7 +301,7 @@ void tst_FlickableInterop::touchDragSlider() // Drag the slider in the allowed (vertical) direction tappedSpy.clear(); - QPoint p1 = knob->mapToScene(knob->clipRect().center()).toPoint(); + QPoint p1 = knob->mapToScene(knob->clipRect().center()).toPoint() - QPoint(0, 8); QTest::touchEvent(window, touchDevice).press(1, p1, window); QQuickTouchUtils::flush(window); QTRY_VERIFY(slider->property("pressed").toBool()); @@ -343,12 +343,12 @@ void tst_FlickableInterop::mouseDragSlider_data() QTest::addColumn("expectedDragHandlerActive"); QTest::addColumn("expectedFlickableMoving"); - QTest::newRow("drag down on knob of knobSlider") << "knobSlider" << QPoint(0, 0) << QPoint(0, 1) << true << true << false; + QTest::newRow("drag down on knob of knobSlider") << "knobSlider" << QPoint(0, -8) << QPoint(0, 1) << true << true << false; QTest::newRow("drag sideways on knob of knobSlider") << "knobSlider" << QPoint(0, 0) << QPoint(1, 0) << true << false << true; QTest::newRow("drag down on groove of knobSlider") << "knobSlider" << QPoint(0, 20) << QPoint(0, 1) << false << false << true; QTest::newRow("drag sideways on groove of knobSlider") << "knobSlider" << QPoint(0, 20) << QPoint(1, 0) << false << false << true; - QTest::newRow("drag down on knob of grooveSlider") << "grooveSlider" << QPoint(0, 0) << QPoint(0, 1) << true << true << false; + QTest::newRow("drag down on knob of grooveSlider") << "grooveSlider" << QPoint(0, -8) << QPoint(0, 1) << true << true << false; QTest::newRow("drag sideways on knob of grooveSlider") << "grooveSlider" << QPoint(0, 0) << QPoint(1, 0) << true << false << true; QTest::newRow("drag down on groove of grooveSlider") << "grooveSlider" << QPoint(0, 20) << QPoint(0, 1) << false << true << false; QTest::newRow("drag sideways on groove of grooveSlider") << "grooveSlider" << QPoint(0, 20) << QPoint(1, 0) << false << false << true; @@ -391,6 +391,10 @@ void tst_FlickableInterop::mouseDragSlider() QCOMPARE(slider->property("value").toInt(), 49); p1 += dragDirection; // one more pixel QTest::mouseMove(window, p1); + // After moving by the drag threshold, the point should still be inside the knob. + // However, QQuickTapHandler::wantsEventPoint() returns false because the drag threshold is exceeded. + // Therefore QQuickTapHandler::setPressed(false, true, point) is called: the active state is canceled. + QCOMPARE(slider->property("pressed").toBool(), false); QCOMPARE(drag->active(), expectedDragHandlerActive); // drag farther, to make sure the knob gets adjusted significantly p1 += QPoint(10 * dragDirection.x(), 10 * dragDirection.y()); -- cgit v1.2.3