Prototype of iTerm2 like image display

[mme]

This is a prototype of adding iTerm2 like image display capabilities to xterm.js, as requested in several tickets:

#614
vercel/hyper#101
microsoft/vscode#44436

Images can also be clicked to be opened in full size in a new window.

It works by slicing the image into patches of scaledCellWidth/scaledCellHeight size and adding the resulting ImageData object directly to the Buffer object. When a cell in the buffer is overwritten, the previous ImageData is thrown away and the memory is freed - e.g. this is useful when showing animated charts by overwriting the previous image.

We are developing an app that would greatly benefit from a cross platform terminal with image display, so I would be quite happy if this feature could make it to xterm.js in one way or another.

Please let me know if I can be of further help to have this feature land in xterm.js!

[Tyriar]

@mme cool!

I have some high level questions before I jump in:

1. How did you get imgcat to work? This one just uses background colors and https://github.com/sindresorhus/term-img is hard coded to only support iterm2.

2. It works by slicing the image into patches of scaledCellWidth/scaledCellHeight size and adding the resulting ImageData object directly to the Buffer object.

   Did you explore any other solutions to the problem like stretching a row to be the size of an image?

3. this is useful when showing animated charts by overwriting the previous image.

   Do gifs work with this implementation?

Here are some notes on how iterm places the image: #614 (comment)

@jerch you'll be interested in the change to CharData:

https://github.com/xtermjs/xterm.js/pull/1577/files#diff-c59b761ad01bc77f0c593de25ec4d847R21

https://github.com/xtermjs/xterm.js/pull/1577/files#diff-c59b761ad01bc77f0c593de25ec4d847R15

I think this is going the opposite direction for memory of the overall buffer to what we want (an extra pointer for every character).

[Tyriar]

Managed to get it working on Linux by commenting out the support if blocks in term-cli:

First bug spotted when changing font family:

Not sure there's an easy way to fix this with the way that it currently works, what if instead of each cell having a reference to an IRenderable there is a single link somewhere indicating a row has an image? That would allow changing the font size to be done and the number of rows to expand/contract dynamically?

[jerch]

Yupp thats a really cool feature addon, also Im kinda surprised it worked this straight forward. Still I have 2 issues with the current impl:

• To be able to tell in the core lib, if a cell will be covered by image data, the image needs to be processed, which currently introduced a dep to Image. This will make the core/frontend separation more complicated - could we get this done with some "delegation abstraction" (read as interface to get image metrics from internal API xy). This way we keep it flexible, e.g. a standalone offscreen nodejs impl could use it own image processing module while a fullstack impl with frontend part can use the browser's Image class.
• The additional data slot in the term buffer is a problem - it will raise memory consumption alot. Also I think it is a waste of memory and not needed here since most cells never hold image data. Imho it should not applied this way. Question is how to store rarely occuring cell based data efficiently.

[Tyriar]

which currently introduced a dep to Image

@jerch Good point, src/InputHandler.ts shouldn't touch DOM APIs, I guess the the core/ should store the base64 string and ui/ should own the HTMLImageElement.

@mme to give you more context on this, we're moving towards this architecture:

#1507 (comment)
#1507 (comment)

Question is how to store rarely occuring cell based data efficiently.

@jerch Well the IMarker APIs were added to track rows so maybe there could be a map somewhere containing this info:

inlineImages: { [markerId: number]: SomeImage };

[mme]

@Tyriar @jerch thanks for your in depth comments!

@Tyriar regarding your initial questions:

1. imgcat just emits the special iTerm2 1337 escape sequence which are picked up in InputHandler.displayImage() where the base64 image is decoded.

2. I did not explore any alternative solutions yet. How would stretching rows work? Would it still be possible to write characters on top of the image when stretching?

3. GIFs do not work, allthough it would be easy to display just the first frame of the GIF using this approach. But I think its absolutely doable to add code to update animated cells periodically in the canvas. This would be very cool too, there is another library which displays matplotlib animations in the terminal (https://github.com/daleroberts/itermplot)

Regarding the bug when changing the font size and @jerch remarks about the memory usage of the additional slot in buffer: I agree that having the image directly in buffer is not an ideal solution. Slicing the image is probably also not flexible enough.

The reason I chose this initial approach is that I want to support clearing up the memory used by the image once it is not visible on the screen. In our specific use case we are developing a deep learning toolkit and want to display an animated chart while training a model, by moving the cursor up, overwriting the previous chart.

When we keep the image outside of the buffer, what would be the best way of determining if the image has been completely overwritten by either text or other images, so it can be thrown away?

I see that IMarker is an IDisposable, can you explain how these classes can be used?

Reading the image through a delegate of some kind should be no problem. I have read through your plans for the new architecture which seems clear, but I think I need some guidance on where to put this abstraction.

One issue which made implementing this a little more difficult was that loading an Image is not synchronous on Chrome and Firefox, even when the image src is a data url. That's why we need to read the image size manually to determine the area covered by the image, since I think OSC handlers are expected to be finished once the handler function returns. We then put placeholders which are replaced once the image has loaded.

Is there a way to tell InputHandler to somehow pause processing until the asynchronous onload event is fired and we get the image parsed by the browser?

[jerch]

Is there a way to tell InputHandler to somehow pause processing until the asynchronous onload event is fired and we get the image parsed by the browser?

Eww, thats kinda impossible since down the road from InputHandler.parse to the fully applied state changes xterm.js is synchronous (most if not all events are sync too). You are kinda asking for a conditional WAIT statement within a big block of sync code, a thing that JS does not support by any means (since it would block the one and only main thread). To get this done the event loop would have to step in by promisifying or applying the async/await syntatic sugar to all code (entering async stuff is viral, all stuff caller upwards will be pulled into this paradigm).

[mme]

@jerch I see. This not a big deal since the current implementation deals with this already.

[Tyriar]

Thanks for the effort on this but I don't think we'll be moving forward with this any time soon. I'd say we should hold off on supporting this until we get more of the current stories finished up, have a good plan on how to handle varying height rows and we're sure we want to commit to supporting this (is it a security issue for apps being able to add tracking images?).

[j-f1]

is it a security issue for apps being able to add tracking images?

I’d say no, since they could just make a tracking request using normal HTTP APIs.