[Kde-pim] Review Request 114629: Separated referencing of collections from the selection in FavoriteCollectionsModel.

[Kevin Krammer]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114629/#review46760
-----------------------------------------------------------



akonadi/favoritecollectionsmodel.cpp
<https://git.reviewboard.kde.org/r/114629/#comment33382>

    if you put the model manipulation into, say, dereferenceHelper() then you could call it from here and from clearReferences(), avoiding the lookup overhead when clearing



akonadi/favoritecollectionsmodel.cpp
<https://git.reviewboard.kde.org/r/114629/#comment33381>

    Maybe use referencedCollections.find() in line 159 and use referencedCollections.erase() here?
    not sure how often this is called but that sounds like a simple optimization



akonadi/favoritecollectionsmodel.cpp
<https://git.reviewboard.kde.org/r/114629/#comment33384>

    is the order of references important? otherwise a QSet might be better, i.e. faster then linear lookups


- Kevin Krammer


On Dec. 23, 2013, 7:11 a.m., Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114629/
> -----------------------------------------------------------
> 
> (Updated Dec. 23, 2013, 7:11 a.m.)
> 
> 
> Review request for KDEPIM-Libraries.
> 
> 
> Bugs: 327865
>     http://bugs.kde.org/show_bug.cgi?id=327865
> 
> 
> Repository: kdepimlibs
> 
> 
> Description
> -------
> 
> Separated referencing of collections from the selection in FavoriteCollectionsModel.
> 
> The FavoritCollectionsModel used an internal selection model (that isn't used in any GUI),
>  to reuse the referencing of the Akonadi::SelectionProxyModel that corresponds to the current selection.
> However, the selection is frequently cleared and restored during
> layoutChanged signals, resulting in the collections frequently getting
> dereferenced and rereferenced.
> Since this happens for each favorite collection, this resultet in the
> complete buffer getting cleared on every layoutChanged result, essentially
> killing the buffering
> (if 10 favorite collections get dereferenced, each of them ends up in the
> buffer pushing other buffered collections out of the buffer).
> 
> Since a QSortProxyModel that filters dynamically translates each dataChanged
> signal into a layoutChanged, the buffer was more or less constantly cleared.
> 
> We could try to avoid clearing the selection on layoutChanged signals in KSelectionProxyModel,
> but not relying on the internal selection in the first place seem much more
> robust and straightforward. The FavoriteCollectionsModel remains a subclass
> of SelectionProxyModel for binary compatibility only, and should eventually be
> turned into a KRecursiveFilterModel.
> 
> 
> Diffs
> -----
> 
>   akonadi/favoritecollectionsmodel.h 17faff1275d500782fccd4ec52c8db5b9f467bd1 
>   akonadi/favoritecollectionsmodel.cpp 884a206088edad5d2699dd8570e3822323e89a27 
>   akonadi/tests/CMakeLists.txt 76d49ad903287c23b0db9097b09667571f759e98 
>   akonadi/tests/favoriteproxytest.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/114629/diff/
> 
> 
> Testing
> -------
> 
> Tried with kmail.
> 
> 
> Thanks,
> 
> Christian Mollekopf
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/