Bug 322093 - Previews stop being generated while scrolling
Summary: Previews stop being generated while scrolling
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: general (show other bugs)
Version: 4.10.90
Platform: Compiled Sources Linux
: NOR minor
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-08 01:16 UTC by Christoph Feck
Modified: 2013-09-14 13:02 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 4.11.2


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Feck 2013-07-08 01:16:40 UTC
To reproduce:
- clear .thumbnails cache
- visit a folder with many images and quickly grab the scroll bar
- while it generates previews, scroll down slowly

Actual results:
- while you scroll, no previews are generated
- if you release the scroll bar, or stop moving, it waits about 500 ms before resuming with the tumbnail generation

Expected results:
- it tries to generate thumbnails for visible icons as fast as possible

The second issue mentioned above is also true when paging through a list of files (mouse of keyboard). A very noticable delay is visible before it resumes generating thumbnails for the visible portion of the view.
Comment 1 Frank Reininghaus 2013-07-08 09:04:33 UTC
Thanks for the report, Christoph.

The delay that you observe is caused by the following two issues (the first one, and also the second one in slightly different form exist at least since 4.8, BTW, and maybe even longer because KFilePreviewGenerator in kdelibs, which was also written by Peter, shares some design considerations with KFileItemModelRolesUpdater):

(a) When the scroll offset changes, KFileItemListView does not notify KFileItemModelRolesUpdater (the class which generates previews and does a few more things) immediately. It rather "pauses" KFileItemModelRolesUpdater and starts a 50 ms timer, which triggers the preview generation (look for m_updateVisibleIndexRangeTimer in dolphin/src/kitemviews/kfileitemlistview.cpp). I did not write that code myself, but I think that the reason for this is that scrolling should always feel smooth, and that KFileItemModelRolesUpdater should therefore not be allowed to block the GUI thread while the user is scrolling.

(b) When the update is triggered in KFileItemModelRolesUpdater, it first tries to determine the "final" icons for the visible items, and then tries to load at least "fast icons" (without full mime type determination) for the invisible ones to reduce the risk that the user sees "unknown" icons when scrolling. It blocks the GUI thread for up to 200 ms for these things. After that, the preview job is started.

[There might be other factors that cause delays, but finding out about this requires some careful profiling, or at least adding debug output and QElapsedTimers to the relevant functions. I don't have the time to do that at the moment, but I'll try to have a look.]

I don't think we can do much about issue (a). We could reduce the 50 ms delay slightly, but if we remove it completely, it probably won't take long until we get "scrolling feels sluggish while icons/previews are being loaded" bug reports.

However, we might be able to reduce the delay caused by (b). Actually, my patch

https://git.reviewboard.kde.org/r/111396/

removes the "load invisible icons fast" thing completely, and rather loads these icons on demand after scrolling the view to an area which still has "unknown" icons. With David's recent changes, KFileItem::iconName() should be fast enough that this works without making scrolling feel slow. With that patch, KFileItemModelRolesUpdater won't  block the GUI thread for 200 ms any more, at least not if determining the visible icons works fast enough. Another benefit of that patch is that there will never be "unknown" icons any more, no matter how many files there are and how fast the user scrolls to the bottom.

However, at this point of the release cycle, I'd prefer if someone != Frank confirms that this patch does not cause any serious problems. Maybe you could test it and check if it at least makes the problem that you are observing a bit smaller?

If you have further ideas how we could improve the situation, please let us know! The obvious solution "move MIME type determination/preview job handling to another thread" might be a bit dangerous. AFAIK, nothing in KFileItem is thread-safe, so this could cause all sorts of subtle and not-so-subtle bugs.
Comment 2 Christoph Feck 2013-08-27 09:57:07 UTC
> If you have further ideas how we could improve the situation, please let us know!

One idea would be to handle the case, where you hit PgDn, or clicked into the free space of the scrollbar to switch to the next page. In these cases, it maybe could use a lower timeout before showing any new thumbnails. The chance for the scroll offset changing again in a very short time is relatively small in these cases.

See also http://qt-project.org/doc/qt-4.8/qabstractslider.html#sliderDown-prop

I will check if I can find the place where this could be added :)
Comment 3 Christoph Feck 2013-08-27 14:03:59 UTC
> When the scroll offset changes, KFileItemListView [...] starts a 50 ms timer

It also uses a value of 300 ms (LongInterval) as explained in the comment at line 388:

    // The ShortInterval is used for cases like switching the directory: If the
    // model is empty and filled later the creation of the previews should be done
    // as soon as possible. The LongInterval is used when the model already contains
    // items and assures that operations like zooming don't result in too many temporary
    // recreations of the previews.

    const int interval = (model()->count() <= 0) ? ShortInterval : LongInterval;

Since during scrolling no temporary previews are generated (they are "final", unlike with zooming), the check above looks wrong.

I temporarily changed the code line to always use ShortInterval, and now paging in a view, where previews are currently being generated is much more responsive.

I guess the code should be changed to always use the ShortInterval for the updateVisibleIndexRange() case, but keep the LongInterval for the updateIconSize() case, so that the code matches the intention described at the comment.
Comment 4 Frank Reininghaus 2013-09-14 13:02:54 UTC
Git commit bf2a0d6957d0d60fe449b5445ebfd69d27c1b4e7 by Frank Reininghaus.
Committed on 14/09/2013 at 12:52.
Pushed by freininghaus into branch 'KDE/4.11'.

Make preview loading faster when scrolling

KFileItemListView notifies KFileItemModelRolesUpdater of changes of the
visible index range and the icon size with a delay, to prevent that
expensive operations are triggered repeatedly, and that scrolling feels
sluggish because the GUI thread is blocked by icon loading.

This patch ensures that the "long" delay of 300 ms is only used when
the zoom level is changed, and the "short" delay if only the visible
index range has changed.

Thanks to Christoph Feck for helping to analyze this problem!
FIXED-IN: 4.11.2
REVIEW: 112580

M  +25   -40   dolphin/src/kitemviews/kfileitemlistview.cpp
M  +0    -2    dolphin/src/kitemviews/kfileitemlistview.h

http://commits.kde.org/kde-baseapps/bf2a0d6957d0d60fe449b5445ebfd69d27c1b4e7