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

Refresh thumbnail after file change #2340

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented Oct 10, 2023

Fixes #2327

  • Handle CHANGES_DONE_HINT events by updating icon
  • Ignore duplicate events and events from transient files
  • Avoid issues causes by caching of thumbnails internally by only caching remote thumbnails
  • Improve performance by avoiding unnecessary icon lookups when scrolling
  • Move some cache and thumbnail related code from AbstractDirectoryView to File

The issue was fundamentally caused by CHANGES_DONE_HINT events from the directory monitor being ignored. However it was found that just fixing that did not reliably fix the problem - icons did not always update at all sizes. This was found to be due to the way Files caches icons internally. Trying to work round this proved intractable. It was found that dropping caching for local files and themeable icons combined with reducing the number of lookups being performed unnecessarily resulted in no discernible performance hit on modern hardware (i5 processor and SSD storage).

Caching was retained for remote files to maintain performance on slow connections. Remote locations cannot be monitored for changes by the directory monitor so the linked issue will always occur for remote files anyway.

@jeremypw jeremypw marked this pull request as ready for review October 11, 2023 17:57
@jeremypw jeremypw marked this pull request as draft October 11, 2023 17:59
@jeremypw
Copy link
Collaborator Author

Investigating regression.

return;
/* Icon/thumnail related functions */

public GLib.Icon? get_icon_user_special_dirs (string path) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this covered by FileAttribute.STANDARD_ICON in FileInfo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look at this. You are probably right - I'll look into it. The scope of this PR seems to have grown substantially, into an extensive review of icon handling as fixing the issue proved more complicated than envisaged. A lot of the code is old and hard to understand. Simplifying/clarifying will aid future porting to Gtk4.

@jeremypw
Copy link
Collaborator Author

@danirabbit Looks like FileAttribute.STANDARD_ICON only returns a standard folder icon for user folders so cannot substitute for that function I am afraid. However, while checking this, I noticed there are some new? attributes related to thumbnail status that might be useful. I guess they might not work for non-native/remote files though unless gvfs generates them locally.

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.

The thumbnail does not update?
2 participants