Basic frame loading works

docs/graphics-protocol.rst

@@ -548,8 +548,6 @@ Key      Value                 Default    Description
 -----------------------------------------------------------
 ``x``    Positive integer      ``0``      The left edge (in pixels) of where the frame data should be updated
 ``y``    Positive integer      ``0``      The top edge (in pixels) of where the frame data should be updated
-``w``    Positive integer      ``0``      The width (in pixels) of the frame area to update. By default, the entire width is used
-``h``    Positive integer      ``0``      The height (in pixels) of the frame area to update. By default, the entire height is used
 ``c``    Positive integer      ``0``      The 1-based frame number of the frame whose image data serves as the base data
                                           when creating a new frame, by default the base data is black, fully transparent pixels
 ``r``    Positive integer      ``0``      The 1-based frame number of the frame that is being edited. By default, a new frame is created

kitty/disk-cache.h

@@ -26,5 +26,5 @@ static inline void* disk_cache_malloc_allocator(void *x, size_t sz) {
 static inline bool
 read_from_disk_cache_simple(PyObject *self_, const void *key, size_t key_sz, void **data, size_t *data_sz) {
     *data = read_from_disk_cache(self_, key, key_sz, disk_cache_malloc_allocator, data_sz);
-    return PyErr_Occurred();
+    return PyErr_Occurred() == NULL;
 }

kitty/graphics.c

@@ -980,20 +980,19 @@ handle_animation_frame_load_command(GraphicsManager *self, GraphicsCommand *g, I
     self->currently_loading_data_for = (const ImageAndFrame){0};
     img = process_image_data(self, img, g, tt, fmt);
     if (!img) return NULL;
-    size_t data_width = g->data_width ? g->data_width : img->width, data_height = g->data_height ? g->data_height : img->height;
     img->width = w; img->height = h;
 #define FAIL(errno, ...) { free_load_data(&img->load_data); ABRT(errno, __VA_ARGS__); }
     if (img->data_loaded) {
         const unsigned bytes_per_pixel = img->is_opaque ? 3 : 4;
         const size_t expected_data_sz = img->width * img->height * bytes_per_pixel;
-        ImageAndFrame key = { .image_id = img->internal_id, .frame_idx = frame_number - 1 };
+        const ImageAndFrame key = { .image_id = img->internal_id, .frame_idx = frame_number - 1 };
 
         if (img->load_data.is_opaque != img->is_opaque)
             FAIL("EINVAL", "Transparency for frames must match that of the base image");
         if (img->load_data.is_4byte_aligned != img->is_4byte_aligned)
             FAIL("EINVAL", "Data type for frames must match that of the base image");
-        if (img->load_data.data_sz < bytes_per_pixel * data_width * data_height)
+        if (img->load_data.data_sz < bytes_per_pixel * g->data_width * g->data_height)
-            FAIL("ENODATA", "Insufficient image data %zu < %zu", img->load_data.data_sz, bytes_per_pixel * data_width, data_height);
+            FAIL("ENODATA", "Insufficient image data %zu < %zu", img->load_data.data_sz, bytes_per_pixel * g->data_width, g->data_height);
         if (is_new_frame && cache_size(self) + expected_data_sz > self->storage_limit * 5) {
             remove_images(self, trim_predicate, img->internal_id);
             if (is_new_frame && cache_size(self) + expected_data_sz > self->storage_limit * 5)
@@ -1004,7 +1003,7 @@ handle_animation_frame_load_command(GraphicsManager *self, GraphicsCommand *g, I
         size_t data_sz = 0;
         if (is_new_frame) {
             if (g->num_cells) {
-                ImageAndFrame other = { .image_id = img->internal_id, .frame_idx = g->num_cells - 1 };
+                const ImageAndFrame other = { .image_id = img->internal_id, .frame_idx = g->num_cells - 1 };
                 if (!read_from_cache(self, other, &base_data, &data_sz)) {
                     FAIL("ENODATA", "No data for frame with number: %u found in image: %u", g->num_cells, img->client_id);
                 }
@@ -1026,11 +1025,11 @@ handle_animation_frame_load_command(GraphicsManager *self, GraphicsCommand *g, I
             memcpy(base_data, img->load_data.data, data_sz);
         } else {
             const size_t dest_width = img->width > g->x_offset ? img->width - g->x_offset : 0;
-            const size_t stride = MIN(data_width, dest_width) * bytes_per_pixel;
+            const size_t stride = MIN(g->data_width, dest_width) * bytes_per_pixel;
-            for (size_t src_y = 0, dest_y = g->y_offset; src_y < data_height && dest_y < img->height; src_y++, dest_y++) {
+            for (size_t src_y = 0, dest_y = g->y_offset; src_y < g->data_height && dest_y < img->height; src_y++, dest_y++) {
                 memcpy(
                     (uint8_t*)base_data + dest_y * bytes_per_pixel * dest_width,
-                    img->load_data.data + src_y * bytes_per_pixel * data_width,
+                    img->load_data.data + src_y * bytes_per_pixel * g->data_width,
                     stride
                 );
             }

kitty_tests/graphics.py

@@ -594,7 +594,7 @@ class TestGraphics(BaseTest):
         def t(code='OK', image_id=1, frame_number=2, **kw):
             res = li(**kw)
             if code is not None:
-                self.assertEqual(code, res.code)
+                self.assertEqual(code, res.code, f'{code} != {res.code}: {res.msg}')
             if image_id is not None:
                 self.assertEqual(image_id, res.image_id)
             if frame_number is not None:
@@ -614,6 +614,28 @@ class TestGraphics(BaseTest):
         t(payload='2' * 36)
         img = g.image_for_client_id(1)
         self.assertEqual(img['extra_frames'], ({'gap': 40, 'data': b'2' * 36},))
+        # test editing a frame
+        t(payload='3' * 36, r=2)
+        img = g.image_for_client_id(1)
+        self.assertEqual(img['extra_frames'], ({'gap': 40, 'data': b'3' * 36},))
+        # test editing part of a frame
+        t(payload='4' * 12, r=2, s=2, v=2)
+        img = g.image_for_client_id(1)
+        self.assertEqual(img['extra_frames'], ({'gap': 40, 'data': b'444444333333444444333333333333333333'},))
+        t(payload='5' * 12, r=2, s=2, v=2, x=1, y=1)
+        img = g.image_for_client_id(1)
+        self.assertEqual(img['extra_frames'], ({'gap': 40, 'data': b'444444333555555444555555333333333333'},))
+        t(payload='3' * 36, r=2)
+        img = g.image_for_client_id(1)
+        self.assertEqual(img['extra_frames'], ({'gap': 40, 'data': b'3' * 36},))
+        # test loading from previous frame
+        t(payload='4' * 12, c=2, s=2, v=2, z=101, frame_number=3)
+        img = g.image_for_client_id(1)
+        self.assertEqual(img['extra_frames'], (
+            {'gap': 40, 'data': b'3' * 36},
+            {'gap': 101, 'data': b'444444333333444444333333333333333333'},
+        ))
 
-        # self.ae(g.image_count, 0)
+        li(a='d', d='A')
-        # self.assertEqual(g.disk_cache.total_size, 0)
+        self.ae(g.image_count, 0)
+        self.assertEqual(g.disk_cache.total_size, 0)