Fix test failure for horizontal layout in the list view. Also, simplify the code a bit and get rid of some internal functions.

6 files changed, 108 insertions, 99 deletions

diff --git a/src/experimental/qgraphicsflowview.cpp b/src/experimental/qgraphicsflowview.cpp
index 23b7099..68d4a6a 100644
--- a/src/experimental/qgraphicsflowview.cpp
+++ b/src/experimental/qgraphicsflowview.cpp
@@ -174,33 +174,33 @@ void QtGraphicsFlowView::doLayout()
     const int visibleRows = qMin(totalRows, qCeil(area.height() / (d->itemSize.height() + MARGIN)));
     const int amountNeeded = qMin(d->model->count() - d->firstIndex, visibleRows * itemsPerRow);
 
-    if (!d->viewItems.isEmpty()) {
+    if (!d->items.isEmpty()) {
         // reuse old items by moving them around. Starting from firstIndex, ending at firstIndex + amountNeeded
         const int index = d->firstIndex;
-        while (d->viewItems.first()->index() < index) { // scrolling down
-            QtGraphicsListViewItem *item = d->viewItems.takeFirst();
-            item->setIndex(d->viewItems.last()->index() + 1);
-            d->viewItems.append(item);
+        while (d->items.first()->index() < index) { // scrolling down
+            QtGraphicsListViewItem *item = d->items.takeFirst();
+            item->setIndex(d->items.last()->index() + 1);
+            d->items.append(item);
         }
-        while (d->viewItems.last()->index() > index + d->viewItems.count()) { // scrolling up
-            QtGraphicsListViewItem *item = d->viewItems.takeLast();
-            item->setIndex(d->viewItems.first()->index() - 1);
-            d->viewItems.prepend(item);
+        while (d->items.last()->index() > index + d->items.count()) { // scrolling up
+            QtGraphicsListViewItem *item = d->items.takeLast();
+            item->setIndex(d->items.first()->index() - 1);
+            d->items.prepend(item);
         }
     }
 
-    while (amountNeeded < d->viewItems.count()) // remove old ones
-        delete d->viewItems.takeLast();
+    while (amountNeeded < d->items.count()) // remove old ones
+        delete d->items.takeLast();
     qreal x = 0;
     qreal y = 0;
     for (int index = d->firstIndex; index < d->firstIndex + amountNeeded; ++index) {
         QtGraphicsListViewItem *item = 0;
         const int itemIndex = index - d->firstIndex;
-        if (itemIndex >= d->viewItems.count()) {
+        if (itemIndex >= d->items.count()) {
             item = new QtGraphicsFlowViewItem(index, this);
-            d->viewItems.append(item);
+            d->items.append(item);
         } else {
-            item = d->viewItems.at(itemIndex);
+            item = d->items.at(itemIndex);
         }
         item->setGeometry(x, y, d->itemSize.width(), d->itemSize.height());
         x += d->itemSize.width();
diff --git a/src/experimental/qgraphicsgridview.cpp b/src/experimental/qgraphicsgridview.cpp
index 084224b..b1809ea 100644
--- a/src/experimental/qgraphicsgridview.cpp
+++ b/src/experimental/qgraphicsgridview.cpp
@@ -346,11 +346,14 @@ void QtGraphicsGridView::doLayout()
     const QRectF viewport(QPointF(-offset.x(), -offset.y()), size());
     const int firstVisibleIndex = d->firstVisibleIndex(viewport);
     const int lastVisibleIndex = qMin(d->lastVisibleIndex(viewport), count - 1);
-    d->recycleViewItems(firstVisibleIndex);
+    d->scrollItems(firstVisibleIndex);
     int index = firstVisibleIndex;
     for (; index <= lastVisibleIndex; ++index) {
         initStyleOption(&option, index);
-        QtGraphicsListViewItem *item = d->viewItemAt(index, firstVisibleIndex);
+        const int i = index - firstVisibleIndex;
+        if (i >= d->items.count())
+            d->items.append(d->creator->create(index, this));
+        QtGraphicsListViewItem *item = d->items.at(i);
         const QPointF pos = d->gridPosition(index) + offset;
         const QSizeF size = item->sizeHint(index, &option, Qt::PreferredSize, viewport.size());
         const QRectF grid(pos.x(), pos.y(), gridSize, gridSize);
@@ -360,7 +363,10 @@ void QtGraphicsGridView::doLayout()
                                                      grid.toRect());
         item->setGeometry(aligned);
     }
-    d->removeViewItemsFrom(index - firstVisibleIndex);
+    // remove unused items
+    const int from = index - firstVisibleIndex;
+    while (from < d->items.count())
+        d->creator->destroy(d->items.takeLast());
 }
 
 /*!
diff --git a/src/experimental/qgraphicspathview.cpp b/src/experimental/qgraphicspathview.cpp
index 32bba08..9768444 100644
--- a/src/experimental/qgraphicspathview.cpp
+++ b/src/experimental/qgraphicspathview.cpp
@@ -164,11 +164,11 @@ void QtGraphicsPathView::doLayout()
 
         // ### FIXME: look up and recycle the items
         QtGraphicsListViewItem *item = 0;
-        if (counter >= d->viewItems.count()) {
+        if (counter >= d->items.count()) {
             item = d->creator->create(index, this);
-            d->viewItems.append(item);
+            d->items.append(item);
         } else {
-            item = d->viewItems.at(counter);
+            item = d->items.at(counter);
             item->setIndex(index); // ###
         }
         item->setGeometry(QRectF(pos, size));
diff --git a/src/qgraphicslistview.cpp b/src/qgraphicslistview.cpp
index 09fcf55..3988bc8 100644
--- a/src/qgraphicslistview.cpp
+++ b/src/qgraphicslistview.cpp
@@ -186,6 +186,22 @@ QtGraphicsListView *QtGraphicsListViewItem::view() const
   \class QtGraphicsListViewItemCreatorBase
  */
 
+QtGraphicsListViewItemCreatorBase::~QtGraphicsListViewItemCreatorBase()
+{
+}
+
+QtGraphicsListViewItem *QtGraphicsListViewItemCreatorBase::recycle(int index, QtGraphicsListViewItem *item) const
+{
+    Q_ASSERT(item);
+    item->setIndex(index);
+    return item;
+}
+
+void QtGraphicsListViewItemCreatorBase::destroy(QtGraphicsListViewItem *item) const
+{
+    delete item;
+}
+
 /*!
   \class QtGraphicsListViewItemCreator
  */
@@ -245,8 +261,8 @@ void QtGraphicsListViewPrivate::_q_itemsChanged(int index, int count, const QLis
     Q_Q(QtGraphicsListView);
     checkCache(index, count);
     //qDebug() << "QtGraphicsListViewPrivate::_q_itemsChanged" << index << count;
-    for (int i = 0; i < viewItems.count(); ++i) {
-        QtGraphicsListViewItem *item = viewItems.at(i);
+    for (int i = 0; i < items.count(); ++i) {
+        QtGraphicsListViewItem *item = items.at(i);
         int itemIndex = item->index();
         if (itemIndex >= index + count) // change happened above
             break;
@@ -297,16 +313,16 @@ void QtGraphicsListViewPrivate::_q_reset()
  */
 void QtGraphicsListViewPrivate::_q_selectionsChanged(const QtListSelectionChange &change)
 {
-    if (viewItems.isEmpty())
+    if (items.isEmpty())
         return;
-    const int from = viewItems.first()->index();
-    const int to = viewItems.last()->index();
+    const int from = items.first()->index();
+    const int to = items.last()->index();
     if (from <= change.index() && (change.index() + change.count() - 1) <= to) {
         const QList<int> indexes = change.indexes();
         for (int i = 0; i < indexes.count(); ++i) {
             const int index = indexes.at(i);
             if (from <= index && index <= to)
-                viewItems.at(index - from)->update();
+                items.at(index - from)->update();
          }
     }
 }
@@ -317,12 +333,12 @@ void QtGraphicsListViewPrivate::_q_selectionsChanged(const QtListSelectionChange
 void QtGraphicsListViewPrivate::_q_currentChanged(int current, int previous)
 {
     //qDebug() << "QtGraphicsListViewPrivate::_q_currentChanged" << current << previous;
-    const int from = viewItems.first()->index();
-    const int to = viewItems.last()->index();
+    const int from = items.first()->index();
+    const int to = items.last()->index();
     if (from <= current && current <= to)
-        viewItems.at(current - from)->update();
+        items.at(current - from)->update();
     if (from <= previous && previous <= to)
-        viewItems.at(current - from)->update();
+        items.at(current - from)->update();
 }
 
 /*!
@@ -381,60 +397,30 @@ int QtGraphicsListViewPrivate::currentItem() const
 /*!
   \internal
  */
-void QtGraphicsListViewPrivate::recycleViewItems(int firstVisibleIndex)
+void QtGraphicsListViewPrivate::scrollItems(int firstVisibleIndex)
 {
-    if (!viewItems.isEmpty()) {
+    // shift items around
+    if (!items.isEmpty()) {
          // scrolling down or right - move from first to last
-        while (viewItems.first()->index() < firstVisibleIndex) {
-            const int lastIndex = viewItems.last()->index();
-            QtGraphicsListViewItem *item = viewItems.takeFirst();
-            item->setIndex(lastIndex + 1);
-            viewItems.append(item);
+        while (items.first()->index() < firstVisibleIndex) {
+            const int index = items.last()->index() + 1;
+            items.append(creator->recycle(index, items.takeFirst()));
         }
         // scrolling up or left - move from last to first
-        const int firstNonVisibleIndex = firstVisibleIndex + viewItems.count();
-        while (viewItems.last()->index() >= firstNonVisibleIndex) {
-            const int firstIndex = viewItems.first()->index();
-            QtGraphicsListViewItem *item = viewItems.takeLast();
-            item->setIndex(firstIndex - 1);
-            viewItems.prepend(item);
+        const int firstNonVisibleIndex = firstVisibleIndex + items.count();
+        while (items.last()->index() >= firstNonVisibleIndex) {
+            const int index = items.first()->index() - 1;
+            items.prepend(creator->recycle(index, items.takeLast()));
         }
     }
 }
 
 /*!
- \internal
- */
-void QtGraphicsListViewPrivate::removeViewItemsFrom(int i)
-{
-    while (i < viewItems.count())
-        delete viewItems.takeLast();
-}
-
-/*!
-  \internal
- */
-QtGraphicsListViewItem *QtGraphicsListViewPrivate::viewItemAt(int index, int firstVisibleIndex)
-{
-    Q_Q(QtGraphicsListView);
-    int i = index - firstVisibleIndex;
-    QtGraphicsListViewItem *item = 0;
-    if (i >= viewItems.count()) {
-        item = creator->create(index, q);
-        viewItems.append(item);
-     } else {
-        item = viewItems.at(i);
-        Q_ASSERT(item->index() == index);
-     }
-     return item;
-}
-
-/*!
   \internal
  */
 QSizeF QtGraphicsListViewPrivate::itemSizeHint(const QStyleOptionViewItemV4 *option, int index, const QSizeF &constraint) const
 {
-    return viewItems.isEmpty() ? QSizeF() : viewItems.first()->sizeHint(index, option, Qt::PreferredSize, constraint);
+    return items.isEmpty() ? QSizeF() : items.first()->sizeHint(index, option, Qt::PreferredSize, constraint);
 }
 
 // QtGraphicsListView
@@ -680,7 +666,7 @@ void QtGraphicsListView::doLayout()
 
     // find the visible items; caching helps us skip this most of the time
     if (d->orientation == Qt::Vertical) {
-        if (y <= 0) { // ###
+        if (y <= 0 && !d->items.isEmpty()) { // ###
             // optimization: use the cached index and offset as starting points
             // The cached values are offsets from the start of the _contents_
             // and the to index found at that offset.
@@ -713,29 +699,38 @@ void QtGraphicsListView::doLayout()
         }
         // we are now at the visible items
         const int firstVisibleIndex = index;
-        d->recycleViewItems(firstVisibleIndex); // ### make this one loop
-        while (index >= 0 && index < count) {
-            QtGraphicsListViewItem *item = d->viewItemAt(index, firstVisibleIndex);
-            Q_ASSERT(item && item->index() == index);
+        d->scrollItems(firstVisibleIndex);
+        // set existing item positions
+        for (int i = 0; i < d->items.count() && index < count && y < area.bottom(); ++i) {
+            QtGraphicsListViewItem *item = d->items.at(i);
+            const QSizeF size = item->size();
+            item->setPos(d->horizontalOffset, y);
+            y += size.height();
+            ++index;
+        }
+        // add more items and set geometry
+        while (index < count && y < area.bottom()) {
+            QtGraphicsListViewItem *item = d->creator->create(index, this);
+            d->items.append(item);
             initStyleOption(&option, index);
             const QSizeF size = item->sizeHint(index, &option, Qt::PreferredSize, constraint);
             item->setGeometry(d->horizontalOffset, y, area.width(), size.height());
             y += size.height();
             ++index;
-            if (y >= area.bottom()) {// no more space
-               break;
-           }
         }
         // remove unused items
-        d->removeViewItemsFrom(index - firstVisibleIndex);
+        const int visibleCount = index - firstVisibleIndex;
+        while (visibleCount < d->items.count())
+            d->creator->destroy(d->items.takeLast());
     } else { // Horizontal
-        if (x <= 0) {
+        if (x <= 0 && !d->items.isEmpty()) {
             // optimization: use the cached index and offset as starting points
 #if 0 // enable or disable the caching
             index += d->cachedIndexOffset;
             x += d->cachedCoordinateOffset;
 #endif
-            if (x < area.x()) { // the cached offset was left of the visible area
+            // the visible area starts at x == 0
+            if (x < 0) { // the cached offset was left of the visible area
                 while (index < count) {
                     initStyleOption(&option, index);
                     const qreal width = d->itemSizeHint(&option, index, constraint).width();
@@ -744,8 +739,8 @@ void QtGraphicsListView::doLayout()
                     x += width;
                     ++index;
                 }
-            } else if (x > area.x()) { // the cached offset was to the right
-                while (index >= 0 && x > area.x()) {
+            } else if (x > 0) { // the cached offset was to the right
+                while (index >= 0 && x > 0) {
                     initStyleOption(&option, index);
                     const qreal width = d->itemSizeHint(&option, index, constraint).width();
                     x -= width;
@@ -758,24 +753,32 @@ void QtGraphicsListView::doLayout()
 #endif
         }
         // we are now at the visible items
-        const QSizeF constraint = geometry().size();
         const int firstVisibleIndex = index;
-        d->recycleViewItems(firstVisibleIndex);
-        while (index >= 0 && index < count) {
-            QtGraphicsListViewItem *item = d->viewItemAt(index, firstVisibleIndex);
-            Q_ASSERT(item && item->index() == index);
+        d->scrollItems(firstVisibleIndex);
+        // set existing item positions
+        for (int i = 0; i < d->items.count() && index < count && x < area.right(); ++i) {
+            QtGraphicsListViewItem *item = d->items.at(i);
+            const QSizeF size = item->size();
+            item->setPos(x, d->verticalOffset);
+            x += size.width();
+            ++index;
+        }
+        // add more items and set geometry
+        while (index < count && x < area.right()) {
+            QtGraphicsListViewItem *item = d->creator->create(index, this);
+            d->items.append(item);
             initStyleOption(&option, index);
             const QSizeF size = item->sizeHint(index, &option, Qt::PreferredSize, constraint);
             item->setGeometry(x, d->verticalOffset, size.width(), area.height());
             x += size.width();
             ++index;
-            if (x >= area.right()) // no more space
-                break;
         }
         // remove unused items
-        d->removeViewItemsFrom(index - firstVisibleIndex);
+        const int visibleCount = index - firstVisibleIndex;
+        while (visibleCount < d->items.count())
+            d->creator->destroy(d->items.takeLast());
     }
-
+    // done
     emit layoutChanged();
 }
 
@@ -1191,7 +1194,7 @@ void QtGraphicsListView::setViewItemCreator(QtGraphicsListViewItemCreatorBase *c
 const QList<QtGraphicsListViewItem*> &QtGraphicsListView::viewItems() const
 {
     Q_D(const QtGraphicsListView);
-    return d->viewItems;
+    return d->items;
 }
 
 /*!
diff --git a/src/qgraphicslistview.h b/src/qgraphicslistview.h
index 31c80c8..9b6a49c 100644
--- a/src/qgraphicslistview.h
+++ b/src/qgraphicslistview.h
@@ -73,8 +73,10 @@ private:
 class QtGraphicsListViewItemCreatorBase
 {
 public:
-    virtual ~QtGraphicsListViewItemCreatorBase() {}
+    virtual ~QtGraphicsListViewItemCreatorBase();
     virtual QtGraphicsListViewItem *create(int index, QtGraphicsListView *view) const = 0;
+    virtual QtGraphicsListViewItem *recycle(int index, QtGraphicsListViewItem *item) const;
+    virtual void destroy(QtGraphicsListViewItem *item) const;
 };
 
 template <class T>
diff --git a/src/qgraphicslistview_p.h b/src/qgraphicslistview_p.h
index 28153b0..7365ff1 100644
--- a/src/qgraphicslistview_p.h
+++ b/src/qgraphicslistview_p.h
@@ -80,9 +80,7 @@ public:
     int currentItem() const;
 
     // used by updateLayout
-    void recycleViewItems(int firstVisibleIndex);
-    void removeViewItemsFrom(int i);
-    QtGraphicsListViewItem *viewItemAt(int index, int firstVisibleIndex);
+    void scrollItems(int firstVisibleIndex);
     QSizeF itemSizeHint(const QStyleOptionViewItemV4 *option, int index, const QSizeF &constraints = QSizeF()) const;
 
     QtGraphicsListView *q_ptr;
@@ -105,7 +103,7 @@ public:
 
     bool layoutsBlocked;
 
-    QList<QtGraphicsListViewItem*> viewItems;
+    QList<QtGraphicsListViewItem*> items;
     QtGraphicsListViewItemCreatorBase *creator;
     QBasicTimer layoutTimer;
 };