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

[Emmanuel Pescosta]

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



dolphin/src/kitemviews/kitemlistview.cpp
<http://git.reviewboard.kde.org/r/111382/#comment26117>

    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.



dolphin/src/kitemviews/private/kitemlistsizehintresolver.cpp
<http://git.reviewboard.kde.org/r/111382/#comment26118>

    Maybe replace it with "m_sizeHintCache.insert(m_sizeHintCache.end(), insertedCount, QSizeF());"?


- Emmanuel Pescosta


On July 3, 2013, 7:47 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111382/
> -----------------------------------------------------------
> 
> (Updated July 3, 2013, 7:47 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/082497d3/attachment.htm>