QHeaderView: properly restore section data after layoutChanged()

QHeaderView is doing a complete rebuild of the sections when the layout changed because everything could have happened. But since layoutChanged is also called during e.g. sorting, the old data must be restored when possible. Task-number: QTBUG-65478 Change-Id: I088d4d843cad362b97df6dc5e0dcb9819b13547f Reviewed-by: Thorbjørn Lund Martsum <[email protected]> Reviewed-by: Richard Moe Gustavsen <[email protected]>
shane-gramlich · Jan 29, 2018 · 3adfcbf · 3adfcbf
1 parent 5a05348
commit 3adfcbf
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 30 deletions.
diff --git a/src/widgets/itemviews/qheaderview.cpp b/src/widgets/itemviews/qheaderview.cpp
@@ -351,7 +351,7 @@ void QHeaderView::setModel(QAbstractItemModel *model)
     if (model == this->model())
         return;
     Q_D(QHeaderView);
-    d->persistentHiddenSections.clear();
+    d->layoutChangePersistentSections.clear();
     if (d->model && d->model != QAbstractItemModelPrivate::staticEmptyModel()) {
     if (d->orientation == Qt::Horizontal) {
         QObject::disconnect(d->model, SIGNAL(columnsInserted(QModelIndex,int,int)),
@@ -2072,40 +2072,86 @@ void QHeaderViewPrivate::_q_layoutAboutToBeChanged()
         || model->columnCount(root) == 0)
         return;
 
-    if (hiddenSectionSize.count() == 0)
-        return;
+    layoutChangePersistentSections.clear();
+    layoutChangePersistentSections.reserve(std::min(10, sectionItems.count()));
+    // after layoutChanged another section can be last stretched section
+    if (stretchLastSection) {
+        const int visual = visualIndex(lastSectionLogicalIdx);
+        sectionItems[visual].size = lastSectionSize;
+    }
+    for (int i = 0; i < sectionItems.size(); ++i) {
+        const auto &s = sectionItems.at(i);
+        // only add if the section is not default and not visually moved
+        if (s.size == defaultSectionSize && !s.isHidden && s.resizeMode == globalResizeMode)
+            continue;
 
-    for (int i = 0; i < sectionItems.count(); ++i)
-        if (isVisualIndexHidden(i)) // ### note that we are using column or row 0
-            persistentHiddenSections.append(orientation == Qt::Horizontal
-                                            ? model->index(0, logicalIndex(i), root)
-                                            : model->index(logicalIndex(i), 0, root));
+        // ### note that we are using column or row 0
+        layoutChangePersistentSections.append({orientation == Qt::Horizontal
+                                                  ? model->index(0, logicalIndex(i), root)
+                                                  : model->index(logicalIndex(i), 0, root),
+                                              s});
+
+        if (layoutChangePersistentSections.size() > 1000)
+            break;
+    }
 }
 
 void QHeaderViewPrivate::_q_layoutChanged()
 {
     Q_Q(QHeaderView);
     viewport->update();
 
-    const auto hiddenSections = persistentHiddenSections;
-    persistentHiddenSections.clear();
-
-    clear();
-    q->initializeSections();
-    invalidateCachedSizeHint();
+    const auto oldPersistentSections = layoutChangePersistentSections;
+    layoutChangePersistentSections.clear();
 
-    if (modelIsEmpty()) {
+    const int newCount = modelSectionCount();
+    const int oldCount = sectionItems.size();
+    if (newCount == 0) {
+        clear();
+        if (oldCount != 0)
+            emit q->sectionCountChanged(oldCount, 0);
         return;
     }
 
-    for (const auto &index : hiddenSections) {
-        if (index.isValid()) {
-            const int logical = (orientation == Qt::Horizontal
-                                 ? index.column()
-                                 : index.row());
-            q->setSectionHidden(logical, true);
+    // adjust section size
+    if (newCount != oldCount) {
+        const int min = qBound(0, oldCount, newCount - 1);
+        q->initializeSections(min, newCount - 1);
+    }
+    // reset sections
+    sectionItems.fill(SectionItem(defaultSectionSize, globalResizeMode), newCount);
+
+    // all hidden sections are in oldPersistentSections
+    hiddenSectionSize.clear();
+
+    for (const auto &item : oldPersistentSections) {
+        const auto &index = item.index;
+        if (!index.isValid())
+            continue;
+
+        const int newLogicalIndex = (orientation == Qt::Horizontal
+                                     ? index.column()
+                                     : index.row());
+        // the new visualIndices are already adjusted / reset by initializeSections()
+        const int newVisualIndex = visualIndex(newLogicalIndex);
+        auto &newSection = sectionItems[newVisualIndex];
+        newSection = item.section;
+
+        if (newSection.isHidden) {
+            // otherwise setSectionHidden will return without doing anything
+            newSection.isHidden = false;
+            q->setSectionHidden(newLogicalIndex, true);
         }
     }
+
+    recalcSectionStartPos();
+    length = headerLength();
+
+    if (stretchLastSection) {
+        // force rebuild of stretched section later on
+        lastSectionLogicalIdx = -1;
+        maybeRestorePrevLastSectionAndStretchLast();
+    }
 }
 
 /*!

diff --git a/src/widgets/itemviews/qheaderview_p.h b/src/widgets/itemviews/qheaderview_p.h
@@ -231,10 +231,6 @@ class QHeaderViewPrivate: public QAbstractItemViewPrivate
                 : model->rowCount(root));
     }
 
-    inline bool modelIsEmpty() const {
-        return (model->rowCount(root) == 0 || model->columnCount(root) == 0);
-    }
-
     inline void doDelayedResizeSections() {
         if (!delayedResize.isActive())
             delayedResize.start(0, q_func());
@@ -304,7 +300,6 @@ class QHeaderViewPrivate: public QAbstractItemViewPrivate
     QLabel *sectionIndicator;
 #endif
     QHeaderView::ResizeMode globalResizeMode;
-    QList<QPersistentModelIndex> persistentHiddenSections;
     mutable bool sectionStartposRecalc;
     int resizeContentsPrecision;
     // header sections
@@ -335,6 +330,11 @@ class QHeaderViewPrivate: public QAbstractItemViewPrivate
     };
 
     QVector<SectionItem> sectionItems;
+    struct LayoutChangeItem {
+        QPersistentModelIndex index;
+        SectionItem section;
+    };
+    QVector<LayoutChangeItem> layoutChangePersistentSections;
 
     void createSectionItems(int start, int end, int size, QHeaderView::ResizeMode mode);
     void removeSectionsFromSectionItems(int start, int end);
@@ -388,6 +388,7 @@ class QHeaderViewPrivate: public QAbstractItemViewPrivate
 
 };
 Q_DECLARE_TYPEINFO(QHeaderViewPrivate::SectionItem, Q_PRIMITIVE_TYPE);
+Q_DECLARE_TYPEINFO(QHeaderViewPrivate::LayoutChangeItem, Q_MOVABLE_TYPE);
 
 QT_END_NAMESPACE
 

diff --git a/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp b/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp
@@ -2251,10 +2251,6 @@ void tst_QHeaderView::QTBUG6058_reset()
 
 void tst_QHeaderView::QTBUG7833_sectionClicked()
 {
-
-
-
-
     QTableView tv;
     QStandardItemModel *sim = new QStandardItemModel(&tv);
     QSortFilterProxyModel *proxyModel = new QSortFilterProxyModel(&tv);
@@ -2278,11 +2274,20 @@ void tst_QHeaderView::QTBUG7833_sectionClicked()
     tv.horizontalHeader()->setSectionResizeMode(QHeaderView::Interactive);
 
     tv.setModel(proxyModel);
+    const int section4Size = tv.horizontalHeader()->sectionSize(4) + 1;
+    tv.horizontalHeader()->resizeSection(4, section4Size);
     tv.setColumnHidden(5, true);
     tv.setColumnHidden(6, true);
     tv.horizontalHeader()->swapSections(8, 10);
     tv.sortByColumn(1, Qt::AscendingOrder);
 
+    QCOMPARE(tv.isColumnHidden(5), true);
+    QCOMPARE(tv.isColumnHidden(6), true);
+    QCOMPARE(tv.horizontalHeader()->sectionsMoved(), true);
+    QCOMPARE(tv.horizontalHeader()->logicalIndex(8), 10);
+    QCOMPARE(tv.horizontalHeader()->logicalIndex(10), 8);
+    QCOMPARE(tv.horizontalHeader()->sectionSize(4), section4Size);
+
     QSignalSpy clickedSpy(tv.horizontalHeader(), SIGNAL(sectionClicked(int)));
     QSignalSpy pressedSpy(tv.horizontalHeader(), SIGNAL(sectionPressed(int)));