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

No sixels in st terminal emulator with sixel patch #1783

Closed
schrmh opened this issue Jun 17, 2021 · 10 comments · Fixed by #2253
Closed

No sixels in st terminal emulator with sixel patch #1783

schrmh opened this issue Jun 17, 2021 · 10 comments · Fixed by #2253
Assignees
Labels
bug Something isn't working
Milestone

Comments

@schrmh
Copy link

schrmh commented Jun 17, 2021

  • export | egrep 'LANG|LC_CTYPE|TERM'
COLORTERM=yes
CSF_LANGUAGE=us
LANG=de_DE.UTF-8
LANGUAGE=de_DE
LESS_TERMCAP_mb=$'\C-[[01;31m'
LESS_TERMCAP_md=$'\C-[[01;31m'
LESS_TERMCAP_me=$'\C-[[0m'
LESS_TERMCAP_se=$'\C-[[0m'
LESS_TERMCAP_so=$'\C-[[01;44;33m'
LESS_TERMCAP_ue=$'\C-[[0m'
LESS_TERMCAP_us=$'\C-[[01;32m'
TERM=st-256color
  • notcurses version (available from notcurses-demo i)
notcurses 2.3.4 by nick black et al on simple terminal
  • terminal name + version
https://github.com/galatolofederico/st-sixel (commit hash 808320d2b18d559c1a162428d40ec4af842b461a)

