Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

appIndicator: Refactor named icons lookup and loading #415

Merged
merged 10 commits into from
Mar 13, 2023

Conversation

3v1n0
Copy link
Collaborator

@3v1n0 3v1n0 commented Mar 10, 2023

From main commit:

We used to always looking up for the icons ourself, somewhat duplicating
the shell work, it allowed to prevent shell caching of some files, but still
we can remove lots of deprecated code and handle things more smartly given
that texture-caching can be good in case of well-known files.

In short:
 - named-only icons are now loaded through a Gio.ThemedIcon (and so cached)
 - icons with custom theme are searched in provided path and behave just
   like any other icon with full path as explained below.
 - icons with a full icon path (provided by the theme or not) may be loaded
   in different ways:
   * Using a Gio.FileIcon if they are in non-writable areas (so we assume
     that there are only a limited number of them) and they are cashed.
   * using a StTextureCacheSkippingFileIcon in newer gjs versions and so
     basically they're non-cachable Gio.FileIcon's
   * using a GdkPixbuf, potentially loaded via the icon theme (for svgs) in
     older gjs versions, and so that's not cached.

@3v1n0
Copy link
Collaborator Author

3v1n0 commented Mar 10, 2023

@patricklibert @JOH451 @stuarthayhurst @luca-e075e @yochananmarqos

Since you've recently experienced #410 could you please if this keeps all working properly to you?

Use: git clone https://github.com/ubuntu/gnome-shell-extension-appindicator -b icon-loading-refactor

(To install see https://github.com/ubuntu/gnome-shell-extension-appindicator/#installation)

Thanks!

@stuarthayhurst
Copy link

Works perfectly for me

3v1n0 added 10 commits March 10, 2023 08:41
The gicon icon size is used only for loading the textures so it only
matters for us when we pass a path to StTextureCache, and it's generally
provided by the theme, however in case that's not the case we need to update
it only when it changes, there's no need to do it more times.
We only can update the icons if we're ready and mapped, otherwise it does
not make sense, so ensure this is the case, waiting in case.
…erences

St.Icon uses the loading size of the icon depending on the theme value for
the icon size, so we should honor this, but also we want to be sure that the
loaded icon will respect the user defined settings, so we override such
value in case the values do not match.
We still prefer to see something than nothing
We used to always looking up for the icons ourself, somewhat duplicating
the shell work, it allowed to prevent shell caching of some files, but still
we can remove lots of deprecated code and handle things more smartly given
that texture-caching can be good in case of well-known files.

In short:
 - named-only icons are now loaded through a Gio.ThemedIcon (and so cached)
 - icons with custom theme are searched in provided path and behave just
   like any other icon with full path as explained below.
 - icons with a full icon path (provided by the theme or not) may be loaded
   in different ways:
   * Using a Gio.FileIcon if they are in non-writable areas (so we assume
     that there are only a limited number of them) and they are cashed.
   * using a StTextureCacheSkippingFileIcon in newer gjs versions and so
     basically they're non-cachable Gio.FileIcon's
   * using a GdkPixbuf, potentially loaded via the icon theme (for svgs) in
     older gjs versions, and so that's not cached.
It's better to a wait a bit more before emitting properties changes so that
they can be grouped together, for example the icon-theme path and the icon
name should go together
@yochananmarqos
Copy link

Yes, it's working. 👍

@patricklibert
Copy link

Seems to be working...

@AnthillSudoku
Copy link

@3v1n0 On mine it doesn't work

When I left-click the Vorta's icon a menu must appear, but it will not. It seems there is no way to open the Vorta's main window

@3v1n0
Copy link
Collaborator Author

3v1n0 commented Mar 13, 2023

@luca-e075e it seems a different issue then given that's not related to the icon itself, but to the menu... That could be due to #416 maybe?

@3v1n0 3v1n0 merged commit 96715aa into master Mar 13, 2023
@AnthillSudoku
Copy link

@luca-e075e it seems a different issue then given that's not related to the icon itself, but to the menu... That could be due to #416 maybe?

I have no idea, but it affects only Vorta. Now the updated version is in the official gnome extensions' rep and it's not working

@3v1n0
Copy link
Collaborator Author

3v1n0 commented Mar 14, 2023

@luca-e075e please open a new bug about it and provide logs if you get errors.

I personally get the vorta icon working properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants