QQuickTableView: use QHash instead of QList to store delegate items

Using a QList to store all loaded delegate items was a legacy solution
inherited from QQuickItemView.  But we look-up items in the list based
on index all the time, so switching to use QHash instead should be more
optimal.

Change-Id: I1aa8d23b3ac208a9424982491faaa5dd42775280
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

3 files changed, 15 insertions, 54 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index bbea895140..c46980eb9c 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -121,7 +121,7 @@ QString QQuickTableViewPrivate::tableLayoutToString() const
 
 void QQuickTableViewPrivate::dumpTable() const
 {
-    auto listCopy = loadedItems;
+    auto listCopy = loadedItems.values();
     std::stable_sort(listCopy.begin(), listCopy.end(),
         [](const FxTableItem *lhs, const FxTableItem *rhs)
         { return lhs->index < rhs->index; });
@@ -293,14 +293,9 @@ FxTableItem *QQuickTableViewPrivate::itemNextTo(const FxTableItem *fxTableItem,
 
 FxTableItem *QQuickTableViewPrivate::loadedTableItem(const QPoint &cell) const
 {
-    for (int i = 0; i < loadedItems.count(); ++i) {
-        FxTableItem *item = loadedItems.at(i);
-        if (item->cell == cell)
-            return item;
-    }
-
-    Q_TABLEVIEW_UNREACHABLE(cell);
-    return nullptr;
+    const int modelIndex = modelIndexAtCell(cell);
+    Q_TABLEVIEW_ASSERT(loadedItems.contains(modelIndex), modelIndex << cell);
+    return loadedItems.value(modelIndex);
 }
 
 FxTableItem *QQuickTableViewPrivate::createFxTableItem(const QPoint &cell, QQmlIncubator::IncubationMode incubationMode)
@@ -402,9 +397,9 @@ void QQuickTableViewPrivate::clear()
 
 void QQuickTableViewPrivate::unloadItem(const QPoint &cell)
 {
-    FxTableItem *item = loadedTableItem(cell);
-    loadedItems.removeOne(item);
-    releaseItem(item);
+    const int modelIndex = modelIndexAtCell(cell);
+    Q_TABLEVIEW_ASSERT(loadedItems.contains(modelIndex), modelIndex << cell);
+    releaseItem(loadedItems.take(modelIndex));
 }
 
 void QQuickTableViewPrivate::unloadItems(const QLine &items)
@@ -932,7 +927,7 @@ void QQuickTableViewPrivate::processLoadRequest()
             return;
         }
 
-        loadedItems.append(fxTableItem);
+        loadedItems.insert(modelIndexAtCell(cell), fxTableItem);
         loadRequest.moveToNextCell();
     }
 
diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h
index 297ef1faa2..9376ad386c 100644
--- a/src/quick/items/qquicktableview_p_p.h
+++ b/src/quick/items/qquicktableview_p_p.h
@@ -192,7 +192,7 @@ public:
     void updatePolish() override;
 
 public:
-    QList<FxTableItem *> loadedItems;
+    QHash<int, FxTableItem *> loadedItems;
 
     // model, delegateModel and modelVariant all points to the same model. modelVariant
     // is the model assigned by the user. And delegateModel is the wrapper model we create
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index 1de40aaae0..6714779fc8 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -75,8 +75,6 @@ public:
     tst_QQuickTableView();
 
     QQuickTableViewAttached *getAttachedObject(const QObject *object) const;
-    FxTableItem *findFxTableItem(int row, int column, const QList<FxTableItem *> items) const;
-    FxTableItem *findLoadedBottomRightItem(const QList<FxTableItem *> items) const;
 
 private slots:
     void initTestCase() override;
@@ -121,32 +119,6 @@ QQuickTableViewAttached *tst_QQuickTableView::getAttachedObject(const QObject *o
     return static_cast<QQuickTableViewAttached *>(attachedObject);
 }
 
-FxTableItem *tst_QQuickTableView::findFxTableItem(int row, int column, const QList<FxTableItem *> items) const
-{
-    for (int i = 0; i < items.count(); ++i) {
-        FxTableItem *fxitem = items[i];
-        auto attached = getAttachedObject(fxitem->item);
-        if (row == attached->row() && column == attached->column())
-            return fxitem;
-    }
-    return nullptr;
-}
-
-FxTableItem *tst_QQuickTableView::findLoadedBottomRightItem(const QList<FxTableItem *> items) const
-{
-    FxTableItem *bottomRightItem = nullptr;
-    int bottomRightIndex = 0;
-
-    for (int i = items.count() - 1; i > 0; --i) {
-        FxTableItem *fxitem = items[i];
-        if (fxitem->index > bottomRightIndex) {
-            bottomRightItem = fxitem;
-            bottomRightIndex = fxitem->index;
-        }
-    }
-    return bottomRightItem;
-}
-
 void tst_QQuickTableView::setAndGetModel_data()
 {
     QTest::addColumn<QVariant>("model");
@@ -418,15 +390,13 @@ void tst_QQuickTableView::checkTableMargins()
 
     WAIT_UNTIL_POLISHED;
 
-    auto const items = tableViewPrivate->loadedItems;
-
-    auto const topLeftFxItem = findFxTableItem(0, 0, items);
-    auto const bottomRightFxItem = findFxTableItem(tableSize.height() - 1, tableSize.width() - 1, items);
-    QVERIFY(topLeftFxItem);
-    QVERIFY(bottomRightFxItem);
+    QCOMPARE(tableViewPrivate->loadedTable.size(), tableSize);
 
+    auto const topLeftFxItem = tableViewPrivate->loadedTableItem(QPoint(0, 0));
+    auto const bottomRightFxItem = tableViewPrivate->loadedTableItem(tableViewPrivate->loadedTable.bottomRight());
     auto const topLeftItem = topLeftFxItem->item;
     auto const bottomRightItem = bottomRightFxItem->item;
+
     qreal leftSpace = topLeftItem->x();
     qreal topSpace = topLeftItem->y();
     qreal rightSpace = tableView->contentWidth() - (bottomRightItem->x() + bottomRightItem->width());
@@ -473,18 +443,14 @@ void tst_QQuickTableView::fillTableViewButNothingMore()
 
     WAIT_UNTIL_POLISHED;
 
-    auto const items = tableViewPrivate->loadedItems;
-
-    auto const topLeftFxItem = findFxTableItem(0, 0, items);
-    QVERIFY(topLeftFxItem);
+    auto const topLeftFxItem = tableViewPrivate->loadedTableItem(QPoint(0, 0));
     auto const topLeftItem = topLeftFxItem->item;
 
     // Check that the top-left item are at the corner of the view
     QCOMPARE(topLeftItem->x(), margins.left());
     QCOMPARE(topLeftItem->y(), margins.top());
 
-    auto const bottomRightFxItem = findLoadedBottomRightItem(items);
-    QVERIFY(bottomRightFxItem);
+    auto const bottomRightFxItem = tableViewPrivate->loadedTableItem(tableViewPrivate->loadedTable.bottomRight());
     auto const bottomRightItem = bottomRightFxItem->item;
     auto bottomRightAttached = getAttachedObject(bottomRightItem);