fzf icon indicating copy to clipboard operation
fzf copied to clipboard

Images preview support for terminals witch sixel

Open timsofteng opened this issue 3 years ago • 6 comments

Hello. Is it possible to implement images preview support for terminal which support sixel?

timsofteng avatar Jun 25 '21 21:06 timsofteng

I was just about to ask the exact same thing. @junegunn, would it be possible?

leo-arch avatar Jul 03 '21 00:07 leo-arch

Since wayland is lacking something like ueberzug, I'm currently using sixels on foot in wayland to display images in a terminal. Therefore I'm also interested in this.

I've never done any golang programming, but looked into the preview window code and noticed that currently Device Control ANSI codes (ESC P .... ST), and hereby also sixels, aren't parsed at all. The relevant parts of code seem to be in src/ansi.go:nextAnsiEscapeSequence where escape codes are filtered. The ESC starting a device control sequence is just removed and any sixel data is written as plain text into the terminal.

Since sixel data is often an enormous control sequence, painting any pixel multiple times, it may not be desirable in something focused on speed like fzf, I'd like to hear whether this feature would even be merged if somebody implements it.

Despite that I thought about getting it to work. I'm trying to keep any sixel escape sequence from the command output until it's printed in the preview.

The problem:

The length and format of a string with sixel data is unpredictable (depends on console, font and font size) and introduces one or more newlines since any text printed after a sixel will be put one line below the last pixel. This messes with the normal handling of strings in go, especially with regard to newlines.

How to go about it:

Any sixel data must therefore be parsed by fzf using (potentially non portable) xterm specific control sequences (ESC [ 14 t) to get the terminal width/height in pixels on stdin (which is another problem when fzf is currently reading stdin), divide that by number of columns/rows to get the width/height of a character and then parse the whole sixel sequence and determine the size of the sixel data in terms of rows and columns. One can then provide an effective width and height for the data that would need to be passed along through the code until it gets printed in the src/terminal.go:renderPreviewText function.

Is it worth it?

If I'm not missing any easy workaround, this is a lot of work. On the upside, one should be able to extend this to also allow for kitty's graphics control. If it has the potential to be merged, I may try my luck, but not in the foreseeable future.

Please correct me if I have misunderstood parts of fzf, or of the sixel graphics escape codes!

pjungkamp avatar Jul 03 '21 19:07 pjungkamp

introduces one or more newlines since any text printed after a sixel will be put one line below the last pixel.

If you use private mode 8452, the cursor will be left to the right, on the last line of the sixel, instead of on a newline.

Since sixel data is often an enormous control sequence, painting any pixel multiple times,

In my benchmarks of foot, it was pretty clear that the bottleneck is the producer. I.e. the one encoding the sixel. Sending the escape and parsing it in the terminal is nowhere near as demanding. I have seen mpv, and notcurses, peg one or more cores while foot is using ~20-25% of a single core.

Thus, one might want to consider pre-generating the sixel escapes, and/or cache already rendered sixels.

Any sixel data must therefore be parsed by fzf using (potentially non portable) xterm specific control sequences (ESC [ 14 t) to get the terminal width/height in pixels on stdin.

An alternative, that doesn't read stdin, is to use the TIOCGWINSZ ioctl. Not all terminals fill in the pixel fields, but all terminals that support sixel do (AFAIK).

dnkl avatar Jul 04 '21 11:07 dnkl

@junegunn any thoughts about this issue?

timsofteng avatar Jul 28 '21 22:07 timsofteng

I'm also interested in ways to do that.

guilhermeprokisch avatar Sep 29 '21 14:09 guilhermeprokisch

While sixel support would be amazing, just a sidenote for people who don't not know it: ANSI based terminal rendering of images is pretty decent meanwhile - and works in fzf previews w/o problems: https://hpjansson.org/chafa/gallery/

axgkl avatar Apr 15 '22 11:04 axgkl

As now ueberzug is not maintained I think it's a great time to rethink this issue. The sixel implementation would be really nice.

ghost avatar Nov 23 '22 22:11 ghost

Is there any thoughts given on this?

niksingh710 avatar Dec 28 '22 07:12 niksingh710

@PJungkamp summarized it well above. I'm not familiar with the spec so I don't even know if it's possible to implement it inside fzf, and even if it is, it's going to be a hell lot of work. So I'm not interested in pursuing the goal, but feel free to send a pull request if you can pull it off.

junegunn avatar Dec 28 '22 08:12 junegunn

I wish you would revisit this.

In terms of the spec, it's not terribly complicated. It's an escape sequence, followed by a prefix, followed by pixel data encoded in a particular way. The main thing to properly forward the escape sequences to the terminal, and to not mangle the pixel data. The terminal does the work of rendering. They're drawn in place at the cursor position. A lot of terminal emulators support full-color sixel graphics now. The more people use it, the more other emulators are likely to adopt it as well. The thing is, it's based on an actual hardware graphics terminal from the 1980s, so it's well-established.

I am not familiar with how FZF's preview window works, but some support for this would really expand the reach of what FZF can do.

emdash avatar Sep 14 '23 15:09 emdash

After adding Kitty graphics support with a minimal code change using pass-through mechanism, @aeghn made me realize a similar approach could be taken to support Sixel as well (https://github.com/junegunn/fzf/discussions/3479#discussioncomment-7284785).

I'm experimenting with the idea in b1a0ab8. Haven't decided if I'm going to keep it. It works fine with a single Sixel image but it's unlikely to work well with anything beyond that.

junegunn avatar Oct 22 '23 16:10 junegunn

After adding Kitty graphics support with a minimal code change using pass-through mechanism, @aeghn made me realize a similar approach could be taken to support Sixel as well (#3479 (reply in thread)).

I'm experimenting with the idea in b1a0ab8. Haven't decided if I'm going to keep it. It works fine with a single Sixel image but it's unlikely to work well with anything beyond that.

FINALLY!! Seeing this is mail gave me so much excitement and relief ......... that now I can ditch my hack for ueberzug to preview images ......... Looking forward with this.

niksingh710 avatar Oct 23 '23 04:10 niksingh710

It works fine with a single Sixel image but it's unlikely to work well with anything beyond that.

So do you have plan to add features like displaying text alongside or scrolling along with the image?

(This video is copied from #3479 from my fork)

https://github.com/junegunn/fzf/assets/51843252/8ed7159a-055a-4701-b022-3abf63eb46ee

I think it would be very cool if we could scroll the images in the fzf preview window (like pdf preview?).

If you decide to keep this option, I will be very happy to continue exploring things about sixel to work with fzf.

aeghn avatar Oct 23 '23 06:10 aeghn

Only if the amount of the required code is small. After all, this is a "preview" window rather than a full-fledged viewer. I would advise users to launch a proper viewer program, examine the contents, and come back to fzf using execute bindings. But I'll take a look at your branch when I have some time.

FWIW, it is currently possible to do those things with Kitty graphics protocol thanks to its "placeholder" mechanism.

There are several competing protocols for image support on the terminal. Honestly, I have little experience with them, but for now, I prefer Kitty protocol because it's noticeably faster than Sixel, and it works on tmux, unlike Sixel.

junegunn avatar Oct 23 '23 07:10 junegunn

Thank you for such a wonderful app and your great effort on the features which would help people living in terminal.

aeghn avatar Oct 23 '23 08:10 aeghn

and it works on tmux, unlike Sixel

tmux officially supports sixel when compiled with ./configure --enable-sixel.

hashworks avatar Oct 23 '23 08:10 hashworks

and it works on tmux, unlike Sixel

tmux officially supports sixel when compiled with ./configure --enable-sixel.

I think @junegunn is referring to the default setting, as someone pointed out in discussion https://github.com/junegunn/fzf/discussions/3479#discussioncomment-7287883.

aeghn avatar Oct 23 '23 09:10 aeghn

I am using sixel in fzf from a few days and it has been working awesomely fine in tmux.

niksingh710 avatar Oct 25 '23 09:10 niksingh710

So here's some progress:

https://github.com/junegunn/fzf/assets/700826/f4da461f-b1dd-4e89-9815-4bffaeabbf79

I have tried to implement the image placement with a minimal amount of Sixel-related code. The code is not polished and it has some issues and limitations some of which are not easy to fix. But it works on a basic level.

junegunn avatar Oct 25 '23 16:10 junegunn

Hey @junegunn, thank you very much for this! Really great. I've faced a few issues however (running on Arch, I3, xterm 388, fzf-git):

  1. Regarding convert(1), -scale is a bit nicer with CPU resources than -resize (same values). Being this just a preview, it might be a nice alternative for low-end machines.
  2. When the image is larger than the preview window's height, we have wrong cursor placement and the items list is messed up. I've arbitrarily reduced the height to something that works for me: $((FZF_PREVIEW_HEIGHT - 50)). This works in most cases, but probably depends on the terminal size (mine: 150x40)
  3. Everything works fine until you set --preview-window to a fixed width (say 120): some images, depending on their size I guess, are not displayed.

EDIT: reducing the width (say $((FZF_PREVIEW_WIDTH - 270))) fixes point 3, but still this is an arbitrary value (works for my setup at least).

  1. When a big image is being loaded (and while we see the "loading..." label) the previous image is reprinted and then replaced by the current one.

leo-arch avatar Oct 25 '23 20:10 leo-arch

Just to be sure, are you testing the latest revision? The previous commit renamed $FZF_PREVIEW_WIDTH to $FZF_PREVIEW_PIXEL_WIDTH.

junegunn avatar Oct 26 '23 00:10 junegunn

Using the latest rev now. Removed clear from --preview-window (not needed anymore, that's cool), and replaced var names with the new names. Still experimenting the same issues (and using the same workaround values to "fix it").

I also noticed that sometimes (specially when the previous file was a text file, tough also happens with images) the preview of the previous file is not properly cleared.

Btw, the whole thing feels snappier and less resource intensive now. Switched to -resize again.

leo-arch avatar Oct 26 '23 01:10 leo-arch

The code is not polished and it has some issues and limitations some of which are not easy to fix.

Are you saying that sometimes the input box gets misaligned or the border gets broken, or sometimes there is an extra line printed by sixel (which may have different behaviors in different terminal emulators)? I encountered this issue before and couldn't find a good solution, so I directly removed the unnecessary borders and kept only the border separating the input box.

Additionally, from my personal standpoint, I would recommend using chafa.

aeghn avatar Oct 26 '23 02:10 aeghn

@aeghn No, no issues with borders. The scrolling beyond an image can't be perfected in a simple way because fzf doesn't know in advance there is an image in the preview contents and how many lines it takes.

chafa looks great. Since it can resize the image by the number of terminal columns and lines instead of pixels, I can remove FZF_PREVIEW_PIXEL_*. That's great!

chafa can even print an image in kitty format, but unfortunately it doesn't seem to work inside tmux. So we're going to keep kitty icat for now.

junegunn avatar Oct 26 '23 02:10 junegunn

@leo-arch

I also noticed that sometimes (specially when the previous file was a text file, tough also happens with images) the preview of the previous file is not properly cleared.

I can reproduce. Let me see what I can do.

junegunn avatar Oct 26 '23 02:10 junegunn

  • Removed $FZF_PREVIEW_PIXEL_*
  • Updated bin/fzf-preview.sh script to use chafa
    • https://github.com/junegunn/fzf/blob/ec208af474fa3eadad5779c3d30db6d2db300932/bin/fzf-preview.sh#L33-L34
  • Clearing of the previous preview text

junegunn avatar Oct 27 '23 02:10 junegunn

Quick testing (latest version, using chafa):

  1. Previous previewed text properly cleared.
  2. Previous previewed image not properly cleared (mostly if it was wider than the current one; even more than two images might overlap).
  3. Again: everything works fine until you set --preview-window to a fixed width (say 120): some images (couldn't find any common pattern among these files) are not displayed, and others (no pattern either) cause the items list to get messed up (wrong cursor position, list scrolled upward, and overwritten items). Subtracting 1 from FZF_PREVIEW_LINES fixes the latter (it seems that for some reason the image is printed past the bottom limit of the preview window, causing the described interface quirks).
  4. Chafa feels slower than convert, but I'm not sure about this one.

I'll keep testing though.

EDIT: Some images are not displayed even if --preview-window isn't set at all.

leo-arch avatar Oct 27 '23 05:10 leo-arch

some images (couldn't find any common pattern among these files) are not displayed

I think that has to do with this part of the code, which I'm not sure of its correctness. It's the code that determines how many lines are used to display the image. And the problem will likely go away if we change ceil to floor. But I'm not sure yet if it's the right thing to do.

https://github.com/junegunn/fzf/blob/ec208af474fa3eadad5779c3d30db6d2db300932/src/terminal.go#L2031-L2034

diff --git a/src/terminal.go b/src/terminal.go
index 918c6e0..7cfc8d2 100644
--- a/src/terminal.go
+++ b/src/terminal.go
@@ -2029,8 +2029,8 @@ Loop:
 				if isSixel {
 					t.previewed.wipe = true
 					if t.termSize.PxHeight > 0 {
-						rows := util.Max(0, strings.Count(passThrough, "-")-1)
-						requiredLines = int(math.Ceil(float64(rows*6*t.termSize.Lines) / float64(t.termSize.PxHeight)))
+						rows := strings.Count(passThrough, "-")
+						requiredLines = int(math.Floor(float64(rows*6*t.termSize.Lines) / float64(t.termSize.PxHeight)))
 					}
 				}
 

junegunn avatar Oct 27 '23 05:10 junegunn

I've found that --height is also involved in this issue, but not sure exactly how.

It's the code that determines how many lines are used to display the image. And the problem will likely go away if we change ceil to floor

That's most likely the issue considering what I've seen so far. I'm willing to test any changes you consider appropriate. Just let me know.

leo-arch avatar Oct 27 '23 05:10 leo-arch

Sadly, it makes no difference at all.

EDIT: I can reproduce the issue by setting --height (and leaving --preview-window completely unset) to different values: at some point near the total amount of terminal lines, the image won't be displayed anymore. I guess this confirms your suspicion: requiredLines and/or rows are not getting the right values.

leo-arch avatar Oct 27 '23 05:10 leo-arch