Image placement using Unicode placeholders

[sergei-grechanik]

This commit introduces the Unicode placeholder image placement method. In particular:

• Virtual placements can be created by passing U=1 in a put command.
• Images with virtual placements can be displayed using the placeholder character U+10EEEE with diacritics indicating rows and columns.
• The image ID is indicated by the foreground color of the placeholder.
• Underline color can be optionally used to specify the placement ID.
• Unicode placeholders are used by the icat kitten when the --unicode-placeholder flag is specified.
• Also the --tmux flag was added to the icat kitten, which enables some tmux workarounds and Unicode placeholders.
• A bug was fixed, which caused incomplete image removal when it was overwritten by another image with the same ID.

[kovidgoyal]

Cool, I will look at this in a while. To start with could you do the follwoing:

1. Resolve the conflicts
2. Make a small unrelated PR that I can merge (could be anything trivial) so that the tests are run on your PRs automatically without waiting for me to approve.

[sergei-grechanik]

Extracted a small fix as https://github.com/kovidgoyal/kitty/pull/5682

[slava-mironov]

Hi! Do I understand it correctly that with this it will be possible to view images in tmux when it's run under kitty?

[sergei-grechanik]

Yes, it will work in tmux. The icat kitty will use this method (and some other tmux workarounds) when the --tmux flag is passed.

[slava-mironov]

That's great! BTW, I tried building kitty with this pr on Mac OS and also tried your experimental branch with upload_ script, couldn't make either work with tmux locally (without tmux works fine), but I didn't dig too much. If you need something tested, I'd be happy to spend some time on this. My usual setup is Mac OS and remote CentOS or Debian servers, I use almost identical environments in tmux locally and remotely.

[sergei-grechanik]

I remember some portability issues with my uploading script on Mac OS, but I think icat should work. Just a guess: check that you have set -gq allow-passthrough in tmux config, in newer versions of tmux it's disabled by default.

[slava-mironov]

Thanks! After setting allow-passthrough on and then running kitty icat --tmux ~/test.jpg the image was shown correctly. I tried doing this in multiple panes, zooming, resizing them, everything works. This is brilliant!

One thing that is a bit inconvenient, running without tmux, but leaving --tmux option on will cause a temporary freeze and then produce an error. This may cause a problem with scripting.

[kovidgoyal]

Comments:

1. At least on my system tmux does report pixels sizes so no need for the workaround. Presumably this was added in more recent tmux versions.

2. Resolve the conflict in docs/changelog.rst

3. It's not clear to me why deletion of virtual images is restricted to only id based.

4. When generating the row/column diacritics for python just use repr(str) rather than \U form, faster to parse. Use tuples instead of lists, more memory efficient and faster to load. rowcolumn_diacritics_utf8 should also be generated directly rather than requiring encoding at runtime. Also rather tham importing this at top level delay import only when needed so that people not using unicode placeholder dont pay the startup cost.

5. Regarding --tmux and --unicode-placeholders for icat, maybe better to have a --mode argument that defaults to auto and has normal, tmux, unicode-placeholders auto mode can use normal unless it detects it is running in tmux in which case it can use tmux mode (see the existing running_in_tmux function) Also when running in tmux it should run tmux set -p allow-passthrough on (see the ssh kitten for an example) otherwise I am going to be flooded by lots of bug reports about it not working in tmux because allow-passthrough defaults to off.

6. in write_gr_command() rather than concatenating long bytestrings just write the tmux prefix and suffix directly to sys.stdout.buffer

7. In fonts.c make IMAGE_PLACEHOLDER_CHAR a case in the switch statement

8. Make IMAGE_PLACEHOLDER_CHAR defined in only one place in C code and the icat python code can import it from there via fast_data_types

9. The documentation needs cleanup but I will do that myself after this PR is merged.

10. I dont like that fact that we are unconditionally iterating over all cells twice. Instead perhaps modify render_line() to set a flag if it finds an image placement char in the line and only if it does call screen_render_line_graphics()

