When opening the URL "man:", there are multiple items with the same
name (for example, _exit is shown twice here). When opening a new tab,
the kioslave reports some items as deleted (I have not quite understood
why). The problem is that it reports some of the duplicate items twice
in the list of deleted items. This confused KFileItemModel and
corrupted the internal data structures, and finally, caused a crash.
The fix is to remove all duplicates from
KItemRangeList::fromSortedContainer(const Container& container).
New unit tests included.
BUG: 335672
REVIEW: 118507
FIXED-IN: 4.13.2
of "m_selectionToggle->contains(pos)" in
KItemListWidget::setHoverPosition(const QPointF& pos).
This wrong check was accidentally introduced by
commit 24326d4473
BUG: 299328
REVIEW: 118320
FIXED-IN: 4.13.2
Before this patch, KDirLister would continue listing any children of
collapsed folders, even though the children themselves were removed
from the model. This could lead to new items being inserted as
top-level items at some later point, because no parent could be found
for them. This inconsistent model state could lead to a crash later on.
Many thanks to Martin Koller for helping to debug this problem!
BUG: 332102
REVIEW: 118055
FIXED-IN: 4.13.2
Before this patch, any (direct or indirect) children that might have
been in m_pendingItemsToInsert, i.e., that were not inserted into the
model yet because KDirLister had not finished listing the directory
yet, would be added to the model later without a proper parent. This
could cause a crash later on.
CCBUG: 332102
FIXED-IN: 4.13.2
If a range of items has been selected by Shift-clicking or by pressing
Shift+Arrow, then each of them is added individually to the selection
before the items are re-sorted.
Before this commit, the first and the last item in the range were
moved, and all items between them were selected, even though these were
not necessarily the items which had been selected before the
re-sorting.
BUG: 333457
REVIEW: 117603
FIXED-IN: 4.13.1
Instead of emitting the directoryLoadingCompleted signal (via slotCompleted) in
KFileItemModel::setShowHiddenFiles directly, we now call
dispatchPendingItemsToInsert instead.
@Frank: Thanks for your help!
FIXED-IN: 4.13.1
BUG: 332159
REVIEW: 117021
The icons of items which are "cut" are shown faded in the view. The
"is cut" state of the widget representing an item is updated if the
clipboard contents change. Before this commit, if the sort order was
changed however, then each widget kept the "is cut" state of the item
which had been shown previously at its position.
The solution is to update the "is cut" state also if any of the
widget's data change.
BUG: 332792
REVIEW: 117451
FIXED-IN: 4.12.5
Before this patch, the destructor of KDirectoryContentsCounter might
delete the worker object, which lives in another thread, while one of
its methods was still being executed. This could cause a crash. Only if
the destroyed KDirectoryContentsCounter was the last one, the worker
thread was stopped, and the destructor waited until all workers are
done.
BUG: 332767
FIXED-IN: 4.13.0
REVIEW: 117209
KItemListViewAnimation::slotFinished() used a QHashIterator to iterate
over a QHash, and then removes an item from the hash using
QHash::remove() inside the loop.
This is quite unusual - the recommended way is to use a
QMutableHashIterator (or std-style iterators and then QHash::erase(it)).
This might be related to the cause of a crash in this function.
BUG: 331876
REVIEW: 116666
FIXED-IN: 4.13.0
Before this patch, pressing one of these buttons while an item is
hovered selected this item. The motivation for this behavior was to
provide a fast way to select items. However, this was counter-intuitive
and confusing for many users.
BUG: 310288
FIXED-IN: 4.13.0
REVIEW: 116469
If the user sets a maximum number of text lines in the settings, this
number was translated into a maximum height in pixels using
QFontMetrics::lineSpacing() before this commit.
In KStandardItemListWidgetInformant::itemSizeHint(), this maximum height
limited the size that is reserved for the item.
However, in KStandardItemListWidget::updateIconsLayoutTextCache(), the
maximum height was translated back into a maximum number of lines,
which limits the number of lines that are created using the QTextLayout.
This approach could lead to problems if the real height of the layouted
text is 1 pixel more or less than QFontMetrics::lineSpacing() times
"number of lines".
Now we do not store a "maximum height" inside the "maximum size"
explicitly, but store a maximum number of lines and a maximum with (for
Compact View) separately, and then use the number of lines also to
calculate the required size in
KStandardItemListWidgetInformant::itemSizeHint(). This should make sure
that the correct height is reserved for each item.
Thanks to Christoph Feck and Emmanuel Pescosta for helping to debug this
problem and testing the patch.
BUG: 323841
FIXED-IN: 4.13
REVIEW: 113871
Also update the font of the meta data widget in InformationPanelContent (smallest readable font).
BUG: 329186
BUG: 315061
FIXED-IN: 4.13
REVIEW: 115958
KItemListViewLayouter uses a KItemListSizeHintResolver to find out how
much space the items will need in the view.
Before this commit, the size hint resolver object could be changed at
runtime, and it could also be null. However, we never made use of these
possibilities, so all the code that checks if m_sizeHintResolver is
null is actually not needed at all.
Moreover, clear the entire hash if items are added or removed.
This saves time and memory when loading a directory, and it fixes
problems that might occur if the model is in an inconsistent state, such
as crashes that can happen when we try to remove individual items from
m_items.
BUG: 329494
FIXED-IN: 4.13.0
REVIEW: 115432
The pointer to QThread object is stored in a global variable, and each
view increments/decrements a reference count when it starts/stops using
the thread. If this thread reaches zero, the thread is stopped.
Note that we cannot just use a smart pointer, like QSharedPointer, to
manage the thread, because we must make sure that the thread is not
running any more before the QThread is deleted.
REVIEW: 115064
The speed up is really small, but theses changes are mostly straightforward and make the code a bit nicer - break
the KStandardItemListWidgetInformant::itemSizeHint beast into three smaller functions.
FIXED-IN: 4.13
REVIEW: 112979
KFileItemModel allows to find out the index of a KFileItem with its
index(const KFileItem&) method. Calling this method is not extremely
expensive, but it's also not free (it looks up the URL of the KFileItem
in a QHash, i.e., it has to call qHash(QString) for the full URL).
In KFileItemModelRolesUpdater, we sometimes converted the known index to
a KFileItem and then back to an index in applyResolvedRoles(KFileItem).
This patch fixes this by modifying applyResolvedRoles such that it takes
the index directly as its argument.
REVIEW: 114847
It is not necessary to save the position of each item as a QPointF
because all items in a row will have the same y-coordinate, and all
items in a column will have the same x-coordinate. Therefore, we can
reduce the number of doubles that we store from
(number of items) * 2
to
(number of rows) + (number of colums)
which is at least 50% less.
REVIEW: 114460
The problem was that the loop in KFileItemModel::createMimeData() which
is supposed to find out if any parent of an item has been added to the
QMimeData already (because it is not necessary to add the item in that
case) did not loop through the parents of the item, but incorrectly
replaced the local variable 'itemData' by its parent.
BUG: 329119
REVIEW: 114562
This fixes the problem that the new file name is not shown in the view
if an item is renamed while it is filtered.
BUG: 329118
FIXED-IN: 4.12.1
REVIEW: 114459
Since Dolphin 4.11, we store not only KFileItems, but also the
corresponding ItemData struct for filtered items. This is required for
keeping track of the parent-child relationships, and has the nice side
effect that the ItemData need not be re-determined when the items are
shown again.
However, this can become a problem if the visible roles or the sort role
change while some items are filtered.
This is fixed by is fixed by clearing the QHash "values" for the
filtered items if the visible roles change. The hash will be
re-populated with all requested data as soon as the items are shown
again and the data(int) method of the model is called.
Moreover, before the items are inserted into the model after filtering,
we have to make sure that the sort role "Permissions"/"User"/etc. is
present in the hash "values". This is achieved by factoring out the code
that currently does this job for new items in createItemDataList() into
a new function, and calling this in insertItems(), because the same
treatment is required for the previously filtered files.
BUG: 328791
FIXED-IN: 4.12.1
REVIEW: 114266
Moreover, this commit ensures that the order of the URLs in the
QMimeData object is the same as the order of the items in the view.
Selecting many items and copying them to the clipboard could take quite
a bit of time. This is because we used
KDirModel::simplifiedUrlList(urls) to remove child items from the list
of URLs, and this function sorts the URLs internally to make it easier
to find out which of them are child URLs.
However, since commit 5c5d87fec4, the
selected indices are already stored in ascending order, and this makes
it easy to detect if an item is a child of the last item that has been
added to the QMimeData.
BUG: 283409
REVIEW: 113515
FIXED-IN: 4.13.0
The recent changes which prevent that all data for each item are saved
in a QHash already when loading the folder (see
https://git.reviewboard.kde.org/r/112725/), which save both memory and
time, do not work yet in Compact View, because
KItemListWidgetInformant::itemSizeHint() calls the model's data(int)
method for every item, which then initializes the hash.
This patch prevents that by accessing the file name directly if only
the "Name" is shown in the view, just like it's done in Icons View.
REVIEW: 113849
When resizing the window and when KItemListContainer::updateGeometries
is called before the scrollbar visibility is updated, a relayout is
triggered in `m_controller->view()->setGeometry` which updates the
scrollbar visibility and calls back to
`KItemListContainer::updateGeometries` again. Since the first call,
which has the wrong geometry (due to the incorrect scrollbar states),
updates the geometries of the scene and viewport after the second call
(which has the right geometry!!) returns, the final result is a size
that corresponded to the old scrollbar state before this commit.
This patch uses the new geometry of the view after updating it (since
it might not be the size we put in) and therefore makes the sizes
consistent.
BUG: 327709
FIXED-IN: 4.11.4
REVIEW: 113939
Before this commit, Dolphin reserved space for the scrollbar spacing
even when no scrollbar is visible resulting in a ugly gap in the view
when:
1. the theme uses QStyle::SH_ScrollView_FrameOnlyAroundContents and
2. the theme has a positive PM_ScrollView_ScrollBarSpacing.
QtCurve can have both while Oxygen have 1 but not 2.
To reproduce the problem with Oxygen style. Replace the
`width += ....` (which returns -2 or 0 for Oxygen) with `width += 2`.
See more info here:
https://github.com/QtCurve/qtcurve-qt4/issues/9#issuecomment-28630517
CCBUG: 306631
FIXED-IN: 4.11.4
REVIEW: 113902
If we detect that the user pressed the back/forward buttons while
hovering the empty space of the view, such that
DolphinView::slotMouseButtonPressed(int, Qt::MouseButtons) will request
that Dolphin navigates back/forward in the history, handling the mouse
press event should stop. This prevents the possible unexpected selection
of items in the new directory.
BUG: 327412
FIXED-IN: 4.11.4
This is a follow-up to commit 0e9f4a3987,
which tries to speed up sorting the items naturally by their name using
the idea that a fast non-natural pre-sorting already sorts the items
mostly correctly and thus reduces the number of expensive natural
comparisons.
This change only makes sense if the view is really sorted by "Name". In
other cases, the pre-sorting will most likely not be useful.
Thanks to Christoph Feck for pointing this out!
Since Dolphin 2.0, we have stored the selected items in a QSet<int>,
which is neither space-efficient nor particularly fast when inserting
many items which are in a consecutive range.
This commit replaces the QSet<int> by a new class "KItemSet", which
stores the items in a sorted list of ranges. For each range, we only
store the first index and the length of the range, so we need a lot
less memory for most common selection patterns, and we also save quite
a few CPU cycles in many situations, because adding an item to the
KItemSet will in many cases not need a memory allocation at all, and
it's particularly easy when inserting sorted items into the KItemSet in
a row.
KItemSet contains a minimal subset of QSet's API which makes it
suitable as a drop-in replacement for our needs. It also has iterators,
such that the items can be iterated through easily, also with foreach.
One advantage of KItemSet compared to QSet<int> is that the items are
always iterated through in ascending order.
REVIEW: 113488
Sort the items in a folder first according to their name, without doing
a natural/locale-aware sorting. This is very fast, but the order of the
items is then already close to the final order in most cases.
The number of expensive natural comparisons required to sort the items
is thus greatly reduced.
In my experiments with a folder with 100,000 items, the time required
to sort the files was reduced by 63% with this patch.
REVIEW: 113485
In KItemListViewLayouter, we have always stored a QRectF for each item,
which is "the area that the item occupies". However, the size of the
QRectF is already stored in the size hint resolver.
Therefore, it is sufficient to store the position of the top left
corner of the QRectF in a QPointF and construct the QRectF on demand.
This patch reduces the memory usage by 16 bytes for each item in the
view:
* a QRectF is 4 doubles -> 32 byes
* a QPointF contains only 2 doubles -> 16 bytes
REVIEW: 113487
This patch actually does two things when collapsing an expanded folder
with expanded children:
(a) It removes all expanded children (which are removed from the model)
from m_expandedDirs.
(b) It remembers the expanded children and restores them if the
top-level folder is re-expanded.
BUG: 304363
FIXED-IN: 4.12.0
REVIEW: 113293
1. Remove the unneeded variable rowCount.
2. Simplify the calculation of the member m_maximumScrollOffset. We can
just use the current value of "y" because this is the offset that
the next row would have.
REVIEW: 113233
When we remove items from the model, we called the function
KFileItemModel::removeItems(const KFileItemList&, RemoveItemsBehavior).
This function then looked up the indexes of the items using the hash
m_items. This is wasteful in the situations when the indexes of the
removed items are known in advance (like when an expanded folder is
collapsed in Details View), and it can cause trouble if one item is
contained in the model multiple times (can happen when searching, and a
file both matches the search and is a child of a folder that matches
the search). Even if expanding folders in the search results list might
not be particularly useful most of the time, it makes sense to make the
model more robust to prevent crashes and other unexpected behavior in
such situations.
This patch makes the following changes to achieve that goal:
* Change the argument of removeItems() from KFileItemList to
KItemRangeList. To make this work, the "look the indexes up in
m_items" code is moved from that function to slotItemsDeleted(). In
the other places where removeItems() is called, the indexes are
calculated directly (which is not more difficult than determining the
removed items as a KFileItemList, if one considers that we needed the
function childItems(KFileItem) for that, which is not needed any more
with this patch).
* Also removeFilteredChildren() takes a KItemRangeList now. Rather than
putting the parent KFileItems into a QSet for O(1) lookup (which
prevents O(N^2) worst case behavior for the entire function), it uses
a QSet<ItemData*> now, which should even be more efficient (hashing a
pointer is cheaper than hashing a KFileItem/KUrl).
BUG: 324371
BUG: 325359
FIXED-IN: 4.12.0
REVIEW: 113070
Before this commit, we only added pressed keys to the search string if
they have no other meaning. This means that files containing a Space in
their name could not be searched because Ctrl+Space toggles the
selection state of the current item, and Space alone selects the
current item.
After this commit, Space is added to the search string if
(a) the key press did not have any other effect, i.e., if Ctrl was not
pressed, and the current item is selected already, and
(b) a keyboard search has been started already (to prevent unexpected
effects when pressing Space accidentally - I think that it's rather
uncommon to have files whose names start with a Space - and to make
the unit test simpler).
I modified the unit test of KItemListController, which did not test
keyboard search yet. This uncovered a small problem in
KItemListController::slotChangeCurrentItem() when NoSelection mode is
used. It's not really relevant for anything that is executed inside
Dolphin, but I still fixed it to make the unit test happy.
BUG: 324479
FIXED-IN: 4.11.3
REVIEW: 113071
To reduce unnecessary memory comsumption and CPU usage, we only fill the
QHash<QByteArray, QVariant> if the methods data(int) or setData(int) are
called for the corresponding index, or the data is necessary for sorting
the model.
According to my tests, this patch reduces the memory usage when loading
a folder with 100,000 items by 17% in Icons View, and by 26% in Details
View.
REVIEW: 112725
