[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: Speedup KIconEngine::actualSize by not loading the
From:       "Albert Astals Cid" <aacid () kde ! org>
Date:       2010-05-19 18:06:19
Message-ID: 20100519180619.4059.73062 () localhost
[Download RAW message or body]


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


Without having a look at the code i think it is somewhat a big deal actualSize() not \
returning the actual size of the icon, even if it is in some corner cases, can we fix \
the slowlyness this by adding a fastButNotReallActualSize() method?

- Albert


On 2010-05-19 13:26:30, David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4052/
> -----------------------------------------------------------
> 
> (Updated 2010-05-19 13:26:30)
> 
> 
> 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
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]