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

Experimental Sixel and embedded graphics support. #1940

Closed
wants to merge 10 commits into from

Conversation

PerBothner
Copy link
Contributor

This is an experimental prototype for Sixel images and potentially other embedded graphics. This is definitely work-in-progress. I've been testing this with the DomTerm embedding but it should probably work for other embeddings. You can then cat images/*.six from libsixel.

This is logically two parts:

  • Support for ElementInset objects, which attach an HtmlElement to the view. This is actually the more interesting and useful (and unfinished) part.

  • Parsing Sixel graphics, using the node-sixel library written by @jerch , converting that to an Image, which is then embedded in an ElementInset. For this prototype I just copied node-sixel to the src/sixel-image.ts - it should probably go elsewhere.

Scrolling is handled by making the ElementInset element be children of the _viewPortScrollArea.

Likecycle management (disposing) and trim are missing. There is also missing handling for inserting/deleting lines: Insert/delete before the image should adjust the position. We also need to delete the image when the owning line(s) are deleted. (That would allow sixel images to be over-written.)

Longer term I think this would be cleaner by allocating a single "tall line" for the image, instead of multiple regular lines - see discussion issue #1901. However, that depends on various issues, including smooth scrolling (issue #1140).

Comments/suggestions appreciated.

@PerBothner
Copy link
Contributor Author

The prototype is getting close to usable now.

@PerBothner
Copy link
Contributor Author

Quake (using SDL-sixel) runs pretty well.

@AndrienkoAleksandr
Copy link
Contributor

AndrienkoAleksandr commented Feb 20, 2019

Quake (using SDL-sixel) runs pretty well.

I guess you mean Guake.

@PerBothner
Copy link
Contributor Author

"I guess you mean Guake."

No, I mean Quake, the game. Specifically sdlquake using sdl-sixel.

You can download a pre-built (Linux 64-bit) binary from here. (Select one of the other files. Then you get a 3-dots menu, and you can click "Download as zip".) This works (at reasonable speed) with my patches (tested in my DomTerm+xterm.js prototype).

@AndrienkoAleksandr
Copy link
Contributor

@PerBothner It would be nice if you record video with this feature, convert to diff and attach to this pr.

@PerBothner
Copy link
Contributor Author

I implemented the DomTerm OSC '72' escape sequence to embed general HTML:

xterm-image

There is no "scrubbing" of the HTML, but of course that can and should be done. (DomTerm has a scrubber that could be ported.)

The screenshot shows a timing glitch with the calculation of the image height: The first time the image is inserted the image height (clientHeight) is reported as 0, which means we don't allocate enough vertical space for it. The second time we do get the correct height. This is somewhat mysterious to me; I'm guessing adding some kind of timer may be needed.

I think the best fix will be to support variable-height lines (as in issue #1901), which is tied in to smooth scrolling (issue #1140) which is gated by webgl. But once we have real variable-height lines then we can also support deferring calculating the height until render-time, and we can support the height changing (based on widow width and magnification).

@Tyriar
Copy link
Member

Tyriar commented Mar 4, 2019

Sorry about the delay, I think the whole team has been busy with other stuff for the past month.

Ultimately I think sixel support is not a priority and should probably be part of an addon instead, this is a lot of code which so few people would actually utilize in the end. In the same vein we wouldn't want to ship OSC 72 as it's non-standard and not well supported by other terminals. While I appreciate the effort, I don't think this is the direction we want to go, at least for now.

Also, after the long discussion in #1901, the team and I developed a roadmap to help keep us focused on what's most important to focus on. The unfortunately truth is that reviewing and merging huge changes like this takes a lot of time that we may not have or that would be better spent on something else.

@Tyriar Tyriar closed this Mar 4, 2019
@Tyriar Tyriar added the reference A closed issue/pr that is expected to be useful later as a reference label Mar 4, 2019
@PerBothner
Copy link
Contributor Author

I created an issue on the freedesktop.org terminal-wg to try to standardize "simple image display". I hope we can get agreement on a suitable semi-portable escape sequence, and that you'd be more willing to accept that.

@Xeddius
Copy link

Xeddius commented Jun 8, 2020

Disappointed to see this was passed over, I very much would like for this to become a standard feature as sixels allow for flexible terminal images and even allows for backwards compatibility with DEC sixels from the old dec terminals.

@Tyriar
Copy link
Member

Tyriar commented Jun 8, 2020

@Xeddius @jerch has been experimenting with this direction some bit more in #2503

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reference A closed issue/pr that is expected to be useful later as a reference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants