Remember the row- and column-information for visible items

Up to now KItemListView has not been aware to which column or row
an item belongs to. This has been handled internally in
KItemListViewLayouter. But for deciding whether a move-animation
might result in overlapping items it is mandatory to be aware about
the column/row. The current approach to guess a column/row change
by comparing the source- and target-rectangle works very well but
breaks on some corner-cases when e.g. zooming.

Now the layouter allows to access the information. This assures
that in the vertical alignment no move-animation is done between
rows and the in the horizontal alignment no move-animation is
done between the columns.

4 files changed, 179 insertions, 62 deletions

diff --git a/src/kitemviews/kitemlistview.cpp b/src/kitemviews/kitemlistview.cpp
index b7d4c2470..bee2f3d6a 100644
--- a/src/kitemviews/kitemlistview.cpp
+++ b/src/kitemviews/kitemlistview.cpp
@@ -67,6 +67,7 @@ KItemListView::KItemListView(QGraphicsWidget* parent) :
     m_styleOption(),
     m_visibleItems(),
     m_visibleGroups(),
+    m_visibleCells(),
     m_sizeHintResolver(0),
     m_layouter(0),
     m_animation(0),
@@ -775,6 +776,11 @@ void KItemListView::slotItemsInserted(const KItemRangeList& itemRanges)
         beginTransaction();
     }
 
+    // Important: Don't read any m_layouter-property inside the for-loop in case if
+    // multiple ranges are given! m_layouter accesses m_sizeHintResolver which is
+    // updated in each loop-cycle and has only a consistent state after the loop.
+    m_layouter->markAsDirty();
+
     int previouslyInsertedCount = 0;
     foreach (const KItemRange& range, itemRanges) {
         // range.index is related to the model before anything has been inserted.
@@ -808,10 +814,14 @@ void KItemListView::slotItemsInserted(const KItemRangeList& itemRanges)
         for (int i = itemsToMove.count() - 1; i >= 0; --i) {
             KItemListWidget* widget = m_visibleItems.value(itemsToMove[i]);
             Q_ASSERT(widget);
-            setWidgetIndex(widget, widget->index() + count);
+            if (hasMultipleRanges) {
+                setWidgetIndex(widget, widget->index() + count);
+            } else {
+                // Try to animate the moving of the item
+                moveWidgetToIndex(widget, widget->index() + count);
+            }
         }
 
-        m_layouter->markAsDirty();
         if (m_model->count() == count && m_activeTransactions == 0) {
             // Check whether a scrollbar is required to show the inserted items. In this case
             // the size of the layouter will be decreased before calling doLayout(): This prevents
@@ -857,6 +867,11 @@ void KItemListView::slotItemsRemoved(const KItemRangeList& itemRanges)
         beginTransaction();
     }
 
+    // Important: Don't read any m_layouter-property inside the for-loop in case if
+    // multiple ranges are given! m_layouter accesses m_sizeHintResolver which is
+    // updated in each loop-cycle and has only a consistent state after the loop.
+    m_layouter->markAsDirty();
+
     for (int i = itemRanges.count() - 1; i >= 0; --i) {
         const KItemRange& range = itemRanges.at(i);
         const int index = range.index;
@@ -908,11 +923,15 @@ void KItemListView::slotItemsRemoved(const KItemRangeList& itemRanges)
             KItemListWidget* widget = m_visibleItems.value(i);
             if (widget) {
                 const int newIndex = i - count;
-                setWidgetIndex(widget, newIndex);
+                if (hasMultipleRanges) {
+                    setWidgetIndex(widget, newIndex);
+                } else {
+                    // Try to animate the moving of the item
+                    moveWidgetToIndex(widget, newIndex);
+                }
             }
         }
 
-        m_layouter->markAsDirty();
         if (!hasMultipleRanges) {
             // The decrease-layout-size optimization in KItemListView::slotItemsInserted()
             // assumes an updated geometry. If items are removed during an active transaction,
@@ -1164,10 +1183,11 @@ void KItemListView::slotVisibleRoleWidthChanged(const QByteArray& role,
         m_layouter->setItemSize(dynamicItemSize);
 
         // Update the role sizes for all visible widgets
-        foreach (KItemListWidget* widget, visibleItemListWidgets()) {
-            widget->setVisibleRolesSizes(m_stretchedVisibleRolesSizes);
+        QHashIterator<int, KItemListWidget*> it(m_visibleItems);
+        while (it.hasNext()) {
+            it.next();
+            it.value()->setVisibleRolesSizes(m_stretchedVisibleRolesSizes);
         }
-
         doLayout(NoAnimation);
     }
 }
@@ -1415,7 +1435,7 @@ void KItemListView::doLayout(LayoutAnimationHint hint, int changedIndex, int cha
             const bool itemsInserted = (changedCount > 0);
             if (itemsRemoved && (i >= changedIndex + changedCount + 1)) {
                 // The item is located after the removed items. Animate the moving of the position.
-                applyNewPos = !moveWidget(widget, itemBounds);
+                applyNewPos = !moveWidget(widget, newPos);
             } else if (itemsInserted && i >= changedIndex) {
                 // The item is located after the first inserted item
                 if (i <= changedIndex + changedCount - 1) {
@@ -1429,11 +1449,11 @@ void KItemListView::doLayout(LayoutAnimationHint hint, int changedIndex, int cha
                     // The item was already there before, so animate the moving of the position.
                     // No moving animation is done if the item is animated by a create animation: This
                     // prevents a "move animation mess" when inserting several ranges in parallel.
-                    applyNewPos = !moveWidget(widget, itemBounds);
+                    applyNewPos = !moveWidget(widget, newPos);
                 }
             } else if (!itemsRemoved && !itemsInserted && !wasHidden) {
                 // The size of the view might have been changed. Animate the moving of the position.
-                applyNewPos = !moveWidget(widget, itemBounds);
+                applyNewPos = !moveWidget(widget, newPos);
             }
         } else {
             m_animation->stop(widget);
@@ -1463,6 +1483,11 @@ void KItemListView::doLayout(LayoutAnimationHint hint, int changedIndex, int cha
                 widget->resize(itemBounds.size());
             }
         }
+
+        // Updating the cell-information must be done as last step: The decision whether the
+        // moving-animation should be started at all is based on the previous cell-information.
+        const Cell cell(m_layouter->itemColumn(i), m_layouter->itemRow(i));
+        m_visibleCells.insert(i, cell);
     }
 
     // Delete invisible KItemListWidget instances that have not been reused
@@ -1528,29 +1553,25 @@ QList<int> KItemListView::recycleInvisibleItems(int firstVisibleIndex,
     return items;
 }
 
-bool KItemListView::moveWidget(KItemListWidget* widget,const QRectF& itemBounds)
+bool KItemListView::moveWidget(KItemListWidget* widget,const QPointF& newPos)
 {
-    const QPointF oldPos = widget->pos();
-    const QPointF newPos = itemBounds.topLeft();
-    if (oldPos == newPos) {
+    if (widget->pos() == newPos) {
         return false;
     }
     
-    bool startMovingAnim = m_itemSize.isEmpty() || widget->size() != itemBounds.size();
-    if (!startMovingAnim) {
-        // When having a grid the moving-animation should only be started, if it is done within
-        // one row in the vertical scroll-orientation or one column in the horizontal scroll-orientation.
-        // Otherwise instead of a moving-animation a create-animation on the new position will be used
-        // instead. This is done to prevent overlapping (and confusing) moving-animations.
-        const QSizeF itemMargin = m_layouter->itemMargin();
-        const qreal xMax = m_itemSize.width() + itemMargin.width();
-        const qreal yMax = m_itemSize.height() + itemMargin.height();
-        qreal xDiff = qAbs(oldPos.x() - newPos.x());
-        qreal yDiff = qAbs(oldPos.y() - newPos.y());
+    bool startMovingAnim = false;
+
+    // When having a grid the moving-animation should only be started, if it is done within
+    // one row in the vertical scroll-orientation or one column in the horizontal scroll-orientation.
+    // Otherwise instead of a moving-animation a create-animation on the new position will be used
+    // instead. This is done to prevent overlapping (and confusing) moving-animations.
+    const int index = widget->index();
+    const Cell cell = m_visibleCells.value(index);
+    if (cell.column >= 0 && cell.row >= 0) {
         if (scrollOrientation() == Qt::Vertical) {
-            startMovingAnim = (xDiff > yDiff && yDiff < yMax);
+            startMovingAnim = (cell.row == m_layouter->itemRow(index));
         } else {
-            startMovingAnim = (yDiff > xDiff && xDiff < xMax);
+            startMovingAnim = (cell.column == m_layouter->itemColumn(index));
         }
     }
 
@@ -1598,6 +1619,7 @@ KItemListWidget* KItemListView::createWidget(int index)
 
     updateWidgetProperties(widget, index);
     m_visibleItems.insert(index, widget);
+    m_visibleCells.insert(index, Cell());
 
     if (m_grouped) {
         updateGroupHeaderForWidget(widget);
@@ -1613,20 +1635,44 @@ void KItemListView::recycleWidget(KItemListWidget* widget)
         recycleGroupHeaderForWidget(widget);
     }
 
-    m_visibleItems.remove(widget->index());
+    const int index = widget->index();
+    m_visibleItems.remove(index);
+    m_visibleCells.remove(index);
+
     m_widgetCreator->recycle(widget);
 }
 
 void KItemListView::setWidgetIndex(KItemListWidget* widget, int index)
 {
     const int oldIndex = widget->index();
+
     m_visibleItems.remove(oldIndex);
+    m_visibleCells.remove(oldIndex);
+
     updateWidgetProperties(widget, index);
+
     m_visibleItems.insert(index, widget);
+    m_visibleCells.insert(index, Cell());
 
     initializeItemListWidget(widget);
 }
 
+void KItemListView::moveWidgetToIndex(KItemListWidget* widget, int index)
+{
+    const int oldIndex = widget->index();
+    const Cell oldCell = m_visibleCells.value(oldIndex);
+
+    setWidgetIndex(widget, index);
+
+    const Cell newCell(m_layouter->itemColumn(index), m_layouter->itemRow(index));
+    const bool vertical = (scrollOrientation() == Qt::Vertical);
+    const bool updateCell = (vertical  && oldCell.row    == newCell.row) ||
+                            (!vertical && oldCell.column == newCell.column);
+    if (updateCell) {
+        m_visibleCells.insert(index, newCell);
+    }
+}
+
 void KItemListView::setLayouterSize(const QSizeF& size, SizeType sizeType)
 {
     switch (sizeType) {
@@ -1877,8 +1923,10 @@ void KItemListView::updateStretchedVisibleRolesSizes()
     }
 
     // Update the role sizes for all visible widgets
-    foreach (KItemListWidget* widget, visibleItemListWidgets()) {
-        widget->setVisibleRolesSizes(m_stretchedVisibleRolesSizes);
+    QHashIterator<int, KItemListWidget*> it(m_visibleItems);
+    while (it.hasNext()) {
+        it.next();
+        it.value()->setVisibleRolesSizes(m_stretchedVisibleRolesSizes);
     }
 }
 
@@ -1912,7 +1960,7 @@ QRectF KItemListView::headerBoundaries() const
 bool KItemListView::changesItemGridLayout(const QSizeF& newGridSize,
                                           const QSizeF& newItemSize,
                                           const QSizeF& newItemMargin) const
-{
+{  
     if (newItemSize.isEmpty() || newGridSize.isEmpty()) {
         return false;
     }
diff --git a/src/kitemviews/kitemlistview.h b/src/kitemviews/kitemlistview.h
index 118ed2f03..293f4b1ec 100644
--- a/src/kitemviews/kitemlistview.h
+++ b/src/kitemviews/kitemlistview.h
@@ -381,15 +381,28 @@ private:
      * the same row or column, otherwise the create-animation is used instead.
      * @return True if the moving-animation has been applied.
      */
-    bool moveWidget(KItemListWidget* widget, const QRectF& itemBounds);
+    bool moveWidget(KItemListWidget* widget, const QPointF& newPos);
 
     void emitOffsetChanges();
 
     KItemListWidget* createWidget(int index);
     void recycleWidget(KItemListWidget* widget);
+
+    /**
+     * Changes the index of the widget to \a index. The cell-information
+     * for the widget gets reset and will be updated in the next doLayout().
+     */
     void setWidgetIndex(KItemListWidget* widget, int index);
 
     /**
+     * Changes the index of the widget to \a index. In opposite to
+     * setWidgetIndex() the cell-information of the widget gets updated.
+     * This update gives doLayout() the chance to animate the moving
+     * of the item visually (see moveWidget()).
+     */
+    void moveWidgetToIndex(KItemListWidget* widget, int index);
+
+    /**
      * Helper method for prepareLayoutForIncreasedItemCount().
      */
     void setLayouterSize(const QSizeF& size, SizeType sizeType);
@@ -550,6 +563,15 @@ private:
     QHash<int, KItemListWidget*> m_visibleItems;
     QHash<KItemListWidget*, KItemListGroupHeader*> m_visibleGroups;
 
+    struct Cell
+    {
+        Cell() : column(-1), row(-1) {}
+        Cell(int c, int r) : column(c), row(r) {}
+        int column;
+        int row;
+    };
+    QHash<int, Cell> m_visibleCells;
+
     int m_scrollBarExtent;
     KItemListSizeHintResolver* m_sizeHintResolver;
     KItemListViewLayouter* m_layouter;
diff --git a/src/kitemviews/kitemlistviewlayouter.cpp b/src/kitemviews/kitemlistviewlayouter.cpp
index 14774c412..744914929 100644
--- a/src/kitemviews/kitemlistviewlayouter.cpp
+++ b/src/kitemviews/kitemlistviewlayouter.cpp
@@ -49,7 +49,7 @@ KItemListViewLayouter::KItemListViewLayouter(QObject* parent) :
     m_groupItemIndexes(),
     m_groupHeaderHeight(0),
     m_groupHeaderMargin(0),
-    m_itemRects()
+    m_itemInfos()
 {
 }
 
@@ -227,14 +227,14 @@ int KItemListViewLayouter::lastVisibleIndex() const
 QRectF KItemListViewLayouter::itemRect(int index) const
 {
     const_cast<KItemListViewLayouter*>(this)->doLayout();
-    if (index < 0 || index >= m_itemRects.count()) {
+    if (index < 0 || index >= m_itemInfos.count()) {
         return QRectF();
     }
 
     if (m_scrollOrientation == Qt::Horizontal) {
         // Rotate the logical direction which is always vertical by 90°
         // to get the physical horizontal direction
-        const QRectF& b = m_itemRects[index];
+        const QRectF& b = m_itemInfos[index].rect;
         QRectF bounds(b.y(), b.x(), b.height(), b.width());
         QPointF pos = bounds.topLeft();
         pos.rx() -= m_scrollOffset;
@@ -242,7 +242,7 @@ QRectF KItemListViewLayouter::itemRect(int index) const
         return bounds;
     }
 
-    QRectF bounds = m_itemRects[index];
+    QRectF bounds = m_itemInfos[index].rect;
     bounds.moveTo(bounds.topLeft() - QPointF(m_itemOffset, m_scrollOffset));
     return bounds;
 }
@@ -271,10 +271,10 @@ QRectF KItemListViewLayouter::groupHeaderRect(int index) const
         // Qt::Horizontal and m_itemRects is accessed directly,
         // the logical height represents the visual width.
         qreal width = minimumGroupHeaderWidth();
-        const qreal y = m_itemRects[index].y();
-        const int maxIndex = m_itemRects.count() - 1;
+        const qreal y = m_itemInfos[index].rect.y();
+        const int maxIndex = m_itemInfos.count() - 1;
         while (index <= maxIndex) {
-            QRectF bounds = m_itemRects[index];
+            QRectF bounds = m_itemInfos[index].rect;
             if (bounds.y() != y) {
                 break;
             }
@@ -291,6 +291,30 @@ QRectF KItemListViewLayouter::groupHeaderRect(int index) const
     return QRectF(pos, size);
 }
 
+int KItemListViewLayouter::itemColumn(int index) const
+{
+    const_cast<KItemListViewLayouter*>(this)->doLayout();
+    if (index < 0 || index >= m_itemInfos.count()) {
+        return -1;
+    }
+
+    return (m_scrollOrientation == Qt::Vertical)
+            ? m_itemInfos[index].column
+            : m_itemInfos[index].row; 
+}
+
+int KItemListViewLayouter::itemRow(int index) const
+{
+    const_cast<KItemListViewLayouter*>(this)->doLayout();
+    if (index < 0 || index >= m_itemInfos.count()) {
+        return -1;
+    }
+
+    return (m_scrollOrientation == Qt::Vertical)
+            ? m_itemInfos[index].row
+            : m_itemInfos[index].column; 
+}
+
 int KItemListViewLayouter::maximumVisibleItems() const
 {
     const_cast<KItemListViewLayouter*>(this)->doLayout();
@@ -371,10 +395,10 @@ void KItemListViewLayouter::doLayout()
             ++rowCount;
         }
 
-        m_itemRects.reserve(itemCount);
+        m_itemInfos.reserve(itemCount);
 
         qreal y = m_headerHeight + itemMargin.height();
-        int rowIndex = 0;
+        int row = 0;
 
         int index = 0;
         while (index < itemCount) {
@@ -419,10 +443,16 @@ void KItemListViewLayouter::doLayout()
                 }
 
                 const QRectF bounds(x, y, itemSize.width(), requiredItemHeight);
-                if (index < m_itemRects.count()) {
-                    m_itemRects[index] = bounds;
+                if (index < m_itemInfos.count()) {
+                    m_itemInfos[index].rect = bounds;
+                    m_itemInfos[index].column = column;
+                    m_itemInfos[index].row = row;
                 } else {
-                    m_itemRects.append(bounds);
+                    ItemInfo itemInfo;
+                    itemInfo.rect = bounds;
+                    itemInfo.column = column;
+                    itemInfo.row = row;
+                    m_itemInfos.append(itemInfo);
                 }
 
                 if (grouped && horizontalScrolling) {
@@ -457,21 +487,21 @@ void KItemListViewLayouter::doLayout()
             }
 
             y += maxItemHeight + itemMargin.height();
-            ++rowIndex;
+            ++row;
         }
-        if (m_itemRects.count() > itemCount) {
-            m_itemRects.erase(m_itemRects.begin() + itemCount,
-                              m_itemRects.end());
+        if (m_itemInfos.count() > itemCount) {
+            m_itemInfos.erase(m_itemInfos.begin() + itemCount,
+                              m_itemInfos.end());
         }
-
+        
         if (itemCount > 0) {
             // Calculate the maximum y-range of the last row for m_maximumScrollOffset
-            m_maximumScrollOffset = m_itemRects.last().bottom();
-            const qreal rowY = m_itemRects.last().y();
+            m_maximumScrollOffset = m_itemInfos.last().rect.bottom();
+            const qreal rowY = m_itemInfos.last().rect.y();
 
-            int index = m_itemRects.count() - 2;
-            while (index >= 0 && m_itemRects.at(index).bottom() >= rowY) {
-                m_maximumScrollOffset = qMax(m_maximumScrollOffset, m_itemRects.at(index).bottom());
+            int index = m_itemInfos.count() - 2;
+            while (index >= 0 && m_itemInfos[index].rect.bottom() >= rowY) {
+                m_maximumScrollOffset = qMax(m_maximumScrollOffset, m_itemInfos[index].rect.bottom());
                 --index;
             }
 
@@ -515,7 +545,7 @@ void KItemListViewLayouter::updateVisibleIndexes()
     int mid = 0;
     do {
         mid = (min + max) / 2;
-        if (m_itemRects[mid].top() < m_scrollOffset) {
+        if (m_itemInfos[mid].rect.top() < m_scrollOffset) {
             min = mid + 1;
         } else {
             max = mid - 1;
@@ -525,13 +555,13 @@ void KItemListViewLayouter::updateVisibleIndexes()
     if (mid > 0) {
         // Include the row before the first fully visible index, as it might
         // be partly visible
-        if (m_itemRects[mid].top() >= m_scrollOffset) {
+        if (m_itemInfos[mid].rect.top() >= m_scrollOffset) {
             --mid;
-            Q_ASSERT(m_itemRects[mid].top() < m_scrollOffset);
+            Q_ASSERT(m_itemInfos[mid].rect.top() < m_scrollOffset);
         }
 
-        const qreal rowTop = m_itemRects[mid].top();
-        while (mid > 0 && m_itemRects[mid - 1].top() == rowTop) {
+        const qreal rowTop = m_itemInfos[mid].rect.top();
+        while (mid > 0 && m_itemInfos[mid - 1].rect.top() == rowTop) {
             --mid;
         }
     }
@@ -548,14 +578,14 @@ void KItemListViewLayouter::updateVisibleIndexes()
     max = maxIndex;
     do {
         mid = (min + max) / 2;
-        if (m_itemRects[mid].y() <= bottom) {
+        if (m_itemInfos[mid].rect.y() <= bottom) {
             min = mid + 1;
         } else {
             max = mid - 1;
         }
     } while (min <= max);
 
-    while (mid > 0 && m_itemRects[mid].y() > bottom) {
+    while (mid > 0 && m_itemInfos[mid].rect.y() > bottom) {
         --mid;
     }
     m_lastVisibleIndex = mid;
diff --git a/src/kitemviews/kitemlistviewlayouter_p.h b/src/kitemviews/kitemlistviewlayouter_p.h
index ebed39a41..017f520fe 100644
--- a/src/kitemviews/kitemlistviewlayouter_p.h
+++ b/src/kitemviews/kitemlistviewlayouter_p.h
@@ -112,6 +112,18 @@ public:
     QRectF itemRect(int index) const;
 
     QRectF groupHeaderRect(int index) const;
+    
+    /**
+     * @return Column of the item with the index \a index.
+     *         -1 is returned if an invalid index is given.
+     */
+    int itemColumn(int index) const;
+    
+    /**
+     * @return Row of the item with the index \a index.
+     *         -1 is returned if an invalid index is given.
+     */
+    int itemRow(int index) const;
 
     /**
      * @return Maximum number of (at least partly) visible items for
@@ -174,7 +186,12 @@ private:
     qreal m_groupHeaderHeight;
     qreal m_groupHeaderMargin;
 
-    QList<QRectF> m_itemRects;
+    struct ItemInfo {
+        QRectF rect;
+        int column;
+        int row;
+    };
+    QList<ItemInfo> m_itemInfos;
 
     friend class KItemListControllerTest;
 };