Review Request: Use QPixmapCache for KStandardItemListWidget::pixmapForIcon

Frank Reininghaus frank78ac at googlemail.com
Thu Oct 25 17:38:11 BST 2012


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

Ship it!


Thanks Emmanuel, very nice work! It's great that you find so many places where optimizations are possible :-)

Feel free to push to master after considering the note below (I know that key collisions are unlikely, but if we can be *sure* that they won't happen, why not do it).


dolphin/src/kitemviews/kstandarditemlistwidget.cpp
<http://git.reviewboard.kde.org/r/107039/#comment16465>

    Just to be sure that we don't get accidental key collisons between "icon1" with size 25 and "icon12" with size 5, or even collisions with other possible future uses of QPixmapCache in Dolphin, I'd suggest that we use a key which is more or less guaranteed to be unique, something like
    "KStandardItemListWidget:" % name  % ":" % QString::number(size). Just like in Qt's example in http://doc.qt.digia.com/qq/qq12-qpixmapcache.html


- Frank Reininghaus


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/1ed86064/attachment.htm>


More information about the kfm-devel mailing list