qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

QQmlTreeModelToTableModel: move logic from modelLayoutChanged() to modelLayoutAboutToBeChanged() 

The QQmlTreeModelToTableModel contains a QList of items that
converts the source model (which typically is a tree model)
into a flat list of items that can be be displayed in a TableView.

When the source model is changing the layout (e.g when doing
a sort), we handle this in QQmlTreeModelToTableModel by listening
to the modelLayoutChanged() signal. The problem is that by the
time we receive that signal, all the TreeItems in the list of
items will have their QPersistentModelIndex updated, before we
have updated the layout of the list to be in sync. Since functions
like itemIndex(index) depends on the list model and the tree model
to be in sync, it cannot be trusted at this point.

To handle this problem, we need to divide the current logic into
two functions. The first part will remove all the affected rows
from the list before the source model has changed (while the models
are still in sync), from modelLayoutAboutToBeChanged(). Then we
add them back again once the source model has finished updating the
layout, which is done from modelLayoutChanged().

This will fix a bug in TreeView, where the view will not reflect
the sorting changes done to a QSortFilterProxyModel applied on
a QAbstractItemModel.

Fixes: QTBUG-103877
Pick-to: 6.4
Change-Id: I9acfc3b7b9a79c2d16e1bc63e56b1d6ea5a53897
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
This commit is contained in:
Richard Moe Gustavsen 2022-10-17 14:11:48 +02:00
parent 605693335f
commit 96e70f2366
6 changed files with 230 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
src/qmlmodels/qqmltreemodeltotablemodel.cpp
View File

@ -672,18 +672,61 @@ void QQmlTreeModelToTableModel::modelDataChanged(const QModelIndex &topLeft, con
void QQmlTreeModelToTableModel::modelLayoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint)
{
ASSERT_CONSISTENCY();
Q_UNUSED(parents)
Q_UNUSED(hint)
// Since the m_items is a list of TreeItems that contains QPersistentModelIndexes, we
// cannot wait until we get a modelLayoutChanged() before we remove the affected rows
// from that list. After the layout has changed, the list (or, the persistent indexes
// that it contains) is no longer in sync with the model (after all, that is what we're
// supposed to correct in modelLayoutChanged()).
// This means that vital functions, like itemIndex(index), cannot be trusted at that point.
// Therefore we need to do the update in two steps; First remove all the affected rows
// from here (while we're still in sync with the model), and then add back the
// affected rows, and notify about it, from modelLayoutChanged().
m_modelLayoutChanged = false;
if (parents.isEmpty() || !parents[0].isValid()) {
// Update entire model
emit layoutAboutToBeChanged();
m_modelLayoutChanged = true;
m_items.clear();
return;
}
for (const QPersistentModelIndex &pmi : parents) {
if (!m_expandedItems.contains(pmi))
continue;
const int row = itemIndex(pmi);
if (row == -1)
continue;
const int rowCount = m_model->rowCount(pmi);
if (rowCount == 0)
continue;
if (!m_modelLayoutChanged) {
emit layoutAboutToBeChanged();
m_modelLayoutChanged = true;
}
const QModelIndex &lmi = m_model->index(rowCount - 1, 0, pmi);
const int lastRow = lastChildIndex(lmi);
removeVisibleRows(row + 1, lastRow, false /*doRemoveRows*/);
}
ASSERT_CONSISTENCY();
}
void QQmlTreeModelToTableModel::modelLayoutChanged(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint)
{
Q_UNUSED(hint)
if (parents.isEmpty()) {
emit layoutAboutToBeChanged();
m_items.clear();
if (!m_modelLayoutChanged) {
// No relevant changes done from modelLayoutAboutToBeChanged()
return;
}
if (m_items.isEmpty()) {
// Entire model has changed. Add back all rows.
showModelTopLevelItems(false /*doInsertRows*/);
const QModelIndex &mi = m_model->index(0, 0);
const int columnCount = m_model->columnCount(mi);
@ -692,30 +735,24 @@ void QQmlTreeModelToTableModel::modelLayoutChanged(const QList<QPersistentModelI
return;
}
bool shouldEmitLayoutChanged = false;
for (const QPersistentModelIndex &pmi : parents) {
if (m_expandedItems.contains(pmi)) {
int row = itemIndex(pmi);
if (row != -1) {
if (!shouldEmitLayoutChanged) {
shouldEmitLayoutChanged = true;
emit layoutAboutToBeChanged();
}
int rowCount = m_model->rowCount(pmi);
if (rowCount > 0) {
const QModelIndex &lmi = m_model->index(rowCount - 1, 0, pmi);
const int lastRow = lastChildIndex(lmi);
const int columnCount = m_model->columnCount(lmi);
removeVisibleRows(row + 1, lastRow, false /*doRemoveRows*/);
showModelChildItems(m_items.at(row), 0, rowCount - 1, false /*doInsertRows*/);
emit dataChanged(index(row + 1, 0), index(lastRow, columnCount - 1));
}
}
}
if (!m_expandedItems.contains(pmi))
continue;
const int row = itemIndex(pmi);
if (row == -1)
continue;
const int rowCount = m_model->rowCount(pmi);
if (rowCount == 0)
continue;
const QModelIndex &lmi = m_model->index(rowCount - 1, 0, pmi);
const int columnCount = m_model->columnCount(lmi);
showModelChildItems(m_items.at(row), 0, rowCount - 1, false /*doInsertRows*/);
const int lastRow = lastChildIndex(lmi);
emit dataChanged(index(row + 1, 0), index(lastRow, columnCount - 1));
}
if (shouldEmitLayoutChanged)
emit layoutChanged();
emit layoutChanged();
ASSERT_CONSISTENCY();
}

1
src/qmlmodels/qqmltreemodeltotablemodel_p_p.h
View File

@ -165,6 +165,7 @@ private:
QList<TreeItem> m_itemsToExpand;
mutable int m_lastItemIndex = 0;
bool m_visibleRowsMoved = false;
bool m_modelLayoutChanged = false;
int m_signalAggregatorStack = 0;
QVector<DataChangedParams> m_queuedDataChanged;
int m_column = 0;

4
tests/auto/qml/qqmltreemodeltotablemodel/tst_qqmltreemodeltotablemodel.cpp
View File

@ -8,6 +8,10 @@
#include "testmodel.h"
/*
* Note: Out of practical reasons, QQmlTreeModelToTableModel is by and large
* tested from tst_qquicktreeview.cpp, where TreeView is available.
*/
class tst_QQmlTreeModelToTableModel : public QObject {
Q_OBJECT

8
tests/auto/quick/qquicktreeview/testmodel.cpp
View File

@ -38,8 +38,14 @@ void TestModel::createTreeRecursive(TreeItem *item, int childCount, int currentD
for (int col = 0; col < m_columnCount; ++col)
childItem->m_entries << QVariant(QString("%1, %2").arg(row).arg(col));
item->m_childItems.append(childItem);
if (row == childCount - 1)
if (row == childCount - 2 && currentDepth != maxDepth()) {
// Add a branch that doesn't recurse
createTreeRecursive(childItem, childCount, maxDepth());
}
if (row == childCount - 1) {
// Add a branch that recurses
createTreeRecursive(childItem, childCount, currentDepth + 1);
}
}
}

2
tests/auto/quick/qquicktreeview/testmodel.h
View File

@ -45,4 +45,6 @@ private:
int m_columnCount = 5;
};
#define TestModelAsVariant(...) QVariant::fromValue(QSharedPointer<TestModel>(new TestModel(__VA_ARGS__)))
#endif // TESTMODEL_H

166
tests/auto/quick/qquicktreeview/tst_qquicktreeview.cpp
View File

@ -79,6 +79,10 @@ private slots:
void selectionBehaviorRows();
void selectionBehaviorColumns();
void selectionBehaviorDisabled();
void sortTreeModel_data();
void sortTreeModel();
void sortTreeModelDynamic_data();
void sortTreeModelDynamic();
};
tst_qquicktreeview::tst_qquicktreeview()
@ -297,7 +301,7 @@ void tst_qquicktreeview::requiredPropertiesChildren()
QCOMPARE(viewProp, treeView);
QCOMPARE(isTreeNode, true);
QCOMPARE(expanded, row == 4);
QCOMPARE(hasChildren, row == 4 || row == 8);
QCOMPARE(hasChildren, model->hasChildren(treeView->modelIndex(0, row)));
QCOMPARE(depth, row <= 4 ? 1 : 2);
}
}
@ -508,7 +512,7 @@ void tst_qquicktreeview::expandRecursivelyChild()
const bool rowToExpandDepth = treeView->depth(rowToExpand);
const int effectiveMaxDepth = depth != -1 ? rowToExpandDepth + depth : model->maxDepth();
// Check that all rows before rowToExpand is not expanded
// Check that none of the rows before rowToExpand are expanded
// (except the root node)
for (int currentRow = 1; currentRow < rowToExpand; ++currentRow)
QVERIFY(!treeView->isExpanded(currentRow));
@ -519,8 +523,8 @@ void tst_qquicktreeview::expandRecursivelyChild()
else
QVERIFY(!treeView->isExpanded(rowToExpand));
// Check that all rows after rowToExpand that is also
// children of that row is expanded (down to depth)
// Check that any row after rowToExpand, that is also
// a child of that row, is expanded (down to depth)
for (int currentRow = rowToExpand + 1; currentRow < treeView->rows(); ++currentRow) {
const int currentDepth = treeView->depth(currentRow);
const bool isChild = currentDepth > rowToExpandDepth;
@ -562,7 +566,9 @@ void tst_qquicktreeview::collapseRecursivelyRoot()
WAIT_UNTIL_POLISHED;
// Verify that the tree is now fully expanded
const int expectedRowCount = 1 + (model->maxDepth() * 4); // root + 4 children per level
// The number of rows should be the root, + 4 children per level. All parents
// (minus the root), will also have a node with 4 non-recursive children.
const int expectedRowCount = 1 + (model->maxDepth() * 8) - 4;
QCOMPARE(treeView->rows(), expectedRowCount);
QSignalSpy spy(treeView, SIGNAL(collapsed(int, bool)));
@ -607,14 +613,18 @@ void tst_qquicktreeview::collapseRecursivelyChild()
WAIT_UNTIL_POLISHED;
// Verify that the tree is now fully expanded
const int expectedRowCount = 1 + (model->maxDepth() * 4); // root + 4 children per level
// The number of rows should be the root, + 4 children per level. All parents
// (minus the root), will also have a node with 4 non-recursive children.
const int expectedRowCount = 1 + (model->maxDepth() * 8) - 4;
QCOMPARE(treeView->rows(), expectedRowCount);
QSignalSpy spy(treeView, SIGNAL(collapsed(int, bool)));
// Collapse the 4th child recursive
const int rowToCollapse = 4;
QCOMPARE(model->data(treeView->modelIndex(0, rowToCollapse), Qt::DisplayRole), QStringLiteral("3, 0"));
// Collapse the 8th child recursive
const int rowToCollapse = 8;
const QModelIndex collapseIndex = treeView->modelIndex(0, rowToCollapse);
const auto expectedLabel = model->data(collapseIndex, Qt::DisplayRole);
QCOMPARE(expectedLabel, QStringLiteral("3, 0"));
treeView->collapseRecursively(rowToCollapse);
QCOMPARE(spy.size(), 1);
@ -624,7 +634,7 @@ void tst_qquicktreeview::collapseRecursivelyChild()
WAIT_UNTIL_POLISHED;
QCOMPARE(treeView->rows(), 5); // root + 4 children
QCOMPARE(treeView->rows(), 9); // root + 4 children + 4 grand children of the 3rd row
// We need to check that all descendants are collapsed as well. But since they're
// now no longer visible in the view, we need to expand each parent one by one again to make
@ -634,9 +644,15 @@ void tst_qquicktreeview::collapseRecursivelyChild()
while (currentRow < treeView->rows()) {
const QModelIndex currentIndex = treeView->modelIndex(0, currentRow);
if (model->hasChildren(currentIndex)) {
QVERIFY(!treeView->isExpanded(currentRow));
treeView->expand(currentRow);
WAIT_UNTIL_POLISHED;
if (treeView->depth(currentRow) == 1 && currentIndex.row() == 2) {
// We did only recursively expand the 4th child, so the
// third should still be expanded
QVERIFY(treeView->isExpanded(currentRow));
} else {
QVERIFY(!treeView->isExpanded(currentRow));
treeView->expand(currentRow);
WAIT_UNTIL_POLISHED;
}
}
currentRow++;
}
@ -986,6 +1002,130 @@ void tst_qquicktreeview::selectionBehaviorDisabled()
QCOMPARE(selectionModel->hasSelection(), false);
}
void tst_qquicktreeview::sortTreeModel_data()
{
QTest::addColumn<QSharedPointer<QAbstractItemModel>>("treeModel");
const auto stringList = QStringList() << "1" << "2" << "3";
QTest::newRow("TreeModel") << QSharedPointer<QAbstractItemModel>(new TestModel());
QTest::newRow("Number model") << QSharedPointer<QAbstractItemModel>(new QStringListModel(stringList));
}
void tst_qquicktreeview::sortTreeModel()
{
// Check that if you assign a QSortFilterProxyModel to to a TreeView, the
// view will end up sorted correctly if the proxy model is sorted.
QFETCH(QSharedPointer<QAbstractItemModel>, treeModel);
LOAD_TREEVIEW("normaltreeview.qml");
QSortFilterProxyModel proxyModel;
proxyModel.setSourceModel(treeModel.data());
treeView->setModel(QVariant::fromValue(&proxyModel));
// Expand some nodes
treeView->expand(0);
treeView->expand(4);
treeView->expand(3);
WAIT_UNTIL_POLISHED;
// Go through all rows in the view, and check that display in the model
// is the same as in the view. That means that QQmlTreeModelToTableModel
// and QSortFilterProxyModel are in sync.
for (int row = 0; row < treeView->rows(); ++row) {
const auto index = treeView->modelIndex(0, row);
const QString modelDisplay = proxyModel.data(index, Qt::DisplayRole).toString();
const auto childFxItem = treeViewPrivate->loadedTableItem(QPoint(0, row));
QVERIFY(childFxItem);
const auto childItem = childFxItem->item;
QVERIFY(childItem);
const auto context = qmlContext(childItem.data());
const auto itemDisplay = context->contextProperty("display").toString();
QCOMPARE(itemDisplay, modelDisplay);
}
// Now sort the model, and do the same test again
proxyModel.sort(0, Qt::DescendingOrder);
WAIT_UNTIL_POLISHED;
for (int row = 0; row < treeView->rows(); ++row) {
const auto index = treeView->modelIndex(0, row);
const QString modelDisplay = proxyModel.data(index, Qt::DisplayRole).toString();
const auto childFxItem = treeViewPrivate->loadedTableItem(QPoint(0, row));
QVERIFY(childFxItem);
const auto childItem = childFxItem->item;
QVERIFY(childItem);
const auto context = qmlContext(childItem.data());
const auto itemDisplay = context->contextProperty("display").toString();
QCOMPARE(itemDisplay, modelDisplay);
}
}
void tst_qquicktreeview::sortTreeModelDynamic_data()
{
QTest::addColumn<QSharedPointer<QAbstractItemModel>>("treeModel");
QTest::addColumn<int>("row");
const auto stringList = QStringList() << "1" << "2" << "3";
QTest::newRow("TreeModel 0") << QSharedPointer<QAbstractItemModel>(new TestModel()) << 0;
QTest::newRow("TreeModel 1") << QSharedPointer<QAbstractItemModel>(new TestModel()) << 1;
QTest::newRow("TreeModel 3") << QSharedPointer<QAbstractItemModel>(new TestModel()) << 3;
QTest::newRow("TreeModel 6") << QSharedPointer<QAbstractItemModel>(new TestModel()) << 6;
QTest::newRow("Number model") << QSharedPointer<QAbstractItemModel>(new QStringListModel(stringList)) << 1;
}
void tst_qquicktreeview::sortTreeModelDynamic()
{
// Check that if you assign a QSortFilterProxyModel to a TreeView, and
// set DynamicSortFilter to true, the view will end up sorted correctly
// if you change the text for one of the items.
QFETCH(QSharedPointer<QAbstractItemModel>, treeModel);
QFETCH(int, row);
LOAD_TREEVIEW("normaltreeview.qml");
QSortFilterProxyModel proxyModel;
proxyModel.setSourceModel(treeModel.data());
proxyModel.setDynamicSortFilter(true);
proxyModel.sort(Qt::AscendingOrder);
treeView->setModel(QVariant::fromValue(&proxyModel));
// Expand some nodes
treeView->expand(0);
treeView->expand(4);
treeView->expand(3);
// Go through all rows in the view, and check that display in the model
// is the same as in the view. That means that QQmlTreeModelToTableModel
// and QSortFilterProxyModel are in sync.
for (int row = 0; row < treeView->rows(); ++row) {
const auto index = treeView->modelIndex(0, row);
const QString modelDisplay = proxyModel.data(index, Qt::DisplayRole).toString();
const auto childFxItem = treeViewPrivate->loadedTableItem(QPoint(0, row));
QVERIFY(childFxItem);
const auto childItem = childFxItem->item;
QVERIFY(childItem);
const auto context = qmlContext(childItem.data());
const auto itemDisplay = context->contextProperty("display").toString();
QCOMPARE(itemDisplay, modelDisplay);
}
// Now change the text in one of the items. This will trigger
// a sort for only one of the parents in the model.
proxyModel.setData(treeView->modelIndex(0, row), u"xxx"_qs, Qt::DisplayRole);
for (int row = 0; row < treeView->rows(); ++row) {
const auto index = treeView->modelIndex(0, row);
const QString modelDisplay = proxyModel.data(index, Qt::DisplayRole).toString();
const auto childFxItem = treeViewPrivate->loadedTableItem(QPoint(0, row));
QVERIFY(childFxItem);
const auto childItem = childFxItem->item;
QVERIFY(childItem);
const auto context = qmlContext(childItem.data());
const auto itemDisplay = context->contextProperty("display").toString();
QCOMPARE(itemDisplay, modelDisplay);
}
}
QTEST_MAIN(tst_qquicktreeview)
#include "tst_qquicktreeview.moc"