QIcon::fromTheme(xxx, someFallback)

[David Faure]

On Sunday 23 August 2015 10:30:13 Olivier Goffart wrote:
> On Friday 21. August 2015 22:30:37 David Faure wrote:
> 
> 
> > > I would also approve a change to Qt to add an overload of QIcon::fromTheme
> > > with only one argument without a fallback. (this would be binary
> > > compatible, and source compatible unless someone took the address of
> > > QIcon::fromTheme)
> 
> > I thought about that, but that would change behavior of existing code, no?
> > 
> > Currently: QIcon::fromTheme("foo").isNull() is true if "foo" isn't found.
> > If we add a one-arg overload: QIcon::fromTheme("foo").isNull() would be
> > false, and QIcon::fromTheme("foo", QIcon()).isNull() would be true. The old
> > behavior (for these rare apps that care) would require adding an explicit
> > null-icon second arg. I wish this would have been done this way from the
> > start, but we can't make that change now.
> 
> Ah ok, i first tought that the problem was the call to availableSizes.
> But other problem is also the fact that we want to return a null icon if not 
> found.

It's the same problem, the QIcon code calls availableSizes in order to determine
if it should return the (null by default) fallback ;)
 
> Fo this mean we could change QIcon::isNull fo be something like
> 
> if (!d) return true;
> if (d->notNull) return false;
> bool null;
> d->virtualHook(CheckNull, &null);
> if (null) {
> 	const_cast(this)->operator=(QIcon())
> } else {
>    d->notNull = true;
> }
> return null;
> 
> 
> So than isNull is also lazy, and the problem is solved... is it not?

Ah!! Yes, that would work. Can you make the change in Qt, since you know that code best?
 
> > Well, there is one solution: QIcon("foo"), the existing fileName ctor,
> > if we add the fact that "if not found in the current dir then the icon
> > will be looked up in the theme". That means one stat() per QIcon,
> > but that's nothing compare to the current QIcon::fromTheme().
> 
> That could be a solution.
>  
> > And from the documentation, it looks like QIcon(QString) already
> > behaves the optimal way, i.e. you don't get isNull() if the file doesn't
> > exist, because "The file will be loaded on demand." right?
> > That's exactly what I want for icons loaded from a theme, and
> > which QIcon::fromTheme() doesn't allow implementing, due
> > to its requirement to immediately find out if a null icon
> > (or a specific fallback) should be returned instead.
> 
> And I think this behaviour is also confusing.
 
Not sure what you meant here. QIcon("file") should be null if
./file doesn't exist?

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