QIcon::fromTheme(xxx, someFallback)

[Olivier Goffart]

On Tuesday 8. September 2015 10:26:20 David Faure wrote:
> On Monday 07 September 2015 15:53:31 Olivier Goffart wrote:
> > But the problem is that QIcon::isNull is likely to be called anyway.
> > And this will again do all the look ups in the file system.
> 
> I don't think so. That's the whole reasoning behind this change.
> 
> I added debug output in QIcon and ran konqueror-kf5 (details below).
> 
> Result: QIcon::fromTheme is called 84 times, for 66 different icon names.
> Among those, 56 do NOT lead to an isNull call.
> 
> I think for icons for QActions in menus aren't loaded (or even checked with
> isNull) until opening the menu. So this really helps speeding up application
> startup.

Right, but we still need to do a lot of stats for the 10 icons left.

Hence the idea to use the GTK+ cache from within Qt.
https://codereview.qt-project.org/125379/

I'll let Volker do the benchmark.


> On the other hand, isNull() can obviously be called multiple times for
> a given icon, so of course we shouldn't make it slower, it should have the
> answer at hand immediately.

I tried to optimize it by 'caching' the isNull value in QIconPrivate.

But then the test failed:
http://code.woboq.org/qt5/qtbase/tests/auto/gui/image/qicon/tst_qicon.cpp.html#633
In that test, the "address-book-new" was looked before and cached, and then we 
expect that looking it up again after changing the theme name changes.
So cahcing the value of isNull would make the test fail, because it would be 
cached as not null.  Yes, the behaviour is that when changing the themeName, 
existing QIcon will be re-looked-up next time we try to render them.


Anyway, I hope that calling a virtual function is not too much overhead and 
that the fact that QIcon::isNull is now slightly slower is not a problem 
compared to the benefit we have to lookup icons lazily.

-- 
Olivier