QIcon::fromTheme(xxx, someFallback)

[David Faure]

On Thursday 10 September 2015 19:29:23 Michael Pyne wrote:
> On Thu, September 10, 2015 09:16:11 David Faure wrote:
> > On Wednesday 09 September 2015 10:24:23 Olivier Goffart wrote:
> > > 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.
> 
> > One solution would be a "change counter" in the icon engine, incremented
> > when changing themes. In QIcon, comparing a local number with that change
> > counter, and skipping the cache if they don't match. But of course this
> > means one more int per icon, so I don't know if it's a good idea.
> 
> It would only need a static int for QIcon's theme engine as a whole, no? In 
> this case, for the "is null" cache.

You're assuming a cache outside QIcon, while Olivier said he was caching the isNull
value in QIconPrivate. So to know if that cache is valid at a given point in time, you also
need an int in QIconPrivate, to compare that with the int in the icon engine.

I think you're assuming a separate "isnull cache" while in his case, it's inside the icon.

Separate is a solution of course... QHash<QString, bool> ?
hash[key]  == QIcon::fromTheme(key).isNull() 
That's of course easier to wipe out when switching icon themes.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5