Closed benjajaja closed 7 months ago
Hi! Very interesting! Would this solution support a combination of an image and a text preview? Like the image widget being on the upper right and the text preview being on the lower right? Would the ratio of this vertical split be configurable? (Maybe a max-height
for the image widget?)
And: is there any plan in “ratatui-image” to support Überzug++ as a backend? At the moment it looks like removing the hooks and using ratatui-image instead would leave many users without image support.
Would be really cool if this works out!
Could probably make some "preview" wrapper here that takes care of text and a max-height
.
Currently there is no Ueberzug support, but I am trying to add it.
The PR has the hooks etc. removed (I was a bit too optimistic), but I will undo that and rather have some option to enable this widget, or only enable it if the hooks aren't configured.
Very interesting indeed! Let me know when this is ready for review!
One question that I have is, will this pull in additional dependencies?
Ideally, I would want joshuto (and ratatui-image
) to continue to compile even if the user does not have libsixel or some other library installed.
One question that I have is, will this pull in additional dependencies? Ideally, I would want joshuto (and
ratatui-image
) to continue to compile even if the user does not have libsixel or some other library installed.
Luckily, someone ported just the relevant part of libsixel to rust in the meantime: https://github.com/mkrueger/icy_sixel and ratatui-image (including this PR's version) now uses that.
@DLFW I refactored the PR so that scripts are unaffected.
There is however one breaking change. The preview_script
hook, for textual previews, had a default value of something like ~/.config/joshuto/preview.sh
which is inconsistent with the other preview config options. Therefore users that had created that file would need to now set the value explicitly, just like the procedure for adding the shown/remove hooks.
I have removed the default value, so that joshuto can render images with ratatui-image by default. Otherwise it would be necessary to 1) attempt to run that script every time an image is focused and check the return value, or 2) check at startup if that script exists (kinda weird thing to do), or 3) do some migration.
Re: additional info, right now just the image is shown, but we could also add some other textual info.
Re: ueberzug support in ratatui-image: no because ratatui-image is "immediate mode", i.e. there is specifically no notion of "removed / stopped rendering". So good reason to leave the scripts support in, as you asked.
@kamiyaa ratatui-image is now using icy_sixel, so all native rust deps. The supported image formats are currently the defaults of the image crate (https://docs.rs/crate/image/0.24.8/features), which could also be trimmed down a bit if desired. But no dynamic linking in any case. I have not tested yet that script support hasn't been broken, once I've done that I'll mark this as ready.
I can't get the preview_shown_hook_script
stuff to work (on main
). I could only get text previews to work with preview_script
, and when that option (or the hook script option) is set then the new built-in-preview is disabled.
I added a commit that attempts the script first, and then renders as image. This way there are both text previews and image previews, and it's not a breaking change. The only required action is for users that have a preview_file.sh
in place and want to use this new feature: they would have to edit the script to exclude previews for image/*
.
I added a commit that attempts the script first, and then renders as image. [...] it's not a breaking change.
Does that mean that ~/.config/joshuto/preview.sh
(or whatever) is again the default value for the preview script?
The only required action is [...] to edit the script to exclude previews for
image/*
.
What does “exclude” mean? Is the return value used somehow? Or does it mean the script must not write anything to stdout
?
Re: additional info, right now just the image is shown, but we could also add some other textual info.
Means ratatui-image
takes the whole right panel? Would it not be possible to just split that panel horizontally and to allow both, image and text (as it is)? I would still find it a charming solution to have both preview-mechanisms working independently from each other in their own area. Then we could drop that hook-thing as soon as ratatui-image
supports enough terminals...
@DLFW sorry I forgot to add/undo the changes to joshuto.toml
and preview_file.sh
, pushed that now.
The way it is now, this PR is not a breaking change (caveat: I could not set up and test the hook scripts myself).
But existing users that previously followed instructions may have to change their configs, if they want to use the new built-in previews:
config/preview_file.sh
to ~/.config/joshuto/preview_file.sh
must edit that script so that image/*
files are NOT handled anymore (see diff), or simply copy over the new script if they didn't modify it.preview_shown_hook_script
and preview_removed_hook_script
from their ~/.config/joshuto/joshuto.toml
.Means ratatui-image takes the whole right panel? Would it not be possible to just split that panel horizontally and to allow both, image and text (as it is)? I would still find it a charming solution to have both preview-mechanisms working independently from each other in their own area.
I guess that also would be an option. In fact, we could run both at the same time (script and image encoding), instead of sequentially, and display any that succeeded.
Then we could drop that hook-thing as soon as ratatui-image supports enough terminals...
So far my idea was that the widgets provided do work like any ratatui widget, as in they require no removed-like hooks. However, seeing how popular ueberzug is, and how it could really complete terminal support, I am now considering adding another widget plus some remove-like hook. But in respect to joshuto, that could then be a next step.
Is this PR ready for review?
@kamiyaa yes. Sorry there was some mistake with ratatui's default-features in the crate, fixed now.
Hi, I built and tested the branch to be merged with kitty, wezterm, alacritty, and st. The two first give me proper images, the two latter some sixel (I guess) block graphics. Otherwise, they all behave the same.
I tested the image preview with 1
and also with 2
from the preview.sh
script. Triggering the image-preview works as described.
However, the solution seems not to work reliably for me.
My observations:
I guess that you have broken some basic functionality somewhere. Seems the preview is just not issued anymore for anything.
One more question: your last commit is titled “can use preview_script alongside image widget”. Does that mean that I should be able to have a text and an image preview together now? If yes, how? I changed my preview.sh
so that it dumps a lot of text and returns with 1
for images, but the stdout from the preview.sh
is not shown as textual preview.
@DLFW thank you for testing this, and pardon me for not getting down to figuring out the existing hooks myself. I have now figured it out, at least for kitty, so I can test it better from here on.
I could reproduce the "hanging" myself now, with the provided preview_file.sh. Visiting some specific .xlsx file, the xlsx2csv commands hangs forever, and thus the preview thread is stuck forever. Previous successful previews do show again since they're cached independently from the thread.
The problem is this manifests also in main
! In main
, a new thread is spawned per visited file, but there is a global lock.
A similar issue occurs when visiting many files very fast, e.g. running the cursor down with J. Each file is processed in sequence (either due to the lock in main
, or due to channel loop in this PR). Some files take relatively long to process, e.g. ZIP files or other file conversions. So basically if one "runs around" visiting many files, the previews queue up to a point where the previews effectively stop working until the queue is processed, which might be several minutes.
Also, sometimes the remove hook is not called when navigating very fast, but that's not relevant here.
I will make changes so that the ratatui-image preview is shown together and independent of preview_file.sh
output. One above the other.
@benjajaja, I have no idea what causes the issue, but let me just emphasis that the problem was with ratatui-image
enabled, as well as with with ratatui-image
disabled. The problem was also not only with the image previews, but also with the text previews. When the text-previews stopped working, of course also the wrapper/hook-based image previews stopped working. I had the wrapper/hook-based image previews enabled only when the ratatui-image
previews were disabled. So, to reproduce this, a hook-based preview setup should not even be necessary.
The files I visited were also the same in my tests and they were not exceptionally big or something. The preview of those files did work fine with the latest upstream main
, but showed the described problem with your branch.
And as I said: the CPU was pretty calm. Your explanation sounds like it's a general performance problem (maybe related to #499 ?), but for me it looks more like some logical problem. However, I'm happy to try the same tests again, once you're there...
BTW, I'm really happy that you're on the way to tackle this problem! It's an amazing feature!
I will make changes so that the ratatui-image preview is shown together and independent of preview_file.sh output. One above the other.
Yay! 👍👍 😉
@DLFW now, it should work as expected with or without hooks.
ratatui-image renders independently of preview_script
, it is only disabled when preview_show/remove_hook
is set. No changes to preview_script.sh
required anymore.
preview_script
is run with a timeout, defaults to 2s, configurable with preview_script_timeout_milliseconds
. This should fix previews getting stuck forever, both with ratatui-image and with hooks.
I'm still not 100% satisfied with how the threads and queue work. When visiting a file, the request to preview should be inserted at the beginning of the queue.
Is it possible to move this behind a feature flag?, or can i fallback to halfblocks, if i don't want to use sixel?
also right now it feels slow (might be because preview logic), also it doesn't recognize the newer avif image format, additionally it broke my video thumbnail generator, that i use in the preview script:
generate_and_display_thumbnail() {
local FILE_PATH="${1}"
local FILE_LINK_PATH
FILE_LINK_PATH="$(readlink -f "$FILE_PATH")"
local CACHE_HASH
CACHE_HASH="$(stat --printf '%n\0%i\0%F\0%s\0%W\0%Y' -- "$FILE_LINK_PATH" | sha256sum | awk '{print $1}')"
local CACHE="$HOME/.cache/joshuto/thumbnail.${CACHE_HASH}"
local CACHE_FILE="${CACHE}.jpg"
# Check if the cache file exists; if not, generate the thumbnail
if [ ! -f "${CACHE_FILE}" ]; then
if ! ffmpegthumbnailer -m -i "${FILE_PATH}" -o "${CACHE_FILE}" -s 720 -q 1 >/dev/null 2>&1 ; then
echo "Thumbnail generation failed."
exit 1
fi
fi
# Display the thumbnail
display_image "${CACHE_FILE}"
}
# Function to display the thumbnail using chafa
display_image() {
local FILE="${1}"
if ! chafa -f symbols -c full -s "${PREVIEW_WIDTH}x${PREVIEW_HEIGHT}" --animate false "${FILE}"; then
echo "Image conversion failed."
exit 1
fi
# If everything is successful, exit with status 0
exit 0
}
also dislike the memory usage for a terminal file manager (probably because of the cached sixel graphics).
Hi, just did another quick test. Unfortunately, with similar results. I hope I don't do anything wrong. What I did and how I can reproduce it:
preview.sh
to exit with 1
on imagesjoshuto.toml
(which should not be necessary, but to avoid side-effects)~/.config/joshuto
directoryicons.toml
and keymap.toml
, the textual preview does not appear anymore. (I think that's even worse than in my last test.) Increasing the new preview_script_timeout_milliseconds
option to 5000
does not help either. I can also see that the text-preview for the smaller toml
files takes way longer than before. It's like waiting almost two seconds just to see a three-line toml or yaml or whatever.If the ratatui-image
loads an image blocking, I can understand it may take some time. But even small images sometimes never appear (and also did not in my previous test), and even 2K toml files never show up. I still have the feeling it's not a simple performance problem.
With the same configuration but a fresh build from upstream main
, the text previews of even the bigger files appear in less than maybe 200ms. I don't think it's a threading problem, unless the new ratatui-image
feature impacts it also for non-image files somehow.
Beside the changes I described above, these are my dotfiles: https://gitlab.com/DLF/dotfiles/-/tree/master/dotfiles/.config/joshuto
Don't you experience any performance issue for the text-previews yourself?
If not, maybe another person can try out your branch also.
also right now it feels slow (might be because preview logic),
Ok, looks like @luteran42 just did some test with a similar experience... 🙃
Yea, the ratatui-image loading was algo blocking the preview_file.sh script. I changed it to two threads, so that the script isn't affected by the image loading.
I also removed the image from the cache to keep memory at bay.
Sixel support could be a config option. Sixels are created with icy_sixel, a native implementation, I don't think it makes sense to put that behind a feature flag.
I tested quickly, but I will run some more deeper tests before marking for review again.
@benjajaja : Nice! One question: Why was/is ratatui-image significantly blocking text-only previews? What keeps ratatui-image busy when I'm just looking at - for example - a toml-file?
Eh, just gave your branch another quick test. Now the “big files” (toml-files >= 2k) are immediately shown when I put the cursor on them the first time! But, the text-preview does not show again a second time when I move the cursor to some other file and then back to “the big file”. 😁
@DLFW
Nice! One question: Why was/is ratatui-image significantly blocking text-only previews? What keeps ratatui-image busy when I'm just looking at - for example - a toml-file?
Before, there was one thread/queue processing visits as script and image. Until both processings have completed, even though they would run in parallel, the next visit will not start for neither script nor image. So as you visit files, the queue keeps growing, until you stop visiting files, and shrinks again to zero. While the queue was not empty, you would get that blocking effect. If you would wait for the queue to be empty, and then just e.g. visit the next file, the script should have been immediately called/rendered. But that is fixed now, with two separate queues.
Eh, just gave your branch another quick test. Now the “big files” (toml-files >= 2k) are immediately shown when I put the cursor on them the first time! But, the text-preview does not show again a second time when I move the cursor to some other file and then back to “the big file”. 😁
Thank you for finding that bug! Some leftover from a previous commit, an image error was mistakenly sent to the cache. For now, image errors are just ignored, since images always reload because they're not in the cache anymore.
Seems to be working! 👍 Tested both the sixel, kitty and halfblocks protocol in foot/alacritty/kitty terminal, all worked fine. only problem i found was in tmux, the sixel protocol was unreliable, but that's just might be tmux weirdness.
also it would be nice if we could set the protocol in the config file. like: image_renderer = auto (guess_protocol()) or image_renderer = pixelated (halfblocks) or image_renderer = disabled (only the preview script)
Thank you for your work! 😎
Next round: Text-previews seem to work now. 👍
But image previews still only work for a certain time for me. Easy to reproducible on st, wezterm and kitty for me, just by jumping through some dirs.
BTW, this also happens if I change from one image to the next only after one is loaded and displayed. I explicitly tried that to be sure that it's not a queuing/performance thing.
weird. I tested it again in st, foot, alacritty, kitty and it worked. only tmux was messing it up.
Added a config option to disable or e.g. force halfblocks
.
@DLFW for me it works fine now, jumping around directories with lots of images or files, I never get a missed image for images that should render (default formats of image crate, and under size limit).
Tmux and friends are problematic, but maybe it will get solved in ratatui-image someday, because it has been reported elsewhere.
So it's good to review now. Please comment if you think the feature should not be enabled by default as the PR currently does, but should be disabled
to test the waters.
I think auto is okay, halfblocks probably the safest. (personally i prefer halfblocks and kitty's protocol over sixel. Sixel seems more computational heavy and slower.)
Disclaimer: I'm the author of ratatui-image.
Use ratatui-image crate to render images. No script or hook setup is required. ratatui-image supports sixel, kitty, and iTerm2 protocols. At least xterm, foot, kitty, iTerm2 and Wezterm work out of the box.
Screenshot using kitty: