-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124811/#review84027
-----------------------------------------------------------


Hi David,

This probably makes it faster (i have no doubt there), but i don't really 
follow how this works.
Could you perhaps explain what you're doing exactly here?

For instance, how can you determine if something is cached based on just an int 
increment?
What "would" make more sense to me is using a bool, for instance this:
KICONTHEMES_EXPORT bool kiconthemes_unittests_searchOnDisk = false;
which would be set to false upon entering the iconPath() function (to always 
make it false by default) and would be set to true when if 
(d->findCachedPixmapWithPath(key, Q_NULLPTR, path) && !path.isEmpty()) { ... } 
eveluates to true.

But then again, i could be missing the clue here :) (likely)

- Mark Gaiser


On aug 19, 2015, 7:25 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124811/
> -----------------------------------------------------------
> 
> (Updated aug 19, 2015, 7:25 a.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Feck and Volker Krause.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> -------
> 
> This makes QIcon::fromTheme() much faster, for all cases where we have loaded 
> the
> icon once before (including in another process).
> 
> 
> Diffs
> -----
> 
>   autotests/kiconloader_unittest.cpp 6b60e7ebfc970c94ae865d56c4e33a8034b4fcee 
>   src/kiconloader.cpp db3aa8f8fd6f8fb706edcb27cce073c36831934d 
> 
> Diff: https://git.reviewboard.kde.org/r/124811/diff/
> 
> 
> Testing
> -------
> 
> The unittest has a way to see if KIconLoader used the cache or searched on 
> disk, with the newly exported global int. I couldn't think of a better way.
> 
> I'll let Volker do the real-world testing and measure the overall impact as 
> he did previously.
> 
> 
> Thanks,
> 
> David Faure
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to