From b40c0de099fc951dbe3423b99310e7c7f799c8c4 Mon Sep 17 00:00:00 2001 From: Eike Hein Date: Wed, 8 Nov 2017 23:08:42 +0900 Subject: [PATCH] When used by QML, only populate after component is complete Summary: This introduces the use of QQmlParserStatus to delay populating the model until all properties have been set, to avoid delegate churn. TasksModel is also meant to be used by C++. There's no good way to determine whether an object is being instanciated by QML during construction time, therefore this patch also introduces a delay in initial population of the model after construction via a single-shot timer. At the time the slot is invoked we know if we're used by QML (because QQmlParserStatus::classBegin has either been called or not by then) so we can decide to populate or wait more for QQmlParserStatus::componentComplete. I'm not super happy with this behavior change for C++ users, however as the model is usually used via QML currently, it's pragmatic to optimize performance for the common case, and it doesn't technically break QAbstractItemModel semantics, as model population isn't required to be sync. There's a decent change this fixes a recently-reported crash as a by-product: CCBUG:386630 Reviewers: #plasma, davidedmundson, mart Subscribers: plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D8723 --- libtaskmanager/tasksmodel.cpp | 67 ++++++++++++++++++++++++++++------- libtaskmanager/tasksmodel.h | 7 +++- 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/libtaskmanager/tasksmodel.cpp b/libtaskmanager/tasksmodel.cpp index ba10a98e2..8be6ffb35 100644 --- a/libtaskmanager/tasksmodel.cpp +++ b/libtaskmanager/tasksmodel.cpp @@ -80,6 +80,9 @@ public: bool groupInline = false; int groupingWindowTasksThreshold = -1; + bool usedByQml = false; + bool componentComplete = false; + void initModels(); void initLauncherTasksModel(); void updateAnyTaskDemandsAttention(); @@ -438,17 +441,6 @@ void TasksModel::Private::initModels() QObject::connect(groupingProxyModel, &QAbstractItemModel::modelReset, q, [this]() { updateAnyTaskDemandsAttention(); } ); - - abstractTasksSourceModel = groupingProxyModel; - q->setSourceModel(groupingProxyModel); - - QObject::connect(q, &QAbstractItemModel::rowsInserted, q, &TasksModel::updateLauncherCount); - QObject::connect(q, &QAbstractItemModel::rowsRemoved, q, &TasksModel::updateLauncherCount); - QObject::connect(q, &QAbstractItemModel::modelReset, q, &TasksModel::updateLauncherCount); - - QObject::connect(q, &QAbstractItemModel::rowsInserted, q, &TasksModel::countChanged); - QObject::connect(q, &QAbstractItemModel::rowsRemoved, q, &TasksModel::countChanged); - QObject::connect(q, &QAbstractItemModel::modelReset, q, &TasksModel::countChanged); } void TasksModel::Private::updateAnyTaskDemandsAttention() @@ -637,6 +629,12 @@ void TasksModel::Private::consolidateManualSortMapForGroup(const QModelIndex &gr void TasksModel::Private::updateGroupInline() { + if (usedByQml && !componentComplete) { + return; + } + + bool hadSourceModel = (q->sourceModel() != nullptr); + if (q->groupMode() != GroupDisabled && groupInline) { if (flattenGroupsProxyModel) { return; @@ -661,7 +659,7 @@ void TasksModel::Private::updateGroupInline() forceResort(); } } else { - if (!flattenGroupsProxyModel) { + if (hadSourceModel && !flattenGroupsProxyModel) { return; } @@ -674,10 +672,28 @@ void TasksModel::Private::updateGroupInline() delete flattenGroupsProxyModel; flattenGroupsProxyModel = nullptr; - if (sortMode == SortManual) { + if (hadSourceModel && sortMode == SortManual) { forceResort(); } } + + // Minor optimization: We only make these connections after we populate for + // the first time to avoid some churn. + if (!hadSourceModel) { + QObject::connect(q, &QAbstractItemModel::rowsInserted, q, + &TasksModel::updateLauncherCount, Qt::UniqueConnection); + QObject::connect(q, &QAbstractItemModel::rowsRemoved, q, + &TasksModel::updateLauncherCount, Qt::UniqueConnection); + QObject::connect(q, &QAbstractItemModel::modelReset, q, + &TasksModel::updateLauncherCount, Qt::UniqueConnection); + + QObject::connect(q, &QAbstractItemModel::rowsInserted, q, + &TasksModel::countChanged, Qt::UniqueConnection); + QObject::connect(q, &QAbstractItemModel::rowsRemoved, q, + &TasksModel::countChanged, Qt::UniqueConnection); + QObject::connect(q, &QAbstractItemModel::modelReset, q, + &TasksModel::countChanged, Qt::UniqueConnection); + } } QModelIndex TasksModel::Private::preFilterIndex(const QModelIndex &sourceIndex) const { @@ -893,6 +909,18 @@ TasksModel::TasksModel(QObject *parent) // Start sorting. sort(0); + + // Private::updateGroupInline() sets our source model, populating the model. We + // delay running this until the QML runtime had a chance to call our implementation + // of QQmlParserStatus::classBegin(), setting Private::usedByQml to true. If used + // by QML, Private::updateGroupInline() will abort if the component is not yet + // complete, instead getting called through QQmlParserStatus::componentComplete() + // only after all properties have been set. This avoids delegate churn in Qt Quick + // views using the model. If not used by QML, Private::updateGroupInline() will run + // directly. + QTimer::singleShot(0, this, [this]() { + d->updateGroupInline(); + }); } TasksModel::~TasksModel() @@ -1684,6 +1712,19 @@ QModelIndex TasksModel::makeModelIndex(int row, int childRow) const return QModelIndex(); } +void TasksModel::classBegin() +{ + d->usedByQml = true; +} + +void TasksModel::componentComplete() +{ + d->componentComplete = true; + + // Sets our source model, populating the model. + d->updateGroupInline(); +} + bool TasksModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const { // All our filtering occurs at the top-level; anything below always diff --git a/libtaskmanager/tasksmodel.h b/libtaskmanager/tasksmodel.h index 1ad4c5583..38319762d 100644 --- a/libtaskmanager/tasksmodel.h +++ b/libtaskmanager/tasksmodel.h @@ -21,6 +21,7 @@ License along with this library. If not, see . #ifndef TASKSMODEL_H #define TASKSMODEL_H +#include #include #include "abstracttasksmodeliface.h" @@ -53,9 +54,10 @@ namespace TaskManager * @author Eike Hein **/ -class TASKMANAGER_EXPORT TasksModel : public QSortFilterProxyModel, public AbstractTasksModelIface +class TASKMANAGER_EXPORT TasksModel : public QSortFilterProxyModel, public AbstractTasksModelIface, public QQmlParserStatus { Q_OBJECT + Q_INTERFACES(QQmlParserStatus) Q_PROPERTY(int count READ rowCount NOTIFY countChanged) Q_PROPERTY(int launcherCount READ launcherCount NOTIFY launcherCountChanged) @@ -809,6 +811,9 @@ public: */ Q_INVOKABLE QModelIndex makeModelIndex(int row, int childRow = -1) const; + void classBegin() override; + void componentComplete() override; + Q_SIGNALS: void countChanged() const; void launcherCountChanged() const;