Detect kitty graphics support via query

[dankamongmen]

@wez is adding Kitty graphics support to WezTerm (wez/wezterm#986) in one of the happier bits of news this year. @kovidgoyal suggests querying for Kitty support somewhere in https://sw.kovidgoyal.net/kitty/protocol-extensions/ (can't find it right now, but the implementation is obvious). We should go ahead and do this to detect support, rather than adding it to WezTerm's heuristics. We'll leave the heuristics in there for Kitty, since we need to differentiate between several levels of support based on version (with that said, a failure to respond to the query ought disable support entirely, even in Kitty).

So yeah, toss this query into our initial query string, and check the result. Hopefully most terminals will consume it, but we'll see.

[dankamongmen]

i can go ahead and do this today; the only worry is that e.g. WezTerm currently has an incomplete implementation; if it answers to this query, but doesn't do the rest, we probably don't yet want to use kitty graphics there.

more fundamentally, since WezTerm also implements sixel, it's going to respond to both of those queries. how do we decide between them? kitty is the more powerful protocol for sure, but in this particular case, we'd want sixel for now. hrmm.

we could theoretically send a full animation sequence on a dummy image (this seems to be what mr. goyal recommends), but this might add some latency to our all-important initialization time (though it might not).

[dankamongmen]

this looks like a duplicate of #1908

[wez]

Currently, wezterm does a terrible job at sending back responses to the kitty protocol, so it may not matter too much :-p
However, I can make a config option that default-disables kitty image protocol handling in wezterm so that someone has to explicitly enable it for testing purposes. Then once it is done enough I can remove or default-enable that option.

[wez]

wez/wezterm@fb9338f adds that option; you can use wezterm --config enable_kitty_graphics=true to turn it on for testing.

[dankamongmen]

Currently, wezterm does a terrible job at sending back responses to the kitty protocol, so it may not matter too much :-p
However, I can make a config option that default-disables kitty image protocol handling in wezterm so that someone has to explicitly enable it for testing purposes. Then once it is done enough I can remove or default-enable that option.

given the pragmatic reality that you are terminal #2 to publicly attempt kitty support, i will likely just hold off until i've tested your implementation by hand, i'm thinking. no need to play this like procedural automatons.

[WSLUser]

Nice to see another terminal picking up Kitty's graphic protocol support. I would love to see it on Windows but when I filed an issue it was closed (since they don't even have Sixel yet) and the guy I would of thought may of been able to add it decided not to. So any push for someone to get this support on Windows would be greatly appreciated.

I do think you should still switch to the query method. It's only a matter of time before more terminals adopt it. Any tuning of it can be worked out later when use cases show up.

[wez]

FWIW, this had me scratching my head for a while; wezterm's large number of color registers took precedence over kitty img support:

