Kitty image protocol fails to delete images after resize by placement id

[aslpavel]

What Operating System(s) are you seeing this problem on?

Linux Wayland

Which Wayland compositor or X11 Window manager(s) are you using?

Reproducible on X and Wayland with any window manager, tested on:

• Wayland Sway, Gnome
• X11 Gnome

WezTerm version

20220814-222723-f2ee48bd

Did you try the latest nightly build to see if the issue is better (or worse!) than your current version?

Yes, and I updated the version box above to show the version of the nightly that I tried

Describe the bug

Deleting image by placement id via kitty protocol \x1b_Ga=d,d=i,i={image_id},p={placement_id}\x1b\\ right after resize (this is done in response to SIGWINCH to redraw the screen) leaves them behind sometimes. Looking at kitty_remove_placement it looks like placement_id is resolved to coordinates which might change after resize?

To Reproduce

I do not have small reproducible example, I have a program that renders scalable graphic icons and displays them as glyphs when I resize it leaves some glyphs behind

$ git clone https://github.com/aslpavel/sweep-rs.git
$ cd sweep-rs/
$ ./scripts/demo.py
# resize wezterm multiple times

Broken output (images left behind)

How it looks on kitty

Configuration

local wezterm = require 'wezterm';

return {
   -- environment
   set_environment_variables = {
      COLORTERM="truecolor",
   },
   term = "xterm-24bit",

   -- font
   font_size = 11.0,
   font = wezterm.font_with_fallback({
         -- "PragmataPro",
         "Iosevka",
         -- "Victor Mono",
         "Material Design Icons",
         "Symbols Nerd Font",
   }),
   bold_brightens_ansi_colors = false,

   -- colors
   colors = {
      foreground = "#ebdbb2",
      background = "#282828",
      cursor_bg = "#ebdbb2",
      cursor_fg = "#ebdbb2",
      cursor_border = "#ebdbb2",
      scrollbar_thumb = "#504945",
      ansi = {"#282828", "#cc241d", "#98971a", "#d79921", "#458588", "#b16286", "#689d6a", "#ebdbb2"},
      brights = {"#7c6f64", "#fb4934", "#b8bb26", "#fabd2f", "#83a598", "#d3869b", "#8ec07c","#fbf1c7"},
      tab_bar = {
         background = "#32302f",
         active_tab = {
            bg_color = "#076678",
            fg_color = "#ebdbb2",
         },
         inactive_tab = {
            bg_color = "#458588",
            fg_color = "#3c3836",
         },
         inactive_tab_hover = {
            bg_color = "#83a598",
            fg_color = "#3c3836",
         },
         new_tab = {
            bg_color = "#458588",
            fg_color = "#ebdbb2",
         },
         new_tab_hover = {
            bg_color = "#076678",
            fg_color = "#ebdbb2",
         }
      }
   },

   -- tab bar
   use_fancy_tab_bar = false,
   tab_max_width = 20,

   default_cursor_style = "SteadyBar",
   enable_scroll_bar = true,
   enable_wayland = true,
   enable_kitty_graphics = true,
}

Expected Behavior

Image should be deleted by its (id, placement_id) on delete command even after resize

Logs

No response

Anything else?

No response

[aslpavel]

If you need more info on this please let me know

[wez]

This should be resolved now in main.

It typically takes about an hour before commits are available as nightly builds for all platforms. Linux builds are the fastest to build and are often available within about 20 minutes. Windows and macOS builds take a bit longer.

Please take a few moments to try out the fix and let me know how that works out. You can find the nightly downloads for your system in the wezterm installation docs.

If you prefer to use packages provided by your distribution or package manager of choice and don't want to replace that with a nightly download, keep in mind that you can download portable packages (eg: a .dmg file on macOS, a .zip file on Windows and an .AppImage file on Linux) that can be run without permanently installing or replacing an existing package, and can then simply be deleted once you no longer need them.

If you are eager and can build from source then you may be able to try this out more quickly.

[aslpavel]

Hi @wez! I have just tried 20240128-202157-1e552d76 it has not been fixed. It looks like according to the logs the image is removed.

22:23:14.233  TRACE  wezterm_term::terminalstate::kitty > Delete { what: ByImageId { image_id: 3470143523, placement_id: Some(65537), delete: false }, verbosity: Verbose }
22:23:14.233  TRACE  wezterm_term::terminalstate::kitty > remove a placement: image_id 3470143523 placement_id Some(65537) delete false verb Verbose
22:23:14.233  TRACE  wezterm_term::terminalstate::kitty > removed placement 3470143523 Some(65537)
22:23:14.233  TRACE  wezterm_term::terminalstate::kitty > after remove: there are 0 placements, 5 images, 46620 memory

But it must be some kind of race condition, my application on resize deletes all images by placement_id but some of them are not being deleted and remain floating, until full screen is cleared.

[wez]

@jonboh was the kind soul who contributed the recent fixes. @jonboh, what do we need to figure this out?

[jonboh]

my bad, while running the tests I didn't realize this one needed resizing wezterm to surface the issue, for some reason I thought it was about resizing with the parameters as the rest of the issues. I've tested this one again and the issue still persists.
I won't be able to dive into this until next week, but I'll check it out then and see if we can fix it.

[jonboh]

I've been checking this issue again and I have some findings. I have a fix for this particular issue, but it is more of a "hack" as it does not address the underlying issue of the pane geometry making the line indexes stale after geometry changes.

In summary the problem is that the firstRow: StableIndexRow that is passed in PlacementInfo
becomes stale after resizing.
I've opened a Draft-PR with the fix and some more details: #4995
Regarding this issue, with the "hack" the application seems to run correctly:

for reference, this is the current behavior in main: