Removed the redundant class IconEngine

[tsujan]

I found no reason for using IconEngine. Moreover, its removal closes #788

[tsujan]

@palinek
Am I missing something obvious here?

EDIT:

Previously, I only checked the correctness of the IconEngine code, without questioning its purpose.

IconEngine is used to make a QIcon that's an approximate copy of IconInfo::internalQicon(). I can't find any reason to use a copy.

Also, in practice, everything seems fine without IconEngine.

[palinek]

If I see it right, the main purpose is to not loose the underlying IconInfo object and its variability (the GIcon) when handing the icon into Qt components.
E.g. IIRC IconInfo correctly supports list of theme names and correctly handles showing them upon the theme change (there is different name existing in new theme, than in the previously used theme) etc.

[tsujan]

Yes, IconInfo is libfm-qt's way of handling icons by using GLib — like in other parts of libfm-qt, here we use GLib/GIO and "translate" the final result into Qt.

My point is that IconEngine has no part in this. It uses IconInfo::internalQicon() in all of its methods, and by doing so, it just duplicates that QIcon.

So far, I haven't found a difference in practice (except for #788, which is a side effect of the above-mentioned duplication). The GUI also feels faster — although that may be an effect of expectation on observation, because I know that extra computations are removed ;)

I asked you because I thought I might not see the wood for the trees (dealing with various codes). Do you know of a use case which would make a real difference?

[palinek]

My point is that IconEngine has no part in this.

It has. It is the pointer back to IconInfo, which makes it possible to have e.g. multiple fallback icons.

  static const char* fallbackIconNames[] = {
      "unknown",     
      "application-octet-stream",
      "application-x-generic",
      "text-x-generic",                       
      nullptr                                 
  };        

Let's say for particular need there is no icon and a fallback is used. In current icon theme A the "application-x-generic" is present and as so used and showed in GUI. When the icon theme is changed on the fly (w/o restarting lbfm-qt based app) to icon theme B, where there is also the "application-octet-stream" icon present, the "application-octet-stream" will be shown in the GUI. As the GUI QIcon with IconInfo reference is aware of all the fallbacks and their priorities.

On the other side the "plain" QIcon (w/o IconInfo reference) will be still showing the "application-x-generic", or even show nothing, when there won't be such icon in the B theme.

[palinek]

Example:

• creating a .desktop file with some "reallynotexisting" icon
• removing the "unknown" icon from Oxygen

for f in $(find usr/share/icons/oxygen -name unknown.\*); do sudo mv "$f" "${f}1"; done

• pcmanfm-qt with QIcon -> IconInfo (the "unknown" gets back with Numix)

vokoscreenNG-2024-10-14_15-04-56.mp4

• pcmanfm-qt with "plain" QIcon (the "unknown" is not restored back with Numix)

vokoscreenNG-2024-10-14_15-05-38.mp4

[tsujan]

Yes! This was what I overlooked: an on-the-fly change or update of the icon theme. Thanks!

Before seeing your last comment, I did exactly the same test with a desktop entry file containing Icon=. With my current icon theme, it's shown as Unknown. When I changed the theme to HighContrast, Adwaita's application-x-generic was shown because, although HighContrast only has text-x-generic, it inherits Adwaita, which has application-x-generic. So far, so good. But when I went back to my icon theme, application-x-generic was shown, instead of Unknown.

If this was only about fallback icons, the removal of IconEngine might deserve to be considered, but any icon can be affected when the theme changes on the fly.

[tsujan]

Also without a theme change, IconEngine does extra paintings whenever the mouse enters/exits the view, the cursor goes over items, items are clicked, …, and in general, whenever the states of items are changed. That's expected, but these paintings are done in addition to what Qt (QStyle) does.

Anyway, it seems that even with a folder containing many icons, the extra CPU usage is very small.