Flickable: let changing contentItem pos also affect the drag starting pos

When calling the setContentX(qreal) and setContentY(qreal) functions,
the flickable would not update the drag starting position if they were
called in the middle of a dragging operation.

This patch makes those function update the drag starting position, so
that the drag will take into account the external changes to the
contentItem position, and not make any massive leaps on the next call to
drag() after changing the contentItem position directly.

Note that vData.pressPos and hData.pressPos are set to the x and y
position of the contentItem at the beginning of a drag operation.
They are unrelated to the mouse position.

The bug QTBUG-104966 will be fixed from this, since QQuickItemView::setModel() calls
QQuickListViewPrivate::setPosition() which calls
QQuickFlickable::setContentX/Y().

The QQuickFlickable::setContentX/Y functions are public (part of the public
API). They will update the timeline value for the x and y axis, which
will as a result also call setViewportX/Y, which calls setX/Y for the
contentItem itself.

Done-with: Jan Arve Sæther <jan-arve.saether@qt.io>
Fixes: QTBUG-104966
Change-Id: Id5165e1ff37a07b94be8c1cc152e86dfcc13d1c6
Reviewed-by: Oliver Eftevaag <oliver.eftevaag@qt.io>
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>
(cherry picked from commit 5647527a8cde84b51fff66fc482f02435770b3dd)

4 files changed, 153 insertions, 8 deletions

diff --git a/src/quick/items/qquickflickable.cpp b/src/quick/items/qquickflickable.cpp
index f4b2379979..ea39edff18 100644
--- a/src/quick/items/qquickflickable.cpp
+++ b/src/quick/items/qquickflickable.cpp
@@ -326,7 +326,7 @@ void QQuickFlickablePrivate::AxisData::updateVelocity()
     }
 }
 
