Kitty graphics protocol: Incorrect image deletion behaviour

[AnonymouX47]

Describe the bug
The delete action a=d with d=c|C, d=x|X, d=y|Y, d=p|P, d=q|Q all perform what they're specified to do but beyond that, they also delete all images off the screen i.e images that have been scrolled past the top of the window (not the text area i.e including the window title bar if in the topmost pane).

d=x|X might be excluded though, since the scrolled-off image might technically intersect with the column but I expect it to be limited to on-screen images.

This only affect off-screen images.

To Reproduce
Steps to reproduce the behavior:

1. Transmit an image with a=T.
2. Make the terminal scroll till the image is out of the window (not just the text area, if in the topmost pane).
3. Check the image to see it's still displayed.
4. Transmit <ESC>_Ga=d,d=c<ESC>\
5. Check the previously displayed image... Should've been deleted.
6. Repeat the steps above, replacing d=c with d=y,y=N (where N is an integer), etc...

Screenshots

Environment details

kitty 0.25.0 created by Kovid Goyal
Linux <...> 5.16.19-76051619-generic #202204081339~1649696161~21.10~091f44b SMP PREEMPT Mon Apr 11 17 x86_64
Pop!_OS 21.10 <...> /dev/tty

DISTRIB_ID=Pop
DISTRIB_RELEASE=21.10
DISTRIB_CODENAME=impish
DISTRIB_DESCRIPTION="Pop!_OS 21.10"
Running under: X11
Frozen: True
Paths:
  kitty: /usr/local/stow/kitty.app/bin/kitty
  base dir: /usr/local/stow/kitty.app/lib/kitty
  extensions dir: /usr/local/stow/kitty.app/lib/kitty-extensions
  system shell: /bin/bash
Loaded config files:
  /etc/xdg/kitty/kitty.conf
  /home/<...>/.config/kitty/kitty.conf

Config options different from defaults:
background_opacity            0.5
dynamic_background_opacity    True
enabled_layouts               ['splits', 'stack', 'tall', 'grid']
scrollback_pager_history_size 4194304
shell_integration             frozenset({'no-cursor'})
update_check_interval         0.0

<long... truncated>

Additional context
Same results with --config NONE and outside IPython (i.e directly from the shell).

[kovidgoyal]

I cannot replicate with::

icat logo/kitty.png && python -c "print('\n' * 120)" && printf '\e_Ga=d,d=c\e\'

[AnonymouX47]

Thanks so much... I tried with icat and the behaviour is correct just as you've said. In fact, the images from my code got erased while those from icat stayed.

I'll look into my implementation (and probably compare with icat).

[AnonymouX47]

After some further testing, it turns out

icat --scale-up ...

does have the same issue.

[kovidgoyal]

Does not reproduce for me with

icat --scale-up logo/kitty.png && python -c "print('\n' * 120)" && printf '\e_Ga=d,d=c\e'

[AnonymouX47]

The same also occurred here with icat --align=left and a=d,d=c.

So, I realized d=c only affected images intersecting the first column of the window. Then, I tested with the images produced by my code again... the ones intersecting column 1 were deleted but others not intersecting column 1 (even just a column away) were not affected.

Then, I decided to try d=y,y=N... no matter the value of N it cleared all images (regardless of alignment) that had been scrolled off the screen including those produced by icat.

[AnonymouX47]

Does not reproduce for me with

icat --scale-up logo/kitty.png && python -c "print('\n' * 120)" && printf '\e_Ga=d,d=c\e'

Oh, wow!

[AnonymouX47]

Then, I decided to try d=y,y=N... no matter the value of N it cleared all images (regardless of alignment) that had been scrolled off the screen including those produced by icat.

Even outrageous values (though not large enough to wrap around) of N like y=100000 gave the same result.

[AnonymouX47]

Thanks 👍

I'll build from source and check it out as soon as I can.

[kovidgoyal]

Please do as I am travelling at the moment so I dont have the time to
write proper tests for the fix.

[AnonymouX47]

Sorry it took some days...
Just tested it out via the latest nightly build, all the delete modes work fine and according to specification now 😃

All that trouble caused by some fixed-width integers and type casting 🥲.

Thanks so much!

[AnonymouX47]

By the way, here (AnonymouX47/term-image#39) is what I was working on when I discovered the bug.

I planned to use d=c for some aspects but since I plan to support Kitty >= 0.20.0, I guess I'll have to workaround the bug for now and maybe fully utilize the delete control code in a future version.

If I encounter any other difficulties/issues, I'll reach out.
Thanks for your swift response once again.

[kovidgoyal]

You're welcome and thanks for finding the bug. Feel free to ping me if
you have other issues.

[kovidgoyal]

Oh and if and when you release your kitty graphics protocol support feel free to ping me or send a PR to add a reference to your project in https://sw.kovidgoyal.net/kitty/graphics-protocol/

[AnonymouX47]

Ok, will do so.

[AnonymouX47]

Oh and if and when you release your kitty graphics protocol support feel free to ping me or send a PR to add a reference to your project in https://sw.kovidgoyal.net/kitty/graphics-protocol/

Hello! 😃

I've just released a new version of my project term-image with kitty graphics support.

Thanks ❤️

EDIT: Just sent a PR.