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

Use "kitty" instead of "sixels" as default format for WezTerm #149

Merged
merged 2 commits into from
Jun 11, 2023

Conversation

Delgan
Copy link
Contributor

@Delgan Delgan commented Jun 10, 2023

Following #148.

Tested with wezterm 20230408-112425-69ae8472 and the support for Kitty protocol seems pretty good from what I tested.
The animation from GIF images are much smoother than with the sixels protocol. The transparency is supported for static frames (right), however it displays a black background for GIF (left):

20230610_10h58m34s_grim

chafa/chafa-term-db.cf Outdated Show resolved Hide resolved
@hpjansson
Copy link
Owner

Looks good. Thanks!

@hpjansson hpjansson merged commit 9c0175d into hpjansson:master Jun 11, 2023
@hpjansson
Copy link
Owner

The black background you're seeing is the current workaround for #104.

@AnonymouX47
Copy link

AnonymouX47 commented Jun 14, 2023

Just came across this...

Would like to note that the state of kitty graphics on wezterm is still experimental and quite non-compliant with the spec (and the reference implementation)... I think I remember mentioning this on some other issue before.

The major issue that would affect chafa is probably wez/wezterm#2084. wez/wezterm#2761 has been open to fix this for months now with no response from Wez.
I just tested main (wezterm 20230612-063953-baf9d970) and the behaviour is still the same... Basically, try to display two images with chafa and observe what happens to the first.

In other words... until Chafa implements support for Kitty image IDs or the issue is fixed, I don't think the kitty format would be usable on Wezterm.

I wouldn't suggest the iterm2 format either. See wez/wezterm#3715.

@hpjansson
Copy link
Owner

I see, thanks for bringing it up. It's unclear to me how using explicit image IDs could work around it, though. If there are multiple Chafa invocations from a shell and they all use e.g. image number 1, wouldn't we run into the same issue? Or do you think we could use image IDs specifically (as opposed to numbers) somehow?

@AnonymouX47
Copy link

AnonymouX47 commented Jun 14, 2023

If there are multiple Chafa invocations from a shell and they all use e.g. image number 1, wouldn't we run into the same issue?

True. Sorry, "Image numbers" (the I control data key) was my intention.

Anyways, this (Image numbers, not IDs) would have to go with the q=1 control data (to suppress OK responses) which, unfortunately, Wezterm also doesn't implement/respect. 🥲


EDIT: q=2 -> q=1... Only required when using image numbers, not IDs

@hpjansson
Copy link
Owner

Confirmed, it works with I=1. And yeah, I get the OKs. Probs better if we stick with sixels for now, then. I'll back out the change until Wezterm improves the protocol support. With apologies to @Delgan.

@AnonymouX47
Copy link

I guess that'll be best.


I've always had it in mind to curate a list of these issues and report them all (recently did that for the iterm2 protocol) but:

  1. haven't gotten the time for that
  2. a part of me feels bad reporting so many issues without solving any (I don't write Rust yet)
  3. it's not really been that high priority to me until the iterm2 protocol support got messed up

but someday soon I'll get to it.

@hpjansson
Copy link
Owner

One could do the same for sixels (there's some very nice detective work in #148). In that case I think the fix is tiny in most cases. The difficult part is getting everyone to agree on something :-)

@AnonymouX47
Copy link

The difficult part is getting everyone to agree on something :-)

Very true! I think highest chances of convincing the developers would be via demo(s) comparing the terminal's current implementation with the proposed implementation. With the rationale behind the proposed implementation.

Even with that, it might still be difficult to get some terminal emulators to change their behaviour, especially if they've specified/advertised that behaviour to their users already or there's already too much dependence on the behaviour... It's almost like the case with direct color SGR sequences, except that this case may not have existed for as long.

@Delgan
Copy link
Contributor Author

Delgan commented Jun 15, 2023

Confirmed, it works with I=1. And yeah, I get the OKs. Probs better if we stick with sixels for now, then. I'll back out the change until Wezterm improves the protocol support. With apologies to @Delgan.

No problem. My bad, I should have tested more extensively.

Thanks @AnonymouX47 for pointing that tout.

@AnonymouX47
Copy link

AnonymouX47 commented Jul 10, 2023

Thought I should mention that wez/wezterm#3715 has now been fixed... though only in the nightly build for now.

@hpjansson
Copy link
Owner

Great! I think we should stick with sixels until the fixed version becomes widely deployed.

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 this pull request may close these issues.

3 participants