When I execute notcurses-demo within st-sixel (st with sixel patch then it does not make use of sixel.
Tho even the popular lsix claims that the terminal emulator has no sixel support but it definitely has sixel support (my own https://github.com/schrmh/pdfgrepSIXEL works..).

@schrmh schrmh added the bug Something isn't working label Jun 17, 2021
@dankamongmen
Copy link
Owner

So I looked into this yesterday, and am not sure it's possible to detect. #1773

basically, st doesn't allow itself to be discovered via answering any of the standard queries, and it does not define any of its own. it also fails to respond to any of the standard sixel queries =[

@dankamongmen
Copy link
Owner

i wanted to file an issue against the patch, but the repo doesn't seem to accept PRs or even issues =[ this is why lsix is denying that it supports sixel, as well.

@dankamongmen
Copy link
Owner

i can't use TERM=st-256color either, since i presume that would be set on st without this patch. is there any reason why this patch hasn't been merged into st?

@schrmh
Copy link
Author

schrmh commented Jun 18, 2021

I guess sixel support is not in the standard st cause of the suckless philosophy. People that want such a thing have to apply patches themselves.

So I guess the options here are:

  • a proper patch that can be applied to st (originally the sixel patch for st was created by @saitoha so it could be mentioned in the comments there) so that it sends an appropriate response (which would then also make lsix work)
  • an "easy" (but I guess not preferable) patch that would just add a custom TERM variable (or something else) that would be dictated by the notcurses project so that it can detect a st terminal emulator with a sixel patch.
  • an option in notcurses so that no sixel compability checks would be performed (don't know what downsides that would bring).

Maybe I'm missing something. Patching st would not be that much of a problem; I'm not bound to that specific st fork — it just had a sixel patch already applied and it did not get rid of older sixel images (in contrast to xterm or mlterm).

@schrmh
Copy link
Author

schrmh commented Jun 18, 2021

Okay so through hackerb9/lsix#32 I found bakkeby/st-flexipatch#7. A few things regarding the situation with st and sixel are explained in both issues and it seems like there was even some experimenting with sixel on the standard st last year..

This st-flexipatch is actually a up-to-date st fork with an updated sixel patch for the current st version 0.8.4 (tho it seems to have a problem that sixels won't vanish after clear and that they get moved up and can't be reached again via scrolling when the scrollback patch is enabled..).
Even lsix works as intended and doesn't complain with that one (btw. lsix also checks a LSIX_FORCE_SIXEL_SUPPORT variable that users could in case that they use a terminal emulator that has sixel support that is not detected..).
But for some reason notcurses still won't display the dolphin / won't use sixel.

export | egrep 'LANG|LC_CTYPE|TERM' of st-flexipatch:

COLORTERM=yes
CSF_LANGUAGE=us
LANG=de_DE.UTF-8
LANGUAGE=de_DE
LESS_TERMCAP_mb=$'\C-[[01;31m'
LESS_TERMCAP_md=$'\C-[[01;31m'
LESS_TERMCAP_me=$'\C-[[0m'
LESS_TERMCAP_se=$'\C-[[0m'
LESS_TERMCAP_so=$'\C-[[01;44;33m'
LESS_TERMCAP_ue=$'\C-[[0m'
LESS_TERMCAP_us=$'\C-[[01;32m'
TERM=st-256color

so it doesn't seem like there is a difference regarding this.

If I interpret this commit correctly then they actually implemented those needed sequences that notcurses could check: bakkeby/st-flexipatch@8f79391

@dankamongmen
Copy link
Owner

If I interpret this commit correctly then they actually implemented those needed sequences that notcurses could check: bakkeby/st-flexipatch@8f79391

did you try it out with this? if it responds to an XTSMGRAPHICS query, which i issue to all terminals, Notcurses ought automatically support sixel with it.

try running notcurses-info with this one. you ought see a line like "Supports sixel graphics with N color registers". if so, it ought work!

@schrmh
Copy link
Author

schrmh commented Jun 20, 2021

try running notcurses-info with this one. you ought see a line like "Supports sixel graphics with N color registers". if so, it ought work!

Warning: already added escape (got 0x10000c, wanted 0x10007b)
Warning: already added escape (got 0x100009, wanted 0x10007c)
Warning: already added escape (got 0x10000b, wanted 0x10007d)
Warning: already added escape (got 0x10000a, wanted 0x10007e)

 notcurses 2.3.4 by nick black et al on simple terminal
  52 rows (14px) 160 cols (7px) (130,00KiB) 48B crend 256 colors
  compiled with gcc-11.1.0, 16B little-endian cells
  terminfo from ncurses 6.2.20200212
  avformat 58.76.100 avutil 56.70.100 swscale 5.9.100

 Warning! Colors subject to https://github.com/dankamongmen/notcurses/issues/4
  Specify a (correct) TrueColor TERM, or COLORTERM=24bit.


 Colors: 256 rgb: n ccc: y setaf: y setab: y
 sgr: y sgr0: y op: y fgop: y bgop: y
 rows: 52 cols: 160 rpx: 14 cpx: 7 (728x1120)
 didn't detect bitmap graphics support
 UTF8: y quad: n sex: n braille: y images: y videos: y
   Halves { ▀▄█}   Quads { ▘▝▀▖▌▞▛▗▚▐▜▄▙▟█}
 Sextants ⎧ 🬀🬁🬂🬃🬄🬅🬆🬇🬈🬊🬋🬌🬍🬎🬏🬐🬑🬒🬓▌🬔🬕🬖🬗🬘🬙🬚🬛🬜🬝⎫
          ⎩🬟🬠🬡🬢🬣🬤🬥🬦🬧▐🬨🬩🬪🬫🬬🬭🬮🬯🬰🬱🬲🬳🬴🬵🬶🬷🬸🬹🬺🬻█⎭
  Braille {⠀⢀⢠⢰⢸⡀⣀⣠⣰⣸⡄⣄⣤⣴⣼⡆⣆⣦⣶⣾⡇⣇⣧⣷⣿}
 Lower ⅛s { ▁▂▃▄▅▆▇█}
 background isn't interpreted as transparent
 cup: y vpa: y hpa: y

At this point I only enabled the sixel and some of the scrollback patches. I guess I could enable the alpha patch to get rid of at least background isn't interpreted as transparent but I don't think this will solve the problem that sixels don't get displayed by notcurses..

@dankamongmen
Copy link
Owner

yeah the " didn't detect bitmap graphics support" means it didn't get an answer to the necessary queries =[

thanks for helping test. i'll come back to this as soon as i can, grab the code you pointed to, and see what exactly is missing, if you think they'll take a patch.

@dankamongmen
Copy link
Owner

It looks like you might finally get your wish, @schrmh; see #2229.

@dankamongmen dankamongmen added this to the 3.0.0 milestone Oct 9, 2021
@dankamongmen dankamongmen self-assigned this Oct 9, 2021
@dankamongmen
Copy link
Owner

ok, i think this ought work now! we interpret a DA1 with a 4 as advetising 256-creg Sixel in the absence of superior XTSMGRAPHICS information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants