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

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

Closed
Delgan opened this issue May 31, 2023 · 5 comments · Fixed by #148
Closed

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

Delgan opened this issue May 31, 2023 · 5 comments · Fixed by #148

Comments

@Delgan
Copy link
Contributor

Delgan commented May 31, 2023

Hi.

Somewhat related to #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
Copy link
Owner

hpjansson commented Jun 1, 2023

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
Copy link
Contributor Author

Delgan commented Jun 2, 2023

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
Copy link
Owner

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

  • Support varies and you'd not always get what you want; better composite on the client, which is less brittle and gives the application more control.
  • Sixels don't support post-upload translation, multiple placements, etc. so there's no efficiency gain in compositing in the terminal.
  • I haven't seen a modern use case where the ability to composite sixels on the terminal side is critical.
  • A configuration switch would be a sixel-specific feature; I think e.g. Kitty only supports OVER (which is a bit disappointing; I'll have to ask Kovid about SRC support), and symbol mosaics obviously only support the equivalent of SRC. So we couldn't have something like chafa_canvas_config_set_compositing_op () and expect it to work.

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
Copy link
Owner

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
Copy link
Contributor Author

Delgan commented Jun 4, 2023

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.

hpjansson added a commit that referenced this issue Aug 23, 2024
We can clear the background in animation frames by setting the alpha
threshold now, like we do for Kitty.

Ref: #211 (GitHub).
Ref: #147 (GitHub).
hpjansson added a commit that referenced this issue Aug 23, 2024
Stills maintain their transparency (the default alpha threshold is
.5). This can be overridden with the -t flag.

Ref: #211 (GitHub).
Ref: #147 (GitHub).
hpjansson added a commit that referenced this issue Sep 9, 2024
We can clear the background in animation frames by setting the alpha
threshold now, like we do for Kitty.

Ref: #211 (GitHub).
Ref: #147 (GitHub).
hpjansson added a commit that referenced this issue Sep 9, 2024
Stills maintain their transparency (the default alpha threshold is
.5). This can be overridden with the -t flag.

Ref: #211 (GitHub).
Ref: #147 (GitHub).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants