Remove ordered list assumptions in PathView

When flicking fast or jumping around on paths with some items not seen,
the current algorithm makes assumptions about list ordering which are
hard to maintain. Specifically that it has the index of the first item
in the list cached and that all changed will be to either prepend or
append an item to the current ordered set.

This patch removes that assumption, leading to a little more work each
time to identify where new elements will go. There is still a slightly
faster path for the common case of adding elements to the beginning or
end of the path.

Task-number: QTBUG-42716
Change-Id: Ief76c93967d254d405e6656ef27d06b4ecc470c8
Reviewed-by: Jørgen Lind <jorgen.lind@theqtcompany.com>
Reviewed-by: Shawn Rutledge <shawn.rutledge@digia.com>

2 files changed, 160 insertions, 0 deletions

diff --git a/tests/auto/quick/qquickpathview/data/qtbug42716.qml b/tests/auto/quick/qquickpathview/data/qtbug42716.qml
new file mode 100644
index 0000000000..81d52d5ea3
--- /dev/null
+++ b/tests/auto/quick/qquickpathview/data/qtbug42716.qml
@@ -0,0 +1,111 @@
+import QtQuick 2.0
+
+Rectangle {
+    //Note that this file was originally the manual reproduction, MouseAreas were left in.
+    id: qmlBrowser
+
+    width: 500
+    height: 350
+
+    ListModel {
+        id: myModel
+        ListElement {
+            name: "Bill Jones 0"
+        }
+        ListElement {
+            name: "Jane Doe 1"
+        }
+        ListElement {
+            name: "John Smith 2"
+        }
+        ListElement {
+            name: "Bill Jones 3"
+        }
+        ListElement {
+            name: "Jane Doe 4"
+        }
+        ListElement {
+            name: "John Smith 5"
+        }
+        ListElement {
+            name: "John Smith 6"
+        }
+        ListElement {
+            name: "John Smith 7"
+        }
+    }
+
+    Component {
+        id: delegate
+
+        Text {
+            id: nameText
+            height: 33
+            width: parent.width
+            objectName: "delegate"+index
+
+            text: "index: " + index + "   text: " + name
+            font.pointSize: 16
+            color: PathView.isCurrentItem ? "red" : "black"
+        }
+    }
+
+    PathView {
+        id: contentList
+        objectName: "pathView"
+        anchors.fill: parent
+
+        property int maxPathItemCount: 7
+        property real itemHeight: 34
+
+        delegate: delegate
+        model: myModel
+        currentIndex: 5
+        pathItemCount: maxPathItemCount
+        highlightMoveDuration: 0
+
+        path: Path {
+            startX: 30 + contentList.width / 2
+            startY: 30
+            PathLine {
+                relativeX: 0
+                relativeY: contentList.itemHeight * contentList.maxPathItemCount
+            }
+        }
+
+        focus: true
+        Keys.onLeftPressed: decrementCurrentIndex()
+        Keys.onRightPressed: incrementCurrentIndex()
+    }
+
+    Column {
+        anchors.right: parent.right
+        Text {
+            text: "Go 1"
+            font.weight: Font.Bold
+            font.pixelSize: 24
+            MouseArea {
+                anchors.fill: parent
+                onClicked:  contentList.offset = 2.55882
+            }
+        }
+        Text {
+            text: "Go 2"
+            font.weight: Font.Bold
+            font.pixelSize: 24
+            MouseArea {
+                anchors.fill: parent
+                onClicked:  contentList.offset = 0.0882353
+            }
+        }
+        Text {
+            text: "Go 3"
+            font.weight: Font.Bold
+            font.pixelSize: 24
+            MouseArea {
+                anchors.fill: parent
+                onClicked:  contentList.offset = 0.99987
+            }
+        }
+    }
+}
diff --git a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
index 7db15522b5..0910f6cb2e 100644
--- a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
+++ b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
@@ -140,6 +140,7 @@ private slots:
     void nestedinFlickable();
     void flickableDelegate();
     void jsArrayChange();
+    void qtbug42716();
 };
 
 class TestObject : public QObject
@@ -2322,6 +2323,54 @@ void tst_QQuickPathView::jsArrayChange()
     QCOMPARE(spy.count(), 1);
 }
 
+/* This bug was one where if you jump the list such that the sole missing item needed to be
+ * added in the middle of the list, it would instead move an item somewhere else in the list
+ * to the middle (messing it up very badly).
+ *
+ * The test checks correct visual order both before and after the jump.
+ */
+void tst_QQuickPathView::qtbug42716()
+{
+    QScopedPointer<QQuickView> window(createView());
+
+    window->setSource(testFileUrl("qtbug42716.qml"));
+    window->show();
+    QVERIFY(QTest::qWaitForWindowActive(window.data()));
+    QCOMPARE(window.data(), qGuiApp->focusWindow());
+
+    QQuickPathView *pathView = findItem<QQuickPathView>(window->rootObject(), "pathView");
+    QVERIFY(pathView != 0);
+
+    int order1[] = {5,6,7,0,1,2,3};
+    int missing1 = 4;
+    int order2[] = {0,1,2,3,4,5,6};
+    int missing2 = 7;
+
+    qreal lastY = 0.0;
+    for (int i = 0; i<7; i++) {
+        QQuickItem *item = findItem<QQuickItem>(pathView, QString("delegate%1").arg(order1[i]));
+        QVERIFY(item);
+        QVERIFY(item->y() > lastY);
+        lastY = item->y();
+    }
+    QQuickItem *itemMiss = findItem<QQuickItem>(pathView, QString("delegate%1").arg(missing1));
+    QVERIFY(!itemMiss);
+
+    pathView->setOffset(0.0882353);
+    //Note refill is delayed, needs time to take effect
+    QTest::qWait(100);
+
+    lastY = 0.0;
+    for (int i = 0; i<7; i++) {
+        QQuickItem *item = findItem<QQuickItem>(pathView, QString("delegate%1").arg(order2[i]));
+        QVERIFY(item);
+        QVERIFY(item->y() > lastY);
+        lastY = item->y();
+    }
+    itemMiss = findItem<QQuickItem>(pathView, QString("delegate%1").arg(missing2));
+    QVERIFY(!itemMiss);
+}
+
 QTEST_MAIN(tst_QQuickPathView)
 
 #include "tst_qquickpathview.moc"