Review Request: Speedup KIconEngine::actualSize by not loading the actual icon.

[Peter Penz]

> On 2010-05-19 23:45:16, Fredrik Höglund wrote:
> > KFileItemDelegate used to use the nominal size (which is available in the style options passed to sizeHint()), but unfortunately it had to be changed to use the real size.
> > 
> > I don't remember the details, but the actual size can deviate quite a bit from the nominal size when previews are enabled.
> > 
> > sizeHint() is also not the only place where the actual size is used, it's also used during painting to center the icon in the item rect.

@David: I did some tests and Fredrik is right, it has some sideeffects when turning on image previews. However I think the actual size is not required for the time consuming path (only for the painting) and think this can be optimized in KFileItemDelegate. If it is OK for you, I'll have a look on this during the next week.


- Peter


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


On 2010-05-19 23:41:07, David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4052/
> -----------------------------------------------------------
> 
> (Updated 2010-05-19 23:41:07)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> KIconEngine::actualSize was loading the icon (from the cache, fortunately, but that still takes time), just to return its size. However the size is mostly mandated by the caller of KIconLoader::loadIcon...
> 
> Looking at the (complex) KIconLoader code I think there are indeed a few cases where the icon wouldn't have the requested size, but the question is whether it's a big deal for the users of actualSize()... In the case of file manager views, the user is KFileItemDelegate::Private::decorationSizeHint which is surely happier to reserve the same space for every icon, even if a few ones are only available as somewhat different sizes. I just wonder 1) how to test the case where we get a different size than the requested one, and 2) if this faster (but only correct 99% of the time, I suppose) actualSize() would break anything.
> 
> 
> This addresses bug 237668.
>     https://bugs.kde.org/show_bug.cgi?id=237668
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kio/kio/kfileitemdelegate.cpp 1128493 
> 
> Diff: http://reviewboard.kde.org/r/4052/diff
> 
> 
> Testing
> -------
> 
> konqueror /usr/bin
> (after the kio fix r1128475)
> This is much faster now than before. And to make sure actualSize() wasn't returning wrong results, I added asserts in pixmap() and paint().
> 
> Plus existing unittests and adding a rudimentary unit-test for actualSize.
> 
> 
> Thanks,
> 
> David
> 
>