lxqt / libqtxdg

Qt implementation of freedesktop.org xdg specs
https://lxqt.github.io
GNU Lesser General Public License v2.1
72 stars 35 forks source link

Icons of newly installed apps may still not be correct #226

Closed tsujan closed 2 years ago

tsujan commented 4 years ago

In spite of using QFileSystemWatcher in xdgiconloader.cpp for keeping track of updated gtk icon caches, it's still possible that icons of newly installed apps are the fallback "application" icon in the main menu of LXQt Panel. Of course, a panel restart fixes the issue.

I encountered the problem when installing gnome-chess: its icon doesn't exist in my local active icon set but, although the set inherits "hicolor" (it has Inherits=breeze,oxygen,gnome,hicolor in its index) and the app icon was installed in "hicolor" and the gtk icon cache was updated with installation, the "application" icon was shown in the main menu.

Strangely enough, after I uninstalled gnome-chess, restarted the panel and re-installed gnome-chess, its icon was correct. So, I think the problem may be about timing.

The same thing happened with another app too.

NOTE: This is an old issue that was supposed to be fixed by QFileSystemWatcher. Before using QFileSystemWatcher, it always happened; now, it happens less often.

tsujan commented 3 years ago

More thoughts:

As for why, after recent changes, this issue happens with lxqt-panel and not with pcmanfm-qt, my first thought was that it might be because of XdgIcon::fromTheme() but replacing it with QIcon::fromTheme() in the panel's code didn't make a difference.

libfm-qt (on which pcmanfm-qt depends) uses GIcon to find icons of desktop entries. My wild guess is that GIcon is somehow compatible with GTK icon cache, while Qt may have a "sync problem" with it (which isn't compensated for by QFileSystemWatcher).

tsujan commented 2 years ago

The problem may not be in the cache but in how Qt updates icons.

I did a simple test with QToolButton inside a window. The button had a nonexistent main icon plus an existing fallback one:

tb->setIcon(QIcon::fromTheme("test-icon", QIcon::fromTheme("user-home")));

It also showed a debug message about the state of the main icon when clicked:

QObject::connect(tb, &QAbstractButton::clicked, [tb] {
    qDebug() << QIcon::fromTheme("test-icon").isNull();
    ...
});

When the button was shown, its icon was "user-home", as expected.

Then, while the button was shown, I added the main icon to the theme and updated the GTK cache. Nothing happened in the button, but the click message said that the main icon really existed. I tried several times, and the result was the same, i.e., the button's icon was still "user-home".

Then I removed the fallback icon from the code (not from the theme), so that the button text was shown in the absence of the main icon:

tb->setIcon(QIcon::fromTheme("test-icon"/*, QIcon::fromTheme("user-home")*/));

Now, when I added the main icon to the theme and updated the cache, it was shown on the button as soon as its window was activated. The subsequent additions and removals of the main icon followed by cache updates were also reflected by the button correctly.

In this test, the issue was related to the existence of the fallback icon, not to the cache update.

Dami-star commented 2 years ago

I will also try the fallback icon problem you mentioned on the weekend, BTW, I submitted a fix to Qt but it doesn't work for the current issue https://codereview.qt-project.org/c/qt/qtbase/+/419489

tsujan commented 2 years ago

I will also try the fallback icon problem you mentioned on the weekend

I mentioned it only as a possibility; otherwise, I'm not sure whether it plays a role here.

Until now, I've tried several workarounds to no avail β€” watching the icon set directory, instead of the cache, was the most promising, but it didn't make a difference.

tsujan commented 2 years ago

@Dami-star ~A compromise is here: https://github.com/lxqt/libqtxdg/pull/283~

It worked with several apps that I installed (and uninstalled), from light to heavy.

You said that you had a good environment for testing. Could you please test it?

tsujan commented 2 years ago

@Dami-star Sorry, please check https://github.com/lxqt/libqtxdg/pull/284 instead. It doesn't make a compromise.

Dami-star commented 2 years ago

It worked with several apps that I installed (and uninstalled), from light to heavy.

You said that you had a good environment for testing. Could you please test it?

πŸ‘οΈπŸ‘οΈπŸ‘οΈπŸ‘οΈπŸ‘οΈ Ok, I will test and verify #284 as soon as possible and give you feedback