From 268f4615dcf19aad3603833af83ba28eca886aa5 Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Wed, 27 Nov 2013 16:13:29 +1000 Subject: Fix Flickable StopAtBounds drag over, back, over behavior. A Flickable with StopAtBounds failed when: 1. position on a boundary. Without lifting your finger: 2. attempt to drag beyond the boundary -> doesn't drag 3. drag back to initiate dragging 4. attempt to quickly drag beyond the boundary. After 4, the view should be back on the boundary, but it could get stuck a little short of the boundary. Change-Id: I9bfbb4293f4d464bddb97c5c37e9bb91ed7d48e4 Reviewed-by: Robin Burchell --- src/quick/items/qquickflickable.cpp | 74 +++++++++++------- .../quick/qquickflickable/data/stopAtBounds.qml | 22 ++++++ .../quick/qquickflickable/tst_qquickflickable.cpp | 90 +++++++++++++++++++++- 3 files changed, 157 insertions(+), 29 deletions(-) create mode 100644 tests/auto/quick/qquickflickable/data/stopAtBounds.qml diff --git a/src/quick/items/qquickflickable.cpp b/src/quick/items/qquickflickable.cpp index ae174d86e0..5b0c1c6756 100644 --- a/src/quick/items/qquickflickable.cpp +++ b/src/quick/items/qquickflickable.cpp @@ -508,16 +508,27 @@ void QQuickFlickablePrivate::fixup(AxisData &data, qreal minExtent, qreal maxExt data.vTime = timeline.time(); } +static bool fuzzyLessThanOrEqualTo(qreal a, qreal b) +{ + if (a == 0.0 || b == 0.0) { + // qFuzzyCompare is broken + a += 1.0; + b += 1.0; + } + return a <= b || qFuzzyCompare(a, b); +} + void QQuickFlickablePrivate::updateBeginningEnd() { Q_Q(QQuickFlickable); bool atBoundaryChange = false; // Vertical - const int maxyextent = int(-q->maxYExtent()); + const qreal maxyextent = -q->maxYExtent(); + const qreal minyextent = -q->minYExtent(); const qreal ypos = -vData.move.value(); - bool atBeginning = (ypos <= -q->minYExtent()); - bool atEnd = (maxyextent <= ypos); + bool atBeginning = fuzzyLessThanOrEqualTo(ypos, minyextent); + bool atEnd = fuzzyLessThanOrEqualTo(maxyextent, ypos); if (atBeginning != vData.atBeginning) { vData.atBeginning = atBeginning; @@ -529,10 +540,11 @@ void QQuickFlickablePrivate::updateBeginningEnd() } // Horizontal - const int maxxextent = int(-q->maxXExtent()); + const qreal maxxextent = -q->maxXExtent(); + const qreal minxextent = -q->minXExtent(); const qreal xpos = -hData.move.value(); - atBeginning = (xpos <= -q->minXExtent()); - atEnd = (maxxextent <= xpos); + atBeginning = fuzzyLessThanOrEqualTo(xpos, minxextent); + atEnd = fuzzyLessThanOrEqualTo(maxxextent, xpos); if (atBeginning != hData.atBeginning) { hData.atBeginning = atBeginning; @@ -1046,17 +1058,20 @@ void QQuickFlickablePrivate::handleMouseMoveEvent(QMouseEvent *event) // the estimate to be altered const qreal minY = vData.dragMinBound + vData.startMargin; const qreal maxY = vData.dragMaxBound - vData.endMargin; - if (newY > minY) - newY = minY + (newY - minY) / 2; - if (newY < maxY && maxY - minY <= 0) - newY = maxY + (newY - maxY) / 2; - if (boundsBehavior == QQuickFlickable::StopAtBounds && newY <= maxY) { - newY = maxY; - rejectY = vData.pressPos == maxY && dy < 0; - } - if (boundsBehavior == QQuickFlickable::StopAtBounds && newY >= minY) { - newY = minY; - rejectY = vData.pressPos == minY && dy > 0; + if (boundsBehavior == QQuickFlickable::StopAtBounds) { + if (newY <= maxY) { + newY = maxY; + rejectY = vData.pressPos == maxY && vData.move.value() == maxY && dy < 0; + } + if (newY >= minY) { + newY = minY; + rejectY = vData.pressPos == minY && vData.move.value() == minY && dy > 0; + } + } else { + if (newY > minY) + newY = minY + (newY - minY) / 2; + if (newY < maxY && maxY - minY <= 0) + newY = maxY + (newY - maxY) / 2; } if (!rejectY && stealMouse && dy != 0.0) { clearTimeline(); @@ -1077,17 +1092,20 @@ void QQuickFlickablePrivate::handleMouseMoveEvent(QMouseEvent *event) qreal newX = dx + hData.pressPos - hData.dragStartOffset; const qreal minX = hData.dragMinBound + hData.startMargin; const qreal maxX = hData.dragMaxBound - hData.endMargin; - if (newX > minX) - newX = minX + (newX - minX) / 2; - if (newX < maxX && maxX - minX <= 0) - newX = maxX + (newX - maxX) / 2; - if (boundsBehavior == QQuickFlickable::StopAtBounds && newX <= maxX) { - newX = maxX; - rejectX = hData.pressPos == maxX && dx < 0; - } - if (boundsBehavior == QQuickFlickable::StopAtBounds && newX >= minX) { - newX = minX; - rejectX = hData.pressPos == minX && dx > 0; + if (boundsBehavior == QQuickFlickable::StopAtBounds) { + if (newX <= maxX) { + newX = maxX; + rejectX = hData.pressPos == maxX && hData.move.value() == maxX && dx < 0; + } + if (newX >= minX) { + newX = minX; + rejectX = hData.pressPos == minX && hData.move.value() == minX && dx > 0; + } + } else { + if (newX > minX) + newX = minX + (newX - minX) / 2; + if (newX < maxX && maxX - minX <= 0) + newX = maxX + (newX - maxX) / 2; } if (!rejectX && stealMouse && dx != 0.0) { diff --git a/tests/auto/quick/qquickflickable/data/stopAtBounds.qml b/tests/auto/quick/qquickflickable/data/stopAtBounds.qml new file mode 100644 index 0000000000..d8661f3e45 --- /dev/null +++ b/tests/auto/quick/qquickflickable/data/stopAtBounds.qml @@ -0,0 +1,22 @@ +import QtQuick 2.0 + +Flickable { + id: outer + objectName: "flickable" + width: 400 + height: 400 + contentX: 50 + contentY: 50 + contentWidth: 500 + contentHeight: 500 + boundsBehavior: Flickable.StopAtBounds + + Rectangle { + x: 100 + y: 100 + width: 300 + height: 300 + + color: "yellow" + } +} diff --git a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp index 3c518c509b..268cdf7920 100644 --- a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp +++ b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp @@ -94,6 +94,8 @@ private slots: void flickTwiceUsingTouches(); void nestedStopAtBounds(); void nestedStopAtBounds_data(); + void stopAtBounds(); + void stopAtBounds_data(); void nestedMouseAreaUsingTouch(); private: @@ -1322,13 +1324,15 @@ void tst_qquickflickable::flickTwiceUsingTouches() qreal contentYAfterFirstFlick = flickable->contentY(); qDebug() << "contentYAfterFirstFlick " << contentYAfterFirstFlick; QVERIFY(contentYAfterFirstFlick > 50.0f); + // Wait until view stops moving + QTRY_VERIFY(!flickable->isMoving()); flickWithTouch(window.data(), touchDevice, QPoint(100, 400), QPoint(100, 240)); // In the original bug, that second flick would cause Flickable to halt immediately qreal contentYAfterSecondFlick = flickable->contentY(); qDebug() << "contentYAfterSecondFlick " << contentYAfterSecondFlick; - QVERIFY(contentYAfterSecondFlick > (contentYAfterFirstFlick + 80.0f)); + QTRY_VERIFY(contentYAfterSecondFlick > (contentYAfterFirstFlick + 80.0f)); } void tst_qquickflickable::flickWithTouch(QWindow *window, QTouchDevice *touchDevice, const QPoint &from, const QPoint &to) @@ -1419,6 +1423,90 @@ void tst_qquickflickable::nestedStopAtBounds() QTRY_VERIFY(!outer->isMoving()); } +void tst_qquickflickable::stopAtBounds_data() +{ + QTest::addColumn("transpose"); + QTest::addColumn("invert"); + + QTest::newRow("left") << false << false; + QTest::newRow("right") << false << true; + QTest::newRow("top") << true << false; + QTest::newRow("bottom") << true << true; +} + +void tst_qquickflickable::stopAtBounds() +{ + QFETCH(bool, transpose); + QFETCH(bool, invert); + + QQuickView view; + view.setSource(testFileUrl("stopAtBounds.qml")); + QTRY_COMPARE(view.status(), QQuickView::Ready); + QQuickViewTestUtil::centerOnScreen(&view); + QQuickViewTestUtil::moveMouseAway(&view); + view.show(); + view.requestActivate(); + QVERIFY(QTest::qWaitForWindowExposed(&view)); + QVERIFY(view.rootObject()); + + QQuickFlickable *flickable = qobject_cast(view.rootObject()); + QVERIFY(flickable); + + if (transpose) + flickable->setContentY(invert ? 100 : 0); + else + flickable->setContentX(invert ? 100 : 0); + + const int threshold = qApp->styleHints()->startDragDistance(); + + QPoint position(200, 200); + int &axis = transpose ? position.ry() : position.rx(); + + // drag away from the aligned boundary. View should not move + QTest::mousePress(&view, Qt::LeftButton, 0, position); + QTest::qWait(10); + for (int i = 0; i < 3; ++i) { + axis += invert ? -threshold : threshold; + QTest::mouseMove(&view, position); + } + QCOMPARE(flickable->isDragging(), false); + if (invert) + QCOMPARE(transpose ? flickable->isAtYEnd() : flickable->isAtXEnd(), true); + else + QCOMPARE(transpose ? flickable->isAtYBeginning() : flickable->isAtXBeginning(), true); + + // drag back towards boundary + for (int i = 0; i < 24; ++i) { + axis += invert ? threshold / 3 : -threshold / 3; + QTest::mouseMove(&view, position); + } + QTRY_COMPARE(flickable->isDragging(), true); + if (invert) + QCOMPARE(transpose ? flickable->isAtYEnd() : flickable->isAtXEnd(), false); + else + QCOMPARE(transpose ? flickable->isAtYBeginning() : flickable->isAtXBeginning(), false); + + // Drag away from the aligned boundary again. + // None of the mouse movements will position the view at the boundary exactly, + // but the view should end up aligned on the boundary + for (int i = 0; i < 5; ++i) { + axis += invert ? -threshold * 2 : threshold * 2; + QTest::mouseMove(&view, position); + } + QCOMPARE(flickable->isDragging(), true); + + // we should have hit the boundary and stopped + if (invert) { + QCOMPARE(transpose ? flickable->isAtYEnd() : flickable->isAtXEnd(), true); + QCOMPARE(transpose ? flickable->contentY() : flickable->contentX(), 100.0); + } else { + QCOMPARE(transpose ? flickable->isAtYBeginning() : flickable->isAtXBeginning(), true); + QCOMPARE(transpose ? flickable->contentY() : flickable->contentX(), 0.0); + } + + QTest::mouseRelease(&view, Qt::LeftButton, 0, position); +} + void tst_qquickflickable::nestedMouseAreaUsingTouch() { QTouchDevice *touchDevice = new QTouchDevice; -- cgit v1.2.3