Commit 983ea2ac authored by Agata Cacko's avatar Agata Cacko
Browse files

Speed up updateView() in tag selection widget

Before this commit, to get the tags for the resources,
Krita would, for every resource separately, set the resource id
to the filterproxy model and then iterate on that model to get
the tags. This is inefficient because the filter proxy model
iterates on all the items in the original model to get the
matching rows anyway.

This commit ensures that the model has all the resource ids
at once, so the iterating over the base model rows happens
only twice (one to filter out inside the proxy model,
one to iterate on the rows to build the tags list).

NOTE: Setting of the resource ids happens only
in setResourceIds() function, and updateView() assumes
it's already done.
This improves the performance even more (because every
setResourcesFilter() invalidates the filter inside the model,
which causes it to iterate and filter out incorrect rows again,
while having the filter already done means that adding or removing
on those resources will only add or remove some rows), but the
performance imrpovements are only on doing repeated actions on
the same set of resources.
It can potentially make it a bit more error prone, hence the
comment above the loop.

A possibility to make it still faster than before this commit,
but less error prone: just set resources filter just before the
loop, or set it to empty list and check for resource id being
in the selected list manually in updateView().

---
Credit for Grum999 for the idea and noticing the problem.
parent 5d4e48bd
......@@ -65,13 +65,11 @@ void KisWdgTagSelectionControllerOneResource::setResourceIds(QString resourceTyp
}
if (resourceIds.count() == 0) {
QList<KoID> list;
m_tagSelectionWidget->setTagList(m_editable, list, list);
QList<KoID> emptyList;
m_tagSelectionWidget->setTagList(m_editable, emptyList, emptyList);
m_tagSelectionWidget->setEnabled(false);
} else {
if (m_tagResourceModel) {
m_tagResourceModel->setResourcesFilter(resourceIds.toVector());
}
m_tagResourceModel->setResourcesFilter(m_resourceIds.toVector());
m_tagSelectionWidget->setEnabled(true);
updateView();
}
......@@ -130,6 +128,12 @@ void KisWdgTagSelectionControllerOneResource::slotCreateNewTag(QString tag)
void KisWdgTagSelectionControllerOneResource::updateView()
{
if (m_resourceIds.count() == 0) {
QList<KoID> emptyList;
m_tagSelectionWidget->setTagList(m_editable, emptyList, emptyList);
return;
}
QMap<QString, int> tagsCounts;
for (int i = 0; i < m_tagModel->rowCount(); i++) {
QModelIndex idx = m_tagModel->index(i, 0);
......@@ -143,13 +147,14 @@ void KisWdgTagSelectionControllerOneResource::updateView()
}
}
Q_FOREACH(int resourceId, m_resourceIds) {
m_tagResourceModel->setResourcesFilter(QVector<int>() << resourceId);
for (int i = 0; i < m_tagResourceModel->rowCount(); i++) {
QModelIndex idx = m_tagResourceModel->index(i, 0);
KisTagSP tag = m_tagResourceModel->data(idx, Qt::UserRole + KisAllTagResourceModel::Tag).value<KisTagSP>();
tagsCounts[tag->url()] += 1;
}
// IMPORTANT: this only works correctly because there was setResourcesFilter() called in setResourceIds() function
// if at any moment there is situation this needs to work without setResourceIds(),
// call m_tagResourceModel->setResourcesFilter(m_resourceIds.toVector()); before this loop
// (it will make it slightly slower since it invalides filter in the proxy model)
for (int i = 0; i < m_tagResourceModel->rowCount(); i++) {
QModelIndex idx = m_tagResourceModel->index(i, 0);
KisTagSP tag = m_tagResourceModel->data(idx, Qt::UserRole + KisAllTagResourceModel::Tag).value<KisTagSP>();
tagsCounts[tag->url()] += 1;
}
QList<KoID> semiSelected;
QList<KoID> selected;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment