-
Notifications
You must be signed in to change notification settings - Fork 614
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
[WIP] xdg thumbnails fetching with fallback on mimetype icons #1939
Conversation
…or menu entries in filebrowser mode
Thanks, looks good.. What is the license of the md5 code? can we include it? |
it seems very much free to use. from the LICENSE
Don't know if you prefer it as a 3rd party submodule, last commit was almost a year ago |
Great. We should copy the license into the header of the file then, so it is clear how it is licensed. Some things I noted trying it out:
I will try it out further. Thanks! |
Sorry I completely forgot about meson, I tend to use configure. |
That is not what I tried to point out.. for me it looks for thumbnail that does exists, but only larger. I worked around this as a test like this: // build file thumbnail's path using md5 hash of its encoded uri string
-static gchar* rofi_icon_fetcher_get_file_thumbnail(const gchar* file_path,
- int requested_size,
+static gchar *rofi_icon_fetcher_get_file_thumbnail(const gchar *file_path,
+ int requested_size,
int *thumb_size) {
// convert filename to encoded uri string and calc its md5 hash
gchar *encoded_entry_name = g_filename_to_uri(file_path, NULL, NULL);
@@ -319,49 +319,71 @@ static gchar* rofi_icon_fetcher_get_file_thumbnail(const gchar* file_path,
md5_to_hex(md5_hex, hex_size, md5, md5_size);
// determine thumbnail folder based on the request size
- const gchar* cache_dir = g_get_user_cache_dir();
- gchar* thumb_path;
+ const gchar *cache_dir = g_get_user_cache_dir();
+ gchar *thumb_path = NULL;
if (requested_size <= 128) {
*thumb_size = 128;
- thumb_path = g_strconcat(cache_dir, "/thumbnails/normal/",
- md5_hex, ".png", NULL);
- } else if (requested_size <= 256) {
+ thumb_path =
+ g_strconcat(cache_dir, "/thumbnails/normal/", md5_hex, ".png", NULL);
+ }
+ if (thumb_path && g_file_test(thumb_path, G_FILE_TEST_EXISTS)) {
+ return thumb_path;
+ }
+ g_free(thumb_path);
+ thumb_path = NULL;
+
+ if (requested_size <= 256) {
*thumb_size = 256;
- thumb_path = g_strconcat(cache_dir, "/thumbnails/large/",
- md5_hex, ".png", NULL);
- } else if (requested_size <= 512) {
+ thumb_path =
+ g_strconcat(cache_dir, "/thumbnails/large/", md5_hex, ".png", NULL);
+ }
+
+ if (thumb_path && g_file_test(thumb_path, G_FILE_TEST_EXISTS)) {
+ return thumb_path;
+ }
+ g_free(thumb_path);
+ thumb_path = NULL;
+
+ if (requested_size <= 512) {
*thumb_size = 512;
- thumb_path = g_strconcat(cache_dir, "/thumbnails/x-large/",
- md5_hex, ".png", NULL);
- } else {
+ thumb_path =
+ g_strconcat(cache_dir, "/thumbnails/x-large/", md5_hex, ".png", NULL);
+ }
+ if (thumb_path && g_file_test(thumb_path, G_FILE_TEST_EXISTS)) {
+ return thumb_path;
+ }
+ g_free(thumb_path);
+ thumb_path = NULL;
+
+ {
*thumb_size = 1024;
- thumb_path = g_strconcat(cache_dir, "/thumbnails/xx-large/",
- md5_hex, ".png", NULL);
+ thumb_path =
+ g_strconcat(cache_dir, "/thumbnails/xx-large/", md5_hex, ".png", NULL);
}
return thumb_path;
} I did something like this to make it find more thumbnails. |
This is not ideal, I think we should reverse it.. find the biggest one exists (we will auto-scale it down) and work our way down.. Because now I don't see it in a theme with bigger 'icons'. (I only have thumbs in normal folder). Is there a nice way to trigger the generation of thumbnails? are these script shipped? |
That's the next step, it's still a work in progress. There is a specification for thumbnailers that is used by file managers. They are ".desktop" files placed in /usr/share/thumbnailers" and "$HOME/.local/share/thumbnailers", containing a list of compatible mime-types and the corresponding to command to generate their thumbnails.
I'm looking at the implementation in libgnome-desktop and was thinking of using a similar solution. Create an hashmap of mimetypes as keys and compatible thumbnailers as values. This can be placed in the global state of rofi-icon-fetcher-data, and populated at creation reading from the thumbnailers standard directories. Doing this will make thumbnails generated by rofi compatible with other file managers |
Seems to work pretty good now, I suggest you to first install the xapp-thumbnailers, this way you have thumbnailers available for epub, mp3, raw files and others. |
that looks very nice. |
to complete this work, i would like to add the ability to specify a custom thumbnailer command when launching rofi. We could keep saving the thumbnails in the xdg cache directories, the custom script would take as arguments the entry name, the output path of the thumbnail (generated with the md5 hash of the entry name) and an integer size. Then, in the script one could do cool things based on the size requested, like for a list of games fetching their icons for small previews and high res banners for bigger previews. A practical example would be something like this:
%i, %o and %s will then be substituted by entry name, ouptut image path and requested size in rofi-icon-fetcher, the code to do that is already there to use standard thumbnailers. What do you think? |
sounds good.. maybe look at available helper functions.. There is already code to launch applications and do substitution. https://github.com/davatorium/rofi/blob/next/source/modes/ssh.c#L97 example ( need to check, this leaks memory in argsv?) |
You mean helper_parse_setup and helper_execute? |
helper_parse_setup mostly, as it does the replacement. |
I see, it does exactly what I need. Thanks for the pointer! |
I tried using helper_parse_setup to build to list of arguments of the thumbnailers, but I'm facing an issue. I need to replace the strings "%i", "%o", "%s" but the function is leaving them unchanged. |
It does not support replacing '%i'.. it replaces anything between brackets.. so |
okay thanks, now i understand, we can use it for the custom thumbnailer scripts but not for the xdg thumbnailers installed in the system. I was testing performance using the new recursive browser mode, and I got a little worried about performance, cpu is trashed and scrolling becomes very very slow after some pages. UPDATE: solved prepending "nice -n 19" to the command arguments list, much much better performance |
In theory it should be 'async' from the UI, but trashing your I/O it often becomes more difficult for the scheduler to do a good job.. I hope to be able to play with the PR this weekend. Thanks. |
I pushed the latest changes, it now works for me also using a custom script. To test, run something like this: |
took me a while to get it working as on debian apparmour was blocking evince-thumbnailer from writing files when launched by rofi.. It works really well, I will play with it some more later.. I did notice that for some reason fallback icon did not work anymore, but did not get around debugging this. |
… not a valid filename
I had some problems too, some commands were not recognized despite being in /usr/bin, I had to set it as the working directory of the spawned process to make it work. That's not very clean as a workaround to be honest.
That's strange, i see them in filebrowser mode, there was something broken with the last "custom command" addition but it should be fixed with the last commit, I did not touch anything about mimetype icons fallback. |
I think we need to bump the glib dependency to use this method. https://docs.gtk.org/glib/ctor.StrvBuilder.new.html 2.68 so that excludes ubuntu 20.04 LTS |
@DaveDavenport hm good question... In the notes, the filebrowser is presented as experimental so probably it is fine to include it in the release as such. It would just need some light documentation I suppose. |
I can add documentation too, where should I put it? My guess was in rofi-theme |
I think this is a good location: https://github.com/davatorium/rofi/blob/next/doc/rofi-theme.5.markdown#icon-handling I am still wondering if we should make this the 'default' behaviour for loading images, so we re-use preview other programs make.. The thing that worries me a little is that I had to tweak app-armour on my pcs to make it work. |
I can't help much with that at the moment, I use Slackware and it doesnt come with neither apparmor nor selinux. I can test a bit at work with fedora/Ubuntu in the coming days if you can wait a bit for the new release |
No rush, I am crazy busy too. |
I'm investigating the apparmor permission issues. Looking at the gnome code here apparently the thumbnailers programs (the one specified in the Exec key of .thumbnailer files) are not trusted by apparmor/selinux/flatpak etc. and are restricted to write inside a sandbox, that is a temporary folder created in /tmp/. The generated images are then loaded back in memory and saved in the standard thumbnails paths ($HOME/.cache/thumbnails/...). |
can you try adding the following apparmor profile to /etc/apparmor.d/usr.bin.rofi
then run |
I will test this somewhere this week.. Thanks a lot |
did not forget, will get back to testing. |
I think one of the important things left todo is test the above, but mostly document this feature. |
I can write some documentation, perhaps also pointing to the apparmor rule if it works for everyone (I only tested it on my work pc), where do you think is best to put it? |
Currently we have a small section here on icon loading: https://github.com/davatorium/rofi/blob/next/doc/rofi-theme.5.markdown#icon-handling However it might be good to create a new manpage for this, given the questions we get it could not hurt to expand the current section and add the new info.. @lbonn what do you think? |
I agree, it is probably worthy of its own man page, with a link (and adjustments) in the existing section.
|
I would go for rofi-thumbnails, because I think it would be nice to (in a next version) use this more generic . |
I finally got some time to write documentation of the new feature, let me know what you think about it |
@giomatfois62 I've added a few tweaks on the docs. The man page does not display too nicely with the huge |
Thank you very much! |
Is it ready to be merged, or are there more updates you want to make before I hit the button? |
No more updates from me, ready to merge if you want |
Thanks!!!! merged |
Thankyou! |
This implements xdg thumbnails fetching functionality in rofi-icon-fetcher.
When an icon is requested for an entry in filebrowser mode, and its name is prefixed by the string "thumbnail://", an existing thumbnail is searched in
$HOME/.cache/thumbnails/
and loaded as icon for the entry.If the thumbnail does not exists, an icon for the entry mime-type is loaded instead.
If the entry is a ".desktop" file, its icon key is used instead to load the icon.
WIP because it lacks the functionality to generate missing thumbnails using system thumbnailers and an option for users to use their own scripts as custom thumbnailers.