I haven't reviewed the actual algorithms/code in detail yet, will get to that after this round. Generally speaking it looks good to me, thanks for your contribution.

[sergei-grechanik]

Thanks for the review, I'll try to address all the issues over the weekend.

1. At least on my system tmux does report pixels sizes so no need for the workaround. Presumably this was added in more recent tmux versions.

I can confirm that, the workaround is necessary for tmux 3.0, but not for tmux 3.4. I will remove it.

3. It's not clear to me why deletion of virtual images is restricted to only id based.

Virtual placements are not considered visible on screen, so deleting them by screen position (a, c, p, q, x, y, z) doesn't make sense. The visible references created on top of placeholder cells are just copies of the original virtual placement. They can be affected by position-based deletion commands, but it will not result in the deletion of the original virtual placement (doing so would be confusing, in my opinion, because there could be multiple references created from the same virtual placement, and deleting one of them shouldn't affect the others). Also, deleting these copy references doesn't make much sense because they will respawn from the placeholder cells on text modification, so it may be a good idea to forbid deleting them.

(btw, I think there is a bug in the code, I forgot to add the if (ref->is_virtual_ref) check to x/y/z_filter_func)

[kovidgoyal]

On Sat, Dec 17, 2022 at 04:10:50PM -0800, Sergei Grechanik wrote:

Thanks for the review, I'll try to address all the issues over the weekend.

Cool.

1. At least on my system tmux does report pixels sizes so no need for the workaround. Presumably this was added in more recent tmux versions.

I can confirm that, the workaround is necessary for tmux 3.0, but not for tmux 3.4. I will remove it.

3. It's not clear to me why deletion of virtual images is restricted to only id based.

Virtual placements are not considered visible on screen, so deleting them by screen position (a, c, p, q, x, y, z) doesn't make sense. The visible references created on top of placeholder cells are just copies of the original virtual placement. They can be affected by position-based deletion commands, but it will not result in the deletion of the original virtual placement (doing so would be confusing, in my opinion, because there could be multiple references created from the same virtual placement, and deleting one of them shouldn't affect the others). Also, deleting these copy references doesn't make much sense because they will respawn from the placeholder cells on text modification, so it may be a good idea to forbid deleting them.

Makes sense, maybe add a line explaining this to the docs. Mention that if deletion is desired for these then it is important to use ids.

[sergei-grechanik]

Found a bug in cell image rendering logic, this will take some time.

[sergei-grechanik]

Fixed the issues.

When generating the row/column diacritics for python just use repr(str) rather than \U form, faster to parse. Use tuples instead of lists, more memory efficient and faster to load. rowcolumn_diacritics_utf8 should also be generated directly rather than requiring encoding at runtime.

Actually, rowcolumn_diacritics_utf8 is the only thing I need. It's represented with literals like b'\xcc\x85' though, so not sure about the parsing speed.

Regarding --tmux and --unicode-placeholders for icat, maybe better to have a --mode argument that defaults to auto and has normal, tmux, unicode-placeholders

I decided not to combine them into one option, mostly because mode sounds too generic. I changed --tmux to have three states: detect (default), yes, and no.

Also when running in tmux it should run tmux set -p allow-passthrough on (see the ssh kitten for an example)

I changed the logic of how this option is set. It now first checks if it is already set, and sets it to on only if it is not set. This is to avoid overriding the option if it has the recently introduced value all (allow pass-through from invisible panes).

[kovidgoyal]

On Sat, Dec 31, 2022 at 04:41:11PM -0800, Sergei Grechanik wrote:

Fixed the issues.

Thanks, looking good.

When generating the row/column diacritics for python just use repr(str) rather than \U form, faster to parse. Use tuples instead of lists, more memory efficient and faster to load. rowcolumn_diacritics_utf8 should also be generated directly rather than requiring encoding at runtime.

Actually, rowcolumn_diacritics_utf8 is the only thing I need. It's represented with literals like b'\xcc\x85' though, so not sure about the parsing speed.

icat is being re-implemented in Go anyway (see the icat branch) so this becomes irrelevant.

