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

Images preview support for terminals witch sixel #2544

Closed
timsofteng opened this issue Jun 25, 2021 · 55 comments
Closed

Images preview support for terminals witch sixel #2544

timsofteng opened this issue Jun 25, 2021 · 55 comments
Assignees
Labels

Comments

@timsofteng
Copy link

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

@leo-arch
Copy link

leo-arch commented Jul 3, 2021

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

@PJungkamp
Copy link

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!

@dnkl
Copy link

dnkl commented Jul 4, 2021

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).

@timsofteng
Copy link
Author

@junegunn any thoughts about this issue?

@guilhermeprokisch
Copy link

I'm also interested in ways to do that.

@AXGKl
Copy link

AXGKl commented Apr 15, 2022

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/

@ghost
Copy link

ghost commented Nov 23, 2022

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

@niksingh710
Copy link

Is there any thoughts given on this?

@junegunn
Copy link
Owner

@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.

@emdash
Copy link

emdash commented Sep 14, 2023

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.

@junegunn junegunn reopened this Oct 22, 2023
junegunn added a commit that referenced this issue Oct 22, 2023
@junegunn
Copy link
Owner

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.

@junegunn junegunn self-assigned this Oct 22, 2023
@niksingh710
Copy link

niksingh710 commented Oct 23, 2023

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.

@aeghn
Copy link

aeghn commented Oct 23, 2023

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)

preview.webm

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.

@junegunn
Copy link
Owner

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.

@aeghn
Copy link

aeghn commented Oct 23, 2023

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

@hashworks
Copy link

and it works on tmux, unlike Sixel

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

@aeghn
Copy link

aeghn commented Oct 23, 2023

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 #3479 (comment).

@niksingh710
Copy link

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

junegunn added a commit that referenced this issue Oct 25, 2023
Progress:

* Sixel image can now be displayed with other text, and is scrollable
* If an image can't be displayed entirely due to the scroll offset, fzf
  will render a wireframe to indicate that an image should be displayed
* Renamed $FZF_PREVIEW_{WIDTH,HEIGHT} to $FZF_PREVIEW_PIXEL_{WIDTH,HEIGHT}
  for clarity
* Added bin/fzf-preview.sh script to demonstrate how to display an image
  using Kitty or Sixel protocol

An example:

  ls *.jpg | fzf --preview='seq $((FZF_PREVIEW_LINES*9/10)); fzf-preview.sh {}; seq 100'

A known issue:

* If you reduce the size of the preview window, the image may extend
  beyond the preview window
@junegunn
Copy link
Owner

So here's some progress:

sixel-scroll.mov

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.

@leo-arch
Copy link

leo-arch commented Oct 25, 2023

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.

@junegunn
Copy link
Owner

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

@leo-arch
Copy link

leo-arch commented Oct 26, 2023

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.

@aeghn
Copy link

aeghn commented Oct 29, 2023

I've found that all of them fail to produce a sixel image of some images at particular resolutions

@leo-arch May I ask if you could upload those images?

@leo-arch
Copy link

Here's one of the images I'm using for testing:

bluhd-lion

Chafa, for example, can display this at 150x19, but fails with 150x40 (my actual term size), 150x30, 130x40, 130x30, 110x20, and more. Same for convert and img2sixel (though they may fail at different resolutions).

@leo-arch
Copy link

leo-arch commented Oct 29, 2023

