Changeset View
Changeset View
Standalone View
Standalone View
src/kitemviews/kfileitemmodel.cpp
| Show First 20 Lines • Show All 1181 Lines • ▼ Show 20 Line(s) | 1180 | #ifdef KFILEITEMMODEL_DEBUG | |||
|---|---|---|---|---|---|
| 1182 | timer.start(); | 1182 | timer.start(); | ||
| 1183 | qCDebug(DolphinDebug) << "==========================================================="; | 1183 | qCDebug(DolphinDebug) << "==========================================================="; | ||
| 1184 | qCDebug(DolphinDebug) << "Inserting" << newItems.count() << "items"; | 1184 | qCDebug(DolphinDebug) << "Inserting" << newItems.count() << "items"; | ||
| 1185 | #endif | 1185 | #endif | ||
| 1186 | 1186 | | |||
| 1187 | m_groups.clear(); | 1187 | m_groups.clear(); | ||
| 1188 | prepareItemsForSorting(newItems); | 1188 | prepareItemsForSorting(newItems); | ||
| 1189 | 1189 | | |||
| 1190 | if (m_sortRole == NameRole && m_naturalSorting) { | 1190 | // Natural sorting of items can be very slow. However, it becomes much faster | ||
meven: We need to change also ine 1207 :
```
parallelMergeSort(newItems.begin(), newItems.end… | |||||
| 1191 | // Natural sorting of items can be very slow. However, it becomes much | 1191 | // if the input sequence is already mostly sorted. Therefore, we first sort | ||
| 1192 | // faster if the input sequence is already mostly sorted. Therefore, we | 1192 | // 'newItems' according to the QStrings using QString::operator<(), which is quite fast. | ||
| 1193 | // first sort 'newItems' according to the QStrings returned by | 1193 | if (m_naturalSorting) { | ||
| 1194 | // KFileItem::text() using QString::operator<(), which is quite fast. | 1194 | if (m_sortRole == NameRole) { | ||
| 1195 | parallelMergeSort(newItems.begin(), newItems.end(), nameLessThan, QThread::idealThreadCount()); | 1195 | parallelMergeSort(newItems.begin(), newItems.end(), nameLessThan, QThread::idealThreadCount()); | ||
| 1196 | } else if (isRoleValueNatural(m_sortRole)) { | ||||
| 1197 | auto lambdaLessThan = [&] (const KFileItemModel::ItemData* a, const KFileItemModel::ItemData* b) | ||||
meven: No line return between `}` and `else if`
| |||||
| 1198 | { | ||||
| 1199 | const QByteArray role = roleForType(m_sortRole); | ||||
| 1200 | return a->values.value(role).toString() < b->values.value(role).toString(); | ||||
| 1201 | }; | ||||
| 1202 | parallelMergeSort(newItems.begin(), newItems.end(), lambdaLessThan, QThread::idealThreadCount()); | ||||
| 1203 | } | ||||
| 1196 | } | 1204 | } | ||
| 1197 | 1205 | | |||
| 1198 | sort(newItems.begin(), newItems.end()); | 1206 | sort(newItems.begin(), newItems.end()); | ||
| 1199 | 1207 | | |||
| 1200 | #ifdef KFILEITEMMODEL_DEBUG | 1208 | #ifdef KFILEITEMMODEL_DEBUG | ||
| 1201 | qCDebug(DolphinDebug) << "[TIME] Sorting:" << timer.elapsed(); | 1209 | qCDebug(DolphinDebug) << "[TIME] Sorting:" << timer.elapsed(); | ||
| 1202 | #endif | 1210 | #endif | ||
| 1203 | 1211 | | |||
| ▲ Show 20 Lines • Show All 461 Lines • ▼ Show 20 Line(s) | 1671 | } else if (m_requestRole[TypeRole] && isDir) { | |||
| 1665 | data.insert(sharedValue("type"), folderMimeType); | 1673 | data.insert(sharedValue("type"), folderMimeType); | ||
| 1666 | } | 1674 | } | ||
| 1667 | 1675 | | |||
| 1668 | return data; | 1676 | return data; | ||
| 1669 | } | 1677 | } | ||
| 1670 | 1678 | | |||
| 1671 | bool KFileItemModel::lessThan(const ItemData* a, const ItemData* b, const QCollator& collator) const | 1679 | bool KFileItemModel::lessThan(const ItemData* a, const ItemData* b, const QCollator& collator) const | ||
| 1672 | { | 1680 | { | ||
| 1673 | int result = 0; | 1681 | int result = 0; | ||
meven: I wonder if it woulde be better to move this code directly in the lambda code. | |||||
| 1674 | 1682 | | |||
| 1675 | if (a->parent != b->parent) { | 1683 | if (a->parent != b->parent) { | ||
| 1676 | const int expansionLevelA = expandedParentsCount(a); | 1684 | const int expansionLevelA = expandedParentsCount(a); | ||
| 1677 | const int expansionLevelB = expandedParentsCount(b); | 1685 | const int expansionLevelB = expandedParentsCount(b); | ||
| 1678 | 1686 | | |||
| 1679 | // If b has a higher expansion level than a, check if a is a parent | 1687 | // If b has a higher expansion level than a, check if a is a parent | ||
| 1680 | // of b, and make sure that both expansion levels are equal otherwise. | 1688 | // of b, and make sure that both expansion levels are equal otherwise. | ||
| 1681 | for (int i = expansionLevelB; i > expansionLevelA; --i) { | 1689 | for (int i = expansionLevelB; i > expansionLevelA; --i) { | ||
| Show All 39 Lines | |||||
| 1721 | void KFileItemModel::sort(const QList<KFileItemModel::ItemData*>::iterator &begin, | 1729 | void KFileItemModel::sort(const QList<KFileItemModel::ItemData*>::iterator &begin, | ||
| 1722 | const QList<KFileItemModel::ItemData*>::iterator &end) const | 1730 | const QList<KFileItemModel::ItemData*>::iterator &end) const | ||
| 1723 | { | 1731 | { | ||
| 1724 | auto lambdaLessThan = [&] (const KFileItemModel::ItemData* a, const KFileItemModel::ItemData* b) | 1732 | auto lambdaLessThan = [&] (const KFileItemModel::ItemData* a, const KFileItemModel::ItemData* b) | ||
| 1725 | { | 1733 | { | ||
| 1726 | return lessThan(a, b, m_collator); | 1734 | return lessThan(a, b, m_collator); | ||
| 1727 | }; | 1735 | }; | ||
| 1728 | 1736 | | |||
| 1729 | if (m_sortRole == NameRole) { | 1737 | if (m_sortRole == NameRole || isRoleValueNatural(m_sortRole)) { | ||
| 1730 | // Sorting by name can be expensive, in particular if natural sorting is | 1738 | // Sorting by string can be expensive, in particular if natural sorting is | ||
| 1731 | // enabled. Use all CPU cores to speed up the sorting process. | 1739 | // enabled. Use all CPU cores to speed up the sorting process. | ||
| 1732 | static const int numberOfThreads = QThread::idealThreadCount(); | 1740 | static const int numberOfThreads = QThread::idealThreadCount(); | ||
| 1733 | parallelMergeSort(begin, end, lambdaLessThan, numberOfThreads); | 1741 | parallelMergeSort(begin, end, lambdaLessThan, numberOfThreads); | ||
| 1734 | } else { | 1742 | } else { | ||
| 1735 | // Sorting by other roles is quite fast. Use only one thread to prevent | 1743 | // Sorting by other roles is quite fast. Use only one thread to prevent | ||
| 1736 | // problems caused by non-reentrant comparison functions, see | 1744 | // problems caused by non-reentrant comparison functions, see | ||
| 1737 | // https://bugs.kde.org/show_bug.cgi?id=312679 | 1745 | // https://bugs.kde.org/show_bug.cgi?id=312679 | ||
| 1738 | mergeSort(begin, end, lambdaLessThan); | 1746 | mergeSort(begin, end, lambdaLessThan); | ||
| ▲ Show 20 Lines • Show All 91 Lines • ▼ Show 20 Line(s) | 1751 | { | |||
| 1830 | default: { | 1838 | default: { | ||
| 1831 | const QByteArray role = roleForType(m_sortRole); | 1839 | const QByteArray role = roleForType(m_sortRole); | ||
| 1832 | const QString roleValueA = a->values.value(role).toString(); | 1840 | const QString roleValueA = a->values.value(role).toString(); | ||
| 1833 | const QString roleValueB = b->values.value(role).toString(); | 1841 | const QString roleValueB = b->values.value(role).toString(); | ||
| 1834 | if (!roleValueA.isEmpty() && roleValueB.isEmpty()) { | 1842 | if (!roleValueA.isEmpty() && roleValueB.isEmpty()) { | ||
| 1835 | result = -1; | 1843 | result = -1; | ||
| 1836 | } else if (roleValueA.isEmpty() && !roleValueB.isEmpty()) { | 1844 | } else if (roleValueA.isEmpty() && !roleValueB.isEmpty()) { | ||
| 1837 | result = +1; | 1845 | result = +1; | ||
| 1846 | } else if (isRoleValueNatural(m_sortRole)) { | ||||
| 1847 | result = stringCompare(roleValueA, roleValueB); | ||||
| 1838 | } else { | 1848 | } else { | ||
| 1839 | result = QString::compare(roleValueA, roleValueB); | 1849 | result = QString::compare(roleValueA, roleValueB); | ||
| 1840 | } | 1850 | } | ||
| 1841 | break; | 1851 | break; | ||
| 1842 | } | 1852 | } | ||
| 1843 | 1853 | | |||
| 1844 | } | 1854 | } | ||
| 1845 | 1855 | | |||
Since this block is very close to the default case. Maybe we could just have (and remove this added multicase) : default: {
const QByteArray role = roleForType(m_sortRole);
const QString roleValueA = a->values.value(role).toString();
const QString roleValueB = b->values.value(role).toString();
if (!roleValueA.isEmpty() && roleValueB.isEmpty()) {
result = -1;
} else if (roleValueA.isEmpty() && !roleValueB.isEmpty()) {
result = +1;
} else {
result = stringCompare(roleValueA, roleValueB); // <- only line changed
}
break;
}meven: Since this block is very close to the default case.
Maybe we could just have (and remove this… | |||||
+1 Also, we should use the new isRoleValueNatural() function also here, instead of hardcoding again the list of roles. elvisangelaccio: +1
Also, we should use the new `isRoleValueNatural()` function also here, instead of… | |||||
| 1846 | if (result != 0) { | 1856 | if (result != 0) { | ||
| 1847 | // The current sort role was sufficient to define an order | 1857 | // The current sort role was sufficient to define an order | ||
| 1848 | return result; | 1858 | return result; | ||
| 1849 | } | 1859 | } | ||
| 1850 | 1860 | | |||
| 1851 | // Fallback #1: Compare the text of the items | 1861 | // Fallback #1: Compare the text of the items | ||
| 1852 | result = stringCompare(itemA.text(), itemB.text(), collator); | 1862 | result = stringCompare(itemA.text(), itemB.text(), collator); | ||
| 1853 | if (result != 0) { | 1863 | if (result != 0) { | ||
| ▲ Show 20 Lines • Show All 579 Lines • Show Last 20 Lines | |||||
We need to change also ine 1207 :
The nameLess function compare the name role of items, it needs to be adapted or replaced so that this the string comparison function called by parallelMergeSort takes into account the sorted Roles.