-void QQuickFlickablePrivate::itemGeometryChanged(QQuickItem *item, QQuickGeometryChange change, const QRectF &)
+void QQuickFlickablePrivate::itemGeometryChanged(QQuickItem *item, QQuickGeometryChange change, const QRectF &oldGeom)
 {
     Q_Q(QQuickFlickable);
     if (item == contentItem) {
@@ -335,8 +335,14 @@ void QQuickFlickablePrivate::itemGeometryChanged(QQuickItem *item, QQuickGeometr
             orient |= Qt::Horizontal;
         if (change.yChange())
             orient |= Qt::Vertical;
-        if (orient)
+        if (orient) {
             q->viewportMoved(orient);
+            const QPointF deltaMoved = item->position() - oldGeom.topLeft();
+            if (hData.contentPositionChangedExternallyDuringDrag)
+                hData.pressPos += deltaMoved.x();
+            if (vData.contentPositionChangedExternallyDuringDrag)
+                vData.pressPos += deltaMoved.y();
+        }
         if (orient & Qt::Horizontal)
             emit q->contentXChanged();
         if (orient & Qt::Vertical)
@@ -796,8 +802,11 @@ void QQuickFlickable::setContentX(qreal pos)
     d->hData.vTime = d->timeline.time();
     if (isMoving() || isFlicking())
         movementEnding(true, false);
-    if (!qFuzzyCompare(-pos, d->hData.move.value()))
+    if (!qFuzzyCompare(-pos, d->hData.move.value())) {
+        d->hData.contentPositionChangedExternallyDuringDrag = d->hData.dragging;
         d->hData.move.setValue(-pos);
+        d->hData.contentPositionChangedExternallyDuringDrag = false;
+    }
 }
 
 qreal QQuickFlickable::contentY() const
@@ -814,8 +823,11 @@ void QQuickFlickable::setContentY(qreal pos)
     d->vData.vTime = d->timeline.time();
     if (isMoving() || isFlicking())
         movementEnding(false, true);
-    if (!qFuzzyCompare(-pos, d->vData.move.value()))
+    if (!qFuzzyCompare(-pos, d->vData.move.value())) {
+        d->vData.contentPositionChangedExternallyDuringDrag = d->vData.dragging;
         d->vData.move.setValue(-pos);
+        d->vData.contentPositionChangedExternallyDuringDrag = false;
+    }
 }
 
 /*!
@@ -2108,9 +2120,11 @@ void QQuickFlickable::setContentWidth(qreal w)
         d->contentItem->setWidth(w);
     d->hData.markExtentsDirty();
     // Make sure that we're entirely in view.
-    if (!d->pressed && !d->hData.moving && !d->vData.moving) {
+    if ((!d->pressed && !d->hData.moving && !d->vData.moving) || d->hData.dragging) {
+        d->hData.contentPositionChangedExternallyDuringDrag = d->hData.dragging;
         d->fixupMode = QQuickFlickablePrivate::Immediate;
         d->fixupX();
+        d->hData.contentPositionChangedExternallyDuringDrag = false;
     } else if (!d->pressed && d->hData.fixingUp) {
         d->fixupMode = QQuickFlickablePrivate::ExtentChanged;
         d->fixupX();
@@ -2137,9 +2151,11 @@ void QQuickFlickable::setContentHeight(qreal h)
         d->contentItem->setHeight(h);
     d->vData.markExtentsDirty();
     // Make sure that we're entirely in view.
-    if (!d->pressed && !d->hData.moving && !d->vData.moving) {
+    if ((!d->pressed && !d->hData.moving && !d->vData.moving) || d->vData.dragging) {
+        d->vData.contentPositionChangedExternallyDuringDrag = d->vData.dragging;
         d->fixupMode = QQuickFlickablePrivate::Immediate;
         d->fixupY();
+        d->vData.contentPositionChangedExternallyDuringDrag = false;
     } else if (!d->pressed && d->vData.fixingUp) {
         d->fixupMode = QQuickFlickablePrivate::ExtentChanged;
         d->fixupY();
diff --git a/src/quick/items/qquickflickable_p_p.h b/src/quick/items/qquickflickable_p_p.h
index 1d99462e5d..0b990739fb 100644
--- a/src/quick/items/qquickflickable_p_p.h
+++ b/src/quick/items/qquickflickable_p_p.h
@@ -109,6 +109,7 @@ public:
             , fixingUp(false), inOvershoot(false), inRebound(false), moving(false), flicking(false)
             , dragging(false), extentsChanged(false)
             , explicitValue(false), minExtentDirty(true), maxExtentDirty(true)
+            , contentPositionChangedExternallyDuringDrag(false)
             , unused(0)
         {}
 
@@ -169,7 +170,8 @@ public:
         bool explicitValue : 1;
         mutable bool minExtentDirty : 1;
         mutable bool maxExtentDirty : 1;
-        uint unused : 19;
+        bool contentPositionChangedExternallyDuringDrag : 1;
+        uint unused : 18;
     };
 
     bool flickX(qreal velocity);
diff --git a/tests/auto/quick/qquickflickable/data/contentPosWhileDragging.qml b/tests/auto/quick/qquickflickable/data/contentPosWhileDragging.qml
new file mode 100644
index 0000000000..57a4273257
--- /dev/null
+++ b/tests/auto/quick/qquickflickable/data/contentPosWhileDragging.qml
@@ -0,0 +1,22 @@
+import QtQuick 2.14
+
+Item {
+    id: root
+    width: 500
+    height: 500
+    Flickable {
+        anchors.centerIn: parent
+        width: 100
+        height: 100
+        clip: true
+        contentWidth: content.width
+        contentHeight: content.height
+        Rectangle {
+            id: content
+            width: 320
+            height: width
+            color: "#41cd52"
+            radius: width/2
+        }
+    }
+}
diff --git a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
index 9fa51da6f8..d092cd0170 100644
--- a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
+++ b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
@@ -27,10 +27,10 @@
 ****************************************************************************/
 #include <qtest.h>
 #include <QtTest/QSignalSpy>
+#include <QtQuick/qquickview.h>
 #include <QtGui/QStyleHints>
 #include <QtQml/qqmlengine.h>
 #include <QtQml/qqmlcomponent.h>
-#include <QtQuick/qquickview.h>
 #include <private/qquickflickable_p.h>
 #include <private/qquickflickable_p_p.h>
 #include <private/qquickmousearea_p.h>
@@ -206,6 +206,8 @@ private slots:
     void synchronousDrag_data();
     void synchronousDrag();
     void visibleAreaBinding();
+    void setContentPositionWhileDragging_data();
+    void setContentPositionWhileDragging();
 
 private:
     void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to);
@@ -2558,6 +2560,109 @@ void tst_qquickflickable::visibleAreaBinding()
     // Shouldn't crash.
 }
 
