Review Request 112580: Make preview loading faster when scrolling

Frank Reininghaus frank78ac at googlemail.com
Sat Sep 14 14:09:31 BST 2013



> On Sept. 9, 2013, 11:31 p.m., Christoph Feck wrote:
> > Works as advertised, merci :)
> > 
> > The only thing I would change is either the naming of the "Interval" constants to clarify their purpose, or move the deleted comment about them to the constants' declaration. This way, someone looking at the constants does not have to grep their usage to understand why there are two different intervals declared.

Thanks for the suggestion. I've added such a comment.

BTW, in Dolphin < 4.11, the delay of 300 ms which was always used after the initial folder loading probably made sense because each change of the visible items would trigger a sorting of *all* items into "visible items" and "invisible items" in KFileItemModelRolesUpdater, which can be quite expensive if the number of items is large. However, this does not apply any more because KFileItemModelRolesUpdater only loads icons/previews and other data for at most 500 items now.


- Frank


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112580/#review39688
-----------------------------------------------------------


On Sept. 14, 2013, 1:02 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112580/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2013, 1:02 p.m.)
> 
> 
> Review request for Dolphin and Christoph Feck.
> 
> 
> Description
> -------
> 
> 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.
> 
> As far as I can tell, the original idea was to have a delay of 300 ms only if something really expensive (like the re-generation of all previews) would be triggered. However, as Christoph found out, this rather long delay is also used while scrolling, see https://git.reviewboard.kde.org/r/112312/.
> 
> The code was a bit confusing - it used two different timers for "visible index range" changes and "icon size" changes, but both timers were assigned the same interval, which was also updated every time anything interesting happened.
> 
> I think that it gets much simpler (and preview generation more responsive) if both timers have different intervals, and the "long" interval is really only used if the icon size has changed.
> 
> Thanks to Christoph for analyzing this problem!
> 
> 
> This addresses bug 322093.
>     http://bugs.kde.org/show_bug.cgi?id=322093
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemlistview.h 49ff773 
>   dolphin/src/kitemviews/kfileitemlistview.cpp 2da238d 
> 
> Diff: http://git.reviewboard.kde.org/r/112580/diff/
> 
> 
> Testing
> -------
> 
> Previews are loaded much faster when scrolling through the view. I could not see any regressions yet.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130914/88defb6c/attachment.htm>


More information about the kfm-devel mailing list