lf
lf copied to clipboard
Horizontal resizing does not flush cached sixels
When you resize the terminal window horizontally, lf does not flush the cache and reload the images. So you get a cropped image as you can see below.
But if you resize vertically, the cache will be flushed and the images will be reloaded.
According to this commit https://github.com/gokcehan/lf/commit/628bf408b4a8cd55154961a5cf54316762a58e72, it's designed to work that way, but it's unclear why the cache is flushed only on vertical resizing.
I'm using the following previewer:
#!/bin/sh
case "$(readlink -f "$1")" in
*.bmp|*.gif|*.jpg|*.jpeg|*.png|*.webp|*.six|*.svg|*.xpm)
chafa -f sixel -s "$2x$3" --animate false "$1" ;;
*)
bat -pp -f "$1" ;;
esac
Note that I don't use the exit 1
command because I don't want to disable image caching.
Thanks for reporting. I guess that most of the other users who were interested in sixel previews never encountered this issue because they just used exit 1
as that was recommended in the PR description https://github.com/gokcehan/lf/pull/1211#issue-1666086783.
The reason why the file preview cache is only cleared on vertical resizing is because it predates sixel support. Originally, file previews only supported text content, which is implemented as an array of strings, limited to the height of the preview window. For instance if the preview window has a height of 30 lines, then only the first 30 lines of the file would be read and stored for previewing. This also means that if the height of the preview window increases, the preview would no longer be valid and have to be reloaded. Horizontal resizing doesn't matter in this case since entire lines are stored, and lines are simply truncated if they exceed the width of the preview window.
I guess we could fix this by unconditionally invalidating the file preview cache if sixel
is enabled:
diff --git a/eval.go b/eval.go
index ff04308..23df598 100644
--- a/eval.go
+++ b/eval.go
@@ -1940,6 +1940,9 @@ func (e *callExpr) eval(app *app, args []string) {
app.nav.height = app.ui.wins[0].h
app.nav.regCache = make(map[string]*reg)
}
+ if gOpts.sixel {
+ app.nav.regCache = make(map[string]*reg)
+ }
for _, dir := range app.nav.dirs {
dir.boundPos(app.nav.height)
}
We might also have to do something similar in other places where the horizontal size of the preview window can change, for example if the ratios
option is set.
I read the entire PR and found that you have already discussed this issue here https://github.com/gokcehan/lf/pull/1211#issuecomment-1529739837 and here https://github.com/gokcehan/lf/pull/1211#issuecomment-1529812057 but it remained unsolved. I also noticed that @horriblename added the exit 1
command on the same day the PR was merged. So are there other cache issues as well since he wanted to disable it in the example?
To me your solution is fine because it doesn't affect the users who don't use the sixels. But as you guessed, the same problem occurs when you change the ratios
and that problem needs to be solved too.
So are there other cache issues as well since he wanted to disable it in the example?
nope, this is the only problem
@joelim-work I'll make a PR from your patch above, there's one other thing that needs to be done (force filler to redraw)
I just noticed that the same cropping problem occurs when using symbols in chafa:
chafa -s "${2}x${3}" -f symbols --animate false "$1"
So it seems to me that we always need to invalidate the file preview cache regardless of whether we resize the terminal window horizontally or vertically. Because we never know which previewer tools actually depend on the width and height attributes of the preview pane.
@joelim-work I read that you are releasing a new version. Is it too much to ask to have this fixed in there as it is very easy to fix?
I have no idea what the "force filler to redraw" issue @horriblename mentioned is, but I don't think we need to worry about it now since everything seems to be working fine with the suggested fix.
I think the "force filler to redraw" is referring to this code: https://github.com/gokcehan/lf/blob/43c2683ca86d30789b6d2ba82460a568a27a69b9/sixel.go#L60-L65
But alternating between bold/regular to clear the image only happens when drawing the preview for a different file, not if the size of the preview changes for the current file. I found a way to work around it though, by setting lastFile
to an empty string.
That being said, I think alternating between bold/regular to clear sixel images is specific to only some terminal emulators, though I'm not 100% sure about this. Anyway I submitted #1629, hopefully that should fix your issue.
Your fix works as expected. Thanks a lot!