diff --git a/docs/changelog.rst b/docs/changelog.rst index 82613579d..955829bd2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -91,6 +91,8 @@ Detailed list of changes - themes kitten: When using the *Default* theme as an auto switch theme include all the actual settings values (:iss:`8124`) +- Graphics: Fix deleted but not freed images without any references being incorrectly freed on a subsequent delete command (:disc:`8129`) + 0.38.0 [2024-12-15] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/kitty/graphics.c b/kitty/graphics.c index 562baea86..62f45a52a 100644 --- a/kitty/graphics.c +++ b/kitty/graphics.c @@ -1894,9 +1894,9 @@ remove_ref(Image *img, ImageRef *ref) { } static void -filter_refs(GraphicsManager *self, const void* data, bool free_images, bool (*filter_func)(const ImageRef*, Image*, const void*, CellPixelSize), CellPixelSize cell, bool only_first_image) { - bool matched = false; +filter_refs(GraphicsManager *self, const void* data, bool free_images, bool (*filter_func)(const ImageRef*, Image*, const void*, CellPixelSize), CellPixelSize cell, bool only_first_image, bool free_only_matched) { for (image_map_itr ii = vt_first(&self->images_by_internal_id); !vt_is_end(ii); ) { Image *img = ii.data->val; + bool matched = false; for (ref_map_itr ri = vt_first(&img->refs_by_internal_id); !vt_is_end(ri); ) { ImageRef *ref = ri.data->val; if (filter_func(ref, img, data, cell)) { ri = remove_ref_itr(img, ri); @@ -1904,7 +1904,7 @@ filter_refs(GraphicsManager *self, const void* data, bool free_images, bool (*fi matched = true; } else ri = vt_next(ri); } - if (!vt_size(&img->refs_by_internal_id) && (free_images || img->client_id == 0)) ii = remove_image_itr(self, ii); + if ((!free_only_matched || matched) && !vt_size(&img->refs_by_internal_id) && (free_images || img->client_id == 0)) ii = remove_image_itr(self, ii); else ii = vt_next(ii); if (only_first_image && matched) break; } @@ -2004,13 +2004,13 @@ void grman_remove_cell_images(GraphicsManager *self, int32_t top, int32_t bottom) { CellPixelSize dummy = {0}; int32_t data[] = {top, bottom}; - filter_refs(self, data, false, cell_image_row_filter_func, dummy, false); + filter_refs(self, data, false, cell_image_row_filter_func, dummy, false, true); } void grman_remove_all_cell_images(GraphicsManager *self) { CellPixelSize dummy = {0}; - filter_refs(self, NULL, false, cell_image_filter_func, dummy, false); + filter_refs(self, NULL, false, cell_image_filter_func, dummy, false, true); } @@ -2034,7 +2034,7 @@ clear_all_filter_func(const ImageRef *ref UNUSED, Image UNUSED *img, const void void grman_clear(GraphicsManager *self, bool all, CellPixelSize cell) { - filter_refs(self, NULL, true, all ? clear_all_filter_func : clear_filter_func, cell, false); + filter_refs(self, NULL, true, all ? clear_all_filter_func : clear_filter_func, cell, false, false); } static bool @@ -2091,7 +2091,7 @@ handle_delete_command(GraphicsManager *self, const GraphicsCommand *g, Cursor *c if (self->currently_loading.loading_for.image_id) free_load_data(&self->currently_loading); GraphicsCommand d; switch (g->delete_action) { -#define I(u, data, func) filter_refs(self, data, g->delete_action == u, func, cell, false); *is_dirty = true; break +#define I(u, data, func) filter_refs(self, data, g->delete_action == u, func, cell, false, true); *is_dirty = true; break #define D(l, u, data, func) case l: case u: I(u, data, func) #define G(l, u, func) D(l, u, g, func) case 0: @@ -2121,12 +2121,14 @@ handle_delete_command(GraphicsManager *self, const GraphicsCommand *g, Cursor *c } } break; case 'f': - case 'F': - if (handle_delete_frame_command(self, g, is_dirty) != NULL) { - filter_refs(self, g, true, id_filter_func, cell, true); + case 'F': { + Image *img = handle_delete_frame_command(self, g, is_dirty); + if (img != NULL) { + remove_image(self, img); *is_dirty = true; } break; + } default: REPORT_ERROR("Unknown graphics command delete action: %c", g->delete_action); break; diff --git a/kitty/screen.c b/kitty/screen.c index 9a97f71f8..4637f884a 100644 --- a/kitty/screen.c +++ b/kitty/screen.c @@ -184,7 +184,8 @@ screen_reset(Screen *self) { linebuf_clear(self->linebuf, BLANK_CHAR); historybuf_clear(self->historybuf); clear_hyperlink_pool(self->hyperlink_pool); - grman_clear(self->grman, false, self->cell_size); + grman_clear(self->main_grman, false, self->cell_size); // dont delete images in scrollback + grman_clear(self->alt_grman, true, self->cell_size); self->modes = empty_modes; self->saved_modes = empty_modes; self->active_hyperlink_id = 0; diff --git a/kitty_tests/graphics.py b/kitty_tests/graphics.py index dfd88c6f9..e8763b6fa 100644 --- a/kitty_tests/graphics.py +++ b/kitty_tests/graphics.py @@ -1055,6 +1055,11 @@ def delete(ac=None, **kw): delete() self.ae(s.grman.image_count, 1) self.ae(len(layers(s)), 0) + delete('A') + self.ae(s.grman.image_count, 1) + s.reset() + self.ae(s.grman.image_count, 0) + put_image(s, cw, ch) self.ae(s.grman.image_count, 1) delete('A') self.ae(s.grman.image_count, 0) @@ -1104,6 +1109,18 @@ def delete(ac=None, **kw): self.ae(s.grman.image_count, 0) self.assertEqual(s.grman.disk_cache.total_size, 0) + # test delete but not free + s.reset() + iid = 9999999 + self.ae(put_image(s, cw, ch, id=iid), (iid, 'OK')) + self.ae(put_ref(s, id=iid), (iid, ('OK', f'i={iid}'))) + self.ae(put_image(s, cw, ch, id=iid+1), (iid+1, 'OK')) + self.ae(put_ref(s, id=iid+1), (iid+1, ('OK', f'i={iid+1}'))) + delete('i', i=iid) + self.ae(s.grman.image_count, 2) + delete('I', i=iid+1) + self.ae(s.grman.image_count, 1) + def test_animation_frame_loading(self): s = self.create_screen() g = s.grman @@ -1210,6 +1227,7 @@ def expand(*rows): self.assertIsNone(li(a='d', d='f', i=1)) img = g.image_for_client_id(1) self.assertEqual(img['data'], b'5' * 36) + self.ae(g.image_count, 1) self.assertIsNone(li(a='d', d='F', i=1)) self.ae(g.image_count, 0) self.assertEqual(g.disk_cache.total_size, 0)