Flickable: Fix edge hitting detection when the extent is fractional

When the extent is fractional, the updateBeginningEnd() function would
not be able to detect if the contentItem is hitting an edge or not.

This is because the target move position for when a flick is initiated,
is at some point rounded (in QQuickFlickablePrivate::fixup()), while
the extent is not.

This patch will ceil the minExtent/topMargin, before checking whether
the contentItem y position is hitting it or not. This value is normally
negative, and the ceil will thus be closer to 0.
This will mean that if the target move position is rounded to be closer
to 0, it will now count that as hitting the edge, even in cases when the
topMargin is a few subpixels further away (which is not visually
noticeable). The same logic is applied to the maxExtent/bottomMargin,
and for the x axis.

Furthermore, this patch also improves the scrolling logic in wheelEvent,
ensuring that flickingStarted() and movingStarted() are not called unless
1) the contentItem is currently standing still, and 2) the scrolling
isn't directed towards the edge while the contentItem is near it
(no movement should occur in this case).

Here is a more holistic expanation for what was happening in the
reported bug:

Scrolling a flickable will cause an "artificial" flick, unless the user
has scrolled the maximum distance in a particular direction.

When flicking, a timeline is started with a goal for what the contentItem's
position should end up being when the timeline completes.

As the timeline advances, it will callback the fixup function, which
will round the target x and y move position for the contentItem.
Which is used to check whether the contentItem is hitting an edge or
not. A fuzzy compare is being done here, but since the extent isn't being
rounded, it will fail when the extent is fractional.

When you scroll with the wheel, QQuickFlickable::wheelEvent will typically
be called a couple of times, and we don't want to call flickingStarted()
or movementStarted() when the Flickable has already scrolled to its limit
in a particular direction (an edge).

While calling flickY() is undesired in this situation, the larger
problem is movementStarted(). This function will set the the moving flag
in vData to true.

The moving flag in the hData and vData is used in a lot of different
places, to figure out what to do in special situations (like resizing
the window). But the moving flag is only set to false upon a call to
timelineCompleted() (which unsurprisingly is called when the timeline
is completed).

What was happening in the reported bug, is the following:
1. wheelEvent() is called, and starts flicking the listview towards the
   edge.
2. The destination for the flick is rounded, to have a lower absolute
   value than the extent.
