Review Request: Implement a cache pixmap for avatars

[David Edmundson]

> On July 22, 2011, 3:01 p.m., David Edmundson wrote:
> > I don't think your premise for doing this isn't entirely true.
> > When you load a QPixmap from a file, it is automatically added to an implicit QPixmapCache
> > 
> > Relevant:
> > http://doc.trolltech.com/4.7/qpixmap.html#load
> 
> Dario Freddi wrote:
>     True, although this was not the case, as pixmaps were loaded straight from a QImage, hence not allowing the caching part to happen. Switching to QPixmap::load all the calls might as well work for this specific case, such an approach would let us cache also custom-painted pixmaps in the future. If we're not planning to do that, simply using ::load would work.

Just checked again, why on earth does the old code do that? You're right, that it's totally wrong and weird.

I'm not sure I'm comfortable to accept this code into the 0.1 release, as it's quite a large change.

After that fine, in fact it will be good to have this act as a QDeclarativeImageProvider for some situations too, which from the looks of it would be easy to do from this.

Discuss on IRC?


- David


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


On July 22, 2011, 2:42 p.m., Dario Freddi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102050/
> -----------------------------------------------------------
> 
> (Updated July 22, 2011, 2:42 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> At the moment, each time an avatar is requested, a new pixmap from the image file is created. This is of course more than evil, and this patch aims to fix that. PixmapCache is a proxy to QPixmapCache for handling avatars' pixmaps efficiently. If this patch gets in, from now on every avatar consumer (tooltips, list view, etc) should get his avatar pixmaps through the cache. Of course, in the future, this will be extended to support the most common and most unefficiently handled pixmaps in our code.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt de2827f 
>   contact-delegate-compact.cpp 45cd282 
>   contact-delegate.cpp 159894f 
>   pixmap-cache.h PRE-CREATION 
>   pixmap-cache.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102050/diff
> 
> 
> Testing
> -------
> 
> Apparently everything still works
> 
> 
> Thanks,
> 
> Dario
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110722/2a67c6c8/attachment.htm