Sixel graphics support

[wez]

This issue tracks open items around sixel graphics

• [ ] Correctly respect DECSDM sixel scroll mode changes (we always scroll today)
• [ ] Correctly respect sixel aspect ratio. We just use 2:1 today
• [ ] Figure out what the default color register map should be
• [ ] Find some good test programs/data. So far have been using lady of shallot and some capture files from jexer

[wez]

@klamonte: I'd appreciate your feedback/suggestions on this. Current master has basic sixel support and is able to replay the sixel based captures from the jexer download page, but I'm not sure of the best way to validate this!

[ghost]

Hi @wez, some general items (mostly from mistakes I've made :) ):

• Testing: definitely test against the shipped libsixel images (snake.six), animations (img2sixel), and 'lsix'. On animations: it would be ideal to match xterm's animation speed (I can't quite pull it off on my Swing backend, but it's only a little slower.) 'lsix' testing is good because it rounds out responses to queries.

• If you don't already, add response to "OSC ? 10" / "OSC ? 11", used by lsix to determine current text foreground/background colors.

• Consider adding the response to "CSI ? 1;1;0S" (read sixel number of color registers). Don't know if you want to go further and permit an application to set the number of registers, xterm will top out at 1024 no matter what.

• It's up to you how many palette colors you want to max out at. I've found that 256-1024 is pretty good, but oddly going past 1024 seems to make banding/artifacts worse especially on photos (at least for Jexer's simple Floyd-Steinberg dithering).

• Raster attributes: (and this is an active bug I think in my own widget) An application can specify a rectangle with no image data, and get said rectangle on screen.

• Consider whether or not or how to limit the amount of image data that will persist in scrollback. I took the cheap route for my widget and just purge everything above a few screenfuls in the scrollback, mintty probably has a better thought out scheme.

• Pixels-overlapping-text: sixel is supposed to do that as per spec, but in practice no one uses it that way right now. IMHO it would be perfectly acceptable to make text and image data mutually exclusive in a cell if that makes the implementation easier.

It'll be exciting to see more sixel support out there! :)

[yoichi]

WezTerm-macos-20201101-103216-403d002d crashes with following command. Is there some error handling issue?

printf "\x1bPqh"

[ModProg]

Maybe in the future other Escape Codes could be implemented as well, for example kitty jumps in my mind for being quite sensible Kitty . Some thing like iTerm2 or Terminology is probably less interesting.

[wez]

The kitty protocol is complex to implement and model and I have no plans to do that at this time.

wezterm has pretty solid implementations of both sixel and the iTerm2 image protocols already. Sixel has broad application level support, and iTerm2's protocol has growing adoption.

[ModProg]

The kitty protocol is complex to implement and model and I have no plans to do that at this time.

Maybe that would be something that could happen some time in the future, one thing that is a bit annoying with sixel is not being able to display imges in cells, but Kitty's protocol is very involved, that is true.

[ghost]

DECSDM appears to be interpreted incorrectly by xterm, such that using it can break behavior on terminals that were tested against real hardware. Credit to @j4james for documenting this: https://github.com/hackerb9/lsix/issues/41 .

[j4james]

FYI, regarding the DECSDM issue, MLTerm has just accepted a PR to switch its DECSDM implementation to match XTerm. Apparently Thomas was well aware that XTerm was doing it incorrectly but had chosen to keep it broken anyway, so MLTerm has now decided they are better off just matching XTerm's broken behaviour. As of now, I'd estimate there are more broken DECSDM implementations than working ones. I hate everything about this.

[ghost]

I'm surprised Dickey didn't just add a resource to swap the behavior (it's literally 'h' vs 'l'), with the default being to keep doing what it does now. Thing is that xterm is a very reasonable replacement for real hardware, and there aren't too many of those around. But TERM=xterm does imply matching what 'xterm' does first and foremost, even if that has some mistakes compared to what 'xterm' is trying to copy.

[j4james]

But TERM=xterm does imply matching what 'xterm' does first and foremost

In this case though, you've literally got to tell XTerm to switch to a VT340 mode (or one of the other graphics terminals) in order to get any sixel functionality. There's definitely an implication that it is actually emulating a VT340 rather that its own sixel variant, so this behaviour just seems bizarre to me. That said, XTerm's sixel implementation was never really usable as a VT340 emulator anyway (like most other open source terminals), so I suppose one more bug doesn't really matter.

[j4james]

For anyone not following along on the lsix issue, Thomas has just commented to say that he would "wait til you've investigated the VT340", which I take to mean he is willing to update XTerm's behaviour if it can clearly be shown to be incorrect. I'm not sure what problem he had with the previous evidence, but I'm just happy he is still open to be persuaded, and there's a chance we may get a sane outcome here.

[ghost]

wezterm's sixel is very solid, but alas on the slower end. foot, xterm, and (sad to say) my own Swing backend can outpace it.

I am skeptical that I can help, but am interested in poking around. Is the sixel decoder entirely in term/src/terminalstate/sixel.rs and termwiz/src/escape/mod.rs ? Can the decoder be run separately from the full terminal?

[wez]

I'm sure there's room for optimization: that code is unoptimized, and I would love for it to be improved!

Yeah, it's those two files. You could extract the bulk of TerminalState::sixel: https://github.com/wez/wezterm/blob/ebef7dc659ccac6a99c592b0f21401992cfb5bd8/term/src/terminalstate/sixel.rs#L11-L106 into a helper function in that file and then set up a test function at the bottom to benchmark; something like:

#[test]
fn test_sixel_decode() {
   let start = std::time::Instant::now(); 
   // build a Sixel
   println!("sixel build took {:?}", start.elapsed());
   
   let start = std::time::Instant::now();
   let data = call_extracted_function_from_above(sixel);
   println!("sixel decode took {:?}", start.elapsed());

   panic!("just to ensure that the test fails and shows its output");
}

then cargo test -p wezterm-term to build and run the tests from term as a low-ish effort way to isolate and iterate on it

[ghost]

@ModProg

...one thing that is a bit annoying with sixel is not being able to display imges in cells

I haven't yet found a way to write image data to the bottom row (DECSET 8452 didn't do it for me), but otherwise sixel can be placed anywhere on screen, and if one is careful it can be mixed in with text:

xterm with translucency and animations

It doesn't do a proper +/- Z axis though, so you can't easily put image and text in the same cell. You can lay text down and then sixel over, but not sixel down and then text over. Is that what you are referring to?

[ModProg]

You can lay text down and then sixel over, but not sixel down and then text over. Is that what you are referring to?

No, I thought you cannot just say make this image 4x4 cells, but have to use pixels? not sure right now. Haven't really dealt with sixels a lot :D

[ghost]

Ah I see. Yes, iTerm2 and Kitty now, and eventually the future "Good Image Protocol", will be able to say "take this picture, and stick it those cells, stretching/scaling like so". Sixel is pure pixels, so you need to know the text cell aspect ratio to be able to place it.

[j4james]

you cannot just say make this image 4x4 cells

On a real DEC terminal, and on terminal emulators that strictly emulate the protocol, you can usually assume a cell is 10x20 pixels in size, so if you write out an image that is 400x800, it will occupy exactly 4x4 cells.

[ghost]

@j4james BTW how is the DECSDM story? I believe that was a blocker for sixel in Windows Terminal?

[j4james]

@klamonte Sorry, I should have followed up on that here. The DECSDM problem is pretty much under control now I think. Almost everyone that supports DECSDM is implementing it correctly now.

The main reason I'm holding back on the release of my Windows sixel implementation is because it's still conhost-only at the moment - to get it in Windows Terminal we need to get a pass-through mode implemented first.

There are also a few sixel issues that would be nice to get resolved first. For example, I'd like to get other terminals to standardize on the correct cursor positioning, and I think there's an issue with the Jexer palette handling that I was hoping to persuade you to fix. But those aren't really blockers.

[ghost]

@j4james If you get some time, please open an issue for me. I've got a new encoder in git head that uses per-image palettes and can generate sixels with transparent pixels, this would be a great time to fix it up for compatibility.

[j4james]

I haven't yet found a way to write image data to the bottom row

@klamonte This is another one of those things that would be possible in sixel if terminal emulators actually implemented it correctly. Admittedly it's not that straightforward, but if you know the cell height is 20, you can avoid triggering a scroll by starting the image 3 rows (i.e. 60 pixels) from the bottom, and leaving the first 40 scanlines transparent (assuming you only want to affect the last row).

You can get this to work with terminals that use a different cell size, but it make things a lot more complicated. And the main requirement is for the terminal to position cursor correctly when the image is complete, i.e. it should end up on the last row of the image, not below the image (which would otherwise trigger a scroll). Most terminals don't do that correctly.

[ghost]

@wez I'm going to try for that bottom row: https://gitlab.com/klamonte/jexer/-/issues/91

If it works, I'll do a PR for DECSDM for you after.

[ghost]

Hey @wez , I'm putting together the DECSDM PR. Would you be OK with renaming terminalstate.sixel_scrolling to terminalstate.sixel_display_mode and corresponding DecPrivateModeCode::SixelScrolling to DecPrivateModeCode::SixelDisplayMode ? That would be more in line with VT382 doc (credit @j4james for the link) : https://vt100.net/dec/ek-vt38t-ug-001.pdf#page=132 .

[wez]

Yep, that sounds fine!

[ghost]

@wez It looks like putting transparent sixel pixels over other non-transparent sixel pixels is making those non-transparent pixels transparent. Did that make sense? Here's what I mean:

https://user-images.githubusercontent.com/4357501/150208696-83a51d00-9686-4d9d-bca9-ccf8b9c528dc.mp4

You see the image appearing on the bottom row (woohoo!) but all the other images above that row gone.

Is this intended behavior in the current design? It feels like an error to me that we should fix.

Also for performance, it seems like assign_image_to_cells() might not be culling cells with fully-transparent images. Regardless of DECSDM we should get that fixed. (Someone we know likes to blit a certain Segway-riding orca who has a lot of transparent cells around her. ;-) )

[ghost]

From much earlier: https://github.com/wez/wezterm/issues/217#issuecomment-642782747

• Pixels-overlapping-text: sixel is supposed to do that as per spec, but in practice no one uses it that way right now. IMHO it would be perfectly acceptable to make text and image data mutually exclusive in a cell if that makes the implementation easier.

I was wrong, so so wrong. :-)

[ghost]

                match params.style {
                    ImageAttachStyle::Kitty => cell.attrs_mut().attach_image(img),
                    ImageAttachStyle::Sixel | ImageAttachStyle::Iterm => {
                        cell.attrs_mut().set_image(img)
                    }

...

    /// Assign a single image to a cell.
    pub fn set_image(&mut self, image: Box<ImageCell>) -> &mut Self {
        self.allocate_fat_attributes();
        self.fat.as_mut().unwrap().image = vec![image];
        self
    }

That looks like a blind "replace image" rather than a "blit image over and replace". Right?

Need instead something that will do:

                    if (oldCell.isImage()) {
                        // Blit the old cell image underneath this cell's
                        // image.
                        newImage = new BufferedImage(textWidth,
                            textHeight, BufferedImage.TYPE_INT_ARGB);

                        java.awt.Graphics gr = newImage.getGraphics();
                        gr.drawImage(oldCell.getImage(), 0, 0, null, null);
                        gr.drawImage(cells[x][y].getImage(), 0, 0, null, null);
                        gr.dispose();
                        cells[x][y].setImage(newImage);

Probably not attach_image() though, since for sixel/iTerm2 once the old pixels are overwritten they are gone forever, unlike Kitty where one could add/remove intermediate layers.

[wez]

Ah, I'd assumed that sixel would logically the replace the current cell, rather than composite with an existing sixel cell. Compositing for the sixel case should be doable, although it's probably going to be fiddly because of the way that ImageCell and ImageData are structured; I think you'll essentially want a method on ImageCell that:

• for the ImageDataType::Rgba8 case, will return something like https://docs.rs/image/latest/image/trait.GenericImageView.html for the region described by the texture coordinates. Let's call it something like ImageCell::get_view(&self) -> Option<GenericImageView> (returns None for the other ImageDataType variants)
• then the sixel attach/composite case would do something like:

  if has_existing_image {
     let bg_image = existing_cell.get_view();
     let dest_image = RgbaImage::new(dimensions);
     dest_image.copy_from(bg_image);
     let fg_image = new_cell.get_view();
     image::imageops::overlay(dest_image, fg_image, 0, 0);
     // and now attach_image dest_image in its place
  }

[ghost]

@wez I'm not quite following the the trait/match stuff, sorry! I've got a long way to go still on Rust.

Would it be OK if I started a PR that compiles, and then over there we can get the ImageCell/ImageData/ImageDataType worked out?

[wez]

Definitely!