hpjansson / chafa

📺🗿 Terminal graphics for the 21st century.
https://hpjansson.org/chafa/
GNU Lesser General Public License v3.0
2.91k stars 62 forks source link

Rendering issue for GIF with transparency on some terminals (using Sixel encoding) #147

Closed Delgan closed 1 year ago

Delgan commented 1 year ago

Hi.

Somewhat related to https://github.com/hpjansson/chafa/issues/104.

I tested the GIF on the foot terminal, and got the following output: screenshot

New frames are layered on top of the old ones without erasing them, causing an unexpected visual effect.

It appears this is because Chafa uses P2=1 background mode for Sixel images. According to the specs:

P2 selects how the terminal draws the background color. You can use one of three values. P2 Meaning
0 or 2 (default) Pixel positions specified as 0 are set to the current background color.
1 Pixel positions specified as 0 remain at their current color.

This mode seems very appropriate for static images with transparency. However, using P2=0 looks preferable for animated images to make sure the previous frame is erased. This has the drawback of also erasing others images possibly displayed underneath, but there's no other choice with the Sixel protocol, as far as I know.

What's your opinion on this? Would you accept a PR trying to replace P2=1 with P2=0 for animated images?

hpjansson commented 1 year ago

Absolutely. A PR would be very welcome.

It might be possible to do this with a one-liner by just switching to P2=0 for all images. When I wrote the first implementation back in 2018, I landed on P2=1 after some testing, but there were fewer sixel-capable terminals to test with, and what was there was inconsistent. Hence I didn't make any promises in the API wrt. being able to compose Chafa images with each other (OVER op), so I don't think it would be an API break.

It'd be necessary to test it with a few terminals (foot, xterm, wezterm, mlterm, konsole at least) to make sure the background color gets shown correctly. For terminals that support background pixmaps, it would be nice if the pixmap shows through, but I don't think it's a deal-breaker if it doesn't.

Delgan commented 1 year ago

Great!

It might be possible to do this with a one-liner by just switching to P2=0 for all images. When I wrote the first implementation back in 2018, I landed on P2=1 after some testing, but there were fewer sixel-capable terminals to test with, and what was there was inconsistent. Hence I didn't make any promises in the API wrt. being able to compose Chafa images with each other (OVER op), so I don't think it would be an API break.

Yes, it's a one-line fix (I tested it) if if we want to manage all cases of transparency with P2=0. However, this would no longer allow users to overlay static images with transparency. Even if it's not a promise made by the API, do you confirm this is what you want?

I personally don't have any use cases requiring P2=1, so it doesn't really bother me. If we want to give the user the choice, I imagine we'll have to add a new parameter ChafaCanvasConfig. Then, we can use it in chafa.c and switch between P2=0 and P2=1 when animated image is detected.

Just let me know what you prefer.

It'd be necessary to test it with a few terminals (foot, xterm, wezterm, mlterm, konsole at least) to make sure the background color gets shown correctly. For terminals that support background pixmaps, it would be nice if the pixmap shows through, but I don't think it's a deal-breaker if it doesn't.

Sure, I'll provide comparison srcreenshots.

hpjansson commented 1 year ago

I think it makes sense to only support the SRC op (replace alpha) as a sixel client, at least in the short term:

In the longer term, though, it might make sense to add such an option if Kitty gains support for the SRC op. Then there'd be two protocols with nominal support, and an abstraction would start to make sense.

You don't have to screenshot everything, by the way, just use your judgement :-)

hpjansson commented 1 year ago

I also have a tentative road map where you'd be able to make multiple image placements on a ChafaCanvas, which means implementing a ChafaImage class and having compositing be a parameter of each placement. Then implementing update deltas. Let me know if you're interested in working on any of this; I could write it up for discussion.

Delgan commented 1 year ago

I think it makes sense to only support the SRC op (replace alpha) as a sixel client, at least in the short term

That's fine by me! I opened #148.

I also have a tentative road map where you'd be able to make multiple image placements on a ChafaCanvas, which means implementing a ChafaImage class and having compositing be a parameter of each placement. Then implementing update deltas. Let me know if you're interested in working on any of this; I could write it up for discussion.

That sounds interesting. By "update deltas", do you mean it would be possible do optimize GIF rendering and avoid re-drawing the whole frame? It would be a pretty challenging optimization problem, that's for sure.