+void tst_qquickflickable::setContentPositionWhileDragging_data()
+{
+    QTest::addColumn<bool>("isHorizontal");
+    QTest::addColumn<int>("newPos");
+    QTest::addColumn<int>("newExtent");
+    QTest::newRow("horizontal, setContentX") << true << 0 << -1;
+    QTest::newRow("vertical, setContentY") << false << 0 << -1;
+    QTest::newRow("horizontal, setContentWidth") << true << -1 << 200;
+    QTest::newRow("vertical, setContentHeight") << false << -1 << 200;
+}
+
+void tst_qquickflickable::setContentPositionWhileDragging() // QTBUG-104966
+{
+    QFETCH(bool, isHorizontal);
+    QFETCH(int, newPos);
+    QFETCH(int, newExtent);
+
+    QScopedPointer<QQuickView> window(new QQuickView);
+    window->setSource(testFileUrl("contentPosWhileDragging.qml"));
+    QTRY_COMPARE(window->status(), QQuickView::Ready);
+    QQuickViewTestUtil::centerOnScreen(window.data());
+    QQuickViewTestUtil::moveMouseAway(window.data());
+    window->show();
+    QVERIFY(QTest::qWaitForWindowActive(window.data()));
+    QQuickItem *rootItem = window->rootObject();
+    QVERIFY(rootItem);
+
+    QVERIFY(window->isVisible());
+    QQuickFlickable *flickable = rootItem->findChild<QQuickFlickable *>();
+    QVERIFY(flickable);
+
+    const auto contentPos = [flickable]() -> QPoint {
+        return QPoint(flickable->contentX(), flickable->contentY());
+    };
+    const qreal threshold =
+            qApp->styleHints()->startDragDistance() * flickable->parentItem()->scale();
+    const QPoint thresholdPnt(qRound(threshold), qRound(threshold));
+    const auto flickableCenterPos = flickable->mapToScene({flickable->width() / 2, flickable->height() / 2}).toPoint();
+
+    // Drag the mouse until we have surpassed the mouse drag threshold and a drag is initiated
+    // by checking for  flickable->isDragging()
+    QPoint pos = flickableCenterPos;
+    moveAndPress(window.data(), pos);
+    int j = 1;
+    QVERIFY(!flickable->isDragging());
+    while (!flickable->isDragging()) {
+        pos = flickableCenterPos - QPoint(j, j);
+        QTest::mouseMove(window.data(), pos);
+        j++;
+    }
+
+    // Now we have entered the drag state
+    QVERIFY(flickable->isDragging());
+    QCOMPARE(flickable->contentX(), 0);
+    QCOMPARE(flickable->contentY(), 0);
+    QVERIFY(flickable->width() > 0);
+    QVERIFY(flickable->height() > 0);
+
+
+    const int moveLength = 50;
+    const QPoint unitDelta(isHorizontal ? 1 : 0, isHorizontal ? 0 : 1);
+    const QPoint moveDelta = unitDelta * moveLength;
+
+    pos -= 3*moveDelta;
+    QTest::mouseMove(window.data(), pos);
+    // Should be positive because we drag in the opposite direction
+    QCOMPARE(contentPos(), 3 * moveDelta);
+    QPoint expectedContentPos;
+
+    // Set the content item position back to zero *while dragging* (!!)
+    if (newPos >= 0) {
+        if (isHorizontal) {
+            flickable->setContentX(newPos);
+        } else {
+            flickable->setContentY(newPos);
+        }
+        // Continue dragging
+        pos -= moveDelta;
+        expectedContentPos = moveDelta;
+    } else if (newExtent >= 0) {
+        // ...or reduce the content size be be less than current (contentX, contentY) position
+        // This forces the content item to move.
+        expectedContentPos = moveDelta;
+        if (isHorizontal) {
+            flickable->setContentWidth(newExtent);
+        } else {
+            flickable->setContentHeight(newExtent);
+        }
+        // Assumption is that the contentItem is aligned to the bottom of the flickable
+        // We therefore cannot scroll/flick it further down. Drag it up towards the top instead
+        // (by moving mouse down).
+        pos += moveDelta;
+    }
+
+    QTest::mouseMove(window.data(), pos);
+
+    // Make sure that the contentItem was only dragged the delta in mouse movement since the last
+    // setContentX/Y() call.
+    QCOMPARE(contentPos(), expectedContentPos);
+    QTest::mouseRelease(window.data(), Qt::LeftButton, Qt::NoModifier, pos);
+    QVERIFY(!flickable->isDragging());
+}
+
 QTEST_MAIN(tst_qquickflickable)
 
 #include "tst_qquickflickable.moc"