fix kitty image protocol display parameters

[jonboh]

Hi! I've made some changes into the way that images get displayed to properly handle the kitty image protocol. The changes should not affect the iterm2 and sixel implementations.

I've also added in test-data/kitty-png.py some functions with test cases (intended to be checked manually) that should cover all the changes. To run each of the test just uncomment the one to be run and comment the rest.

Here is a list of changes:

• Fix x,y,w,h,c,r parameter behaviour
• Fix cell padding (X, Y). It must be lower than the cell width/height (previously it could pad without limit). Now for bigger input the padding is limited.
• images with id=0 handling

Here are some screenshots comparing kitty, the current wezterm master branch, and this pr branch.

display_parameters (x, y, w, h, c, r) test

Kitty

Wezterm 05eadb7

This PR

cell_offsets (X, Y parameters) test

Kitty

Wezterm 05eadb7

This PR

For these I've used this picture, although any image should generate similar results.

This PR should solve most of the issues described in #986 (comment)

I'll be happy to make any modification if needed :)

[wez]

Thanks for looking into this!
Could you also run a couple of tests to confirm that sixel and iterm2 image protocol handling isn't impacted by this change?
https://wezfurlong.org/wezterm/cli/imgcat.html is the easiest way to play around with the latter.
The former is a bit more ad-hoc.

The thing that caught my eye, although I haven't read your diff in detail yet, is that there were some changes around how the cursor position is handled, so it would be good to confirm that wezterm imgcat --no-move-cursor still functions correctly.

[jonboh]

Here's what I get running img2sixel and wezterm imgcat
I had checked img2sixel, and wezterm imgcat, but not with --no-move-cursor thanks for pointing it out, it seems the behaviour on all three cases is the same 👍.

Wezterm 05eadb7

kitty-image branch (this pr)

[wez]

If you wouldn't mind, could you run through these issues and confirm whether this PR addresses them? Thanks!

• Kitty graphics support does not implement w and h options correctly #1663
• wezterm losing image ids with kitty graphics #1156 (I think this is the id 0 thing you're fixing here?)
• Behaviour of graphics using kitty protocol differs from kitty implementation #2084 this one is definitely related to id 0
• Bad image output when using kitty protocol with width and height specified #4233
• Kitty protocol: Sending a sequence of PNG images shows only the last #3918
• Kitty image protocol fails to delete images after resize by placement id #2422

[jonboh]

Here are the results of the tests. All seem to be displayed correctly.

#1156 (kitty on the left, this branch on the right)

e left, test on the right)

#1156, #2084, #4233

#3918 (kitty on the left, this branch on the right)

#2422

#1663 did not provide a file/script with which to reproduce, but from the problem description and the other results I think we can consider it closed.

[wez]

So this PR will close all of those issues? Excellent work, thank you!

[jonboh]

Yeah, #2761 can also be closed, as its changes are already integrated here (the part about the id=0)
thank you for reviewing it and wezterm as a whole! :D