Flicking behaviour of ListView/GridView SnapOnItem is inconsistent

Improve the response of the views when SnapOneItem/Row is enabled.
In this case it is best to be much more reactive to the user input
since even a small movement in a particular direction indicates
a change to the next/previous item.

Change-Id: I6a8eb689c3b12cdc67f24106032e36bba82d2846
Task-number: QTBUG-19874
Reviewed-by: Bea Lam

1 files changed, 66 insertions, 31 deletions

diff --git a/src/declarative/graphicsitems/qdeclarativegridview.cpp b/src/declarative/graphicsitems/qdeclarativegridview.cpp
index 36f6e29fe1..264280008a 100644
--- a/src/declarative/graphicsitems/qdeclarativegridview.cpp
+++ b/src/declarative/graphicsitems/qdeclarativegridview.cpp
@@ -52,9 +52,13 @@
 
 #include <qmath.h>
 #include <math.h>
+#include "qplatformdefs.h"
 
 QT_BEGIN_NAMESPACE
 
+#ifndef QML_FLICK_SNAPONETHRESHOLD
+#define QML_FLICK_SNAPONETHRESHOLD 30
+#endif
 
 //----------------------------------------------------------------------------
 
@@ -345,9 +349,12 @@ public:
         Q_Q(const QDeclarativeGridView);
         qreal snapPos = 0;
         if (!visibleItems.isEmpty()) {
+            qreal highlightStart = isRightToLeftTopToBottom() && highlightRangeStartValid ? size()-highlightRangeEnd : highlightRangeStart;
+            pos += highlightStart;
             pos += rowSize()/2;
             snapPos = visibleItems.first()->rowPos() - visibleIndex / columns * rowSize();
             snapPos = pos - fmodf(pos - snapPos, qreal(rowSize()));
+            snapPos -= highlightStart;
             qreal maxExtent;
             qreal minExtent;
             if (isRightToLeftTopToBottom()) {
@@ -875,7 +882,7 @@ void QDeclarativeGridViewPrivate::updateHighlight()
 {
     if ((!currentItem && highlight) || (currentItem && !highlight))
         createHighlight();
-    if (currentItem && autoHighlight && highlight && !movingHorizontally && !movingVertically) {
+    if (currentItem && autoHighlight && highlight && !hData.moving && !vData.moving) {
         // auto-update highlight
         highlightXAnimator->to = currentItem->item->x();
         highlightYAnimator->to = currentItem->item->y();
@@ -1062,6 +1069,18 @@ void QDeclarativeGridViewPrivate::fixup(AxisData &data, qreal minExtent, qreal m
 
     if (snapMode != QDeclarativeGridView::NoSnap) {
         qreal tempPosition = isRightToLeftTopToBottom() ? -position()-size() : position();
+        if (snapMode == QDeclarativeGridView::SnapOneRow && moveReason == Mouse) {
+            // if we've been dragged < rowSize()/2 then bias towards the next row
+            qreal dist = data.move.value() - (data.pressPos - data.dragStartOffset);
+            qreal bias = 0;
+            if (data.velocity > 0 && dist > QML_FLICK_SNAPONETHRESHOLD && dist < rowSize()/2)
+                bias = rowSize()/2;
+            else if (data.velocity < 0 && dist < -QML_FLICK_SNAPONETHRESHOLD && dist > -rowSize()/2)
+                bias = -rowSize()/2;
+            if (isRightToLeftTopToBottom())
+                bias = -bias;
+            tempPosition -= bias;
+        }
         FxGridItem *topItem = snapItemAt(tempPosition+highlightStart);
         FxGridItem *bottomItem = snapItemAt(tempPosition+highlightEnd);
         qreal pos;
@@ -1083,25 +1102,13 @@ void QDeclarativeGridViewPrivate::fixup(AxisData &data, qreal minExtent, qreal m
             }
         } else if (bottomItem) {
             if (isRightToLeftTopToBottom())
-                pos = qMax(qMin(-bottomItem->rowPos() + highlightStart - size(), -maxExtent), -minExtent);
+                pos = qMax(qMin(-bottomItem->rowPos() + highlightEnd - size(), -maxExtent), -minExtent);
             else
-                pos = qMax(qMin(bottomItem->rowPos() - highlightStart, -maxExtent), -minExtent);
+                pos = qMax(qMin(bottomItem->rowPos() - highlightEnd, -maxExtent), -minExtent);
         } else {
             QDeclarativeFlickablePrivate::fixup(data, minExtent, maxExtent);
             return;
         }
-        if (currentItem && haveHighlightRange && highlightRange == QDeclarativeGridView::StrictlyEnforceRange) {
-            updateHighlight();
-            qreal currPos = currentItem->rowPos();
-            if (isRightToLeftTopToBottom())
-                pos = -pos-size(); // Transform Pos if required
-            if (pos < currPos + rowSize() - highlightEnd)
-                pos = currPos + rowSize() - highlightEnd;
-            if (pos > currPos - highlightStart)
-                pos = currPos - highlightStart;
-            if (isRightToLeftTopToBottom())
-                pos = -pos-size(); // Untransform
-        }
         qreal dist = qAbs(data.move + pos);
         if (dist > 0) {
             timeline.reset(data.move);
@@ -1158,9 +1165,14 @@ void QDeclarativeGridViewPrivate::flick(AxisData &data, qreal minExtent, qreal m
     if (velocity > 0) {
         if (data.move.value() < minExtent) {
             if (snapMode == QDeclarativeGridView::SnapOneRow) {
-                if (FxGridItem *item = firstVisibleItem()) {
-                    maxDistance = qAbs(item->rowPos() + dataValue);
-                }
+                // if we've been dragged < averageSize/2 then bias towards the next item
+                qreal dist = data.move.value() - (data.pressPos - data.dragStartOffset);
+                qreal bias = dist < rowSize()/2 ? rowSize()/2 : 0;
+                if (isRightToLeftTopToBottom())
+                    bias = -bias;
+                data.flickTarget = -snapPosAt(-dataValue - bias);
+                maxDistance = qAbs(data.flickTarget - data.move.value());
+                velocity = maxVelocity;
             } else {
                 maxDistance = qAbs(minExtent - data.move.value());
             }
@@ -1170,8 +1182,14 @@ void QDeclarativeGridViewPrivate::flick(AxisData &data, qreal minExtent, qreal m
     } else {
         if (data.move.value() > maxExtent) {
             if (snapMode == QDeclarativeGridView::SnapOneRow) {
-                qreal pos = snapPosAt(-dataValue) + (isRightToLeftTopToBottom() ? 0 : rowSize());
-                maxDistance = qAbs(pos + dataValue);
+                // if we've been dragged < averageSize/2 then bias towards the next item
+                qreal dist = data.move.value() - (data.pressPos - data.dragStartOffset);
+                qreal bias = -dist < rowSize()/2 ? rowSize()/2 : 0;
+                if (isRightToLeftTopToBottom())
+                    bias = -bias;
+                data.flickTarget = -snapPosAt(-dataValue + bias);
+                maxDistance = qAbs(data.flickTarget - data.move.value());
+                velocity = -maxVelocity;
             } else {
                 maxDistance = qAbs(maxExtent - data.move.value());
             }
@@ -1181,7 +1199,6 @@ void QDeclarativeGridViewPrivate::flick(AxisData &data, qreal minExtent, qreal m
     }
 
     bool overShoot = boundsBehavior == QDeclarativeFlickable::DragAndOvershootBounds;
-    qreal highlightStart = isRightToLeftTopToBottom() && highlightRangeStartValid ? size()-highlightRangeEnd : highlightRangeStart;
 
     if (maxDistance > 0 || overShoot) {
         // This mode requires the grid to stop exactly on a row boundary.
@@ -1201,9 +1218,20 @@ void QDeclarativeGridViewPrivate::flick(AxisData &data, qreal minExtent, qreal m
             dist = qMin(dist, maxDistance);
             if (v > 0)
                 dist = -dist;
-            qreal distTemp = isRightToLeftTopToBottom() ? -dist : dist;
-            data.flickTarget = -snapPosAt(-(dataValue - highlightStart) + distTemp) + highlightStart;
+            if (snapMode != QDeclarativeGridView::SnapOneRow) {
+                qreal distTemp = isRightToLeftTopToBottom() ? -dist : dist;
+                data.flickTarget = -snapPosAt(-dataValue + distTemp);
+            }
             data.flickTarget = isRightToLeftTopToBottom() ? -data.flickTarget+size() : data.flickTarget;
+            if (overShoot) {
+                if (data.flickTarget >= minExtent) {
+                    overshootDist = overShootDistance(vSize);
+                    data.flickTarget += overshootDist;
+                } else if (data.flickTarget <= maxExtent) {
+                    overshootDist = overShootDistance(vSize);
+                    data.flickTarget -= overshootDist;
+                }
+            }
             qreal adjDist = -data.flickTarget + data.move.value();
             if (qAbs(adjDist) > qAbs(dist)) {
                 // Prevent painfully slow flicking - adjust velocity to suit flickDeceleration
@@ -1224,14 +1252,14 @@ void QDeclarativeGridViewPrivate::flick(AxisData &data, qreal minExtent, qreal m
         timeline.reset(data.move);
         timeline.accel(data.move, v, accel, maxDistance + overshootDist);
         timeline.callback(QDeclarativeTimeLineCallback(&data.move, fixupCallback, this));
-        if (!flickingHorizontally && q->xflick()) {
-            flickingHorizontally = true;
+        if (!hData.flicking && q->xflick()) {
+            hData.flicking = true;
             emit q->flickingChanged();
             emit q->flickingHorizontallyChanged();
             emit q->flickStarted();
         }
-        if (!flickingVertically && q->yflick()) {
-            flickingVertically = true;
+        if (!vData.flicking && q->yflick()) {
+            vData.flicking = true;
             emit q->flickingChanged();
             emit q->flickingVerticallyChanged();
             emit q->flickStarted();
@@ -1555,6 +1583,8 @@ void QDeclarativeGridView::setCurrentIndex(int index)
     if (index == d->currentIndex)
         return;
     if (isComponentComplete() && d->isValid()) {
+        if (d->layoutScheduled)
+            d->layout();
         d->moveReason = QDeclarativeGridViewPrivate::SetIndex;
         d->updateCurrent(index);
     } else {
@@ -2108,7 +2138,8 @@ bool QDeclarativeGridView::event(QEvent *event)
 {
     Q_D(QDeclarativeGridView);
     if (event->type() == QEvent::User) {
-        d->layout();
+        if (d->layoutScheduled)
+            d->layout();
         return true;
     }
 
@@ -2122,7 +2153,7 @@ void QDeclarativeGridView::viewportMoved()
     if (!d->itemCount)
         return;
     d->lazyRelease = true;
-    if (d->flickingHorizontally || d->flickingVertically) {
+    if (d->hData.flicking || d->vData.flicking) {
         if (yflick()) {
             if (d->vData.velocity > 0)
                 d->bufferMode = QDeclarativeGridViewPrivate::BufferBefore;
@@ -2138,7 +2169,7 @@ void QDeclarativeGridView::viewportMoved()
         }
     }
     refill();
-    if (d->flickingHorizontally || d->flickingVertically || d->movingHorizontally || d->movingVertically)
+    if (d->hData.flicking || d->vData.flicking || d->hData.moving || d->vData.moving)
         d->moveReason = QDeclarativeGridViewPrivate::Mouse;
     if (d->moveReason != QDeclarativeGridViewPrivate::SetIndex) {
         if (d->haveHighlightRange && d->highlightRange == StrictlyEnforceRange && d->highlight) {
@@ -2884,7 +2915,6 @@ void QDeclarativeGridView::itemsRemoved(int modelIndex, int count)
     d->itemCount -= count;
     bool currentRemoved = d->currentIndex >= modelIndex && d->currentIndex < modelIndex + count;
     bool removedVisible = false;
-
     // Remove the items from the visible list, skipping anything already marked for removal
     QList<FxGridItem*>::Iterator it = d->visibleItems.begin();
     while (it != d->visibleItems.end()) {
@@ -2918,6 +2948,11 @@ void QDeclarativeGridView::itemsRemoved(int modelIndex, int count)
         }
     }
 
+    // If we removed items before visible items a layout may be
+    // required to ensure item 0 is in the first column.
+    if (!removedVisible && modelIndex < d->visibleIndex)
+        d->scheduleLayout();
+
     // fix current
     if (d->currentIndex >= modelIndex + count) {
         d->currentIndex -= count;