3. Any subsequent calls to wheelEvent() will also call flickY() and
   movementStarted(), setting the moving flag to true, even though
   no movement is occurring (since the contentItem has already reached
   it's maximum or minimum value).
4. Since flickY() isn't really doing much when it's called (since the
   source value == destination value), it will cause the timeline to
   complete instantly, inside the flickY() call. This would lead to
   the moving flag being set to true in the movementStarted() function,
   that is called right after flickY() in wheelEvent(). The moving flag
   would remain true forever, since timelineCompleted() must be called
   for it to become false again, but that function was called before
   movingStarted(). The user would need to flick in a different direction
   for it to go back to false again.
5. Since the vData.moving flag is set to true, fixupY() will not get
   called from QQuickFlickable::geometryChange() (when resizing the
   window). Normally, when resizing the window, the adjustContentPos()
   will get called, which will provide the timeline with the correct new
   contentItem position, and use the Immediate fixupMode, that will
   cause the timeline to complete immediately. If the vData.moving flag is
   true, all of these fixup steps will be skipped during
   geometryChange(), which was the case in the reported bug.

In the above description, I've mostly been talking about the behavior
that happens when scrolling in the vertical direction, but I see no
reason why this couldn't theoretically happen in the horizontal
direction as well. Therefore, I am applying the same fix for scrolling
horizontally.

Fixes: QTBUG-101268
Fixes: QTBUG-101266
Change-Id: I0e56290541a9f28a5489d92251f059e7448184f0
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
(cherry picked from commit 833f83f270e844265905e4657ef74319b7b8b68f)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

3 files changed, 111 insertions, 10 deletions

diff --git a/src/quick/items/qquickflickable.cpp b/src/quick/items/qquickflickable.cpp
index 721dba1769..151bd521bf 100644
--- a/src/quick/items/qquickflickable.cpp
+++ b/src/quick/items/qquickflickable.cpp
@@ -520,8 +520,8 @@ void QQuickFlickablePrivate::updateBeginningEnd()
     const qreal maxyextent = -q->maxYExtent();
     const qreal minyextent = -q->minYExtent();
     const qreal ypos = -vData.move.value();
-    bool atBeginning = fuzzyLessThanOrEqualTo(ypos, minyextent);
-    bool atEnd = fuzzyLessThanOrEqualTo(maxyextent, ypos);
+    bool atBeginning = fuzzyLessThanOrEqualTo(ypos, std::ceil(minyextent));
+    bool atEnd = fuzzyLessThanOrEqualTo(std::floor(maxyextent), ypos);
 
     if (atBeginning != vData.atBeginning) {
         vData.atBeginning = atBeginning;
@@ -540,8 +540,8 @@ void QQuickFlickablePrivate::updateBeginningEnd()
     const qreal maxxextent = -q->maxXExtent();
     const qreal minxextent = -q->minXExtent();
     const qreal xpos = -hData.move.value();
-    atBeginning = fuzzyLessThanOrEqualTo(xpos, minxextent);
-    atEnd = fuzzyLessThanOrEqualTo(maxxextent, xpos);
+    atBeginning = fuzzyLessThanOrEqualTo(xpos, std::ceil(minxextent));
+    atEnd = fuzzyLessThanOrEqualTo(std::floor(maxxextent), xpos);
 
     if (atBeginning != hData.atBeginning) {
         hData.atBeginning = atBeginning;
@@ -1621,9 +1621,9 @@ void QQuickFlickable::wheelEvent(QWheelEvent *event)
             d->vData.addVelocitySample(instVelocity, d->maxVelocity);
             d->vData.updateVelocity();
             if ((yDelta > 0 && contentY() > -minYExtent()) || (yDelta < 0 && contentY() < -maxYExtent())) {
-                d->flickY(d->vData.velocity);
-                d->flickingStarted(false, true);
-                if (d->vData.flicking) {
+                const bool newFlick = d->flickY(d->vData.velocity);
+                if (newFlick && (d->vData.atBeginning != (yDelta > 0) || d->vData.atEnd != (yDelta < 0))) {
+                    d->flickingStarted(false, true);
                     d->vMoved = true;
                     movementStarting();
                 }
@@ -1638,9 +1638,9 @@ void QQuickFlickable::wheelEvent(QWheelEvent *event)
             d->hData.addVelocitySample(instVelocity, d->maxVelocity);
             d->hData.updateVelocity();
             if ((xDelta > 0 && contentX() > -minXExtent()) || (xDelta < 0 && contentX() < -maxXExtent())) {
-                d->flickX(d->hData.velocity);
-                d->flickingStarted(true, false);
-                if (d->hData.flicking) {
+                const bool newFlick = d->flickX(d->hData.velocity);
+                if (newFlick && (d->hData.atBeginning != (xDelta > 0) || d->hData.atEnd != (xDelta < 0))) {
+                    d->flickingStarted(true, false);
                     d->hMoved = true;
                     movementStarting();
                 }
diff --git a/tests/auto/quick/qquickflickable/data/fractionalExtent.qml b/tests/auto/quick/qquickflickable/data/fractionalExtent.qml
new file mode 100644
index 0000000000..2675e8c85a
--- /dev/null
+++ b/tests/auto/quick/qquickflickable/data/fractionalExtent.qml
@@ -0,0 +1,14 @@
+import QtQuick
+
+Flickable {
+    width: 300
+    height: 300
+    contentWidth: content.width; contentHeight: content.height
+    Rectangle {
+        id: content
+        width: 350
+        height: 350
+        color: "darkkhaki"
+    }
+    boundsBehavior: Flickable.StopAtBounds
+}
diff --git a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
index c55537364c..1e8d162d03 100644
--- a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
+++ b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
@@ -214,6 +214,8 @@ private slots:
     void receiveTapOutsideContentItem();
     void flickWhenRotated_data();
     void flickWhenRotated();
+    void scrollingWithFractionalExtentSize_data();
+    void scrollingWithFractionalExtentSize();
 
 private:
     void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to);
@@ -2851,6 +2853,91 @@ void tst_qquickflickable::flickWhenRotated() // QTBUG-99639
     QVERIFY(!flickable->isAtYBeginning());
 }
 
+
+void tst_qquickflickable::scrollingWithFractionalExtentSize_data()
+{
+    QTest::addColumn<QByteArray>("property");
+    QTest::addColumn<bool>("isYAxis");
+    QTest::addColumn<bool>("atBeginning");
+    QTest::addColumn<QQuickFlickable::BoundsBehaviorFlag>("boundsBehaviour");
+
+    QTest::newRow("minyextent") << QByteArray("topMargin") << true << true << QQuickFlickable::StopAtBounds;
+    QTest::newRow("maxyextent") << QByteArray("bottomMargin") << true << false << QQuickFlickable::StopAtBounds;
+    QTest::newRow("minxextent") << QByteArray("leftMargin") << false << true << QQuickFlickable::StopAtBounds;
+    QTest::newRow("maxxextent") << QByteArray("rightMargin") << false << false << QQuickFlickable::StopAtBounds;
+
+    QTest::newRow("minyextent") << QByteArray("topMargin") << true << true << QQuickFlickable::DragAndOvershootBounds;
+    QTest::newRow("maxyextent") << QByteArray("bottomMargin") << true << false << QQuickFlickable::DragAndOvershootBounds;
+    QTest::newRow("minxextent") << QByteArray("leftMargin") << false << true << QQuickFlickable::DragAndOvershootBounds;
+    QTest::newRow("maxxextent") << QByteArray("rightMargin") << false << false << QQuickFlickable::DragAndOvershootBounds;
+}
+
+void tst_qquickflickable::scrollingWithFractionalExtentSize() // QTBUG-101268
+{
+    QFETCH(QByteArray, property);
+    QFETCH(bool, isYAxis);
+    QFETCH(bool, atBeginning);
+    QFETCH(QQuickFlickable::BoundsBehaviorFlag, boundsBehaviour);
+
+    QQuickView window;
+    QVERIFY(QQuickTest::showView(window, testFileUrl("fractionalExtent.qml")));
+    QQuickItem *rootItem = window.rootObject();
+    QVERIFY(rootItem);
+    QQuickFlickable *flickable = qobject_cast<QQuickFlickable *>(rootItem);
+    QVERIFY(flickable);
+    flickable->setBoundsBehavior(boundsBehaviour);
+
+    qreal value = 100.5;
+    flickable->setProperty(property.constData(), 100.5);
+    if (isYAxis)
+        flickable->setContentY(atBeginning ? -value : value + qAbs(flickable->height() - flickable->contentHeight()));
+    else
+        flickable->setContentX(atBeginning ? -value : value + qAbs(flickable->width() - flickable->contentWidth()));
+
+    if (isYAxis) {
+        QVERIFY(atBeginning ? flickable->isAtYBeginning() : flickable->isAtYEnd());
+        QVERIFY(!flickable->isMovingVertically());
+    } else {
+        QVERIFY(atBeginning ? flickable->isAtXBeginning() : flickable->isAtXEnd());
+        QVERIFY(!flickable->isMovingHorizontally());
+    }
+
+    QPointF pos(flickable->x() + flickable->width() / 2, flickable->y() + flickable->height() / 2);
+    QPoint angleDelta(isYAxis ? 0 : (atBeginning ? -50 : 50), isYAxis ? (atBeginning ? -50 : 50) : 0);
+
+    QWheelEvent wheelEvent1(pos, window.mapToGlobal(pos), QPoint(), angleDelta,
+                               Qt::NoButton, Qt::NoModifier, Qt::NoScrollPhase, false);
+
+    QGuiApplication::sendEvent(&window, &wheelEvent1);
+    qApp->processEvents();
+    if (isYAxis) {
+        QVERIFY(flickable->isMovingVertically());
+        QTRY_VERIFY(!flickable->isMovingVertically());
+        QVERIFY(!(atBeginning ? flickable->isAtYBeginning() : flickable->isAtYEnd()));
+    } else {
+        QVERIFY(flickable->isMovingHorizontally());
+        QTRY_VERIFY(!flickable->isMovingHorizontally());
+        QVERIFY(!(atBeginning ? flickable->isAtXBeginning() : flickable->isAtXEnd()));
+    }
+
+    QWheelEvent wheelEvent2(pos, window.mapToGlobal(pos), QPoint(), -angleDelta,
+                               Qt::NoButton, Qt::NoModifier, Qt::NoScrollPhase, false);
+    wheelEvent2.setTimestamp(wheelEvent1.timestamp() + 10);
+
+    QGuiApplication::sendEvent(&window, &wheelEvent2);
+    qApp->processEvents();
+
+    if (isYAxis) {
+        QVERIFY(flickable->isMovingVertically());
+        QTRY_VERIFY(!flickable->isMovingVertically());
+        QVERIFY(atBeginning ? flickable->isAtYBeginning() : flickable->isAtYEnd());
+    } else {
+        QVERIFY(flickable->isMovingHorizontally());
+        QTRY_VERIFY(!flickable->isMovingHorizontally());
+        QVERIFY(atBeginning ? flickable->isAtXBeginning() : flickable->isAtXEnd());
+    }
+}
+
 QTEST_MAIN(tst_qquickflickable)
 
 #include "tst_qquickflickable.moc"