Ensure Flickable.flickEnded signal is emitted after flick-then-drag

hData and vData's flicking flags could get set to false in two places:
movementEnding(), which would also emit flickEnded(); or in
maybeBeginDrag(), which did not emit flickEnded(). So if the user did a
quick flick-flick-flick sequence, usually the flick would be restarted,
and movementEnding would emit the signal after motion stopped. But if
the user dragged too slowly the final time, the flick would not be
restarted, and movementEnding() did not emit flickEnded() because the
two flicking flags were already false. At the time the final drag
starts, we don't know if the user is going to drag fast enough to
qualify as a flick, so that's probably why maybeBeginDrag() doesn't
emit flickEnded(). But information is lost by setting the flicking
flags to false there.

So to make the flickStarted() and flickEnded() signals more symmetric,
we also need flags to remember that when the drag began, there was
residual momentum (it was already flicking). Then movementEnding() can
emit flickEnded() after movement has stopped, which is more consistent
with what the user should expect: all's well that ends well, even if it
started more than once.

It seems the intention was always that flickEnded() means the whole
sequence of flicks and drags had ended; so give a hint in the docs.

Declare the wasFlicking locals const, as a drive-by.

The bool bitfields in AxisData are converted to uint for proper packing
on MSVC.

Fixes: QTBUG-103620
Change-Id: I363aa3a0d95e1fff21b4b09f22e88e51b88c7c16
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit c9b8acba0e49a4d461183d069d12cbf18dfebe41)
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

3 files changed, 102 insertions, 19 deletions

diff --git a/src/quick/items/qquickflickable.cpp b/src/quick/items/qquickflickable.cpp
index e684dae0bc..03ef6142d1 100644
--- a/src/quick/items/qquickflickable.cpp
+++ b/src/quick/items/qquickflickable.cpp
@@ -717,7 +717,8 @@ void QQuickFlickablePrivate::updateBeginningEnd()
 /*!
     \qmlsignal QtQuick::Flickable::flickEnded()
 
-    This signal is emitted when the view stops moving due to a flick.
+    This signal is emitted when the view stops moving after a flick
+    or a series of flicks.
 */
 
 /*!
@@ -1115,7 +1116,9 @@ void QQuickFlickablePrivate::maybeBeginDrag(qint64 currentTimestamp, const QPoin
     pressPos = pressPosn;
     hData.pressPos = hData.move.value();
     vData.pressPos = vData.move.value();
-    bool wasFlicking = hData.flicking || vData.flicking;
+    const bool wasFlicking = hData.flicking || vData.flicking;
+    hData.flickingWhenDragBegan = hData.flicking;
+    vData.flickingWhenDragBegan = vData.flicking;
     if (hData.flicking) {
         hData.flicking = false;
         emit q->flickingHorizontallyChanged();
@@ -2919,7 +2922,7 @@ void QQuickFlickable::movementEnding(bool hMovementEnding, bool vMovementEnding)
     Q_D(QQuickFlickable);
 
     // emit flicking signals
-    bool wasFlicking = d->hData.flicking || d->vData.flicking;
+    const bool wasFlicking = d->hData.flicking || d->vData.flicking;
     if (hMovementEnding && d->hData.flicking) {
         d->hData.flicking = false;
         emit flickingHorizontallyChanged();
@@ -2931,6 +2934,10 @@ void QQuickFlickable::movementEnding(bool hMovementEnding, bool vMovementEnding)
     if (wasFlicking && (!d->hData.flicking || !d->vData.flicking)) {
         emit flickingChanged();
         emit flickEnded();
+    } else if (d->hData.flickingWhenDragBegan || d->vData.flickingWhenDragBegan) {
+        d->hData.flickingWhenDragBegan = !hMovementEnding;
+        d->vData.flickingWhenDragBegan = !vMovementEnding;
+        emit flickEnded();
     }
 
     // emit moving signals
diff --git a/src/quick/items/qquickflickable_p_p.h b/src/quick/items/qquickflickable_p_p.h
index 46d5b0f55d..d6d34c160d 100644
--- a/src/quick/items/qquickflickable_p_p.h
+++ b/src/quick/items/qquickflickable_p_p.h
@@ -71,7 +71,7 @@ public:
             , smoothVelocity(fp), atEnd(false), atBeginning(true)
             , transitionToSet(false)
             , fixingUp(false), inOvershoot(false), inRebound(false), moving(false), flicking(false)
-            , dragging(false), extentsChanged(false)
+            , flickingWhenDragBegan(false), dragging(false), extentsChanged(false)
             , explicitValue(false), minExtentDirty(true), maxExtentDirty(true)
             , contentPositionChangedExternallyDuringDrag(false)
             , unused(0)
@@ -121,21 +121,22 @@ public:
         int vTime;
         QQuickFlickablePrivate::Velocity smoothVelocity;
         QPODVector<qreal,10> velocityBuffer;
-        bool atEnd : 1;
-        bool atBeginning : 1;
-        bool transitionToSet : 1;
-        bool fixingUp : 1;
-        bool inOvershoot : 1;
-        bool inRebound : 1;
-        bool moving : 1;
-        bool flicking : 1;
-        bool dragging : 1;
-        bool extentsChanged : 1;
-        bool explicitValue : 1;
-        mutable bool minExtentDirty : 1;
-        mutable bool maxExtentDirty : 1;
-        bool contentPositionChangedExternallyDuringDrag : 1;
-        uint unused : 18;
+        uint atEnd : 1;
+        uint atBeginning : 1;
+        uint transitionToSet : 1;
+        uint fixingUp : 1;
+        uint inOvershoot : 1;
+        uint inRebound : 1;
+        uint moving : 1;
+        uint flicking : 1;
+        uint flickingWhenDragBegan : 1;
+        uint dragging : 1;
+        uint extentsChanged : 1;
+        uint explicitValue : 1;
+        mutable uint minExtentDirty : 1;
+        mutable uint maxExtentDirty : 1;
+        uint contentPositionChangedExternallyDuringDrag : 1;
+        uint unused : 17;
     };
 
     bool flickX(qreal velocity);
diff --git a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
index 33d8dafc14..af591dd5ce 100644
--- a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
+++ b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
@@ -182,6 +182,7 @@ private slots:
     void movingAndDragging_data();
     void flickOnRelease();
     void pressWhileFlicking();
+    void dragWhileFlicking();
     void disabled();
     void flickVelocity();
     void margins();
@@ -1458,6 +1459,8 @@ void tst_qquickflickable::pressWhileFlicking()
     QSignalSpy hFlickSpy(flickable, SIGNAL(flickingHorizontallyChanged()));
     QSignalSpy vFlickSpy(flickable, SIGNAL(flickingVerticallyChanged()));
     QSignalSpy flickSpy(flickable, SIGNAL(flickingChanged()));
+    QSignalSpy flickStartSpy(flickable, &QQuickFlickable::flickStarted);
+    QSignalSpy flickEndSpy(flickable, &QQuickFlickable::flickEnded);
 
     // flick then press while it is still moving
     // flicking == false, moving == true;
@@ -1475,6 +1478,8 @@ void tst_qquickflickable::pressWhileFlicking()
     QCOMPARE(vFlickSpy.count(), 1);
     QCOMPARE(hFlickSpy.count(), 0);
     QCOMPARE(flickSpy.count(), 1);
+    QCOMPARE(flickStartSpy.count(), 1);
+    QCOMPARE(flickEndSpy.count(), 0);
 
     QTest::mousePress(window.data(), Qt::LeftButton, Qt::NoModifier, QPoint(20, 50));
     QTRY_VERIFY(!flickable->isFlicking());
@@ -1487,6 +1492,76 @@ void tst_qquickflickable::pressWhileFlicking()
     QVERIFY(!flickable->isFlickingVertically());
     QTRY_VERIFY(!flickable->isMoving());
     QVERIFY(!flickable->isMovingVertically());
+    QCOMPARE(flickStartSpy.size(), 1);
+    QCOMPARE(flickEndSpy.size(), 1);
+    // Stop on a full pixel after user interaction
+    QCOMPARE(flickable->contentX(), (qreal)qRound(flickable->contentX()));
+}
+
+void tst_qquickflickable::dragWhileFlicking()
+{
+    QQuickView window;
+    QVERIFY(QQuickTest::showView(window, testFileUrl("flickable03.qml")));
+
+    QQuickFlickable *flickable = qobject_cast<QQuickFlickable*>(window.rootObject());
+    QVERIFY(flickable != nullptr);
+
+    QSignalSpy vMoveSpy(flickable, &QQuickFlickable::movingVerticallyChanged);
+    QSignalSpy hMoveSpy(flickable, &QQuickFlickable::movingHorizontallyChanged);
+    QSignalSpy moveSpy(flickable, &QQuickFlickable::movingChanged);
+    QSignalSpy hFlickSpy(flickable, &QQuickFlickable::flickingHorizontallyChanged);
+    QSignalSpy vFlickSpy(flickable, &QQuickFlickable::flickingVerticallyChanged);
+    QSignalSpy flickSpy(flickable, &QQuickFlickable::flickingChanged);
+    QSignalSpy flickStartSpy(flickable, &QQuickFlickable::flickStarted);
+    QSignalSpy flickEndSpy(flickable, &QQuickFlickable::flickEnded);
+
+    // flick first, let it keep moving
+    flick(&window, QPoint(20,190), QPoint(20, 50), 200);
+    QVERIFY(flickable->verticalVelocity() > 0.0);
+    QTRY_VERIFY(flickable->isFlicking());
+    QVERIFY(flickable->isFlickingVertically());
+    QCOMPARE(flickable->isFlickingHorizontally(), false);
+    QVERIFY(flickable->isMoving());
+    QVERIFY(flickable->isMovingVertically());
+    QCOMPARE(flickable->isMovingHorizontally(), false);
+    QCOMPARE(vMoveSpy.size(), 1);
+    QCOMPARE(hMoveSpy.size(), 0);
+    QCOMPARE(moveSpy.size(), 1);
+    QCOMPARE(vFlickSpy.size(), 1);
+    QCOMPARE(hFlickSpy.size(), 0);
+    QCOMPARE(flickSpy.size(), 1);
+    QCOMPARE(flickStartSpy.size(), 1);
+    QCOMPARE(flickEndSpy.size(), 0);
+
+    // then drag slowly while it's still flicking and moving
+    const int dragStepDelay = 100;
+    QTest::mousePress(&window, Qt::LeftButton, Qt::NoModifier, QPoint(20, 70));
+    QTRY_COMPARE(flickable->isFlicking(), false);
+    QCOMPARE(flickable->isFlickingVertically(), false);
+    QVERIFY(flickable->isMoving());
+    QVERIFY(flickable->isMovingVertically());
+
+    for (int y = 70; y > 50; y -= 5) {
+        QTest::mouseMove(&window, QPoint(20, y), dragStepDelay);
+        QVERIFY(flickable->isMoving());
+        QVERIFY(flickable->isMovingVertically());
+        // Flickable's timeline is real-time, so spoofing timestamps isn't enough
+        QTest::qWait(dragStepDelay);
+    }
+
+    QTest::mouseRelease(&window, Qt::LeftButton, Qt::NoModifier, QPoint(20, 50), dragStepDelay);
+
+    QCOMPARE(flickable->isFlicking(), false);
+    QCOMPARE(flickable->isFlickingVertically(), false);
+    QTRY_COMPARE(flickable->isMoving(), false);
+    QCOMPARE(flickable->isMovingVertically(), false);
+    QCOMPARE(flickStartSpy.size(), 1);
+    QCOMPARE(flickEndSpy.size(), 1);
+    QCOMPARE(vMoveSpy.size(), 2);
+    QCOMPARE(hMoveSpy.size(), 0);
+    QCOMPARE(moveSpy.size(), 2);
+    QCOMPARE(vFlickSpy.size(), 2);
+    QCOMPARE(hFlickSpy.size(), 0);
     // Stop on a full pixel after user interaction
     QCOMPARE(flickable->contentX(), (qreal)qRound(flickable->contentX()));
 }