Regarding --tmux and --unicode-placeholders for icat, maybe better to have a --mode argument that defaults to auto and has normal, tmux, unicode-placeholders

I decided not to combine them into one option, mostly because mode sounds too generic. I changed --tmux to have three states: detect (default), yes, and no.

Maybe name it something like --passthrough (after all in the future we might want to add support for other multiplexers, not just tmux). --passthrough=detect|tmux|none (this can anyway be changed when porting to Go).

Also when running in tmux it should run tmux set -p allow-passthrough on (see the ssh kitten for an example)

I changed the logic of how this option is set. It now first checks if it is already set, and sets it to on only if it is not set. This is to avoid overriding the option if it has the recently introduced value all (allow pass-through from invisible panes).

This looks OK.

My remaining concerns are:

1. I dont like the fact that we are using ids since they can collide. Maybe instead let U=<any 23 bit number>. This number should be set as fg color in the unicode cells. When kitty sees these cells it checks the fg color, if bit 24 is not set, it maps the 23-bit fg to a unique 23 bit id and sets the fg to that id with the 24th bit also set. And stores the mapping so that future cells can re-use it.

Any future graphics commands that dont create a new image with U=that number will refer to the image with the mapped id.

If a U number is re-used in a command creating a new image, it will now refer to the latest image just like image numbers work currently.

When deleting an image we remove the stored mapping. Although I guess for placeholder based images delete is not really well defined. So maybe instead just prune entries that point to non existent ids from the mapping when creating a new mapping.

This means we have to use RGB colors not 256 colors but I dont think that's a problem. Nowadays, most things support RGB colors.

Or maybe this is too much bother and we can just live with the occasional collision. I am not totally convinced either way.

2. The icat part of this PR should be separated as in a day or two I will merge the icat branch which is a re-implementation of icat in Go. You can either port your changes to that or if you are not comfortable in Go I will do it for you after merging the non-icat parts of this PR.

3. There need to be some tests for how kitty handles unicode placeholder images in kitty_tests/graphics.py

[trygveaa]

This means we have to use RGB colors not 256 colors but I dont think that's a problem. Nowadays, most things support RGB colors.

This would absolutely be a problem for me :/ I would like to use it in WeeChat, and that only supports 256 colors unfortunately.

[kovidgoyal]

On Thu, Jan 05, 2023 at 05:30:49AM -0800, Trygve Aaberge wrote:

This means we have to use RGB colors not 256 colors but I dont think that's a problem. Nowadays, most things support RGB colors.

This would absolutely be a problem for me :/ I would like to use it in WeeChat, and that only supports 256 colors unfortunately.

The end application doesnt need to generate the colors, only tmux/whatever else is in the middle needs to pass them through.

Presumably you will be writing some kind of script/plugin/extension for weechat to generate these codes, since I dont think weechat can generate them natively. That script can simply set the foreground color using an RGB code and then restore it once it is done transmitting the graphics code.

[kovidgoyal]

Actually it doesnt matter you can use 256 colors as well, it just means your chances of collision with a previously used U number are higher, but that is true when using ids as well, and the effect of collision is less severe with U numbers. You will need to restrict to only those colors from the 256 color set that dont set the 24th bit however, so you should have around 128 unique ids.

[trygveaa]

The end application doesnt need to generate the colors, only tmux/whatever else is in the middle needs to pass them through. Presumably you will be writing some kind of script/plugin/extension for weechat to generate these codes, since I dont think weechat can generate them natively. That script can simply set the foreground color using an RGB code and then restore it once it is done transmitting the graphics code.

I need WeeChat to handle displaying the unicode characters, otherwise they won't be placed correctly when scrolling, changing buffers etc. Or am I misunderstanding something?

My plan was to call icat from my WeeChat script, capture the output characters, and print them with the WeeChat API. I see that does not work anymore though, since icat seem to print it directly to the terminal now, so I can't capture the output (unlike tupimage which I tested earlier, with which it worked to do it like that).

