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, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 _______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
