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

David Faure faure at kde.org
Wed May 19 14:26:30 BST 2010


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

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/kdeui/icons/kiconengine.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





More information about the kde-core-devel mailing list