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 graphics should be ignored unless supported #120

Closed
mobluse opened this issue Feb 21, 2018 · 10 comments · Fixed by #6328
Closed

Sixel graphics should be ignored unless supported #120

mobluse opened this issue Feb 21, 2018 · 10 comments · Fixed by #6328
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@mobluse
Copy link

mobluse commented Feb 21, 2018

  • Your Windows build number: (Type ver at a Windows Command Prompt)
    Microsoft Windows [Version 10.0.17101.1000]

  • What you're doing and what's happening: (Copy & paste specific commands and their output, or include screen shots)
    I run a file containing this (you could also just paste the three lines starting with printf):

#!/bin/bash
printf '\ePq#1NNNN#2NNNN#3NNNN$oooo'\
'#1oooo#2oooo-#3BBBB#1BBBB#2BBBB${{{{'\
'#3{{{{#1{{{{????\e\\'

The result is that it prints:
q#1NNNN#2NNNN#3NNNN$oooo#1oooo#2oooo-#3BBBB#1BBBB#2BBBB${{{{#3{{{{#1{{{{????

  • What's wrong / what should be happening instead:
    Since ConHost doesn't support sixel graphics (at this time) it should be ignored and nothing should be printed. It should be as if the entire output of the program was sent to /dev/null.

"Non-graphics terminals generally silently ignore sixel escape sequences." -- https://en.wikipedia.org/wiki/Sixel

In WSLTTY (that supports sixel graphics) the program prints a 3x3 red, green and blue colored square:
brg
gbr
rgb

xterm in WSL Ubuntu with VcXsrv ignores sixel graphics unless started as a VT340 terminal: xterm -ti vt340, then it prints the colored square.

@WSLUser
Copy link
Contributor

WSLUser commented Mar 2, 2018

Since we're mentioning Sixel here, there's a UserVoice to get Sixel graphics support for ConHost here Recommend upvoting it to gain priority.

@zadjii-msft
Copy link
Member

I guess I didn't directly respond about sixels in microsoft/WSL#1099 - It'd be cool, it's on the backlog.

@zadjii-msft zadjii-msft added backlog Product-Conhost For issues in the Console codebase labels Mar 5, 2018
@zadjii-msft zadjii-msft added this to the Backlog milestone Mar 5, 2018
mikemaccana pushed a commit to mikemaccana/console that referenced this issue May 16, 2018
@oising
Copy link
Collaborator

oising commented Apr 10, 2019

I should leave this here for stopgap graphical fun, although the performance of conhost makes it very painful: https://hpjansson.org/chafa/

@miniksa
Copy link
Member

miniksa commented Apr 10, 2019

Whoa whoa whoa. You can't just rag on conhost's performance without giving me a repro @oising. :P I work pretty dang hard to optimize things when I have a concrete hot path to work on.

@oising
Copy link
Collaborator

oising commented Apr 10, 2019

@miniksa -- you're right, I'm sorry. I'm on your side, but I was definitely way too terse here. That links to a super fun tool for emitting VT for animating gifs/pngs/mp4s etc to a terminal. I think you're doing an outstanding job of adding value without making conhost worse, but until the old GDI+ crud is ripped out, it'll never compete with xterm/alacritty/sakura/(insert GL/SDL powered terminal here.)

That said, here's your repro (WSL):

  1. sudo apt install chafa
  2. curl https://media.giphy.com/media/12UwsVgQCYL3H2/giphy.gif --output winanim.gif
  3. chafa winanim.gif --font-ratio 1/3

Now fire up X410, VcVxsrv, etc, spawn an xterm or a fancy accelerated shell like sakura and try again.

I've attached two short vids (one sakura over x410), the other wsl on conhost (18362.53).

sakura 2019-04-10 14-31-22.zip
Ubuntu 18.04 2019-04-10 14-33-05.mp4.zip
xterm 2019-04-10 14-38-29.zip

Vids recorded with window 10 game bar (win+g).

edit: added plain old xterm. I'm on a suface book i7 btw. Even xterm smokes conhost.

@oising
Copy link
Collaborator

oising commented Apr 10, 2019

I'd be curious to know how much of this difference is down to WSL's known problems with I/O, though I assumed that was mainly disk writes - but with the architecture of linux being all files and sockets, perhaps it's part of the problem. There's no win32 native version of Chafa, at least nothing comparable that I know of. I can't imagine it being much of a blocker though - what's the throughput here for animating with text? It's gotta be absolutely pitiful compared to running npm install ;)

@miniksa
Copy link
Member

miniksa commented Apr 11, 2019

OK. I did a WPR trace of what's going on with chafa and we should make a new issue. Going to do that and come back and link it here.

See #410

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 17, 2019
@miniksa miniksa added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. labels May 29, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 29, 2019
@ghost ghost added the In-PR This issue has a related PR label Jun 3, 2020
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Aug 17, 2020
DHowett pushed a commit that referenced this issue Aug 17, 2020
As the title suggests, this commit adds initial support for the VT DCS
sequences. The parameters are parsed but not yet used. The pass through
data is yet to be handled. This effectively fixes #120 by making Sixel
graphics sequences *ignored* instead of printed.

* https://vt100.net/docs/vt510-rm/chapter4.html
* https://vt100.net/emu/dec_ansi_parser

Tests added.

References #448
Closes #120
@ghost
Copy link

ghost commented Aug 26, 2020

🎉This issue was addressed in #6328, which has now been successfully released as Windows Terminal Preview v1.3.2382.0.:tada:

Handy links:

@voronoipotato
Copy link

Is there an issue for tracking sixel support?

@zadjii-msft
Copy link
Member

Yep, #448

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants