From 352f6441590a050099ee685b2284d1679f733c97 Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Wed, 4 Jun 2014 21:48:19 +0200 Subject: [PATCH 1/2] Fix possible crash if a kioslave adds multiple items with the same URL When opening the URL "man:", there are multiple items with the same name (for example, _exit is shown twice here). When opening a new tab, the kioslave reports some items as deleted (I have not quite understood why). The problem is that it reports some of the duplicate items twice in the list of deleted items. This confused KFileItemModel and corrupted the internal data structures, and finally, caused a crash. The fix is to remove all duplicates from KItemRangeList::fromSortedContainer(const Container& container). New unit tests included. BUG: 335672 REVIEW: 118507 FIXED-IN: 4.13.2 --- src/kitemviews/kfileitemmodel.cpp | 36 ++++++++++++++- src/kitemviews/kitemrange.h | 10 ++++- src/tests/CMakeLists.txt | 7 +++ src/tests/kfileitemmodeltest.cpp | 22 +++++++++ src/tests/kitemrangetest.cpp | 75 +++++++++++++++++++++++++++++++ 5 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 src/tests/kitemrangetest.cpp diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp index de3c3eb22..b3b926c3a 100644 --- a/src/kitemviews/kfileitemmodel.cpp +++ b/src/kitemviews/kfileitemmodel.cpp @@ -395,7 +395,41 @@ int KFileItemModel::index(const KUrl& url) const index = m_items.value(urlToFind, -1); } - Q_ASSERT(index >= 0 || m_items.count() == m_itemData.count()); + if (index < 0) { + // The item could not be found, even though all items from m_itemData + // should be in m_items now. We print some diagnostic information which + // might help to find the cause of the problem, but only once. This + // prevents that obtaining and printing the debugging information + // wastes CPU cycles and floods the shell or .xsession-errors. + static bool printDebugInfo = true; + + if (m_items.count() != m_itemData.count() && printDebugInfo) { + printDebugInfo = false; + + kWarning() << "The model is in an inconsistent state."; + kWarning() << "m_items.count() ==" << m_items.count(); + kWarning() << "m_itemData.count() ==" << m_itemData.count(); + + // Check if there are multiple items with the same URL. + QMultiHash indexesForUrl; + for (int i = 0; i < m_itemData.count(); ++i) { + indexesForUrl.insert(m_itemData.at(i)->item.url(), i); + } + + foreach (const KUrl& url, indexesForUrl.uniqueKeys()) { + if (indexesForUrl.count(url) > 1) { + kWarning() << "Multiple items found with the URL" << url; + foreach (int index, indexesForUrl.values(url)) { + const ItemData* data = m_itemData.at(index); + kWarning() << "index" << index << ":" << data->item; + if (data->parent) { + kWarning() << "parent" << data->parent->item; + } + } + } + } + } + } return index; } diff --git a/src/kitemviews/kitemrange.h b/src/kitemviews/kitemrange.h index 70927b915..ecc03988d 100644 --- a/src/kitemviews/kitemrange.h +++ b/src/kitemviews/kitemrange.h @@ -78,7 +78,10 @@ KItemRangeList KItemRangeList::fromSortedContainer(const Container& container) int index = *it; int count = 1; - ++it; + // Remove duplicates, see https://bugs.kde.org/show_bug.cgi?id=335672 + while (it != end && *it == index) { + ++it; + } while (it != end) { if (*it == index + count) { @@ -89,6 +92,11 @@ KItemRangeList KItemRangeList::fromSortedContainer(const Container& container) count = 1; } ++it; + + // Remove duplicates, see https://bugs.kde.org/show_bug.cgi?id=335672 + while (it != end && *it == *(it - 1)) { + ++it; + } } result << KItemRange(index, count); diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 4ba68dd73..c1f4124ff 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -12,6 +12,13 @@ set(kitemsettest_SRCS kde4_add_unit_test(kitemsettest TEST ${kitemsettest_SRCS}) target_link_libraries(kitemsettest dolphinprivate ${KDE4_KIO_LIBS} ${QT_QTTEST_LIBRARY}) +# KItemRangeTest +set(kitemrangetest_SRCS + kitemrangetest.cpp +) +kde4_add_unit_test(kitemrangetest TEST ${kitemrangetest_SRCS}) +target_link_libraries(kitemrangetest dolphinprivate ${KDE4_KIO_LIBS} ${QT_QTTEST_LIBRARY}) + # KItemListSelectionManagerTest set(kitemlistselectionmanagertest_SRCS kitemlistselectionmanagertest.cpp diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp index 48e72e83f..c584c5e62 100644 --- a/src/tests/kfileitemmodeltest.cpp +++ b/src/tests/kfileitemmodeltest.cpp @@ -95,6 +95,7 @@ private slots: void testRefreshFilteredItems(); void testCollapseFolderWhileLoading(); void testCreateMimeData(); + void testDeleteFileMoreThanOnce(); private: QStringList itemsInModel() const; @@ -1697,6 +1698,27 @@ void KFileItemModelTest::testCollapseFolderWhileLoading() QVERIFY(!m_model->isExpanded(1)); } +void KFileItemModelTest::testDeleteFileMoreThanOnce() +{ + QStringList files; + files << "a.txt" << "b.txt" << "c.txt" << "d.txt"; + m_testDir->createFiles(files); + + m_model->loadDirectory(m_testDir->url()); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "b.txt" << "c.txt" << "d.txt"); + + const KFileItem fileItemB = m_model->fileItem(1); + + // Tell the model that a list of items has been deleted, where "b.txt" appears twice in the list. + KFileItemList list; + list << fileItemB << fileItemB; + m_model->slotItemsDeleted(list); + + QVERIFY(m_model->isConsistent()); + QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "c.txt" << "d.txt"); +} + QStringList KFileItemModelTest::itemsInModel() const { QStringList items; diff --git a/src/tests/kitemrangetest.cpp b/src/tests/kitemrangetest.cpp new file mode 100644 index 000000000..9f3f79980 --- /dev/null +++ b/src/tests/kitemrangetest.cpp @@ -0,0 +1,75 @@ +/*************************************************************************** + * Copyright (C) 2014 by Frank Reininghaus * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * + ***************************************************************************/ + +#include + +#include "kitemviews/kitemrange.h" + +#include + +Q_DECLARE_METATYPE(QVector); +Q_DECLARE_METATYPE(KItemRangeList); + +class KItemRangeTest : public QObject +{ + Q_OBJECT + +private slots: + void testFromSortedContainer_data(); + void testFromSortedContainer(); +}; + +void KItemRangeTest::testFromSortedContainer_data() +{ + QTest::addColumn >("sortedNumbers"); + QTest::addColumn("expected"); + + QTest::newRow("empty") << QVector() << KItemRangeList(); + QTest::newRow("[1]") << (QVector() << 1) << (KItemRangeList() << KItemRange(1, 1)); + QTest::newRow("[9]") << (QVector() << 9) << (KItemRangeList() << KItemRange(9, 1)); + QTest::newRow("[1-2]") << (QVector() << 1 << 2) << (KItemRangeList() << KItemRange(1, 2)); + QTest::newRow("[1-3]") << (QVector() << 1 << 2 << 3) << (KItemRangeList() << KItemRange(1, 3)); + QTest::newRow("[1] [4]") << (QVector() << 1 << 4) << (KItemRangeList() << KItemRange(1, 1) << KItemRange(4, 1)); + QTest::newRow("[1-3] [5]") << (QVector() << 1 << 2 << 3 << 5) << (KItemRangeList() << KItemRange(1, 3) << KItemRange(5, 1)); + QTest::newRow("[1] [5-6]") << (QVector() << 1 << 5 << 6) << (KItemRangeList() << KItemRange(1, 1) << KItemRange(5, 2)); + QTest::newRow("duplicates: 1 1") << (QVector() << 1 << 1) << (KItemRangeList() << KItemRange(1, 1)); + QTest::newRow("duplicates: 1 1 1") << (QVector() << 1 << 1 << 1) << (KItemRangeList() << KItemRange(1, 1)); + QTest::newRow("duplicates: 1 1 5") << (QVector() << 1 << 1 << 5) << (KItemRangeList() << KItemRange(1, 1) << KItemRange(5, 1)); + QTest::newRow("duplicates: 1 5 5") << (QVector() << 1 << 5 << 5) << (KItemRangeList() << KItemRange(1, 1) << KItemRange(5, 1)); + QTest::newRow("duplicates: 1 1 1 5") << (QVector() << 1 << 1 << 1 << 5) << (KItemRangeList() << KItemRange(1, 1) << KItemRange(5, 1)); + QTest::newRow("duplicates: 1 5 5 5") << (QVector() << 1 << 5 << 5 << 5) << (KItemRangeList() << KItemRange(1, 1) << KItemRange(5, 1)); + QTest::newRow("duplicates: 1 1 2") << (QVector() << 1 << 1 << 2) << (KItemRangeList() << KItemRange(1, 2)); + QTest::newRow("duplicates: 1 2 2") << (QVector() << 1 << 2 << 2) << (KItemRangeList() << KItemRange(1, 2)); + QTest::newRow("duplicates: 1 1 2 3") << (QVector() << 1 << 1 << 2 << 3) << (KItemRangeList() << KItemRange(1, 3)); + QTest::newRow("duplicates: 1 2 2 3") << (QVector() << 1 << 2 << 2 << 3) << (KItemRangeList() << KItemRange(1, 3)); + QTest::newRow("duplicates: 1 2 3 3") << (QVector() << 1 << 2 << 3 << 3) << (KItemRangeList() << KItemRange(1, 3)); +} + +void KItemRangeTest::testFromSortedContainer() +{ + QFETCH(QVector, sortedNumbers); + QFETCH(KItemRangeList, expected); + + const KItemRangeList result = KItemRangeList::fromSortedContainer(sortedNumbers); + QCOMPARE(expected, result); +} + +QTEST_KDEMAIN(KItemRangeTest, NoGUI) + +#include "kitemrangetest.moc" From e07468c7840caeff97360edf08826aa38e8d96ae Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Thu, 5 Jun 2014 08:50:52 +0200 Subject: [PATCH 2/2] Separate width and height info in the layouting code By separating the width and height info, we can save some unnecessary overhead in terms of memory and CPU cycles, and make the calculation of the height of a row (or the width of a column in Compact View) a bit simpler. To achieve this, this patch extends the concept of "logical rows" (which are actually columns in Compact View) to "logical width" and "logical height" (which is the actual height and width, respectively, in Compact View). The distinction between rows/columns and "logical" rows/columns may be a bit confusing, but the confusion is already in the current code, and I hope that it will be mitigated a bit by prefixing the corresponding variables with "logical". REVIEW: 118454 --- src/kitemviews/kitemlistview.cpp | 4 +- src/kitemviews/kitemlistview.h | 16 ++++--- src/kitemviews/kitemlistwidget.h | 2 +- src/kitemviews/kstandarditemlistwidget.cpp | 40 ++++++++--------- src/kitemviews/kstandarditemlistwidget.h | 8 ++-- .../private/kitemlistsizehintresolver.cpp | 45 ++++++++++--------- .../private/kitemlistsizehintresolver.h | 3 +- .../private/kitemlistviewlayouter.cpp | 7 ++- 8 files changed, 65 insertions(+), 60 deletions(-) diff --git a/src/kitemviews/kitemlistview.cpp b/src/kitemviews/kitemlistview.cpp index 222a29cf5..281258898 100644 --- a/src/kitemviews/kitemlistview.cpp +++ b/src/kitemviews/kitemlistview.cpp @@ -459,9 +459,9 @@ int KItemListView::lastVisibleIndex() const return m_layouter->lastVisibleIndex(); } -void KItemListView::calculateItemSizeHints(QVector& sizeHints) const +void KItemListView::calculateItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint) const { - widgetCreator()->calculateItemSizeHints(sizeHints, this); + widgetCreator()->calculateItemSizeHints(logicalHeightHints, logicalWidthHint, this); } void KItemListView::setSupportsItemExpanding(bool supportsExpanding) diff --git a/src/kitemviews/kitemlistview.h b/src/kitemviews/kitemlistview.h index 8a522a686..cf6f27c03 100644 --- a/src/kitemviews/kitemlistview.h +++ b/src/kitemviews/kitemlistview.h @@ -194,12 +194,14 @@ public: int lastVisibleIndex() const; /** - * @return Required size for all items in the model. - * The returned value might be larger than KItemListView::itemSize(). + * @return Calculates the required size for all items in the model. + * It might be larger than KItemListView::itemSize(). * In this case the layout grid will be stretched to assure an * unclipped item. + * NOTE: the logical height (width) is actually the + * width (height) if the scroll orientation is Qt::Vertical! */ - void calculateItemSizeHints(QVector& sizeHints) const; + void calculateItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint) const; /** * If set to true, items having child-items can be expanded to show the child-items as @@ -805,7 +807,7 @@ public: virtual void recycle(KItemListWidget* widget); - virtual void calculateItemSizeHints(QVector& sizeHints, const KItemListView* view) const = 0; + virtual void calculateItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const = 0; virtual qreal preferredRoleColumnWidth(const QByteArray& role, int index, @@ -824,7 +826,7 @@ public: virtual KItemListWidget* create(KItemListView* view); - virtual void calculateItemSizeHints(QVector& sizeHints, const KItemListView* view) const; + virtual void calculateItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const; virtual qreal preferredRoleColumnWidth(const QByteArray& role, int index, @@ -857,9 +859,9 @@ KItemListWidget* KItemListWidgetCreator::create(KItemListView* view) } template -void KItemListWidgetCreator::calculateItemSizeHints(QVector& sizeHints, const KItemListView* view) const +void KItemListWidgetCreator::calculateItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const { - return m_informant->calculateItemSizeHints(sizeHints, view); + return m_informant->calculateItemSizeHints(logicalHeightHints, logicalWidthHint, view); } template diff --git a/src/kitemviews/kitemlistwidget.h b/src/kitemviews/kitemlistwidget.h index cfb9155eb..a06bb5c9c 100644 --- a/src/kitemviews/kitemlistwidget.h +++ b/src/kitemviews/kitemlistwidget.h @@ -49,7 +49,7 @@ public: KItemListWidgetInformant(); virtual ~KItemListWidgetInformant(); - virtual void calculateItemSizeHints(QVector& sizeHints, const KItemListView* view) const = 0; + virtual void calculateItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const = 0; virtual qreal preferredRoleColumnWidth(const QByteArray& role, int index, diff --git a/src/kitemviews/kstandarditemlistwidget.cpp b/src/kitemviews/kstandarditemlistwidget.cpp index 037226997..e0375480e 100644 --- a/src/kitemviews/kstandarditemlistwidget.cpp +++ b/src/kitemviews/kstandarditemlistwidget.cpp @@ -55,19 +55,19 @@ KStandardItemListWidgetInformant::~KStandardItemListWidgetInformant() { } -void KStandardItemListWidgetInformant::calculateItemSizeHints(QVector& sizeHints, const KItemListView* view) const +void KStandardItemListWidgetInformant::calculateItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const { switch (static_cast(view)->itemLayout()) { case KStandardItemListWidget::IconsLayout: - calculateIconsLayoutItemSizeHints(sizeHints, view); + calculateIconsLayoutItemSizeHints(logicalHeightHints, logicalWidthHint, view); break; case KStandardItemListWidget::CompactLayout: - calculateCompactLayoutItemSizeHints(sizeHints, view); + calculateCompactLayoutItemSizeHints(logicalHeightHints, logicalWidthHint, view); break; case KStandardItemListWidget::DetailsLayout: - calculateDetailsLayoutItemSizeHints(sizeHints, view); + calculateDetailsLayoutItemSizeHints(logicalHeightHints, logicalWidthHint, view); break; default: @@ -138,7 +138,7 @@ QFont KStandardItemListWidgetInformant::customizedFontForLinks(const QFont& base return baseFont; } -void KStandardItemListWidgetInformant::calculateIconsLayoutItemSizeHints(QVector& sizeHints, const KItemListView* view) const +void KStandardItemListWidgetInformant::calculateIconsLayoutItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const { const KItemListStyleOption& option = view->styleOption(); const QFont& normalFont = option.font; @@ -154,8 +154,8 @@ void KStandardItemListWidgetInformant::calculateIconsLayoutItemSizeHints(QVector QTextOption textOption(Qt::AlignHCenter); textOption.setWrapMode(QTextOption::WrapAtWordBoundaryOrAnywhere); - for (int index = 0; index < sizeHints.count(); ++index) { - if (!sizeHints.at(index).isEmpty()) { + for (int index = 0; index < logicalHeightHints.count(); ++index) { + if (logicalHeightHints.at(index) > 0.0) { continue; } @@ -186,11 +186,13 @@ void KStandardItemListWidgetInformant::calculateIconsLayoutItemSizeHints(QVector // Add one line for each additional information textHeight += additionalRolesSpacing; - sizeHints[index] = QSizeF(itemWidth, textHeight + spacingAndIconHeight); + logicalHeightHints[index] = textHeight + spacingAndIconHeight; } + + logicalWidthHint = itemWidth; } -void KStandardItemListWidgetInformant::calculateCompactLayoutItemSizeHints(QVector& sizeHints, const KItemListView* view) const +void KStandardItemListWidgetInformant::calculateCompactLayoutItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const { const KItemListStyleOption& option = view->styleOption(); const QFontMetrics& normalFontMetrics = option.fontMetrics; @@ -204,8 +206,8 @@ void KStandardItemListWidgetInformant::calculateCompactLayoutItemSizeHints(QVect const QFontMetrics linkFontMetrics(customizedFontForLinks(option.font)); - for (int index = 0; index < sizeHints.count(); ++index) { - if (!sizeHints.at(index).isEmpty()) { + for (int index = 0; index < logicalHeightHints.count(); ++index) { + if (logicalHeightHints.at(index) > 0.0) { continue; } @@ -232,22 +234,18 @@ void KStandardItemListWidgetInformant::calculateCompactLayoutItemSizeHints(QVect width = maxWidth; } - sizeHints[index] = QSizeF(width, height); + logicalHeightHints[index] = width; } + + logicalWidthHint = height; } -void KStandardItemListWidgetInformant::calculateDetailsLayoutItemSizeHints(QVector& sizeHints, const KItemListView* view) const +void KStandardItemListWidgetInformant::calculateDetailsLayoutItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const { const KItemListStyleOption& option = view->styleOption(); const qreal height = option.padding * 2 + qMax(option.iconSize, option.fontMetrics.height()); - - for (int index = 0; index < sizeHints.count(); ++index) { - if (!sizeHints.at(index).isEmpty()) { - continue; - } - - sizeHints[index] = QSizeF(-1, height); - } + logicalHeightHints.fill(height); + logicalWidthHint = -1.0; } KStandardItemListWidget::KStandardItemListWidget(KItemListWidgetInformant* informant, QGraphicsItem* parent) : diff --git a/src/kitemviews/kstandarditemlistwidget.h b/src/kitemviews/kstandarditemlistwidget.h index ba426d054..4f7a9136e 100644 --- a/src/kitemviews/kstandarditemlistwidget.h +++ b/src/kitemviews/kstandarditemlistwidget.h @@ -38,7 +38,7 @@ public: KStandardItemListWidgetInformant(); virtual ~KStandardItemListWidgetInformant(); - virtual void calculateItemSizeHints(QVector& sizeHints, const KItemListView* view) const; + virtual void calculateItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const; virtual qreal preferredRoleColumnWidth(const QByteArray& role, int index, @@ -73,9 +73,9 @@ protected: */ virtual QFont customizedFontForLinks(const QFont& baseFont) const; - void calculateIconsLayoutItemSizeHints(QVector& sizeHints, const KItemListView* view) const; - void calculateCompactLayoutItemSizeHints(QVector& sizeHints, const KItemListView* view) const; - void calculateDetailsLayoutItemSizeHints(QVector& sizeHints, const KItemListView* view) const; + void calculateIconsLayoutItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const; + void calculateCompactLayoutItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const; + void calculateDetailsLayoutItemSizeHints(QVector& logicalHeightHints, qreal& logicalWidthHint, const KItemListView* view) const; friend class KStandardItemListWidget; // Accesses roleText() }; diff --git a/src/kitemviews/private/kitemlistsizehintresolver.cpp b/src/kitemviews/private/kitemlistsizehintresolver.cpp index 029beddf9..1d8067026 100644 --- a/src/kitemviews/private/kitemlistsizehintresolver.cpp +++ b/src/kitemviews/private/kitemlistsizehintresolver.cpp @@ -23,7 +23,8 @@ KItemListSizeHintResolver::KItemListSizeHintResolver(const KItemListView* itemListView) : m_itemListView(itemListView), - m_sizeHintCache(), + m_logicalHeightHintCache(), + m_logicalWidthHint(0.0), m_needsResolving(false) { } @@ -35,7 +36,7 @@ KItemListSizeHintResolver::~KItemListSizeHintResolver() QSizeF KItemListSizeHintResolver::sizeHint(int index) { updateCache(); - return m_sizeHintCache.at(index); + return QSizeF(m_logicalWidthHint, m_logicalHeightHintCache.at(index)); } void KItemListSizeHintResolver::itemsInserted(const KItemRangeList& itemRanges) @@ -45,15 +46,15 @@ void KItemListSizeHintResolver::itemsInserted(const KItemRangeList& itemRanges) insertedCount += range.count; } - const int currentCount = m_sizeHintCache.count(); - m_sizeHintCache.reserve(currentCount + insertedCount); + const int currentCount = m_logicalHeightHintCache.count(); + m_logicalHeightHintCache.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()); + m_logicalHeightHintCache.insert(m_logicalHeightHintCache.end(), insertedCount, 0.0); int sourceIndex = currentCount - 1; - int targetIndex = m_sizeHintCache.count() - 1; + int targetIndex = m_logicalHeightHintCache.count() - 1; int itemsToInsertBeforeCurrentRange = insertedCount; for (int rangeIndex = itemRanges.count() - 1; rangeIndex >= 0; --rangeIndex) { @@ -62,33 +63,33 @@ void KItemListSizeHintResolver::itemsInserted(const KItemRangeList& itemRanges) // First: move all existing items that must be put behind 'range'. while (targetIndex >= itemsToInsertBeforeCurrentRange + range.index + range.count) { - m_sizeHintCache[targetIndex] = m_sizeHintCache[sourceIndex]; + m_logicalHeightHintCache[targetIndex] = m_logicalHeightHintCache[sourceIndex]; --sourceIndex; --targetIndex; } // Then: insert QSizeF() for the items which are inserted into 'range'. while (targetIndex >= itemsToInsertBeforeCurrentRange + range.index) { - m_sizeHintCache[targetIndex] = QSizeF(); + m_logicalHeightHintCache[targetIndex] = 0.0; --targetIndex; } } m_needsResolving = true; - Q_ASSERT(m_sizeHintCache.count() == m_itemListView->model()->count()); + Q_ASSERT(m_logicalHeightHintCache.count() == m_itemListView->model()->count()); } void KItemListSizeHintResolver::itemsRemoved(const KItemRangeList& itemRanges) { - const QVector::iterator begin = m_sizeHintCache.begin(); - const QVector::iterator end = m_sizeHintCache.end(); + const QVector::iterator begin = m_logicalHeightHintCache.begin(); + const QVector::iterator end = m_logicalHeightHintCache.end(); KItemRangeList::const_iterator rangeIt = itemRanges.constBegin(); const KItemRangeList::const_iterator rangeEnd = itemRanges.constEnd(); - QVector::iterator destIt = begin + rangeIt->index; - QVector::iterator srcIt = destIt + rangeIt->count; + QVector::iterator destIt = begin + rangeIt->index; + QVector::iterator srcIt = destIt + rangeIt->count; ++rangeIt; @@ -104,33 +105,33 @@ void KItemListSizeHintResolver::itemsRemoved(const KItemRangeList& itemRanges) } } - m_sizeHintCache.erase(destIt, end); + m_logicalHeightHintCache.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()); + if (!m_logicalHeightHintCache.isEmpty() && m_itemListView->model()) { + Q_ASSERT(m_logicalHeightHintCache.count() == m_itemListView->model()->count()); } } void KItemListSizeHintResolver::itemsMoved(const KItemRange& range, const QList& movedToIndexes) { - QVector newSizeHintCache(m_sizeHintCache); + QVector newLogicalHeightHintCache(m_logicalHeightHintCache); const int movedRangeEnd = range.index + range.count; for (int i = range.index; i < movedRangeEnd; ++i) { const int newIndex = movedToIndexes.at(i - range.index); - newSizeHintCache[newIndex] = m_sizeHintCache.at(i); + newLogicalHeightHintCache[newIndex] = m_logicalHeightHintCache.at(i); } - m_sizeHintCache = newSizeHintCache; + m_logicalHeightHintCache = newLogicalHeightHintCache; } void KItemListSizeHintResolver::itemsChanged(int index, int count, const QSet& roles) { Q_UNUSED(roles); while (count) { - m_sizeHintCache[index] = QSizeF(); + m_logicalHeightHintCache[index] = 0.0; ++index; --count; } @@ -140,14 +141,14 @@ void KItemListSizeHintResolver::itemsChanged(int index, int count, const QSetcalculateItemSizeHints(m_sizeHintCache); + m_itemListView->calculateItemSizeHints(m_logicalHeightHintCache, m_logicalWidthHint); m_needsResolving = false; } } diff --git a/src/kitemviews/private/kitemlistsizehintresolver.h b/src/kitemviews/private/kitemlistsizehintresolver.h index 86580bf7b..a0ad033f3 100644 --- a/src/kitemviews/private/kitemlistsizehintresolver.h +++ b/src/kitemviews/private/kitemlistsizehintresolver.h @@ -48,7 +48,8 @@ public: private: const KItemListView* m_itemListView; - mutable QVector m_sizeHintCache; + mutable QVector m_logicalHeightHintCache; + mutable qreal m_logicalWidthHint; bool m_needsResolving; }; diff --git a/src/kitemviews/private/kitemlistviewlayouter.cpp b/src/kitemviews/private/kitemlistviewlayouter.cpp index 9da5384d4..04325c7d0 100644 --- a/src/kitemviews/private/kitemlistviewlayouter.cpp +++ b/src/kitemviews/private/kitemlistviewlayouter.cpp @@ -239,6 +239,7 @@ QRectF KItemListViewLayouter::itemRect(int index) const // to get the physical horizontal direction QPointF pos(y, x); pos.rx() -= m_scrollOffset; + sizeHint.transpose(); return QRectF(pos, sizeHint); } @@ -282,7 +283,9 @@ QRectF KItemListViewLayouter::groupHeaderRect(int index) const break; } - const qreal itemWidth = m_sizeHintResolver->sizeHint(index).width(); + const qreal itemWidth = (m_scrollOrientation == Qt::Vertical) + ? m_sizeHintResolver->sizeHint(index).width() + : m_sizeHintResolver->sizeHint(index).height(); if (itemWidth > headerWidth) { headerWidth = itemWidth; @@ -461,7 +464,7 @@ void KItemListViewLayouter::doLayout() while (index < itemCount && column < m_columnCount) { qreal requiredItemHeight = itemSize.height(); const QSizeF sizeHint = m_sizeHintResolver->sizeHint(index); - const qreal sizeHintHeight = horizontalScrolling ? sizeHint.width() : sizeHint.height(); + const qreal sizeHintHeight = sizeHint.height(); if (sizeHintHeight > requiredItemHeight) { requiredItemHeight = sizeHintHeight; }