Review Request: Use QPixmapCache for KStandardItemListWidget::pixmapForIcon

[Michael Pyne]

> On Oct. 25, 2012, 9:05 p.m., Christoph Feck wrote:
> > Sorry, QPixmapCache is obsolete, please port to KImageCache.
> 
> Christoph Feck wrote:
>     Before you ask, here is what the API docs say:
>     
>     Deprecated:
>     KPixmapCache is susceptible to various non-trivial locking bugs and inefficiencies, and is supported for backward compatibility only (since it exposes a QDataStream API for subclasses). Users should port to KImageCache for a very close work-alike, or KSharedDataCache if they need more control.

In fairness, QPixmapCache is nowhere near the same as KPixmapCache. QPixmapCache is a thread-shared cache (i.e. different processes will have different QPixmapCaches). There are plausible scenarios where you might not need either the additional features or different semantics of KImageCache.

However, given that QPixmapCache has no means for namespacing it should not be your first choice for library code (e.g. if the library adds a pixmap called "icon_back", what happens if an application using that library also tries to add or use a pixmap called "icon_back"). If used in library code the key should be modified to include a namespace element e.g. key = QLatin1String("org.kde.KSILW.") + key. But doing this will make the cache itself slower as it has to fight with many different elements with a long common prefix (using a common suffix is probably OK but introduces a minor risk of collision again).


- Michael


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


On Oct. 25, 2012, 3:43 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107039/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2012, 3:43 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Use QPixmapCache for KStandardItemListWidget::pixmapForIcon(const QString& name, int size) -> Avoid KIcon loading and rescaleing
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kstandarditemlistwidget.cpp 72d10cf 
> 
> Diff: http://git.reviewboard.kde.org/r/107039/diff/
> 
> 
> Testing
> -------
> 
> According to valgrind/callgrind lesser cpu time and according to valgrind/massif lesser memory consume
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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