diff --git a/src/lib/termdesc.c b/src/lib/termdesc.c
index f727e5b..28a33ea 100644
--- a/src/lib/termdesc.c
+++ b/src/lib/termdesc.c
@@ -871,7 +872,7 @@ int interrogate_terminfo(tinfo* ti, int fd, const char* termname, unsigned utf8,
   // our current sixel quantization algorithm requires at least 64 color
   // registers. we make use of no more than 256. this needs to happen
   // after heuristics, since the choice of sixel_init() depends on it.
-  if(ti->color_registers >= 64){
+  if(ti->color_registers >= 64 && ti->pixel_scrub != kitty_scrub){
     setup_sixel_bitmaps(ti, fd, invertsixel);
   }
   return 0;

[dankamongmen]

FWIW, this had me scratching my head for a while; wezterm's large number of color registers took precedence over kitty img support:

not sure i'm following. i'm not yet doing the kitty test; it's only enabled (by hand) for kitty itself. wezterm ought always be getting sixel at the moment. was it not?

[wez]

I added a call to setup_kitty_bitmaps in the wezterm case for local testing purposes and found that it was being overridden by that check because wezterm advertises 64k color registers

[dankamongmen]

ahhh well that explains things perfectly, thanks. yes, the work i'm doing to send the kitty query addresses this issue, have no fears!

[wez]

The state of wezterm main is probably good enough now that you could safely code up detection; wezterm will only respond to the probe when enable_kitty_graphics=true is set in its config, and the default is false, so we needn't worry about ruining current or future wezterm user's sixel experience with my half-baked kitty implementation.

I've confirmed that kitty +kitten icat --detect-support runs successfully in wezterm.

Ignoring some quirks in the wezterm implementation that need to get run down and resolved, the reduction in bandwidth makes a nice impact in the FPS in the demo!

FWIW, I've been working locally with this diff applied for testing:

diff --git a/src/lib/termdesc.c b/src/lib/termdesc.c
index f727e5b..731de53 100644
--- a/src/lib/termdesc.c
+++ b/src/lib/termdesc.c
@@ -573,6 +573,7 @@ apply_term_heuristics(tinfo* ti, const char* termname, int fd,
     }
     // we don't yet want to use the iterm2 protocol in place of sixel
     //setup_iterm_bitmaps(ti, fd);
+    setup_kitty_bitmaps(ti, fd, KITTY_SELFREF);
   }else if(qterm == TERMINAL_XTERM){
     termname = "XTerm";
     // xterm 357 added color palette escapes XT{PUSH,POP,REPORT}COLORS
@@ -871,7 +872,7 @@ int interrogate_terminfo(tinfo* ti, int fd, const char* termname, unsigned utf8,
   // our current sixel quantization algorithm requires at least 64 color
   // registers. we make use of no more than 256. this needs to happen
   // after heuristics, since the choice of sixel_init() depends on it.
-  if(ti->color_registers >= 64){
+  if(ti->color_registers >= 64 && ti->pixel_scrub != kitty_scrub){
     setup_sixel_bitmaps(ti, fd, invertsixel);
   }
   return 0;

It's also worth noting that notcurses-info also uses ti->color_registers to determine that sixel is preferred, even though kitty's bitmap callbacks are being used.

[dankamongmen]

excellent @wez; i'll play around with it today, if possible! if it all works, i'm gonna see if i can't get a bottle of goodly laphroaig out to you

[dankamongmen]

so here's what i'm thinking we do: we already need to send a a=d "clear all graphics" message on startup unless NCOPTION_PRESERVE_BITMAPS has been passed. so to kill two birds with one stone, let's send the "clear all" in our identification string unless PRESERVE was used; in that case, we can send whatever else will elicit a response and have no effect. the minimal such query i've found thus far is

'\x1b_Gi=1;\x1b\\'

which ought always generate EINVAL:Zero width/height not allowed.

we can then match any kitty-looking APC response, and turn on kitty graphics in that case. then, clear any sixel results in deference to kitty at the end of ncinputlayer_init(). that ought be a complete solution.

[dankamongmen]

@wez things initially look very good indeed! i'm seeing one possible problem, but it might be nothing at all, or it might be my bug. it has to do with restoring cell-sized sections of a graphic, which i do using the "composition" element of the animation protocol. i'm seeing the following (notes to self; i don't expect you to look into these):

(a) a single misplaced braille cell about halfway through intro, up near the head of the orca, well above where any braille is used,
(b) the statepixel PoC, when run via ./statepixel ../data/worldmap.png, seems to have two active cells rather than the expected one. the leading cell looks kinda grayish?
(c) the bitmapstates PoC doesn't seem to fill in squares after cleaning them out

let me try it with a lower conformance level and see if these behaviors change (i'm using the highest level, KITTY_SELFREF, at the moment. they're explained in the large comment atop src/lib/kitty.c).

btw, having dug around in it a little, how does the overall code/project look to you? be honest!

[kovidgoyal]

On Thu, Aug 05, 2021 at 01:15:10AM -0700, nick black wrote: so here's what i'm thinking we do: we already need to send a `a=d` "clear all graphics" message on startup unless `NCOPTION_PRESERVE_BITMAPS` has been passed. so to kill two birds with one stone, let's send the "clear all" in our identification string unless `PRESERVE` was used; in that case, we can send whatever else will elicit a response and have no effect. the minimal such query i've found thus far is `'\x1b_Gi=1;\x1b\\'` which ought always generate `EINVAL:Zero width/height not allowed`. we can then match any kitty-looking APC response, and turn on kitty graphics in that case. then, clear any sixel results in deference to kitty at the end of `ncinputlayer_init()`. that ought be a complete solution.

Use the query mode of the protocol: https://sw.kovidgoyal.net/kitty/graphics-protocol/#querying-support-and-available-transmission-mediums

[dankamongmen]

Use the query mode of the protocol: https://sw.kovidgoyal.net/kitty/graphics-protocol/#querying-support-and-available-transmission-mediums

i'm pretty sure that's exactly what i proposed? the example there is

<ESC>_Gi=31,s=10,v=2,t=s;<encoded /some-shared-memory-name><ESC>\

and i was going to send

\x1b_Gi=1;\x1b\\

which suffices to get a response:

[schwarzgerat](130) $ echo -e '\x1b_Gi=1;\x1b\\'
[schwarzgerat](0) $ '\x1b_Gi=1;\x1b\\'Gi=1;EINVAL:Zero width/height not allowed

it's just your query stripped down.

i was going to send this only if i wasn't sending an Ga=d at startup, but it looks like the latter isn't enough to elicit a response, so i'll always send this query. let me go ahead and wrap this up right now.

[dankamongmen]

pump_control_read:1019:state:  0 char:    27 1b
pump_control_read:1019:state:  1 char: _  95 5f
pump_control_read:1019:state:  1 char: G  71 47
pump_control_read:1019:state:  1 char: i 105 69
pump_control_read:1019:state:  1 char: =  61 3d
pump_control_read:1019:state:  1 char: 1  49 31
pump_control_read:1019:state: 12 char: ;  59 3b
pump_control_read:1019:state: 12 char: E  69 45
pump_control_read:1019:state: 12 char: I  73 49
pump_control_read:1019:state: 12 char: N  78 4e
pump_control_read:1019:state: 12 char: V  86 56
pump_control_read:1019:state: 12 char: A  65 41
pump_control_read:1019:state: 12 char: L  76 4c
pump_control_read:1019:state: 12 char: :  58 3a
pump_control_read:1019:state: 12 char: Z  90 5a
pump_control_read:1019:state: 12 char: e 101 65
pump_control_read:1019:state: 12 char: r 114 72
pump_control_read:1019:state: 12 char: o 111 6f
pump_control_read:1019:state: 12 char:    32 20
pump_control_read:1019:state: 12 char: w 119 77
pump_control_read:1019:state: 12 char: i 105 69
pump_control_read:1019:state: 12 char: d 100 64
pump_control_read:1019:state: 12 char: t 116 74
pump_control_read:1019:state: 12 char: h 104 68
pump_control_read:1019:state: 12 char: /  47 2f
pump_control_read:1019:state: 12 char: h 104 68
pump_control_read:1019:state: 12 char: e 101 65
pump_control_read:1019:state: 12 char: i 105 69
pump_control_read:1019:state: 12 char: g 103 67
pump_control_read:1019:state: 12 char: h 104 68
pump_control_read:1019:state: 12 char: t 116 74
pump_control_read:1019:state: 12 char:    32 20
pump_control_read:1019:state: 12 char: n 110 6e
pump_control_read:1019:state: 12 char: o 111 6f
pump_control_read:1019:state: 12 char: t 116 74
pump_control_read:1019:state: 12 char:    32 20
pump_control_read:1019:state: 12 char: a  97 61
pump_control_read:1019:state: 12 char: l 108 6c
pump_control_read:1019:state: 12 char: l 108 6c
pump_control_read:1019:state: 12 char: o 111 6f
pump_control_read:1019:state: 12 char: w 119 77
pump_control_read:1019:state: 12 char: e 101 65
pump_control_read:1019:state: 12 char: d 100 64
pump_control_read:1019:state: 12 char:    27 1b
pump_control_read:1019:state:  1 char: \  92 5c
pump_control_read:1019:state:  0 char:    27 1b

[dankamongmen]

// query for kitty graphics. if they are supported, we'll get a response to
// this using the kitty response syntax. otherwise, we'll get nothing.
#define KITTYQUERY "\x1b_Gi=1,a=q;\x1b\\"

this suffices to get a response from both wezterm and kitty

[dankamongmen]

ncinputlayer_init:1543:advertised kitty; disabling sixel
notcurses 2.3.13 on WezTerm 20210804-162508-e3acbd59
rgba pixel graphics support

there we go

[wez]

btw, having dug around in it a little, how does the overall code/project look to you? be honest!

I've not looked too deeply; it was pretty easy to grep and find the bit I needed to change, so I haven't needed to look far beyond that. It looks like a well-commented C/C++ project, which is nice: it was easy to find what I needed without having to read much, and it didn't make my eyes bleed. A solid first impression :-)

I appreciate the breadth of the demo and all of the metrics it captures and dumps at the end: those imply a solid foundation and attention to detail.

[wez]

(b) the statepixel PoC, when run via ./statepixel ../data/worldmap.png, seems to have two active cells rather than the expected one. the leading cell looks kinda grayish?

If you zoom in on this, it looks like there is a slightly offset transparent region that is showing the text that is behind the image (that splash of magenta is text from my prompt).

Not sure yet where this is coming from.