Set up the elements collection's items on the GUI thread

ElementsCollectionModel::loadCollections() runs setUpData() for every
item on worker threads (QtConcurrent::map), and setUpData() calls
setText(), setFlags(), setData() and setToolTip() on items that are
already in the model. Each of these changes the model and emits its
dataChanged() signal from a worker thread, which QAbstractItemModel
does not allow. Loading the shipped collection (8838 elements) emitted
dataChanged() 38958 times, all from worker threads.

The expensive part (reading every element file) stays on the worker
threads. Only the result is moved: ElementCollectionItem::setData()
keeps a value set from a worker thread on the item, data() returns it
to the same worker so setUpData() still reads back what it has set,
and the model applies the kept values on the GUI thread when the map
is finished, before emitting loadingFinished(). setUpData() called on
the GUI thread (macros collection, a single added or changed element)
is unchanged.

With this change the same load emits dataChanged() 38958 times, all on
the GUI thread.

Revives the still-needed part of #516, closed only to clear a review
backlog. Its other two changes are left out: the wait in
loadMacrosCollection() guarded a model shared with a running map,
which no longer happens (the macros always get a model of their own),
and qetinformation.h's static QString constants are a size clean-up,
not a bug.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
ispyisail
2026-09-30 10:35:19 +13:00
parent bae336ef5b
commit 79e87cb3b8
3 changed files with 90 additions and 2 deletions
@@ -18,6 +18,9 @@
#include "elementcollectionitem.h"
#include <QCoreApplication>
#include <QThread>
/**
@brief ElementCollectionItem::ElementCollectionItem
Constructor
@@ -247,6 +250,73 @@ QList<ElementCollectionItem *> ElementCollectionItem::items() const
return list;
}
/**
@brief onGuiThread
@return true when called from the thread the application lives in
*/
static bool onGuiThread()
{
return QThread::currentThread() == QCoreApplication::instance()->thread();
}
/**
@brief ElementCollectionItem::setData
ElementsCollectionModel::loadCollections() runs setUpData() for every
item on worker threads (QtConcurrent::map), while the items are
already in the model. QStandardItem::setData() updates the model and
emits its dataChanged() signal, which is not safe from a worker
thread. So a value set from a worker thread is kept on the item, and
the model applies it on the GUI thread with applyDeferredData() once
every item is set up. setText(), setFlags(), setToolTip() and setIcon()
all end up here.
@param value
@param role
*/
void ElementCollectionItem::setData(const QVariant &value, int role)
{
if (role == Qt::EditRole)
role = Qt::DisplayRole;
if (onGuiThread())
QStandardItem::setData(value, role);
else
m_deferred_data.insert(role, value);
}
/**
@brief ElementCollectionItem::data
On a worker thread, a value set by setData() but not applied yet is
returned, so setUpData() reads back what it has just set
(localName() tests text() for example).
@param role
@return
*/
QVariant ElementCollectionItem::data(int role) const
{
if (role == Qt::EditRole)
role = Qt::DisplayRole;
if (!onGuiThread())
{
const auto it = m_deferred_data.constFind(role);
if (it != m_deferred_data.constEnd())
return it.value();
}
return QStandardItem::data(role);
}
/**
@brief ElementCollectionItem::applyDeferredData
Apply the values set from a worker thread, see setData().
Must be called on the GUI thread, after the worker is done.
*/
void ElementCollectionItem::applyDeferredData()
{
for (auto it = m_deferred_data.constBegin() ; it != m_deferred_data.constEnd() ; ++it)
QStandardItem::setData(it.value(), it.key());
m_deferred_data.clear();
}
void setUpData(ElementCollectionItem *eci) {
eci->setUpData();
}
@@ -18,6 +18,7 @@
#ifndef ELEMENTCOLLECTIONITEM2_H
#define ELEMENTCOLLECTIONITEM2_H
#include <QHash>
#include <QStandardItem>
/**
@@ -56,6 +57,16 @@ class ElementCollectionItem : public QStandardItem
QList<ElementCollectionItem *> elementsChild() const;
QList<ElementCollectionItem *> directoriesChild() const;
QList<ElementCollectionItem *> items() const;
QVariant data(int role = Qt::UserRole + 1) const override;
void setData(const QVariant &value, int role = Qt::UserRole + 1) override;
void applyDeferredData();
private:
/// Values set by setUpData() while it runs on a worker thread,
/// see setData(). Only ever touched by that one worker thread,
/// until applyDeferredData() empties it on the GUI thread.
QHash<int, QVariant> m_deferred_data;
};
void setUpData(ElementCollectionItem *eci);
@@ -312,8 +312,15 @@ void ElementsCollectionModel::loadCollections(bool common_collection,
this, &ElementsCollectionModel::loadingProgressValueChanged);
connect(watcher, &QFutureWatcher<void>::progressRangeChanged,
this, &ElementsCollectionModel::loadingProgressRangeChanged);
connect(watcher, &QFutureWatcher<void>::finished,
this, &ElementsCollectionModel::loadingFinished);
//setUpData() ran on worker threads, which only kept the values on
//the items (ElementCollectionItem::setData()): apply them here, on
//the GUI thread, before anyone is told the loading is finished.
connect(watcher, &QFutureWatcher<void>::finished, this, [this]()
{
for (ElementCollectionItem *eci : std::as_const(m_items_list_to_setUp))
eci->applyDeferredData();
emit loadingFinished();
});
connect(watcher, &QFutureWatcher<void>::finished, watcher, &QFutureWatcher<void>::deleteLater);