Drag over bounds errors when ListView has variable height content

There are two related problems:
- when dragging variable height content up beyond bounds items are
destroyed, causing the maximum extent to change, which in turn causes
skipping.  This is fixed by storing the extents on press so that later
changes do not affect drag behavior.
- when variable height content is dragged beyond the bounds and
and released, items are created as the view returns to bounds.  This
changes the extents, however the bound target was not updated.  If the
viewport height changes during fixup the bounds animation is now
updated.

Change-Id: Ib37ec9e51f8ceb71af1e2e58f25fd8da18dcd632
Task-number: QTBUG-17769
Reviewed-by: Michael Brasser

4 files changed, 84 insertions, 43 deletions

diff --git a/src/declarative/graphicsitems/qdeclarativeflickable.cpp b/src/declarative/graphicsitems/qdeclarativeflickable.cpp
index 364d76f..d64c347 100644
--- a/src/declarative/graphicsitems/qdeclarativeflickable.cpp
+++ b/src/declarative/graphicsitems/qdeclarativeflickable.cpp
@@ -143,7 +143,7 @@ QDeclarativeFlickablePrivate::QDeclarativeFlickablePrivate()
     , stealMouse(false), pressed(false), interactive(true), calcVelocity(false)
     , deceleration(500), maxVelocity(2000), reportedVelocitySmoothing(100)
     , delayedPressEvent(0), delayedPressTarget(0), pressDelay(0), fixupDuration(600)
-    , vTime(0), visibleArea(0)
+    , fixupMode(Normal), vTime(0), visibleArea(0)
     , flickableDirection(QDeclarativeFlickable::AutoFlickDirection)
     , boundsBehavior(QDeclarativeFlickable::DragAndOvershootBounds)
 {
@@ -219,6 +219,7 @@ void QDeclarativeFlickablePrivate::flick(AxisData &data, qreal minExtent, qreal
 {
     Q_Q(QDeclarativeFlickable);
     qreal maxDistance = -1;
+    data.fixingUp = false;
     bool overShoot = boundsBehavior == QDeclarativeFlickable::DragAndOvershootBounds;
     // -ve velocity means list is moving up
     if (velocity > 0) {
@@ -288,24 +289,45 @@ void QDeclarativeFlickablePrivate::fixup(AxisData &data, qreal minExtent, qreal
     if (data.move.value() > minExtent || maxExtent > minExtent) {
         timeline.reset(data.move);
         if (data.move.value() != minExtent) {
-            if (fixupDuration) {
-                qreal dist = minExtent - data.move;
-                timeline.move(data.move, minExtent - dist/2, QEasingCurve(QEasingCurve::InQuad), fixupDuration/4);
-                timeline.move(data.move, minExtent, QEasingCurve(QEasingCurve::OutExpo), 3*fixupDuration/4);
-            } else {
+            switch (fixupMode) {
+            case Immediate:
                 timeline.set(data.move, minExtent);
+                break;
+            case ExtentChanged:
+                // The target has changed. Don't start from the beginning; just complete the
+                // second half of the animation using the new extent.
+                timeline.move(data.move, minExtent, QEasingCurve(QEasingCurve::OutExpo), 3*fixupDuration/4);
+                data.fixingUp = true;
+                break;
+            default: {
+                    qreal dist = minExtent - data.move;
+                    timeline.move(data.move, minExtent - dist/2, QEasingCurve(QEasingCurve::InQuad), fixupDuration/4);
+                    timeline.move(data.move, minExtent, QEasingCurve(QEasingCurve::OutExpo), 3*fixupDuration/4);
+                    data.fixingUp = true;
+                }
             }
         }
     } else if (data.move.value() < maxExtent) {
         timeline.reset(data.move);
-        if (fixupDuration) {
-            qreal dist = maxExtent - data.move;
-            timeline.move(data.move, maxExtent - dist/2, QEasingCurve(QEasingCurve::InQuad), fixupDuration/4);
-            timeline.move(data.move, maxExtent, QEasingCurve(QEasingCurve::OutExpo), 3*fixupDuration/4);
-        } else {
+        switch (fixupMode) {
+        case Immediate:
             timeline.set(data.move, maxExtent);
+            break;
+        case ExtentChanged:
+            // The target has changed. Don't start from the beginning; just complete the
+            // second half of the animation using the new extent.
+            timeline.move(data.move, maxExtent, QEasingCurve(QEasingCurve::OutExpo), 3*fixupDuration/4);
+            data.fixingUp = true;
+            break;
+        default: {
+                qreal dist = maxExtent - data.move;
+                timeline.move(data.move, maxExtent - dist/2, QEasingCurve(QEasingCurve::InQuad), fixupDuration/4);
+                timeline.move(data.move, maxExtent, QEasingCurve(QEasingCurve::OutExpo), 3*fixupDuration/4);
+                data.fixingUp = true;
+            }
         }
     }
+    fixupMode = Normal;
     vTime = timeline.time();
 }
 
@@ -688,6 +710,12 @@ void QDeclarativeFlickablePrivate::handleMousePressEvent(QGraphicsSceneMouseEven
     vData.velocity = 0;
     hData.dragStartOffset = 0;
     vData.dragStartOffset = 0;
+    hData.dragMinBound = q->minXExtent();
+    vData.dragMinBound = q->minYExtent();
+    hData.dragMaxBound = q->maxXExtent();
+    vData.dragMaxBound = q->maxYExtent();
+    hData.fixingUp = false;
+    vData.fixingUp = false;
     lastPos = QPoint();
     QDeclarativeItemPrivate::start(lastPosTime);
     pressPos = event->pos();
@@ -716,8 +744,8 @@ void QDeclarativeFlickablePrivate::handleMouseMoveEvent(QGraphicsSceneMouseEvent
             if (!vMoved)
                 vData.dragStartOffset = dy;
             qreal newY = dy + vData.pressPos - vData.dragStartOffset;
-            const qreal minY = q->minYExtent();
-            const qreal maxY = q->maxYExtent();
+            const qreal minY = vData.dragMinBound;
+            const qreal maxY = vData.dragMaxBound;
             if (newY > minY)
                 newY = minY + (newY - minY) / 2;
             if (newY < maxY && maxY - minY <= 0)
@@ -748,8 +776,8 @@ void QDeclarativeFlickablePrivate::handleMouseMoveEvent(QGraphicsSceneMouseEvent
             if (!hMoved)
                 hData.dragStartOffset = dx;
             qreal newX = dx + hData.pressPos - hData.dragStartOffset;
-            const qreal minX = q->minXExtent();
-            const qreal maxX = q->maxXExtent();
+            const qreal minX = hData.dragMinBound;
+            const qreal maxX = hData.dragMaxBound;
             if (newX > minX)
                 newX = minX + (newX - minX) / 2;
             if (newX < maxX && maxX - minX <= 0)
@@ -1065,10 +1093,8 @@ void QDeclarativeFlickable::geometryChanged(const QRectF &newGeometry,
         }
         // Make sure that we're entirely in view.
         if (!d->pressed && !d->movingHorizontally && !d->movingVertically) {
-            int oldDuration = d->fixupDuration;
-            d->fixupDuration = 0;
+            d->fixupMode = QDeclarativeFlickablePrivate::Immediate;
             d->fixupX();
-            d->fixupDuration = oldDuration;
         }
     }
     if (newGeometry.height() != oldGeometry.height()) {
@@ -1080,10 +1106,8 @@ void QDeclarativeFlickable::geometryChanged(const QRectF &newGeometry,
         }
         // Make sure that we're entirely in view.
         if (!d->pressed && !d->movingHorizontally && !d->movingVertically) {
-            int oldDuration = d->fixupDuration;
-            d->fixupDuration = 0;
+            d->fixupMode = QDeclarativeFlickablePrivate::Immediate;
             d->fixupY();
-            d->fixupDuration = oldDuration;
         }
     }
 
@@ -1258,10 +1282,11 @@ void QDeclarativeFlickable::setContentWidth(qreal w)
         d->contentItem->setWidth(w);
     // Make sure that we're entirely in view.
     if (!d->pressed && !d->movingHorizontally && !d->movingVertically) {
-        int oldDuration = d->fixupDuration;
-        d->fixupDuration = 0;
+        d->fixupMode = QDeclarativeFlickablePrivate::Immediate;
+        d->fixupX();
+    } else if (!d->pressed && d->hData.fixingUp) {
+        d->fixupMode = QDeclarativeFlickablePrivate::ExtentChanged;
         d->fixupX();
-        d->fixupDuration = oldDuration;
     }
     emit contentWidthChanged();
     d->updateBeginningEnd();
@@ -1285,10 +1310,11 @@ void QDeclarativeFlickable::setContentHeight(qreal h)
         d->contentItem->setHeight(h);
     // Make sure that we're entirely in view.
     if (!d->pressed && !d->movingHorizontally && !d->movingVertically) {
-        int oldDuration = d->fixupDuration;
-        d->fixupDuration = 0;
+        d->fixupMode = QDeclarativeFlickablePrivate::Immediate;
+        d->fixupY();
+    } else if (!d->pressed && d->vData.fixingUp) {
+        d->fixupMode = QDeclarativeFlickablePrivate::ExtentChanged;
         d->fixupY();
-        d->fixupDuration = oldDuration;
     }
     emit contentHeightChanged();
     d->updateBeginningEnd();
@@ -1660,6 +1686,7 @@ void QDeclarativeFlickable::movementXEnding()
                 emit movementEnded();
         }
     }
+    d->hData.fixingUp = false;
 }
 
 void QDeclarativeFlickable::movementYEnding()
@@ -1682,6 +1709,7 @@ void QDeclarativeFlickable::movementYEnding()
                 emit movementEnded();
         }
     }
+    d->vData.fixingUp = false;
 }
 
 void QDeclarativeFlickablePrivate::updateVelocity()
diff --git a/src/declarative/graphicsitems/qdeclarativeflickable_p_p.h b/src/declarative/graphicsitems/qdeclarativeflickable_p_p.h
index 1b6081c..38a5eb3 100644
--- a/src/declarative/graphicsitems/qdeclarativeflickable_p_p.h
+++ b/src/declarative/graphicsitems/qdeclarativeflickable_p_p.h
@@ -94,17 +94,21 @@ public:
     struct AxisData {
         AxisData(QDeclarativeFlickablePrivate *fp, void (QDeclarativeFlickablePrivate::*func)(qreal))
             : move(fp, func), viewSize(-1), smoothVelocity(fp), atEnd(false), atBeginning(true)
+            , fixingUp(false)
         {}
 
         QDeclarativeTimeLineValueProxy<QDeclarativeFlickablePrivate> move;
         qreal viewSize;
         qreal pressPos;
         qreal dragStartOffset;
+        qreal dragMinBound;
+        qreal dragMaxBound;
         qreal velocity;
         qreal flickTarget;
         QDeclarativeFlickablePrivate::Velocity smoothVelocity;
         bool atEnd : 1;
         bool atBeginning : 1;
+        bool fixingUp : 1;
     };
 
     void flickX(qreal velocity);
@@ -161,6 +165,9 @@ public:
     int pressDelay;
     int fixupDuration;
 
+    enum FixupMode { Normal, Immediate, ExtentChanged };
+    FixupMode fixupMode;
+
     static void fixupY_callback(void *);
     static void fixupX_callback(void *);
 
diff --git a/src/declarative/graphicsitems/qdeclarativegridview.cpp b/src/declarative/graphicsitems/qdeclarativegridview.cpp
index 00e5b3f..1d2ad1c 100644
--- a/src/declarative/graphicsitems/qdeclarativegridview.cpp
+++ b/src/declarative/graphicsitems/qdeclarativegridview.cpp
@@ -912,8 +912,7 @@ void QDeclarativeGridViewPrivate::fixup(AxisData &data, qreal minExtent, qreal m
         || (flow == QDeclarativeGridView::LeftToRight && &data == &hData))
         return;
 
-    int oldDuration = fixupDuration;
-    fixupDuration = moveReason == Mouse ? fixupDuration : 0;
+    fixupMode = moveReason == Mouse ? fixupMode : Immediate;
 
     if (snapMode != QDeclarativeGridView::NoSnap) {
         FxGridItem *topItem = snapItemAt(position()+highlightRangeStart);
@@ -932,7 +931,6 @@ void QDeclarativeGridViewPrivate::fixup(AxisData &data, qreal minExtent, qreal m
             pos = qMax(qMin(bottomItem->rowPos() - highlightRangeStart, -maxExtent), -minExtent);
         } else {
             QDeclarativeFlickablePrivate::fixup(data, minExtent, maxExtent);
-            fixupDuration = oldDuration;
             return;
         }
         if (currentItem && haveHighlightRange && highlightRange == QDeclarativeGridView::StrictlyEnforceRange) {
@@ -947,10 +945,12 @@ void QDeclarativeGridViewPrivate::fixup(AxisData &data, qreal minExtent, qreal m
         qreal dist = qAbs(data.move + pos);
         if (dist > 0) {
             timeline.reset(data.move);
-            if (fixupDuration)
+            if (fixupMode != Immediate) {
                 timeline.move(data.move, -pos, QEasingCurve(QEasingCurve::InOutQuad), fixupDuration/2);
-            else
+                data.fixingUp = true;
+            } else {
                 timeline.set(data.move, -pos);
+            }
             vTime = timeline.time();
         }
     } else if (haveHighlightRange && highlightRange == QDeclarativeGridView::StrictlyEnforceRange) {
@@ -965,23 +965,26 @@ void QDeclarativeGridViewPrivate::fixup(AxisData &data, qreal minExtent, qreal m
 
             timeline.reset(data.move);
             if (viewPos != position()) {
-                if (fixupDuration)
+                if (fixupMode != Immediate) {
                     timeline.move(data.move, -viewPos, QEasingCurve(QEasingCurve::InOutQuad), fixupDuration/2);
-                else
+                    data.fixingUp = true;
+                } else {
                     timeline.set(data.move, -viewPos);
+                }
             }
             vTime = timeline.time();
         }
     } else {
         QDeclarativeFlickablePrivate::fixup(data, minExtent, maxExtent);
     }
-    fixupDuration = oldDuration;
+    fixupMode = Normal;
 }
 
 void QDeclarativeGridViewPrivate::flick(AxisData &data, qreal minExtent, qreal maxExtent, qreal vSize,
                                         QDeclarativeTimeLineCallback::Callback fixupCallback, qreal velocity)
 {
     Q_Q(QDeclarativeGridView);
+    data.fixingUp = false;
     moveReason = Mouse;
     if ((!haveHighlightRange || highlightRange != QDeclarativeGridView::StrictlyEnforceRange)
         && snapMode == QDeclarativeGridView::NoSnap) {
diff --git a/src/declarative/graphicsitems/qdeclarativelistview.cpp b/src/declarative/graphicsitems/qdeclarativelistview.cpp
index e8bc1bf..e369ba6 100644
--- a/src/declarative/graphicsitems/qdeclarativelistview.cpp
+++ b/src/declarative/graphicsitems/qdeclarativelistview.cpp
@@ -1198,8 +1198,7 @@ void QDeclarativeListViewPrivate::fixup(AxisData &data, qreal minExtent, qreal m
         return;
 
     correctFlick = false;
-    int oldDuration = fixupDuration;
-    fixupDuration = moveReason == Mouse ? fixupDuration : 0;
+    fixupMode = moveReason == Mouse ? fixupMode : Immediate;
 
     if (currentItem && haveHighlightRange && highlightRange == QDeclarativeListView::StrictlyEnforceRange) {
         updateHighlight();
@@ -1212,10 +1211,12 @@ void QDeclarativeListViewPrivate::fixup(AxisData &data, qreal minExtent, qreal m
 
         timeline.reset(data.move);
         if (viewPos != position()) {
-            if (fixupDuration)
+            if (fixupMode != Immediate) {
                 timeline.move(data.move, -viewPos, QEasingCurve(QEasingCurve::InOutQuad), fixupDuration/2);
-            else
+                data.fixingUp = true;
+            } else {
                 timeline.set(data.move, -viewPos);
+            }
         }
         vTime = timeline.time();
     } else if (snapMode != QDeclarativeListView::NoSnap) {
@@ -1231,23 +1232,24 @@ void QDeclarativeListViewPrivate::fixup(AxisData &data, qreal minExtent, qreal m
            pos = qMax(qMin(bottomItem->position() - highlightRangeStart, -maxExtent), -minExtent);
         } else {
             QDeclarativeFlickablePrivate::fixup(data, minExtent, maxExtent);
-            fixupDuration = oldDuration;
             return;
         }
 
         qreal dist = qAbs(data.move + pos);
         if (dist > 0) {
             timeline.reset(data.move);
-            if (fixupDuration)
+            if (fixupMode != Immediate) {
                 timeline.move(data.move, -pos, QEasingCurve(QEasingCurve::InOutQuad), fixupDuration/2);
-            else
+                data.fixingUp = true;
+            } else {
                 timeline.set(data.move, -pos);
+            }
             vTime = timeline.time();
         }
     } else {
         QDeclarativeFlickablePrivate::fixup(data, minExtent, maxExtent);
     }
-    fixupDuration = oldDuration;
+    fixupMode = Normal;
 }
 
 void QDeclarativeListViewPrivate::flick(AxisData &data, qreal minExtent, qreal maxExtent, qreal vSize,
@@ -1255,6 +1257,7 @@ void QDeclarativeListViewPrivate::flick(AxisData &data, qreal minExtent, qreal m
 {
     Q_Q(QDeclarativeListView);
 
+    data.fixingUp = false;
     moveReason = Mouse;
     if ((!haveHighlightRange || highlightRange != QDeclarativeListView::StrictlyEnforceRange) && snapMode == QDeclarativeListView::NoSnap) {
         correctFlick = true;