Review Request 111382: Make sure that KItemListSizeHintResolver is always in a consistent state

[Emmanuel Pescosta]

> On July 4, 2013, 11:32 a.m., Emmanuel Pescosta wrote:
> > dolphin/src/kitemviews/kitemlistview.cpp, line 1549
> > <http://git.reviewboard.kde.org/r/111382/diff/2/?file=167956#file167956line1549>
> >
> >     Why not use m_sizeHintResolver->clearCache() here and change clearCache() to call m_sizeHintCache.clear()? 
> >     
> >     I think it would make it more obvious at first sight and removes the "if (!m_sizeHintCache.isEmpty() && m_itemListView->model()) {" in KItemListSizeHintResolver::itemsRemoved.
> 
> Frank Reininghaus wrote:
>     Because clearCache() does not actually remove the items from the cache. It just replaces their calculated size hints by QSizeF(), which indicates that a new size hint has to be calculated. Note that clearCache() is called by KItemListView every time anything that affects the item size changes.
>     
>     However, if the model changes, we really want to remove the size hints from the cache, and add a new QSizeF() for every item of the new model.

> Note that clearCache() is called by KItemListView every time anything that affects the item size changes.
Ok sry, I missed this.


- Emmanuel


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


On July 4, 2013, 12:21 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111382/
> -----------------------------------------------------------
> 
> (Updated July 4, 2013, 12:21 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> The other day, I found out what the real cause for bug 317827 is. The assert verifies that the layouter is "dirty", i.e., that no function that triggers a layouting has been called in the loop in KItemListView::slotItemsRemoved(const KItemRangeList&). This is done because the item ranges are removed step by step from KItemListSizeHintResolver, such that it is in an inconsistent state inside the loop, and any attempt to start a layout may yield incorrect results. However, if an animation is ongoing, calling "m_animation->stop(widget)" will trigger KItemListView::slotAnimationFinished(), which calls m_layouter->firstVisibleIndex(), which then triggers KItemListViewLayouter::doLayout().
> 
> I think the best way to fix this is to make sure that all item ranges are always inserted to and removed from KItemListSizeHintResolver at once. A nice side-effect is that there is one source less for O(N^2) behaviour if many item ranges are inserted/removed (but there are still more, which I will look at).
> 
> Moreover, this fixed some more inconsistencies in KItemListView::setModel(), which caused KItemListSizeHintResolver to have more items than actually needed .
> 
> 
> This addresses bug 317827.
>     http://bugs.kde.org/show_bug.cgi?id=317827
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistview.cpp b5e1058 
>   dolphin/src/kitemviews/private/kitemlistsizehintresolver.h 6f50d67 
>   dolphin/src/kitemviews/private/kitemlistsizehintresolver.cpp e075b46 
> 
> Diff: http://git.reviewboard.kde.org/r/111382/diff/
> 
> 
> Testing
> -------
> 
> Works. Note that the patch still contains some debug output that makes it easier to see if the items are moved and initialized correctly. I'll remove it before pushing the patch.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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