Actually it doesnt matter you can use 256 colors as well, it just means your chances of collision with a previously used U number are higher, but that is true when using ids as well, and the effect of collision is less severe with U numbers. You will need to restrict to only those colors from the 256 color set that dont set the 24th bit however, so you should have around 128 unique ids.

Hm, 128 ids is even more limiting than 256 though. I plan to display many small images (custom emojis from Slack, to be exact), so I would rather manage the 256 ids myself than be limited to 128.

[kovidgoyal]

On Thu, Jan 05, 2023 at 10:48:29AM -0800, Trygve Aaberge wrote:

The end application doesnt need to generate the colors, only tmux/whatever else is in the middle needs to pass them through. Presumably you will be writing some kind of script/plugin/extension for weechat to generate these codes, since I dont think weechat can generate them natively. That script can simply set the foreground color using an RGB code and then restore it once it is done transmitting the graphics code.

I need WeeChat to handle displaying the unicode characters, otherwise they won't be placed correctly when scrolling, changing buffers etc. Or am I misunderstanding something?

My plan was to call icat from my WeeChat script, capture the output characters, and print them with the WeeChat API. I see that does not work anymore though, since icat seem to print it directly to the terminal now, so I can't capture the output (unlike tupimage which I tested earlier, with which it worked to do it like that).

You can still do that with icat, though you would have to create a temporary pty pair for it.

Actually it doesnt matter you can use 256 colors as well, it just means your chances of collision with a previously used U number are higher, but that is true when using ids as well, and the effect of collision is less severe with U numbers. You will need to restrict to only those colors from the 256 color set that dont set the 24th bit however, so you should have around 128 unique ids.

Hm, 128 ids is even more limiting than 256 though. I plan to display many small images (custom emojis from Slack, to be exact), so I would rather manage the 256 ids myself than be limited to 128.

If you are sending emojis they are fire and forget so it doesnt matter if the ids conflict.

[kovidgoyal]

In fact for your use case, numbers are strictly better than ids. Since with an id conflict the previous image gets changed. With a number conflict all it means is you can no longer make changes to the previous image.

[sergei-grechanik]

Maybe instead let U=<any 23 bit number>. This number should be set as fg color in the unicode cells. When kitty sees these cells it checks the fg color, if bit 24 is not set, it maps the 23-bit fg to a unique 23 bit id and sets the fg to that id with the 24th bit also set.

Not sure I understand this part. If kitty changes the fg of a placeholder to the actual image id, it will work only for that specific placeholder, and if the user switches windows in tmux or scrolls the display, the placeholder will be rerendered with the old 23-bit id, because tmux wouldn't be aware of the fg change.

In general collisions are unavoidable, for example the user can store the placeholder from icat in a file, then cat it in a year and get a different image in the placeholder. I see the following options we can try to minimize the probability of collisions further:

• Expand the size of the id:
  • We can split the underline color so that some of its bits are used for image id (we'll have less bits for the placement id, but it's less important). Not every middle-man application supports underline colors though, even tmux needs some weird line in the config to enable them.
  • We can use a range of placeholder characters instead of a single placeholder character. I think getting 8 additional bits this way is reasonable. Higher risk of character collision though.
• Ask the terminal to allocate the id. The problem is that we need to wait for the response from the terminal, which may be slow over ssh, and may also be directed to the wrong tmux pane if the user switches panes at the right moment.
• Store the id <-> image mapping locally. This is what I'm doing with tupimage. The problem is that you are out of luck if you use multiple image-uploading tools that don't know about each other. And also sshing from within tmux is also problematic.

Also it's possible to minimize the damage from collisions. When we have a collision, we see the wrong image in the wrong place, but we may not know that it is the wrong image. But, for example, if we randomly shuffle the row numbers for each image, the wrong image will look obviously incorrect with a high probability.

[kovidgoyal]

On Thu, Jan 05, 2023 at 09:50:24PM -0800, Sergei Grechanik wrote:

