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

Sixel preview are not interpreted, but displayed as plain text, #675

Closed
adrienbarlier opened this issue Jul 30, 2021 · 26 comments
Closed

Sixel preview are not interpreted, but displayed as plain text, #675

adrienbarlier opened this issue Jul 30, 2021 · 26 comments
Labels

Comments

@adrienbarlier
Copy link

Hi,
Is there anything in particular to do in order to let lf display sixel preview ?

WM: Sway
Terminal: Foot
displaying preview with chafa, options in pictures

2021-07-30_17-07-1627658876
2021-07-30_17-07-1627658926

@gokcehan
Copy link
Owner

@adrienbarlier Short answer, sixel images are not working in lf.

Long answer, we interpret escape codes ourselves for the previews instead of pushing them directly to the terminal. Currently we only have standard color escape codes and other things are ignored. At some point, I thought about pushing unknown escape codes directly to the terminal so things like sixels might work. However, sixel images were not working properly with tcell when I tried it on xterm. Things might be different on foot terminal. There's also some complication for such escape codes as previews are stored as arrays of lines and escape codes may not obey that model. I have also seen some unmerged sixel patches on tcell side which may be of interest.

@adrienbarlier
Copy link
Author

Thank you for this quite clear answer @gokcehan ,

"we interpret escape codes ourselves for the previews instead of pushing them directly to the terminal"

I am curious to understand this behavior more deeply, in which section of your work can i see the code responsible of that please ?

@gokcehan
Copy link
Owner

gokcehan commented Aug 1, 2021

@adrienbarlier You can take a look at win.print method in ui.go which further calls applyAnsiCodes in colors.go.

@elmarsto
Copy link

+1 on this, still very interested.

@cain-dev
Copy link

Can i haz some sixels? Please! ç.ç

@horriblename
Copy link
Contributor

horriblename commented May 17, 2022

The sixel library for tcell seems to be abandoned, so I think the only we can do sixel rendering is directly writing to stdout/stderr, for reference, photon by ghost08 (the only tcell based project I can find that uses sixel) writes directly to stdout. So I wrote some code to test if it would work on lf, and it did. From what I can tell, it doesn't interfere with the rest of the application.

Screenshot and previewer script on the right, tested on foot and konsole in wayland.

sixel-demo

Here's a snippet that I added to win.print in ui.go:

		if r == gEscapeCode && i+1 < len(s) && s[i+1] == 'P' {
                       // idk how to match the closing pattern `ESC \` so I'm not using j below
			j := strings.IndexAny(s[i+1:len(s)], "\x1b")
			if j == -1 { 
				continue
			}
			j ++

			// ANSI sequence that moves cursor to where we want
			moveCursor := fmt.Sprintf("\033[%d;%dH", win.y+y, win.x+x)
			os.Stdout.WriteString(moveCursor)
			os.Stdout.WriteString(s)

			i += j
			continue
		}

Now we just need to somehow clean up the sixel image afterwards, which should be doable. For this we need to calculate the area being drawn on (the photon project I mentioned might leave some clue), or we could expose some kind of command to let the user clean up sections of the pane manually. or worst case scenario redraw the whole pane. I'll take a look and hopefully it'll work out


Possibly helpful:
cleaning up after sixel graphics
photon, RSS reader - seriously, the app is really cool, and the imgproc library might solve all our problems

@cain-dev
Copy link

I tried with sptv and lfimg to get this to work using chafa -f sixel but i noticed that the lf just shows the string.

I think that showing images with sixel protocol and some kind of persistent cache for the converted image without the need to convert things everytime would make this veeery fast... My pc is too slow plz help

and thanks for the contribution btw

@horriblename
Copy link
Contributor

I tried with sptv and lfimg to get this to work using chafa -f sixel but i noticed that the lf just shows the string.

To clarify, I made some changes to lf itself for it to work, I'll upload the code later, it's getting late...

I think that showing images with sixel protocol and some kind of persistent cache for the converted image without the need to convert things everytime would make this veeery fast... My pc is too slow plz help

lf already caches the output of your preview panel and I can confirm that it works as intended with sixel.

Also, we would probably need to increase the bandwidth on the previewer-reader, I keep getting this error when I use a larger size of sixel image or a more colorful image. Not sure if it's possible though.

2022/05/17 02:10:56 loading file: bufio.Scanner: token too long

@cain-dev
Copy link

"I'll upload the code later, it's getting late..." take your time xD
If isn't asking too much now what previewer are you using?

@horriblename
Copy link
Contributor

I use this config
https://github.com/cirala/lfimg

@cain-dev
Copy link

Got it, same here. I am so anxious to see this implemented xD

@dhasial
Copy link

dhasial commented May 22, 2022

Same here, god I love lf !

@AtomicFS
Copy link

AtomicFS commented Apr 1, 2023

Hi there, is there any chance of implementing sixel in near future? Maybe merging @horriblename 's fork?

I love lf and I would love to see it support sixel.

@horriblename
Copy link
Contributor

for reasons mentioned in the PR, the fork won't be merged

@AtomicFS
Copy link

AtomicFS commented Apr 3, 2023

I feared that would be the case, kinda hoped it changed since then.

@ilyagr
Copy link
Collaborator

ilyagr commented Apr 3, 2023

@horriblename If you like, you can add your fork or patch to https://github.com/gokcehan/lf/wiki/Patches-and-community-forks.

@AtomicFS
Copy link

AtomicFS commented Apr 3, 2023

I know this is probably not the right place, but @horriblename the fork is also rather out-of-date, do you still maintain it and do you plan to update it?

@horriblename
Copy link
Contributor

yeah I still intend to maintain it, but I just update occasionally and forgot to do it lately, thanks for reminding me lol

@ilyagr thanks for mentioning the fork section, I'll add it soon

@DusanLesan
Copy link

Regarding @horriblename fork, I see it was declined because of the maintaining could be difficult. Since I see most of the code are new functions and not much changes, would it be possible to have it as a plugging/optional package or conditionally compiled? That would make it easy to switch off/remove it is makes any problems.

@gokcehan
Copy link
Owner

gokcehan commented Apr 8, 2023

@DusanLesan I have taken another look at the PR and unfortunately I still think this is a non-trivial patch. Specifically, it seems to integrate heavily into nav.preview. I guess it might be possible refactor out some of this complexity. For example, it might be worth to add a new file sixel.go and move all sixel related additions to that file. Maybe a new boolean option sixel could be added to keep the existing behavior as it is. If @horriblename decides to update the fork and send it as a PR again, I will let the contributors review the patch and make a decision without my involvement this time.

@DusanLesan
Copy link

@gokcehan Ok. I do not feel strongly about sixel graphics format support as I do not actually need it myself. It just looks like important feature to users wanting to switch to wayland. It looked trivial to me since changes in nav.go looked like mostly additions that might be circumvented by few if statements or compilation options. But I do not know go and whatever you guys decide, it is ok by me

@horriblename
Copy link
Contributor

I'll try to smooth out the code a bit and make a PR soon

@joelim-work
Copy link
Collaborator

No longer an issue, Sixel previews are now officially supported as of dd82949

@Lassebq
Copy link

Lassebq commented Aug 27, 2023

Running e0abb29, previews don't work.
Terminal: foot, konsole
Using chafa, just like the wiki mentions it

@horriblename
Copy link
Contributor

Running e0abb29, previews don't work.
Terminal: foot, konsole
Using chafa, just like the wiki mentions it

can you elaborate? I'm guessing you didn't add set sixel to your config

@Lassebq
Copy link

Lassebq commented Aug 28, 2023

That was it 🤦‍♂️ thanks

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