mirror of
https://invent.kde.org/system/dolphin
synced 2024-08-22 01:40:02 +00:00
Make sure that KItemListSizeHintResolver is always consistent
This was the root cause of bug 317827. The assert tried to make sure that we never access KItemListSizeHintResolver from KItemListViewLayouter inside the loop over the item ranges. This would be dangerous because it might be in an inconsistent state - the removed item ranges were removed step by step, so accessing the item size hints before the operation was finished could lead to wrong results. The solution is to insert/remove all item ranges immediately. A nice side effect is that there are no sources of O(N^2) complexity in KItemListSizeHintResolver any more if many item ranges are inserted/removed. BUG: 317827 FIXED-IN: 4.11.0 REVIEW: 111382
This commit is contained in:
parent
508661100f
commit
786cea0076
|
@ -958,6 +958,8 @@ void KItemListView::slotItemsInserted(const KItemRangeList& itemRanges)
|
|||
|
||||
m_layouter->markAsDirty();
|
||||
|
||||
m_sizeHintResolver->itemsInserted(itemRanges);
|
||||
|
||||
int previouslyInsertedCount = 0;
|
||||
foreach (const KItemRange& range, itemRanges) {
|
||||
// range.index is related to the model before anything has been inserted.
|
||||
|
@ -971,8 +973,6 @@ void KItemListView::slotItemsInserted(const KItemRangeList& itemRanges)
|
|||
}
|
||||
previouslyInsertedCount += count;
|
||||
|
||||
m_sizeHintResolver->itemsInserted(index, count);
|
||||
|
||||
// Determine which visible items must be moved
|
||||
QList<int> itemsToMove;
|
||||
QHashIterator<int, KItemListWidget*> it(m_visibleItems);
|
||||
|
@ -1030,12 +1030,6 @@ void KItemListView::slotItemsInserted(const KItemRangeList& itemRanges)
|
|||
}
|
||||
|
||||
if (hasMultipleRanges) {
|
||||
#ifndef QT_NO_DEBUG
|
||||
// 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.
|
||||
Q_ASSERT(m_layouter->isDirty());
|
||||
#endif
|
||||
m_endTransactionAnimationHint = NoAnimation;
|
||||
endTransaction();
|
||||
|
||||
|
@ -1074,6 +1068,8 @@ void KItemListView::slotItemsRemoved(const KItemRangeList& itemRanges)
|
|||
removedItemsCount += itemRanges[i].count;
|
||||
}
|
||||
|
||||
m_sizeHintResolver->itemsRemoved(itemRanges);
|
||||
|
||||
for (int i = itemRanges.count() - 1; i >= 0; --i) {
|
||||
const KItemRange& range = itemRanges[i];
|
||||
const int index = range.index;
|
||||
|
@ -1083,8 +1079,6 @@ void KItemListView::slotItemsRemoved(const KItemRangeList& itemRanges)
|
|||
continue;
|
||||
}
|
||||
|
||||
m_sizeHintResolver->itemsRemoved(index, count);
|
||||
|
||||
const int firstRemovedIndex = index;
|
||||
const int lastRemovedIndex = index + count - 1;
|
||||
const int lastIndex = m_model->count() - 1 + removedItemsCount;
|
||||
|
@ -1153,15 +1147,6 @@ void KItemListView::slotItemsRemoved(const KItemRangeList& itemRanges)
|
|||
}
|
||||
|
||||
if (hasMultipleRanges) {
|
||||
#ifndef QT_NO_DEBUG
|
||||
// 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.
|
||||
// TODO: This assert can be hit when filtering in Icons and Compact view,
|
||||
// see https://bugs.kde.org/show_bug.cgi?id=317827 comments 2 and 3.
|
||||
// We should try to figure out if the assert is wrong or if there is a bug in the code.
|
||||
//Q_ASSERT(m_layouter->isDirty());
|
||||
#endif
|
||||
m_endTransactionAnimationHint = NoAnimation;
|
||||
endTransaction();
|
||||
updateSiblingsInformation();
|
||||
|
@ -1540,9 +1525,9 @@ void KItemListView::setModel(KItemModelBase* model)
|
|||
this, SLOT(slotSortOrderChanged(Qt::SortOrder,Qt::SortOrder)));
|
||||
disconnect(m_model, SIGNAL(sortRoleChanged(QByteArray,QByteArray)),
|
||||
this, SLOT(slotSortRoleChanged(QByteArray,QByteArray)));
|
||||
}
|
||||
|
||||
m_sizeHintResolver->clearCache();
|
||||
m_sizeHintResolver->itemsRemoved(KItemRangeList() << KItemRange(0, m_model->count()));
|
||||
}
|
||||
|
||||
m_model = model;
|
||||
m_layouter->setModel(model);
|
||||
|
@ -1566,7 +1551,6 @@ void KItemListView::setModel(KItemModelBase* model)
|
|||
|
||||
const int itemCount = m_model->count();
|
||||
if (itemCount > 0) {
|
||||
m_sizeHintResolver->itemsInserted(0, itemCount);
|
||||
slotItemsInserted(KItemRangeList() << KItemRange(0, itemCount));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -20,7 +20,6 @@
|
|||
#include "kitemlistsizehintresolver.h"
|
||||
|
||||
#include <kitemviews/kitemlistview.h>
|
||||
#include <KDebug>
|
||||
|
||||
KItemListSizeHintResolver::KItemListSizeHintResolver(const KItemListView* itemListView) :
|
||||
m_itemListView(itemListView),
|
||||
|
@ -42,22 +41,77 @@ QSizeF KItemListSizeHintResolver::sizeHint(int index) const
|
|||
return size;
|
||||
}
|
||||
|
||||
void KItemListSizeHintResolver::itemsInserted(int index, int count)
|
||||
void KItemListSizeHintResolver::itemsInserted(const KItemRangeList& itemRanges)
|
||||
{
|
||||
const int currentCount = m_sizeHintCache.count();
|
||||
m_sizeHintCache.reserve(currentCount + count);
|
||||
while (count > 0) {
|
||||
m_sizeHintCache.insert(index, QSizeF());
|
||||
++index;
|
||||
--count;
|
||||
int insertedCount = 0;
|
||||
foreach (const KItemRange& range, itemRanges) {
|
||||
insertedCount += range.count;
|
||||
}
|
||||
|
||||
const int currentCount = m_sizeHintCache.count();
|
||||
m_sizeHintCache.reserve(currentCount + insertedCount);
|
||||
|
||||
// We build the new list from the end to the beginning to mimize the
|
||||
// number of moves.
|
||||
m_sizeHintCache.insert(m_sizeHintCache.end(), insertedCount, QSizeF());
|
||||
|
||||
int sourceIndex = currentCount - 1;
|
||||
int targetIndex = m_sizeHintCache.count() - 1;
|
||||
int itemsToInsertBeforeCurrentRange = insertedCount;
|
||||
|
||||
for (int rangeIndex = itemRanges.count() - 1; rangeIndex >= 0; --rangeIndex) {
|
||||
const KItemRange& range = itemRanges.at(rangeIndex);
|
||||
itemsToInsertBeforeCurrentRange -= range.count;
|
||||
|
||||
// First: move all existing items that must be put behind 'range'.
|
||||
while (targetIndex >= itemsToInsertBeforeCurrentRange + range.index + range.count) {
|
||||
m_sizeHintCache[targetIndex] = m_sizeHintCache[sourceIndex];
|
||||
--sourceIndex;
|
||||
--targetIndex;
|
||||
}
|
||||
|
||||
// Then: insert QSizeF() for the items which are inserted into 'range'.
|
||||
while (targetIndex >= itemsToInsertBeforeCurrentRange + range.index) {
|
||||
m_sizeHintCache[targetIndex] = QSizeF();
|
||||
--targetIndex;
|
||||
}
|
||||
}
|
||||
|
||||
Q_ASSERT(m_sizeHintCache.count() == m_itemListView->model()->count());
|
||||
}
|
||||
|
||||
void KItemListSizeHintResolver::itemsRemoved(int index, int count)
|
||||
void KItemListSizeHintResolver::itemsRemoved(const KItemRangeList& itemRanges)
|
||||
{
|
||||
const QVector<QSizeF>::iterator begin = m_sizeHintCache.begin() + index;
|
||||
const QVector<QSizeF>::iterator end = begin + count;
|
||||
m_sizeHintCache.erase(begin, end);
|
||||
const QVector<QSizeF>::iterator begin = m_sizeHintCache.begin();
|
||||
const QVector<QSizeF>::iterator end = m_sizeHintCache.end();
|
||||
|
||||
KItemRangeList::const_iterator rangeIt = itemRanges.constBegin();
|
||||
const KItemRangeList::const_iterator rangeEnd = itemRanges.constEnd();
|
||||
|
||||
QVector<QSizeF>::iterator destIt = begin + rangeIt->index;
|
||||
QVector<QSizeF>::iterator srcIt = destIt + rangeIt->count;
|
||||
|
||||
++rangeIt;
|
||||
|
||||
while (srcIt != end) {
|
||||
*destIt = *srcIt;
|
||||
++destIt;
|
||||
++srcIt;
|
||||
|
||||
if (rangeIt != rangeEnd && srcIt == begin + rangeIt->index) {
|
||||
// Skip the items in the next removed range.
|
||||
srcIt += rangeIt->count;
|
||||
++rangeIt;
|
||||
}
|
||||
}
|
||||
|
||||
m_sizeHintCache.erase(destIt, end);
|
||||
|
||||
// Note that the cache size might temporarily not match the model size if
|
||||
// this function is called from KItemListView::setModel() to empty the cache.
|
||||
if (!m_sizeHintCache.isEmpty() && m_itemListView->model()) {
|
||||
Q_ASSERT(m_sizeHintCache.count() == m_itemListView->model()->count());
|
||||
}
|
||||
}
|
||||
|
||||
void KItemListSizeHintResolver::itemsMoved(int index, int count)
|
||||
|
|
|
@ -22,7 +22,7 @@
|
|||
|
||||
#include <libdolphin_export.h>
|
||||
|
||||
#include <QByteArray>
|
||||
#include <kitemviews/kitemmodelbase.h>
|
||||
#include <QSizeF>
|
||||
#include <QVector>
|
||||
|
||||
|
@ -38,8 +38,8 @@ public:
|
|||
virtual ~KItemListSizeHintResolver();
|
||||
QSizeF sizeHint(int index) const;
|
||||
|
||||
void itemsInserted(int index, int count);
|
||||
void itemsRemoved(int index, int count);
|
||||
void itemsInserted(const KItemRangeList& itemRanges);
|
||||
void itemsRemoved(const KItemRangeList& itemRanges);
|
||||
void itemsMoved(int index, int count);
|
||||
void itemsChanged(int index, int count, const QSet<QByteArray>& roles);
|
||||
|
||||
|
|
Loading…
Reference in a new issue