Tried with mltem, and it works great! The problem seems to be related to xterm (I'm using version 388).

EDIT: I still need to subtract 1 from FZF_PREVIEW_LINES to prevent the interface from becoming crazy. Here's my chafa command:

chafa -f sixel -s ${FZF_PREVIEW_COLUMNS}x$((FZF_PREVIEW_LINES - 1)) {}

This is required when you add a border to the preview window, say --preview-window=border-left (it seems to happen only with border-left, border-right, and border-vertical). I guess it would be easy to make fzf inform 1 line less in FZF_PREVIEW_LINES when one of these preview window borders is set.

junegunn added a commit that referenced this issue Oct 29, 2023
@junegunn
Copy link
Owner

It looks like we were not on the same page. Reverted the height calculation. Use math.Ceil instead of math.Floor; i.e. if the calculated number of required lines is something like 25.88, fzf will regard it as 26 lines.

I still need to subtract 1 from FZF_PREVIEW_LINES to prevent the interface from becoming crazy. Here's my chafa command:

I can't reproduce the problem.

@leo-arch
Copy link

Hey, not a big deal anyway @junegunn, but it's definitely there (at least for me).

@leo-arch
Copy link

leo-arch commented Oct 30, 2023

Here's an image that triggers this behavior (my term is xterm (same thing on mlterm), size 150x40):
1252cd2755b8216d849b2805891dd65a

And here's the command used:

ls | ~/build/fzf/target/fzf-linux_amd64 --preview='~/build/fzf/bin/fzf-preview.sh {}' --preview-window=border-left

@junegunn
Copy link
Owner

@leo-arch I can't reproduce the problem on my iTerm2 with any type of border.

# Press space to change the border style
FZF_DEFAULT_OPTS= fzf --preview 'fzf-preview.sh {}' --bind 'space:change-preview-window(border-left|border-top|border-bottom|border-horizontal|border-vertical|border-double|border-block|noborder)+clear-screen+refresh-preview'

Please post a screenshot (or preferably a screencast) of the situation.

@junegunn
Copy link
Owner

@leo-arch Oh wait, I'm noticing a rendering problem when I move between the selections.

image

@junegunn
Copy link
Owner

junegunn commented Oct 31, 2023

I see the pattern. The issue arises when there is no border at the bottom.

  • border-left
  • border-right
  • border-top
  • border-vertical
  • border-none

@aeghn
Copy link

aeghn commented Oct 31, 2023

@leo-arch Oh wait, I'm noticing a rendering problem when I move between the selections.
image

This is the main problem I have run into in my fork previously, I just remove the bottom booder and decrease the image size. Hope you could find the cause and solve it.

        chafa "$1" -f sixel -s $((${FZF_PREVIEW_COLUMNS}-2))x$(($FZF_PREVIEW_LINES/9*10)) --animate false

23-10-31_12 02 23

@junegunn
Copy link
Owner

Hmm, it looks like there's no simple solution to the problem. The whole screen shifts up by a row as soon as fzf passes the Sixel image data to the underlying terminal. The situation is also reproducible without using fzf.

# The image is shifted up. The cursor is on the last line which is empty.
chafa -f sixel -s ${COLUMNS}x${LINES} long-image.jpg; sleep 5

I guess it would be easy to make fzf inform 1 line less in FZF_PREVIEW_LINES when one of these preview window borders is set.

This might be the only solution at hand, but it's not ideal because fzf can use the last line for other text data just fine. So we're reporting an incorrect value just for Sixel.

# Make sure that the preview window covers the last line
fzf --no-margin --no-padding --no-border --preview-window border-none --preview 'echo -n "$FZF_PREVIEW_LINES / ";seq $FZF_PREVIEW_LINES'

@leo-arch
Copy link

leo-arch commented Oct 31, 2023

This might be the only solution at hand, but it's not ideal because fzf can use the last line for other text data just fine. So we're reporting an incorrect value just for Sixel.

Agreed. So, what about exporting an environment variable and then testing against it to construct the chafa command in fzf_preview.sh? Something like this:

if [ -n "$FZF_PREVIEW_NO_BOTTOM_BORDER" ]; then
    chafa -f sixel -s "${FZF_PREVIEW_COLUMNS}x$((FZF_PREVIEW_LINES - 1))" "$file"
else
    chafa -f sixel -s "${FZF_PREVIEW_COLUMNS}x${FZF_PREVIEW_LINES}" "$file"
fi

I know, it's far from elegant and hacky, but should work.

Another alternative (cleaner but less exact) would be to always perform the subtraction: it's not even a noticeable loss after all.

junegunn added a commit that referenced this issue Oct 31, 2023
When a Sixel image touches the bottom of the screen, the whole screen
scrolls up one line to make room for the cursor. Add an ANSI escape
code to compensate for the movement. Unfortunately, the movement of the
screen is sometimes noticeable.

  fzf --preview='fzf-preview.sh {}' --preview-window border-left
@junegunn
Copy link
Owner

278dce9 fixes the issue, at least on iTerm2 where I tested it. As noted in the commit message, the momentary movement of the screen is sometimes noticeable, but I can't think of a better solution.

@leo-arch
Copy link

Using latest revision. Screen movement is noticeable, true, but not annoying at all. However, there are still a few glitches:
screenshot-0

@junegunn
Copy link
Owner

junegunn commented Nov 1, 2023

Do you have any idea under what conditions this happens?

@leo-arch
Copy link

leo-arch commented Nov 1, 2023

What can be seen in the picture is a directory containing cached images of various sizes (from 400x80 to 1920x1360 aprox). The terminal is xterm (version 388, size 150x40). The misbehavior is triggered by a few images (three of them attached) for which no preview is generated (just a black background). The command used is ls | ~/build/fzf/target/fzf-linux_amd64 --preview='~/build/fzf/bin/fzf-preview.sh {}' --preview-window=border-left
3e0c62d1580d0197cdd4bcbce22c792f
10fee51ed4141c114e562b1622c079d8
231ecd090a732f71c2624e87beff0fde

This issue is avoided by just reducing FZF_PREVIEW_LINES by 1 (though previews are still not generated).

It's important to note that I've seen images (cannot find the guilty ones right now) overlapping the left border (which is why I normally reduce FZF_PREVIEW_COLUMNS by 2).

@veltza
Copy link

veltza commented Nov 1, 2023

When a Sixel image touches the bottom of the screen, the whole
screen scrolls up one line to make room for the cursor.

Some terminals like Wezterm leave the cursor on the last row of a sixel to avoid this behavior. Because of that, the last commit 278dce9 breaks the UI on Wezterm when using --preview-window border-left.

I think the easiest solution to this is to just use the shorter preview window and leave room for the cursor. Another solution is to use \e[?8452h sequence which leaves the cursor on the right side of the the sixel, but not all terminals support it.

@dnkl
Copy link

dnkl commented Nov 1, 2023 via email

@junegunn
Copy link
Owner

junegunn commented Nov 1, 2023

Some terminals like Wezterm leave the cursor on the last row of a sixel to avoid this behavior. Because of that, the last commit 278dce9 breaks the UI on Wezterm when using --preview-window border-left.

Huh, that was what I was worried about, the inconsistencies between the terminal emulators. I'm going to have to revert the patch then.

I was not completely happy with the result anyway; while scrolling up fixes the whole viewport, the top line is lost.

image

junegunn added a commit that referenced this issue Nov 1, 2023
So that it can determine if it should subtract 1 from $FZF_PREVIEW_LINES
to avoid scrolling issue of Sixel image that touches the bottom of the
screen.
@junegunn
Copy link
Owner

junegunn commented Nov 1, 2023

21ab64e is another take on the issue. We export $FZF_PREVIEW_TOP to the preview command so that it can determine whether it should subtract 1 from $FZF_PREVIEW_LINES or not. Removed the scrolling trick.

\e[?8452h would make things much simpler, but it doesn't seem to work on iTerm2 which I currently use for testing.

@leo-arch
Copy link

leo-arch commented Nov 1, 2023

It works great now. Thanks @junegunn!

@junegunn
Copy link
Owner

junegunn commented Nov 2, 2023

Thank you for your help and feedback. I'm going to close this thread. Please open a new one if you encounter any problems or have any suggestions.

@junegunn junegunn closed this as completed Nov 2, 2023
junegunn added a commit that referenced this issue Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests