Fix QQuickListViewPrivate::snapItemAt for lists with spacing

If lists had spacing, snapping would have gaps where snapItemAt would
return nullptr eventhough there were items above and below in the list.

I.e. if we have a list with items of size 100, and a spacing of 100, and
scroll to 110 (right below the first item), then the first item would fail
because this is false:

    itemTop+itemSize()/2 >= pos (50 >= 110)

And the next item (the one we should have snapped to), would fail because
this is false:

    itemTop-prevItemSize/2 < pos (200-100/2 < 110)

The meaning of first part was probably to see if the middle of the current
item is inside the view.

The meaning of the second part was probably to see if the middle of the
previous item is outside the view.

This patch refactors the code so it's more visible what's happening and
fixes the bug by taking the spacing into account when computing the position of
the halfway positions.

Task-number: QTBUG-59852
Change-Id: I60eec0d16e91d2f22d70b97d11bcde5d88ac2997
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

2 files changed, 44 insertions, 0 deletions

diff --git a/tests/auto/quick/qquicklistview/data/snapToItemWithSpacing.qml b/tests/auto/quick/qquicklistview/data/snapToItemWithSpacing.qml
new file mode 100644
index 0000000000..50b5abb206
--- /dev/null
+++ b/tests/auto/quick/qquicklistview/data/snapToItemWithSpacing.qml
@@ -0,0 +1,18 @@
+import QtQuick 2.0
+
+ListView {
+    width: 100
+    height: 300
+    snapMode: ListView.SnapToItem
+    spacing: 100
+    model: 10
+    delegate: Rectangle {
+        height: 100
+        width: 100
+        color: "blue"
+        Text {
+            anchors.centerIn: parent
+            text: index
+        }
+    }
+}
diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
index 8bc5575e34..4d3a665255 100644
--- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
+++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
@@ -176,6 +176,7 @@ private slots:
     void creationContext();
     void snapToItem_data();
     void snapToItem();
+    void snapToItemWithSpacing_QTBUG_59852();
     void snapOneItemResize_QTBUG_43555();
     void snapOneItem_data();
     void snapOneItem();
@@ -5197,6 +5198,31 @@ void tst_QQuickListView::snapToItem()
 
     releaseView(window);
 }
+void tst_QQuickListView::snapToItemWithSpacing_QTBUG_59852()
+{
+    QQuickView *window = getView();
+
+    window->setSource(testFileUrl("snapToItemWithSpacing.qml"));
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window));
+
+    auto *listView = qobject_cast<QQuickListView*>(window->rootObject());
+    QVERIFY(listView);
+
+    QTRY_COMPARE(QQuickItemPrivate::get(listView)->polishScheduled, false);
+
+    // each item in the list is 100 pixels tall, and the spacing is 100
+
+    listView->setContentY(110); // this is right below the first item
+    listView->returnToBounds();
+    QCOMPARE(listView->contentY(), 200); // the position of the second item
+
+    listView->setContentY(60); // this is right below the middle of the first item
+    listView->returnToBounds();
+    QCOMPARE(listView->contentY(), 0); // it's farther to go to the next item, so snaps to the first
+
+    releaseView(window);
+}
 
 void tst_QQuickListView::snapOneItemResize_QTBUG_43555()
 {