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

[David Faure]

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

(Updated 2010-05-19 23:41:07.875333)


Review request for kdelibs.


Changes
-------

OK, that's the same as fixing it one layer up, in KFileItemDelegate. This is indeed "less dangerous" because it only affects file views and not every single user of KIcon. What about this fix, then?


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 (updated)
-----

  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