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

Dinamically load icons if cache is being rebuilt (issue #2209) #2238

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

edwood-grant
Copy link
Contributor

@edwood-grant edwood-grant commented Jan 2, 2025

Short summary

This aims to fix #2209

What I've found is that are some methods that will temporarily erase the cached icons folder in order to repopulate. This happens:

  • On the first time the app is loaded and there is no cache
  • When the cache is old enough that it warrants another download. Right now is set up to one hour, but the problem is easy to trigger every-time you open if you set this to zero. This is why "sometimes" the icons are bad, because the metadata pre-process only happens when the cache is old enough to trigger this process again.

The method in question is preprocess_metadata. This, for every user and system remote, delete s its cache folder, and checks for the integrity of everything before re-creating the folder icons symbolics links again.

Given some discussion, the conclusion was that it was best to add a signal to detect as soon as possible that each remote manages to update itself and then update the corresponding icon accordingly. The aforementioned icons now use the default icon if it fails to

This PR also addresses the icon load procedure in Package.vala, in order to have priority on the recommended icons to load according to the AppStream API. The order in question is:

  • Stock icon
  • Cached Icon
  • Local Icon
  • Remote Icon (not being loaded/used right now)

Right now, we are not loading remote icons because of a blocking event when querying the URI. I think it's perfectly possible to dynamically load these, similarly to when the cache is being rebuilt. Worthy of a future PR. This change should be a basic preparation for that.

Tests

Here are some videos of the results. Note that I'm using for some of these fake cache times, just to show stuff.

This video tests loading icons for the first time. Then it reloads to show that, if cache is good, won't load them up again.

2025-01-02-01.13.16.723112574.mp4

This one shows that if you quickly go to the AppInfoView, the loading icon event can still be seen.

2025-01-02-01.13.59.877832494.mp4

Additional info

Sorry about the commit history being a bit polluted, I was doing and undoing stuff. If you need me to squash some commits, I can do that.

italo-capasso added 14 commits December 31, 2024 07:54
…ith a borken icon.

This also attmepts to load all available icons, including flatpak emote icons via priority according to freedekstop specs.
…rify cache existence).

Also removing a not needed critical message
…d icon.

Then, change it to the correct icon when you are sure its correspondent cahce remote has been updated properly. This happens aof Banner icons, ListPackageRowGrid icons, and AppInfoView Icons
blocking method.

Also, ensure no spinner shows when an icon is not
available (instead of failing to load icon)
@alainm23
Copy link

alainm23 commented Jan 3, 2025

I think it's a great solution, but I'm not sure if we should use Gtk.Spinner. Maybe just use the default icon until the actual one loads and leverage Gtk.Stack to make the transition smoother.

@edwood-grant
Copy link
Contributor Author

edwood-grant commented Jan 3, 2025

So I implemented the Gtk.Stack way. Wanted some input on the result though.

I suppose the CROSSFADE transition would be the best, smooth but not too much (but maybe other transitions might be better? There are slides and 3d box rotations, but not sure about that).

I also decided to blur and desaturate the default icon a bit via CSS, to avoid confusion if an actual icon cannot be found, so those use the normal default icons (at the moment some flatpaks that use remote icons only have no icon on the AppCenter because we are not dynamically downloading remote icons yet.)

Here is the result:

2025-01-03.18-06-47.mp4

Let me know what you think 👍

Edit: I made some change to removed using named Gtk.Stack children to ref widgets, for performance reasons 👍

@teamcons
Copy link

teamcons commented Jan 6, 2025

I feel stupid for asking this, but how would it be best to test this out ?

when testing this branch, deleting the appstream or flatpak, or both, folders in cache,
only makes appcenter window take up ~17s (if killed before) or 5s (if wasnt) to appear

it appears fully loaded, no temporary icon or anything.

deleting io.elementary.appcenter only causes #2246

@edwood-grant
Copy link
Contributor Author

edwood-grant commented Jan 7, 2025

Hey @teamcons.

So yeah, it's a bit annoying to test this. The "real test" is to, literally, wait for one hour after opening the AppCenter and closing it. The cache gets invalidated after one hour, so that's how you should test it "for real".

But if you want to do the quick check, you can modify the MAX_APPSTREAM_AGE constant value, located in src/Core/FlatpakBackend.vala, which currently is at 3600 seconds (e.g. one hour).

You can put a small value, like 30, so the cache gets invalidated every 30 seconds or something like that (so you can test that a reload of the AppCenter should not trigger this behavior in between the invalidation timer because the cache is OK). You can also put 0, and it will always invalidate.

In theory, both tests should yield the same results, as the only difference is the AppCenter Flatpak backend defined time for the cache to be invalid.

If you have more questions, let me know 👍

@teamcons
Copy link

teamcons commented Jan 7, 2025

I mean i cam retry, but that is also how ive proceeded a few times, waiting some time before reopening appcenter, and the behaviour is similar - probably it loads too fast ?

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.

AppCenter does not load images/icons on first load
3 participants