Maybe instead let U=<any 23 bit number>. This number should be set as fg color in the unicode cells. When kitty sees these cells it checks the fg color, if bit 24 is not set, it maps the 23-bit fg to a unique 23 bit id and sets the fg to that id with the 24th bit also set.

Not sure I understand this part. If kitty changes the fg of a placeholder to the actual image id, it will work only for that specific placeholder, and if the user switches windows in tmux or scrolls the display, the placeholder will be rerendered with the old 23-bit id, because tmux wouldn't be aware of the fg change.

Ah yes, that's true. I guess that makes my idea unworkable.

In general collisions are unavoidable, for example the user can store the placeholder from icat in a file, then cat it in a year and get a different image in the placeholder. I see the following options we can try to minimize the probability of collisions further:

Yes, but we want to minimize the chance of collisions in the common use case of applications outputting images during their normal operations.

• Expand the size of the id:
  • We can split the underline color so that some of its bits are used for image id (we'll have less bits for the placement id, but it's less important). Not every middle-man application supports underline colors though, even tmux needs some weird line in the config to enable them.
  • We can use a range of placeholder characters instead of a single placeholder character. I think getting 8 additional bits this way is reasonable. Higher risk of character collision though.

Not a fan of either of these, as you say underline color is a lot less supported and I really dont want to make more PUA characters unuseable for other things than we absolutely have to. Another option is to use a third combining character since kitty now supports three, that will easily give us an additional 8 bits. So we can use 32 bits the combining char the lower 8 and the foreground the upper 24. This will also work in the case of applications that support only 256 colors to give us 16 bits.

• Ask the terminal to allocate the id. The problem is that we need to wait for the response from the terminal, which may be slow over ssh, and may also be directed to the wrong tmux pane if the user switches panes at the right moment.

No, I dont think requiring a roundtrip is workable. tmux wont pass back the responses anyway, and other applications may filter them out as well.

• Store the id <-> image mapping locally. This is what I'm doing with tupimage. The problem is that you are out of luck if you use multiple image-uploading tools that don't know about each other. And also sshing from within tmux is also problematic.

You mean each tool maintains a list of ids it has used in the past? This will require persistent state and wont work over ssh, also not worth it IMO.

Also it's possible to minimize the damage from collisions. When we have a collision, we see the wrong image in the wrong place, but we may not know that it is the wrong image. But, for example, if we randomly shuffle the row numbers for each image, the wrong image will look obviously incorrect with a high probability.

I dont quite follow what you are suggesting here, can you elaborate.

[sergei-grechanik]

Another option is to use a third combining character since kitty now supports three, that will easily give us an additional 8 bits.

Not every application supports three combining characters, but it looks like tmux does, so it may be a good idea. @trygveaa will it work for WeeChat? I'm also a bit worried about the total size of the placeholder, but if it's ever a problem, we can use the same trick as with row numbers and specify the third combining character only for the first column.

tmux wont pass back the responses anyway, and other applications may filter them out as well.

Yes, it's a problem for fzf, for example.

You mean each tool maintains a list of ids it has used in the past? This will require persistent state and wont work over ssh, also not worth it IMO.

In my opinion it's actually quite a good solution. I don't insist on having it in icat though. The problem with ssh is only when we have multiple ssh sessions attached to the same terminal (and even then I see some possible solutions, like assigning non-overlapping id ranges).

I dont quite follow what you are suggesting here, can you elaborate.

We can specify a random permutation of rows when we create an image placement. Then the placeholder has to use the same permutation, otherwise the image will appear broken, something like this: Probably not worth it unless we really expect collisions to happen.

Ok, I think I'll try the third combining char approach.

[kovidgoyal]

On Sat, Jan 07, 2023 at 09:51:57PM -0800, Sergei Grechanik wrote:

Another option is to use a third combining character since kitty now supports three, that will easily give us an additional 8 bits.

Not every application supports three combining characters, but it looks like tmux does, so it may be a good idea. @trygveaa will it work for WeeChat? I'm also a bit worried about the total size of the placeholder, but if it's ever a problem, we can use the same trick as with row numbers and specify the third combining character only for the first column.

It's 10 bytes per cell with the third char being 2 of those bytes. For a giant image of 160x50 cells thats 78KB with the third char and 62KB without. Assuming a cell has an area of 200 pixels, the image data would be something on the order of 4-5MB as a comparison. Of course, the image data only has to be transmitted once and can use a side channel.

Most use cases for this will involve much smaller images. So I think the overhead is acceptable, and yes one could use only the first column, at a cost in robustness, which will make the overhead truly negligible.

In my opinion it's actually quite a good solution. I don't insist on having it in icat though. The problem with ssh is only when we have multiple ssh sessions attached to the same terminal (and even then I see some possible solutions, like assigning non-overlapping id ranges).

I mean one could run icat on the local machine in a session, then ssh into another machine and run icat then logout, and ssh into a third machine and run icat and so on... From the terminals perspective all the ids used in all those invocations are in the same namespace. And then one would have to manage the issue of stale ids. Seems cleaner to just rely on the low likelihood of collision with a random number in 32 bits.

I dont quite follow what you are suggesting here, can you elaborate.

We can specify a random permutation of rows when we create an image placement. Then the placeholder has to use the same permutation, otherwise the image will appear broken, something like this: Probably not worth it unless we really expect collisions to happen.

Ah, got it. Yes I think we can table that for now. Lets see how it works in practice first.

Ok, I think I'll try the third combining char approach.

Cool.

[trygveaa]

You can still do that with icat, though you would have to create a temporary pty pair for it.

How can I do that? I tried with pty.spawn in python, but then kitty doesn't receive the control commands since they are just sent to the new pty, so I get "Terminal does not support reporting screen sizes via the TIOCGWINSZ ioctl".

Not every application supports three combining characters, but it looks like tmux does, so it may be a good idea. @trygveaa will it work for WeeChat?

Yes, looks like it does. E.g. when printing a\u0300\u0301\u0320 I see all three in WeeChat.

I'm also a bit worried about the total size of the placeholder, but if it's ever a problem, we can use the same trick as with row numbers and specify the third combining character only for the first column.

Would all columns need all information independently, in case they are not displayed together, or the first column is not displayed at all? E.g. if you scroll horizontally in less so the first column is out of view (it doesn't seem like displaying the image works in less though, but it's just an example).

By the way, I wondered about the placement ID. When do you need to use that when using unicode placeholders? Isn't the placements just determined by the unicode characters? I'm asking because WeeChat doesn't support underline color, but I can't see when it would be necessary.

[kovidgoyal]

On Sun, Jan 08, 2023 at 03:22:14AM -0800, Trygve Aaberge wrote:

You can still do that with icat, though you would have to create a temporary pty pair for it.

How can I do that? I tried with pty.spawn in python, but then kitty doesn't receive the control commands since they are just sent to the new pty, so I get "Terminal does not support reporting screen sizes via the TIOCGWINSZ ioctl".

You dont need to anymore I already changed icat to write only to stdout. Pass in --transfer-mode and it wont send anything on the tty device.

[sergei-grechanik]

Sorry for a long silence. So, since last time:

• I implemented the support for the third diacritic to expand image IDs to 24 bits (or 16 bits in 256 color mode).
• Added some unit tests.
• Simplified some logic in screen.c: now the image placeholder character is detected in draw_codepoint instead of render_line. This made the changes in screen_update_cell_data less intrusive.
• Removed the changes for the old python icat kitten. Right now I don't have the bandwidth to port the changes to Go.

[kovidgoyal]

OK post the icat python diff and I will take care of porting it to Go.

[sergei-grechanik]

Here is the commit with icat changes: https://github.com/sergei-grechanik/kitty/commit/7468171c8b2f64c88fdf584300d368063e320851

[kovidgoyal]

I have merged and ported the icat changes. I dont use tmux so I leave testing that to those who do. Basic icat of a simple PNG file worked for me inside tmux. I didnt test beyond that.

[sergei-grechanik]

Thank, looks great! I'll try to test it more thoroughly tomorrow.