From 7e026ba119cc33a0fb60aa6705287347f5faddff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 2 Oct 2020 20:54:45 +0200 Subject: [PATCH 01/33] sixel: fold long line --- sixel.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sixel.c b/sixel.c index c6fe1a10..f9c11037 100644 --- a/sixel.c +++ b/sixel.c @@ -484,7 +484,9 @@ sixel_unhook(struct terminal *term) sixel_overwrite_by_rectangle( term, cursor->row, image.pos.col, image.rows, image.cols); - LOG_DBG("generating %dx%d pixman image at %d-%d", image.width, image.height, image.pos.row, image.pos.row + image.rows); + LOG_DBG("generating %dx%d pixman image at %d-%d", + image.width, image.height, + image.pos.row, image.pos.row + image.rows); image.pix = pixman_image_create_bits_no_clear( PIXMAN_a8r8g8b8, From 96405c2ca97aed6bb126a5a39d81b97100743b78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 2 Oct 2020 20:55:10 +0200 Subject: [PATCH 02/33] =?UTF-8?q?sixel:=20overwrite-by-rectangle=20expects?= =?UTF-8?q?=20=E2=80=98width=E2=80=99=20to=20not=20exceed=20screen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes a crash when the emitted sixel extends beyond the right margin. The crash only happens when there are other sixel images already emitted. Fixes part of #151 --- CHANGELOG.md | 2 ++ sixel.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f919972d..87e6440f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,8 @@ * Crash when `foot.ini` contains an invalid section name (https://codeberg.org/dnkl/foot/issues/159). * Background opacity when in _reverse video_ mode. +* Crash when writing a sixel image that extends outside the terminal's + right margin (https://codeberg.org/dnkl/foot/issues/151). ### Security diff --git a/sixel.c b/sixel.c index f9c11037..0600a434 100644 --- a/sixel.c +++ b/sixel.c @@ -482,7 +482,8 @@ sixel_unhook(struct terminal *term) }; sixel_overwrite_by_rectangle( - term, cursor->row, image.pos.col, image.rows, image.cols); + term, cursor->row, image.pos.col, + image.rows, min(image.cols, term->cols - image.pos.col)); LOG_DBG("generating %dx%d pixman image at %d-%d", image.width, image.height, From bc75f4744c02b60d896f445f6c2278416aa33ed2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 2 Oct 2020 20:56:44 +0200 Subject: [PATCH 03/33] sixel: fix sheared image when image crosses scrollback wrap-around MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a sixel image crosses the scrollback wrap-around, it is split up into (at least) two pieces. We use cursor->point.col for all pieces’ x-coordinate. This caused the final image to appear sheared, since we do a carriage-return (after a number of linefeeds) after each piece - this causes the cursor’s position to be reset to the left margin. The solution is simple; remember the cursor’s initial x-coordinate, and use that to position all image pieces. Closes #151. --- CHANGELOG.md | 3 +++ sixel.c | 12 +++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87e6440f..668b2c01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,9 @@ * Background opacity when in _reverse video_ mode. * Crash when writing a sixel image that extends outside the terminal's right margin (https://codeberg.org/dnkl/foot/issues/151). +* Sixel image at non-zero column positions getting sheared at + seemingly random occasions + (https://codeberg.org/dnkl/foot/issues/151). ### Security diff --git a/sixel.c b/sixel.c index 0600a434..747e3d00 100644 --- a/sixel.c +++ b/sixel.c @@ -448,6 +448,16 @@ sixel_unhook(struct terminal *term) int pixel_rows_left = term->sixel.image.height; const int stride = term->sixel.image.width * sizeof(uint32_t); + /* + * Need to 'remember' current cursor column. + * + * If we split up the sixel (to avoid scrollback wrap-around), we + * will emit a carriage-return (after several linefeeds), which + * will reset the cursor column to 0. If we use _that_ column for + * the subsequent image parts, the image will look sheared. + */ + const int start_col = term->grid->cursor.point.col; + /* We do not allow sixels to cross the scrollback wrap-around, as * this makes intersection calculations much more complicated */ while (pixel_rows_left > 0) { @@ -478,7 +488,7 @@ sixel_unhook(struct terminal *term) .height = height, .rows = (height + term->cell_height - 1) / term->cell_height, .cols = (width + term->cell_width - 1) / term->cell_width, - .pos = (struct coord){cursor->col, cur_row}, + .pos = (struct coord){start_col, cur_row}, }; sixel_overwrite_by_rectangle( From 6cf86d67d9fafe6ac4efc2626e2f0174c938f35f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 3 Oct 2020 22:41:20 +0200 Subject: [PATCH 04/33] =?UTF-8?q?term:=20re-calculate=20sixel=20images?= =?UTF-8?q?=E2=80=99=20rows/cols=20values=20when=20cell=20size=20changes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- terminal.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/terminal.c b/terminal.c index 077d15f0..a286161f 100644 --- a/terminal.c +++ b/terminal.c @@ -1526,6 +1526,20 @@ term_font_size_adjust(struct terminal *term, double amount) * So we delete them. */ sixel_destroy_all(term); + } else if (term->cell_width != old_cell_width || + term->cell_height != old_cell_height) + { + tll_foreach(term->normal.sixel_images, it) { + struct sixel *six = &it->item; + six->rows = (six->height + term->cell_height - 1) / term->cell_height; + six->cols = (six->width + term->cell_width - 1) / term->cell_width; + } + + tll_foreach(term->alt.sixel_images, it) { + struct sixel *six = &it->item; + six->rows = (six->height + term->cell_height - 1) / term->cell_height; + six->cols = (six->width + term->cell_width - 1) / term->cell_width; + } } return true; } From a53a81cebfa50b093cfec93a10171ed2f698e4b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 3 Oct 2020 22:42:03 +0200 Subject: [PATCH 05/33] sixel: overwrite: remove asserts Sixels may extend outside the visible screen area --- sixel.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/sixel.c b/sixel.c index 747e3d00..84a3ec96 100644 --- a/sixel.c +++ b/sixel.c @@ -211,11 +211,6 @@ static void sixel_overwrite(struct terminal *term, struct sixel *six, int row, int col, int height, int width) { - assert(row >= 0); - assert(row + height <= term->grid->num_rows); - assert(col >= 0); - assert(col + width <= term->grid->num_cols); - int rel_above = min(max(row - six->pos.row, 0), six->rows); int rel_below = max(min(row + height - six->pos.row, six->rows), 0); int rel_left = min(max(col - six->pos.col, 0), six->cols); @@ -315,11 +310,6 @@ static void _sixel_overwrite_by_rectangle( struct terminal *term, int row, int col, int height, int width) { - assert(row >= 0); - assert(row + height <= term->grid->num_rows); - assert(col >= 0); - assert(col + width <= term->grid->num_cols); - const int start = row; const int end = row + height - 1; @@ -492,8 +482,7 @@ sixel_unhook(struct terminal *term) }; sixel_overwrite_by_rectangle( - term, cursor->row, image.pos.col, - image.rows, min(image.cols, term->cols - image.pos.col)); + term, cursor->row, image.pos.col, image.rows, image.cols); LOG_DBG("generating %dx%d pixman image at %d-%d", image.width, image.height, From 5a6b96817d6e99afce4166d3cd1b311380f83fdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 3 Oct 2020 22:42:34 +0200 Subject: [PATCH 06/33] =?UTF-8?q?sixel:=20overwrite:=20calculate=20split-u?= =?UTF-8?q?p=20image=20pieces=E2=80=99=20rows/cols=20from=20their=20width/?= =?UTF-8?q?height?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sixel.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/sixel.c b/sixel.c index 84a3ec96..88da4e28 100644 --- a/sixel.c +++ b/sixel.c @@ -232,8 +232,6 @@ sixel_overwrite(struct terminal *term, struct sixel *six, imgs[0] = (struct sixel){ .width = six->width, .height = rel_above * term->cell_height, - .rows = rel_above, - .cols = six->cols, .pos = six->pos, }; imgs[0].data = xmalloc(imgs[0].width * imgs[0].height * sizeof(uint32_t)); @@ -244,8 +242,6 @@ sixel_overwrite(struct terminal *term, struct sixel *six, imgs[1] = (struct sixel){ .width = six->width, .height = six->height - rel_below * term->cell_height, - .rows = six->rows - rel_below, - .cols = six->cols, .pos = (struct coord){ six->pos.col, (six->pos.row + rel_below) & (term->grid->num_rows - 1)}, @@ -261,8 +257,6 @@ sixel_overwrite(struct terminal *term, struct sixel *six, imgs[2] = (struct sixel){ .width = rel_left * term->cell_width, .height = min(term->cell_height, six->height - rel_above * term->cell_height), - .rows = 1, - .cols = rel_left, .pos = (struct coord){ six->pos.col, (six->pos.row + rel_above) & (term->grid->num_rows - 1)}, @@ -279,8 +273,6 @@ sixel_overwrite(struct terminal *term, struct sixel *six, imgs[3] = (struct sixel){ .width = six->width - rel_right * term->cell_width, .height = min(term->cell_height, six->height - rel_above * term->cell_height), - .rows = 1, - .cols = six->cols - rel_right, .pos = (struct coord){ six->pos.col + rel_right, (six->pos.row + rel_above) & (term->grid->num_rows - 1)}, @@ -297,6 +289,9 @@ sixel_overwrite(struct terminal *term, struct sixel *six, if (imgs[i].data == NULL) continue; + imgs[i].rows = (imgs[i].height + term->cell_height - 1) / term->cell_height; + imgs[i].cols = (imgs[i].width + term->cell_width - 1) / term->cell_width; + imgs[i].pix = pixman_image_create_bits_no_clear( PIXMAN_a8r8g8b8, imgs[i].width, imgs[i].height, From 323119a6451445119c8e314e6bd18c19c7db9682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 3 Oct 2020 23:00:34 +0200 Subject: [PATCH 07/33] grid: reflow: re-insert sixels *after* new grid offset has been set Also make sure to destroy sixels that are too big to fit in the scrollback. Fixes issues with the sixel list not being sorted correctly. --- grid.c | 91 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 51 insertions(+), 40 deletions(-) diff --git a/grid.c b/grid.c index a79b4493..84ee5313 100644 --- a/grid.c +++ b/grid.c @@ -83,9 +83,10 @@ grid_reflow(struct grid *grid, int new_rows, int new_cols, * at the output that is *oldest* */ int offset = grid->offset + old_screen_rows; - tll(struct sixel) old_sixels = tll_init(); + tll(struct sixel) untranslated_sixels = tll_init(); + tll(struct sixel) translated_sixels = tll_init(); tll_foreach(grid->sixel_images, it) - tll_push_back(old_sixels, it->item); + tll_push_back(untranslated_sixels, it->item); tll_free(grid->sixel_images); /* Turn cursor coordinates into grid absolute coordinates */ @@ -108,7 +109,6 @@ grid_reflow(struct grid *grid, int new_rows, int new_cols, for (size_t i = 0; i < tracking_points_count; i++) tll_push_back(tracking_points, _tracking_points[i]); - /* * Walk the old grid */ @@ -122,7 +122,7 @@ grid_reflow(struct grid *grid, int new_rows, int new_cols, continue; /* Map sixels on current "old" row to current "new row" */ - tll_foreach(old_sixels, it) { + tll_foreach(untranslated_sixels, it) { if (it->item.pos.row != old_row_idx) continue; @@ -131,47 +131,17 @@ grid_reflow(struct grid *grid, int new_rows, int new_cols, /* Make sure it doesn't cross the wrap-around after being re-based */ int end = (sixel.pos.row + sixel.rows - 1) & (new_rows - 1); + if (end < sixel.pos.row) { /* TODO: split instead of destroying */ sixel_destroy(&it->item); } else { - - /* Insert sixel into the *sorted* list. */ - - /* Based on rebase_row() in sixel.c */ - /* Uses 'old' offset to ensure old sixels are treated as such */ -#define rebase_row(t, row) \ - (((row) - (grid->offset + new_screen_rows) + new_rows) & (new_rows - 1)) - - int end_row = rebase_row(term, sixel.pos.row + sixel.rows - 1); - - /* - * TODO: this is basically sixel_insert(), except we - * cannot use it since: - * - * a) we don't have a 'term' reference - * b) the grid hasn't been fully * updated yet - * (e.g. grid->num_rows is invalid etc). - */ - - bool inserted = false; - tll_foreach(grid->sixel_images, it2) { - const struct sixel *s = &it2->item; - if (rebase_row(term, s->pos.row + s->rows - 1) < end_row) { - tll_insert_before(grid->sixel_images, it2, sixel); - inserted = true; - break; - } - } - - if (!inserted) - tll_push_back(grid->sixel_images, sixel); - + /* Insert sixel into the translated, but *unsorted* list. */ + tll_push_back(translated_sixels, sixel); } /* Sixel has been either re-mapped, or destroyed */ - tll_remove(old_sixels, it); -#undef rebase_row + tll_remove(untranslated_sixels, it); } #define line_wrap() \ @@ -372,9 +342,50 @@ grid_reflow(struct grid *grid, int new_rows, int new_cols, grid->saved_cursor.lcf = false; /* Free sixels we failed to "map" to the new grid */ - tll_foreach(old_sixels, it) + tll_foreach(untranslated_sixels, it) sixel_destroy(&it->item); - tll_free(old_sixels); + tll_free(untranslated_sixels); + + /* Based on rebase_row() in sixel.c */ + /* Uses 'old' offset to ensure old sixels are treated as such */ +#define rebase_row(t, row) \ + (((row) - (grid->offset + new_screen_rows) + new_rows) & (new_rows - 1)) + + /* Re-add translated sixels, sorted according to the new grid offset */ + tll_foreach(translated_sixels, it) { + int end_row = rebase_row(term, it->item.pos.row + it->item.rows - 1); + + /* + * TODO: this is basically sixel_insert(), except we + * cannot use it since: + * + * a) we don't have a 'term' reference + * b) the grid hasn't been fully * updated yet + * (e.g. grid->num_rows is invalid etc). + */ + + if (it->item.rows >= new_rows) { + sixel_destroy(&it->item); + continue; + } + + assert(it->item.rows < new_rows); + + bool inserted = false; + tll_foreach(grid->sixel_images, it2) { + const struct sixel *s = &it2->item; + if (rebase_row(term, s->pos.row + s->rows - 1) < end_row) { + tll_insert_before(grid->sixel_images, it2, it->item); + inserted = true; + break; + } + } + + if (!inserted) + tll_push_back(grid->sixel_images, it->item); + } + tll_free(translated_sixels); +#undef rebase_row tll_free(tracking_points); } From cf48d1dc4ca4f61273d71bbde497caf4f36f471c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 11:00:50 +0200 Subject: [PATCH 08/33] sixel: debug: more fine-grained verification of sixel image list * Verify no sixel crosses the scrollback wrap-around * Verify no sixels overlap --- sixel.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/sixel.c b/sixel.c index 88da4e28..9cec4f62 100644 --- a/sixel.c +++ b/sixel.c @@ -110,7 +110,7 @@ rebase_row(const struct terminal *term, int abs_row) } static bool -verify_sixel_list_order(const struct terminal *term) +verify_list_order(const struct terminal *term) { #if defined(_DEBUG) int prev_row = INT_MAX; @@ -146,6 +146,65 @@ verify_sixel_list_order(const struct terminal *term) return true; } +static bool +verify_no_wraparound_crossover(const struct terminal *term) +{ +#if defined(_DEBUG) + tll_foreach(term->grid->sixel_images, it) { + const struct sixel *six = &it->item; + + assert(six->pos.row >= 0); + assert(six->pos.row < term->grid->num_rows); + + int end = (six->pos.row + six->rows) & (term->grid->num_rows - 1); + assert(end >= six->pos.row); + } +#endif + return true; +} + +#include + +static bool +verify_no_overlap(const struct terminal *term) +{ +#if defined(_DEBUG) + tll_foreach(term->grid->sixel_images, it) { + const struct sixel *six1 = &it->item; + + pixman_region32_t rect1; + pixman_region32_init_rect( + &rect1, six1->pos.col, six1->pos.row, six1->cols, six1->rows); + + tll_foreach(term->grid->sixel_images, it2) { + const struct sixel *six2 = &it2->item; + + if (six1 == six2) + continue; + + pixman_region32_t rect2; + pixman_region32_init_rect( + &rect2, six2->pos.col, + six2->pos.row, six2->cols, six2->rows); + + pixman_region32_t intersection; + pixman_region32_intersect(&intersection, &rect1, &rect2); + + assert(!pixman_region32_not_empty(&intersection)); + } + } +#endif + return true; +} + +static bool +verify_sixels(const struct terminal *term) +{ + return (verify_no_wraparound_crossover(term) && + verify_list_order(term) && + verify_no_overlap(term)); +} + static void sixel_insert(struct terminal *term, struct sixel sixel) { @@ -166,9 +225,8 @@ out: tll_foreach(term->grid->sixel_images, it) { LOG_DBG(" rows=%d+%d", it->item.pos.row, it->item.rows); } -#else - verify_sixel_list_order(term); #endif + verify_sixels(term); } void @@ -185,7 +243,7 @@ sixel_scroll_up(struct terminal *term, int rows) break; } - verify_sixel_list_order(term); + verify_sixels(term); } void @@ -204,7 +262,7 @@ sixel_scroll_down(struct terminal *term, int rows) break; } - verify_sixel_list_order(term); + verify_sixels(term); } static void From 52a715589748b12f379b0d9b67dc57efd43f2589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:09:24 +0200 Subject: [PATCH 09/33] sixel: add sixel_cell_size_changed() This function should be called *after* the cell dimensions have changed, but *before* resizing/reflowing the grids. --- sixel.c | 24 ++++++++++++++++++++++++ sixel.h | 1 + 2 files changed, 25 insertions(+) diff --git a/sixel.c b/sixel.c index 9cec4f62..110c8ab5 100644 --- a/sixel.c +++ b/sixel.c @@ -484,6 +484,30 @@ sixel_overwrite_at_cursor(struct terminal *term, int width) term, term->grid->cursor.point.row, term->grid->cursor.point.col, width); } +void +sixel_cell_size_changed(struct terminal *term) +{ + struct grid *g = term->grid; + + term->grid = &term->normal; + tll_foreach(term->normal.sixel_images, it) { + struct sixel *six = &it->item; + six->rows = (six->height + term->cell_height - 1) / term->cell_height; + six->cols = (six->width + term->cell_width - 1) / term->cell_width; + } + verify_sixels(term); + + term->grid = &term->alt; + tll_foreach(term->alt.sixel_images, it) { + struct sixel *six = &it->item; + six->rows = (six->height + term->cell_height - 1) / term->cell_height; + six->cols = (six->width + term->cell_width - 1) / term->cell_width; + } + verify_sixels(term); + + term->grid = g; +} + void sixel_unhook(struct terminal *term) { diff --git a/sixel.h b/sixel.h index 666b8c40..ac763bb0 100644 --- a/sixel.h +++ b/sixel.h @@ -16,6 +16,7 @@ void sixel_destroy_all(struct terminal *term); void sixel_scroll_up(struct terminal *term, int rows); void sixel_scroll_down(struct terminal *term, int rows); +void sixel_cell_size_changed(struct terminal *term); /* * Remove sixel data from the specified location. Used when printing * or erasing characters, and when emitting new sixel images, to From 14b4231c0929cb794619acd431b5c8aa363863e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:10:06 +0200 Subject: [PATCH 10/33] =?UTF-8?q?sixel:=20verify-no-wraparound-crossover:?= =?UTF-8?q?=20fix=20calculation=20of=20=E2=80=98end=E2=80=99=20row?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sixel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sixel.c b/sixel.c index 110c8ab5..32af3a65 100644 --- a/sixel.c +++ b/sixel.c @@ -156,7 +156,7 @@ verify_no_wraparound_crossover(const struct terminal *term) assert(six->pos.row >= 0); assert(six->pos.row < term->grid->num_rows); - int end = (six->pos.row + six->rows) & (term->grid->num_rows - 1); + int end = (six->pos.row + six->rows - 1) & (term->grid->num_rows - 1); assert(end >= six->pos.row); } #endif From 910219484679a5b5ae5edb55a26412c13df26fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:10:25 +0200 Subject: [PATCH 11/33] sixel: verify-sixels: check for bad list order last --- sixel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sixel.c b/sixel.c index 32af3a65..84b889fd 100644 --- a/sixel.c +++ b/sixel.c @@ -201,8 +201,8 @@ static bool verify_sixels(const struct terminal *term) { return (verify_no_wraparound_crossover(term) && - verify_list_order(term) && - verify_no_overlap(term)); + verify_no_overlap(term) && + verify_list_order(term)); } static void From 892730e5b94723304f9ee8a01c681733e02b2e1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:10:56 +0200 Subject: [PATCH 12/33] term: update sixel rows/cols *after* cell dimension change, *before* resize This fixes an issue where we resized+reflowed with bad rows/cols values in the sixels. --- terminal.c | 66 ++++++++++++++++++++++-------------------------------- 1 file changed, 27 insertions(+), 39 deletions(-) diff --git a/terminal.c b/terminal.c index a286161f..0afe39d5 100644 --- a/terminal.c +++ b/terminal.c @@ -605,12 +605,38 @@ term_set_fonts(struct terminal *term, struct fcft_font *fonts[static 4]) term->fonts[i] = fonts[i]; } + const int old_cell_width = term->cell_width; + const int old_cell_height = term->cell_height; + term->cell_width = term->fonts[0]->space_advance.x > 0 ? term->fonts[0]->space_advance.x : term->fonts[0]->max_advance.x; term->cell_height = max(term->fonts[0]->height, term->fonts[0]->ascent + term->fonts[0]->descent); LOG_INFO("cell width=%d, height=%d", term->cell_width, term->cell_height); + if (term->cell_width < old_cell_width || + term->cell_height < old_cell_height) + { + /* + * The cell size has decreased. + * + * This means sixels, which we cannot resize, no longer fit + * into their "allocated" grid space. + * + * To be able to fit them, we would have to change the grid + * content. Inserting empty lines _might_ seem acceptable, but + * we'd also need to insert empty columns, which would break + * existing layout completely. + * + * So we delete them. + */ + sixel_destroy_all(term); + } else if (term->cell_width != old_cell_width || + term->cell_height != old_cell_height) + { + sixel_cell_size_changed(term); + } + /* Use force, since cell-width/height may have changed */ render_resize_force(term, term->width / term->scale, term->height / term->scale); return true; @@ -1503,45 +1529,7 @@ term_font_size_adjust(struct terminal *term, double amount) term->font_sizes[i].px_size = -1; } - const int old_cell_width = term->cell_width; - const int old_cell_height = term->cell_height; - - if (!reload_fonts(term)) - return false; - - if (term->cell_width < old_cell_width || - term->cell_height < old_cell_height) - { - /* - * The cell size has decreased. - * - * This means sixels, which we cannot resize, no longer fit - * into their "allocated" grid space. - * - * To be able to fit them, we would have to change the grid - * content. Inserting empty lines _might_ seem acceptable, but - * we'd also need to insert empty columns, which would break - * existing layout completely. - * - * So we delete them. - */ - sixel_destroy_all(term); - } else if (term->cell_width != old_cell_width || - term->cell_height != old_cell_height) - { - tll_foreach(term->normal.sixel_images, it) { - struct sixel *six = &it->item; - six->rows = (six->height + term->cell_height - 1) / term->cell_height; - six->cols = (six->width + term->cell_width - 1) / term->cell_width; - } - - tll_foreach(term->alt.sixel_images, it) { - struct sixel *six = &it->item; - six->rows = (six->height + term->cell_height - 1) / term->cell_height; - six->cols = (six->width + term->cell_width - 1) / term->cell_width; - } - } - return true; + return reload_fonts(term); } bool From dbfc636ade8bd59a4a5af4531c19af8a163e8c09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:12:44 +0200 Subject: [PATCH 13/33] sixel: implement reflow Move sixel reflow from grid_reflow() to sixel_reflow(). This lets us use internal sixel functions to update the sixels. Note that grid_reflow() still needs to remap the sixelss coordinates. --- grid.c | 55 +------------------------------------------------------ sixel.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ sixel.h | 2 ++ 3 files changed, 51 insertions(+), 54 deletions(-) diff --git a/grid.c b/grid.c index 84ee5313..a779c1b3 100644 --- a/grid.c +++ b/grid.c @@ -84,7 +84,6 @@ grid_reflow(struct grid *grid, int new_rows, int new_cols, int offset = grid->offset + old_screen_rows; tll(struct sixel) untranslated_sixels = tll_init(); - tll(struct sixel) translated_sixels = tll_init(); tll_foreach(grid->sixel_images, it) tll_push_back(untranslated_sixels, it->item); tll_free(grid->sixel_images); @@ -129,18 +128,7 @@ grid_reflow(struct grid *grid, int new_rows, int new_cols, struct sixel sixel = it->item; sixel.pos.row = new_row_idx; - /* Make sure it doesn't cross the wrap-around after being re-based */ - int end = (sixel.pos.row + sixel.rows - 1) & (new_rows - 1); - - if (end < sixel.pos.row) { - /* TODO: split instead of destroying */ - sixel_destroy(&it->item); - } else { - /* Insert sixel into the translated, but *unsorted* list. */ - tll_push_back(translated_sixels, sixel); - } - - /* Sixel has been either re-mapped, or destroyed */ + tll_push_back(grid->sixel_images, sixel); tll_remove(untranslated_sixels, it); } @@ -346,46 +334,5 @@ grid_reflow(struct grid *grid, int new_rows, int new_cols, sixel_destroy(&it->item); tll_free(untranslated_sixels); - /* Based on rebase_row() in sixel.c */ - /* Uses 'old' offset to ensure old sixels are treated as such */ -#define rebase_row(t, row) \ - (((row) - (grid->offset + new_screen_rows) + new_rows) & (new_rows - 1)) - - /* Re-add translated sixels, sorted according to the new grid offset */ - tll_foreach(translated_sixels, it) { - int end_row = rebase_row(term, it->item.pos.row + it->item.rows - 1); - - /* - * TODO: this is basically sixel_insert(), except we - * cannot use it since: - * - * a) we don't have a 'term' reference - * b) the grid hasn't been fully * updated yet - * (e.g. grid->num_rows is invalid etc). - */ - - if (it->item.rows >= new_rows) { - sixel_destroy(&it->item); - continue; - } - - assert(it->item.rows < new_rows); - - bool inserted = false; - tll_foreach(grid->sixel_images, it2) { - const struct sixel *s = &it2->item; - if (rebase_row(term, s->pos.row + s->rows - 1) < end_row) { - tll_insert_before(grid->sixel_images, it2, it->item); - inserted = true; - break; - } - } - - if (!inserted) - tll_push_back(grid->sixel_images, it->item); - } - tll_free(translated_sixels); -#undef rebase_row - tll_free(tracking_points); } diff --git a/sixel.c b/sixel.c index 84b889fd..52eaa406 100644 --- a/sixel.c +++ b/sixel.c @@ -508,6 +508,54 @@ sixel_cell_size_changed(struct terminal *term) term->grid = g; } +void +sixel_reflow(struct terminal *term) +{ + struct grid *g = term->grid; + + for (size_t i = 0; i < 2; i++) { + struct grid *grid = i == 0 ? &term->normal : &term->alt; + + term->grid = grid; + + /* Need the “real” list to be empty from the beginning */ + tll(struct sixel) copy = tll_init(); + tll_foreach(grid->sixel_images, it) + tll_push_back(copy, it->item); + tll_free(grid->sixel_images); + + tll_rforeach(copy, it) { + struct sixel *six = &it->item; + int start = six->pos.row; + int end = (start + six->rows - 1) & (grid->num_rows - 1); + + if (end < start) { + /* Crosses scrollback wrap-around */ + /* TOOD: split image */ + sixel_destroy(six); + continue; + } + + if (six->rows > grid->num_rows) { + /* Image too large */ + /* TODO: keep bottom part? */ + sixel_destroy(six); + continue; + } + + /* Sixels that didn’t overlap may now do so, which isn’t + * allowed of course */ + _sixel_overwrite_by_rectangle( + term, six->pos.row, six->pos.col, six->rows, six->cols); + sixel_insert(term, it->item); + } + + tll_free(copy); + } + + term->grid = g; +} + void sixel_unhook(struct terminal *term) { diff --git a/sixel.h b/sixel.h index ac763bb0..6cd7324e 100644 --- a/sixel.h +++ b/sixel.h @@ -17,6 +17,8 @@ void sixel_scroll_up(struct terminal *term, int rows); void sixel_scroll_down(struct terminal *term, int rows); void sixel_cell_size_changed(struct terminal *term); +void sixel_reflow(struct terminal *term); + /* * Remove sixel data from the specified location. Used when printing * or erasing characters, and when emitting new sixel images, to From 47298776dcd25fb82b16a69bd67c268289776932 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:13:40 +0200 Subject: [PATCH 14/33] sixel: unhook: only call render_refresh() once --- sixel.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sixel.c b/sixel.c index 52eaa406..0d69e658 100644 --- a/sixel.c +++ b/sixel.c @@ -618,11 +618,10 @@ sixel_unhook(struct terminal *term) image.width, image.height, img_data, stride); - + /* Allocate space *first*, then insert */ for (size_t i = 0; i < image.rows; i++) term_linefeed(term); term_carriage_return(term); - render_refresh(term); sixel_insert(term, image); @@ -635,6 +634,8 @@ sixel_unhook(struct terminal *term) term->sixel.image.height = 0; term->sixel.max_col = 0; term->sixel.pos = (struct coord){0, 0}; + + render_refresh(term); } static unsigned From f2da822e9a05f6dade9004fb327555dd97b9f5f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:14:09 +0200 Subject: [PATCH 15/33] render: resize: call sixel_reflow() after reflowing grids --- render.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/render.c b/render.c index 427c8d44..4a006e10 100644 --- a/render.c +++ b/render.c @@ -20,6 +20,7 @@ #include "grid.h" #include "quirks.h" #include "selection.h" +#include "sixel.h" #include "shm.h" #include "util.h" #include "xmalloc.h" @@ -2197,6 +2198,8 @@ maybe_resize(struct terminal *term, int width, int height, bool force) term->cols = new_cols; term->rows = new_rows; + sixel_reflow(term); + LOG_DBG("resize: %dx%d, grid: cols=%d, rows=%d " "(left-margin=%d, right-margin=%d, top-margin=%d, bottom-margin=%d)", term->width, term->height, term->cols, term->rows, From 2f5df30ef51b2429144d22810ce69ee8b0d49451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:32:29 +0200 Subject: [PATCH 16/33] =?UTF-8?q?sixel:=20verify-no-overlap:=20initialize?= =?UTF-8?q?=20=E2=80=98intersection=E2=80=99=20pixman=20region?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sixel.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sixel.c b/sixel.c index 0d69e658..bea90414 100644 --- a/sixel.c +++ b/sixel.c @@ -188,6 +188,7 @@ verify_no_overlap(const struct terminal *term) six2->pos.row, six2->cols, six2->rows); pixman_region32_t intersection; + pixman_region32_init(&intersection); pixman_region32_intersect(&intersection, &rect1, &rect2); assert(!pixman_region32_not_empty(&intersection)); From 8129ff69c9d73ada39d6579ec740aff86601c8d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:32:53 +0200 Subject: [PATCH 17/33] sixel: verify-no-overlap: free pixman regions --- sixel.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sixel.c b/sixel.c index bea90414..54eea50c 100644 --- a/sixel.c +++ b/sixel.c @@ -192,7 +192,12 @@ verify_no_overlap(const struct terminal *term) pixman_region32_intersect(&intersection, &rect1, &rect2); assert(!pixman_region32_not_empty(&intersection)); + + pixman_region32_fini(&intersection); + pixman_region32_fini(&rect2); } + + pixman_region32_fini(&rect1); } #endif return true; From 42cc84eab7e74642ffdf7f91ea51243f61d05a58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 13:33:56 +0200 Subject: [PATCH 18/33] sixel: TOOD -> TODO (fixes codespell build error) --- sixel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sixel.c b/sixel.c index 54eea50c..157bcc09 100644 --- a/sixel.c +++ b/sixel.c @@ -537,7 +537,7 @@ sixel_reflow(struct terminal *term) if (end < start) { /* Crosses scrollback wrap-around */ - /* TOOD: split image */ + /* TODO: split image */ sixel_destroy(six); continue; } From 8168fc19dff6616312496a194fb70dff5827e5f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 14:03:45 +0200 Subject: [PATCH 19/33] sixel: scroll: call sixel_erase() instead of sixel_destroy() This ensures the screen is updated correctly. Without this, the sixel image would remain on screen until force-refreshed by some other means. --- sixel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sixel.c b/sixel.c index 157bcc09..4de17c11 100644 --- a/sixel.c +++ b/sixel.c @@ -243,7 +243,7 @@ sixel_scroll_up(struct terminal *term, int rows) int six_start = rebase_row(term, six->pos.row); if (six_start < rows) { - sixel_destroy(six); + sixel_erase(term, six); tll_remove(term->grid->sixel_images, it); } else break; @@ -262,7 +262,7 @@ sixel_scroll_down(struct terminal *term, int rows) int six_end = rebase_row(term, six->pos.row + six->rows - 1); if (six_end >= term->grid->num_rows - rows) { - sixel_destroy(six); + sixel_erase(term, six); tll_remove(term->grid->sixel_images, it); } else break; From 675f3e56c82524cf45526d6856c906133f341c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 14:05:37 +0200 Subject: [PATCH 20/33] =?UTF-8?q?sixel:=20overwrite-by-rectangle:=20assert?= =?UTF-8?q?=20sixels=20don=E2=80=99t=20cross=20scrollback=20wrap-around?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sixel.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sixel.c b/sixel.c index 4de17c11..8c95e70b 100644 --- a/sixel.c +++ b/sixel.c @@ -372,23 +372,26 @@ _sixel_overwrite_by_rectangle( const int start = row; const int end = row + height - 1; + /* We should never generate scrollback wrapping sixels */ + assert(end < term->grid->num_rows); + const int scrollback_rel_start = rebase_row(term, start); tll_foreach(term->grid->sixel_images, it) { struct sixel *six = &it->item; const int six_start = six->pos.row; - const int six_end = (six_start + six->rows - 1) & (term->grid->num_rows - 1); + const int six_end = (six_start + six->rows - 1); const int six_scrollback_rel_end = rebase_row(term, six_end); + /* We should never generate scrollback wrapping sixels */ + assert(six_end < term->grid->num_rows); + if (six_scrollback_rel_end < scrollback_rel_start) { /* All remaining sixels are *before* our rectangle */ break; } - /* We should never generate scrollback wrapping sixels */ - assert(six_end >= six_start); - if ((start <= six_start && end >= six_start) || /* Crosses sixel start boundary */ (start <= six_end && end >= six_end) || /* Crosses sixel end boundary */ (start >= six_start && end <= six_end)) /* Fully within sixel range */ From e870a0068f1967c98f5044890b5f9c0683f240dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 19:10:54 +0200 Subject: [PATCH 21/33] =?UTF-8?q?sixel:=20verify-list-order:=20add=20an=20?= =?UTF-8?q?=E2=80=98index=E2=80=99=20variable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will make it easier when debugging assertions in this function. --- sixel.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sixel.c b/sixel.c index 8c95e70b..07b16031 100644 --- a/sixel.c +++ b/sixel.c @@ -117,6 +117,9 @@ verify_list_order(const struct terminal *term) int prev_col = -1; int prev_col_count = 0; + /* To aid debugging */ + size_t idx = 0; + tll_foreach(term->grid->sixel_images, it) { int row = rebase_row(term, it->item.pos.row + it->item.rows - 1); int col = it->item.pos.col; @@ -141,6 +144,7 @@ verify_list_order(const struct terminal *term) prev_row = row; prev_col = col; prev_col_count = col_count; + idx++; } #endif return true; From b66e235e84f37d1995334580e0b7074fcc9e16a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 19:11:40 +0200 Subject: [PATCH 22/33] =?UTF-8?q?sixel:=20scroll-up:=20don=E2=80=99t=20bre?= =?UTF-8?q?ak=20out=20early=20of=20loop?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function loops the list of sixels, and discards those that would be scrolled out if the grid offset is moved forward by the specified number of rows. The criteria is when the rebased row value is less than the number of rows to scroll. A rebased row number is a zero-based number starting at the beginning of the scrollback. Thus, when scrolling 5 rows, rows with a rebased row number between 0-4 will be scrolled out. For performance reasons, we used to break out of the loop as soon as a row number *larger* than the scroll count. This however does not work. The sixels are sorted by their *end* row. While this works in most cases (think images outputted in the shell in the normal screen), it doesn’t always. In the alt screen, where applications can print images just about anywhere, it is possible to have *any* start row number anywhere in the sixel list. Just as long as their *end* row numbers are sorted. For example, a huuuge sixel that covers the entire scrollback. This sixel will naturally be first in the list (and thus sixel_scroll_up() will visit it *last*, since it iterates the list in reverse), but should still be destroyed when scrolling. --- sixel.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/sixel.c b/sixel.c index 07b16031..e584d677 100644 --- a/sixel.c +++ b/sixel.c @@ -246,11 +246,21 @@ sixel_scroll_up(struct terminal *term, int rows) struct sixel *six = &it->item; int six_start = rebase_row(term, six->pos.row); + if (six_start < rows) { sixel_erase(term, six); tll_remove(term->grid->sixel_images, it); - } else - break; + } else { + /* + * Unfortunately, we cannot break here. + * + * The sixels are sorted on their *end* row. This means + * there may be a sixel with a top row that will be + * scrolled out *anywhere* in the list (think of a huuuuge + * sixel that covers the entire scrollback) + */ + //break; + } } verify_sixels(term); From cf620cf3d002c67a4d4bbf418a6c4230d1ab97c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 19:17:33 +0200 Subject: [PATCH 23/33] sixel: unhook: look total number of sixels This helps debug sixel overwrites, as it makes it more visible when there are sixels that _should_ have been removed. --- sixel.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sixel.c b/sixel.c index e584d677..2696f211 100644 --- a/sixel.c +++ b/sixel.c @@ -658,6 +658,9 @@ sixel_unhook(struct terminal *term) term->sixel.max_col = 0; term->sixel.pos = (struct coord){0, 0}; + LOG_DBG("you now have %zu sixels in current grid", + tll_length(term->grid->sixel_images)); + render_refresh(term); } From 984083bf19d15db5972bfda1e9d2e205f6015ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 19:22:47 +0200 Subject: [PATCH 24/33] =?UTF-8?q?sixel:=20cell-size-changed:=20don?= =?UTF-8?q?=E2=80=99t=20verify=20sixels=20here?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The state after this function is an intermediate state and isn’t necessarily valid. This sixels needs to be ‘reflowed’ to ensure a valid state. This is something that should be done by the caller after the text grid has been reflowed and the sixel coordinates have been re-mapped to the new grid. TODO: can/should we update the sixel cols/rows in sixel_reflow() instead? --- sixel.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sixel.c b/sixel.c index 2696f211..ee34df0a 100644 --- a/sixel.c +++ b/sixel.c @@ -518,7 +518,6 @@ sixel_cell_size_changed(struct terminal *term) six->rows = (six->height + term->cell_height - 1) / term->cell_height; six->cols = (six->width + term->cell_width - 1) / term->cell_width; } - verify_sixels(term); term->grid = &term->alt; tll_foreach(term->alt.sixel_images, it) { @@ -526,7 +525,6 @@ sixel_cell_size_changed(struct terminal *term) six->rows = (six->height + term->cell_height - 1) / term->cell_height; six->cols = (six->width + term->cell_width - 1) / term->cell_width; } - verify_sixels(term); term->grid = g; } From 0a5a6cb7fa58577989716976a3430a9de4cac860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 19:28:22 +0200 Subject: [PATCH 25/33] sixel: scroll up/down: early return when list is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch tagged as ‘likely’ for performance reason --- sixel.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sixel.c b/sixel.c index ee34df0a..76b42b25 100644 --- a/sixel.c +++ b/sixel.c @@ -242,6 +242,9 @@ out: void sixel_scroll_up(struct terminal *term, int rows) { + if (likely(tll_length(term->grid->sixel_images) == 0)) + return; + tll_rforeach(term->grid->sixel_images, it) { struct sixel *six = &it->item; @@ -269,6 +272,9 @@ sixel_scroll_up(struct terminal *term, int rows) void sixel_scroll_down(struct terminal *term, int rows) { + if (likely(tll_length(term->grid->sixel_images) == 0)) + return; + assert(term->grid->num_rows >= rows); tll_foreach(term->grid->sixel_images, it) { From 5607e5d6583220b680186edc4b574015a312c0b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Oct 2020 19:29:48 +0200 Subject: [PATCH 26/33] =?UTF-8?q?sixel:=20verify-*:=20don=E2=80=99t=20retu?= =?UTF-8?q?rn=20anything;=20rely=20on=20asserts=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sixel.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/sixel.c b/sixel.c index 76b42b25..2ae0c042 100644 --- a/sixel.c +++ b/sixel.c @@ -109,7 +109,7 @@ rebase_row(const struct terminal *term, int abs_row) return rebased_row; } -static bool +static void verify_list_order(const struct terminal *term) { #if defined(_DEBUG) @@ -126,8 +126,6 @@ verify_list_order(const struct terminal *term) int col_count = it->item.cols; assert(row <= prev_row); - if (row > prev_row) - return false; if (row == prev_row) { /* Allowed to be on the same row only if their columns @@ -135,10 +133,6 @@ verify_list_order(const struct terminal *term) assert(col + col_count <= prev_col || prev_col + prev_col_count <= col); - - if (!(col + col_count <= prev_col || - prev_col + prev_col_count <= col)) - return false; } prev_row = row; @@ -147,10 +141,9 @@ verify_list_order(const struct terminal *term) idx++; } #endif - return true; } -static bool +static void verify_no_wraparound_crossover(const struct terminal *term) { #if defined(_DEBUG) @@ -164,12 +157,9 @@ verify_no_wraparound_crossover(const struct terminal *term) assert(end >= six->pos.row); } #endif - return true; } -#include - -static bool +static void verify_no_overlap(const struct terminal *term) { #if defined(_DEBUG) @@ -204,15 +194,14 @@ verify_no_overlap(const struct terminal *term) pixman_region32_fini(&rect1); } #endif - return true; } -static bool +static void verify_sixels(const struct terminal *term) { - return (verify_no_wraparound_crossover(term) && - verify_no_overlap(term) && - verify_list_order(term)); + verify_no_wraparound_crossover(term); + verify_no_overlap(term); + verify_list_order(term); } static void From 11760e9071e831bd883d6f502aae4fbf38b4851f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 5 Oct 2020 18:28:53 +0200 Subject: [PATCH 27/33] sixel: add comments to verify_*() functions --- sixel.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/sixel.c b/sixel.c index 2ae0c042..e55b52b0 100644 --- a/sixel.c +++ b/sixel.c @@ -99,6 +99,15 @@ sixel_erase(struct terminal *term, struct sixel *sixel) sixel_destroy(sixel); } +/* + * Calculates the scrollback relative row number, given an absolute row number. + * + * The scrollback relative row number 0 is the *first*, and *oldest* + * row in the scrollback history (and thus the *first* row to be + * scrolled out). Thus, a higher number means further *down* in the + * scrollback, with the *highest* number being at the bottom of the + * screen, where new input appears. + */ static int rebase_row(const struct terminal *term, int abs_row) { @@ -109,6 +118,12 @@ rebase_row(const struct terminal *term, int abs_row) return rebased_row; } +/* + * Verify the sixels are sorted correctly. + * + * The sixels are sorted on their *end* row, in descending order. This + * invariant means the most recent sixels appear first in the list. + */ static void verify_list_order(const struct terminal *term) { @@ -143,6 +158,11 @@ verify_list_order(const struct terminal *term) #endif } +/* + * Verifies there aren't any sixels that cross the scrollback + * wrap-around. This invariant means a sixel's absolute row numbers + * are strictly increasing. + */ static void verify_no_wraparound_crossover(const struct terminal *term) { @@ -159,6 +179,9 @@ verify_no_wraparound_crossover(const struct terminal *term) #endif } +/* + * Verifies no sixel overlap with any other sixels. + */ static void verify_no_overlap(const struct terminal *term) { From 5ebab9dea94c1fd06293e71ea3b904bc9902ad1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 5 Oct 2020 18:29:24 +0200 Subject: [PATCH 28/33] sixel: verify scrollback consistency: new verify function Verifies sixels have been scrolled out correctly --- sixel.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/sixel.c b/sixel.c index e55b52b0..6c378546 100644 --- a/sixel.c +++ b/sixel.c @@ -179,6 +179,31 @@ verify_no_wraparound_crossover(const struct terminal *term) #endif } +/* + * Verify there aren't any sixels that cross the scrollback end. This + * invariant means a sixel's rebased row numbers are strictly + * increasing. + */ +static void +verify_scrollback_consistency(const struct terminal *term) +{ +#if defined(_DEBUG) + tll_foreach(term->grid->sixel_images, it) { + const struct sixel *six = &it->item; + + int last_row = -1; + for (int i = 0; i < six->rows; i++) { + int row_no = rebase_row(term, six->pos.row + i); + + if (last_row != -1) + assert(last_row < row_no); + + last_row = row_no; + } + } +#endif +} + /* * Verifies no sixel overlap with any other sixels. */ @@ -223,6 +248,7 @@ static void verify_sixels(const struct terminal *term) { verify_no_wraparound_crossover(term); + verify_scrollback_consistency(term); verify_no_overlap(term); verify_list_order(term); } From 91c0aed26e387097dddadeee05b2af1773f972a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 5 Oct 2020 18:30:23 +0200 Subject: [PATCH 29/33] sixel: ovewrite-by-rectangle: in debug builds, cross-reference against pixman Use pixman to calculate the intersection of the rectangle being overwritten, and the sixel(s). Verify our code matches. --- sixel.c | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/sixel.c b/sixel.c index 6c378546..53bd3e75 100644 --- a/sixel.c +++ b/sixel.c @@ -427,6 +427,13 @@ static void _sixel_overwrite_by_rectangle( struct terminal *term, int row, int col, int height, int width) { + verify_sixels(term); + +#if defined(_DEBUG) + pixman_region32_t overwrite_rect; + pixman_region32_init_rect(&overwrite_rect, col, row, width, height); +#endif + const int start = row; const int end = row + height - 1; @@ -435,6 +442,8 @@ _sixel_overwrite_by_rectangle( const int scrollback_rel_start = rebase_row(term, start); + bool UNUSED would_have_breaked = false; + tll_foreach(term->grid->sixel_images, it) { struct sixel *six = &it->item; @@ -447,9 +456,23 @@ _sixel_overwrite_by_rectangle( if (six_scrollback_rel_end < scrollback_rel_start) { /* All remaining sixels are *before* our rectangle */ + would_have_breaked = true; break; } +#if defined(_DEBUG) + pixman_region32_t six_rect; + pixman_region32_init_rect(&six_rect, six->pos.col, six->pos.row, six->cols, six->rows); + + pixman_region32_t intersection; + pixman_region32_init(&intersection); + pixman_region32_intersect(&intersection, &six_rect, &overwrite_rect); + + const bool collides = pixman_region32_not_empty(&intersection); +#else + const bool UNUSED collides = false; +#endif + if ((start <= six_start && end >= six_start) || /* Crosses sixel start boundary */ (start <= six_end && end >= six_end) || /* Crosses sixel end boundary */ (start >= six_start && end <= six_end)) /* Fully within sixel range */ @@ -461,14 +484,27 @@ _sixel_overwrite_by_rectangle( (col <= col_end && col + width - 1 >= col_end) || (col >= col_start && col + width - 1 <= col_end)) { + assert(!would_have_breaked); + struct sixel to_be_erased = *six; tll_remove(term->grid->sixel_images, it); sixel_overwrite(term, &to_be_erased, start, col, height, width); sixel_erase(term, &to_be_erased); - } - } + } else + assert(!collides); + } else + assert(!collides); + +#if defined(_DEBUG) + pixman_region32_fini(&intersection); + pixman_region32_fini(&six_rect); +#endif } + +#if defined(_DEBUG) + pixman_region32_fini(&overwrite_rect); +#endif } void From 93f5e743cc70473eb9c528ea8ef89cbc3ed2d73e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 5 Oct 2020 18:31:25 +0200 Subject: [PATCH 30/33] sixel: overwrite: use pixman to calculate new the sixel boundaries When punching a hole in a sixel (and thus splitting it up into up to four new sixels), use pixman to calculate the new sixel coordinates and sizes. --- sixel.c | 153 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 76 insertions(+), 77 deletions(-) diff --git a/sixel.c b/sixel.c index 53bd3e75..b7bcb65a 100644 --- a/sixel.c +++ b/sixel.c @@ -333,93 +333,92 @@ static void sixel_overwrite(struct terminal *term, struct sixel *six, int row, int col, int height, int width) { - int rel_above = min(max(row - six->pos.row, 0), six->rows); - int rel_below = max(min(row + height - six->pos.row, six->rows), 0); - int rel_left = min(max(col - six->pos.col, 0), six->cols); - int rel_right = max(min(col + width - six->pos.col, six->cols), 0); + pixman_region32_t six_rect; + pixman_region32_init_rect( + &six_rect, + six->pos.col * term->cell_width, six->pos.row * term->cell_height, + six->width, six->height); - assert(rel_above >= 0); - assert(rel_below >= 0); - assert(rel_left >= 0); - assert(rel_right >= 0); + pixman_region32_t overwrite_rect; + pixman_region32_init_rect( + &overwrite_rect, + col * term->cell_width, row * term->cell_height, + width * term->cell_width, height * term->cell_height); - LOG_DBG("SPLIT: six (%p): %dx%d-%dx%d, %dx%d-%dx%d, rel: above=%d, below=%d, left=%d, right=%d", - (void *)six, six->pos.row, six->pos.col, six->rows, six->cols, - row, col, height, width, - rel_above, rel_below, rel_left, rel_right); +#if defined(_DEBUG) + pixman_region32_t intersection; + pixman_region32_init(&intersection); + pixman_region32_intersect(&intersection, &six_rect, &overwrite_rect); + assert(pixman_region32_not_empty(&intersection)); + pixman_region32_fini(&intersection); +#endif - struct sixel imgs[4] = {0}; + pixman_region32_t diff; + pixman_region32_init(&diff); + pixman_region32_subtract(&diff, &six_rect, &overwrite_rect); - if (rel_above > 0) { - imgs[0] = (struct sixel){ - .width = six->width, - .height = rel_above * term->cell_height, - .pos = six->pos, - }; - imgs[0].data = xmalloc(imgs[0].width * imgs[0].height * sizeof(uint32_t)); - memcpy(imgs[0].data, six->data, imgs[0].width * imgs[0].height * sizeof(uint32_t)); - } + pixman_region32_fini(&six_rect); + pixman_region32_fini(&overwrite_rect); - if (rel_below < six->rows) { - imgs[1] = (struct sixel){ - .width = six->width, - .height = six->height - rel_below * term->cell_height, - .pos = (struct coord){ - six->pos.col, - (six->pos.row + rel_below) & (term->grid->num_rows - 1)}, - }; - imgs[1].data = xmalloc(imgs[1].width * imgs[1].height * sizeof(uint32_t)); - memcpy( - imgs[1].data, - &((const uint32_t *)six->data)[rel_below * term->cell_height * six->width], - imgs[1].width * imgs[1].height * sizeof(uint32_t)); - } + int n_rects = -1; + pixman_box32_t *boxes = pixman_region32_rectangles(&diff, &n_rects); - if (rel_left > 0) { - imgs[2] = (struct sixel){ - .width = rel_left * term->cell_width, - .height = min(term->cell_height, six->height - rel_above * term->cell_height), - .pos = (struct coord){ - six->pos.col, - (six->pos.row + rel_above) & (term->grid->num_rows - 1)}, - }; - imgs[2].data = xmalloc(imgs[2].width * imgs[2].height * sizeof(uint32_t)); - for (size_t i = 0; i < imgs[2].height; i++) + for (int i = 0; i < n_rects; i++) { + LOG_DBG("box #%d: x1=%d, y1=%d, x2=%d, y2=%d", i, + boxes[i].x1, boxes[i].y1, boxes[i].x2, boxes[i].y2); + + assert(boxes[i].x1 % term->cell_width == 0); + assert(boxes[i].y1 % term->cell_height == 0); + + /* New image's position, in cells */ + const int new_col = boxes[i].x1 / term->cell_width; + const int new_row = boxes[i].y1 / term->cell_height; + + assert(new_row < term->grid->num_rows); + + /* New image's width and height, in pixels */ + const int new_width = boxes[i].x2 - boxes[i].x1; + const int new_height = boxes[i].y2 - boxes[i].y1; + + uint32_t *new_data = xmalloc(new_width * new_height * sizeof(uint32_t)); + const uint32_t *old_data = six->data; + + /* Pixel offsets into old image backing memory */ + const int x_ofs = boxes[i].x1 - six->pos.col * term->cell_width; + const int y_ofs = boxes[i].y1 - six->pos.row * term->cell_height; + + /* Copy image data, one row at a time */ + for (size_t j = 0; j < new_height; j++) { memcpy( - &((uint32_t *)imgs[2].data)[i * imgs[2].width], - &((const uint32_t *)six->data)[(rel_above * term->cell_height + i) * six->width], - imgs[2].width * sizeof(uint32_t)); - } + &new_data[(0 + j) * new_width], + &old_data[(y_ofs + j) * six->width + x_ofs], + new_width * sizeof(uint32_t)); + } - if (rel_right < six->cols) { - imgs[3] = (struct sixel){ - .width = six->width - rel_right * term->cell_width, - .height = min(term->cell_height, six->height - rel_above * term->cell_height), - .pos = (struct coord){ - six->pos.col + rel_right, - (six->pos.row + rel_above) & (term->grid->num_rows - 1)}, - }; - imgs[3].data = xmalloc(imgs[3].width * imgs[3].height * sizeof(uint32_t)); - for (size_t i = 0; i < imgs[3].height; i++) - memcpy( - &((uint32_t *)imgs[3].data)[i * imgs[3].width], - &((const uint32_t *)six->data)[(rel_above * term->cell_height + i) * six->width + rel_right * term->cell_width], - imgs[3].width * sizeof(uint32_t)); - } - - for (size_t i = 0; i < sizeof(imgs) / sizeof(imgs[0]); i++) { - if (imgs[i].data == NULL) - continue; - - imgs[i].rows = (imgs[i].height + term->cell_height - 1) / term->cell_height; - imgs[i].cols = (imgs[i].width + term->cell_width - 1) / term->cell_width; - - imgs[i].pix = pixman_image_create_bits_no_clear( + pixman_image_t *new_pix = pixman_image_create_bits_no_clear( PIXMAN_a8r8g8b8, - imgs[i].width, imgs[i].height, - imgs[i].data, imgs[i].width * sizeof(uint32_t)); - sixel_insert(term, imgs[i]); + new_width, new_height, new_data, new_width * sizeof(uint32_t)); + + struct sixel new_six = { + .data = new_data, + .pix = new_pix, + .width = new_width, + .height = new_height, + .pos = {.col = new_col, .row = new_row}, + .cols = (new_width + term->cell_width - 1) / term->cell_width, + .rows = (new_height + term->cell_height - 1) / term->cell_height, + }; + +#if defined(_DEBUG) + /* Assert we don't cross the scrollback wrap-around */ + const int new_end = new_six.pos.row + new_six.rows - 1; + assert(new_end < term->grid->num_rows); +#endif + + sixel_insert(term, new_six); } + + pixman_region32_fini(&diff); } /* Row numbers are absolute */ From 2c952761f2b113bf2c3f45abfe8119f243f38f88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 5 Oct 2020 18:32:44 +0200 Subject: [PATCH 31/33] sixel: unhook: do overwrite *after* linefeeding This ensures the overwrite is done when the scrollback history is in the same state as when we then insert the new image. --- sixel.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sixel.c b/sixel.c index b7bcb65a..0b971a90 100644 --- a/sixel.c +++ b/sixel.c @@ -706,8 +706,8 @@ sixel_unhook(struct terminal *term) .pos = (struct coord){start_col, cur_row}, }; - sixel_overwrite_by_rectangle( - term, cursor->row, image.pos.col, image.rows, image.cols); + assert(image.rows < term->grid->num_rows); + assert(image.pos.row + image.rows - 1 < term->grid->num_rows); LOG_DBG("generating %dx%d pixman image at %d-%d", image.width, image.height, @@ -723,6 +723,9 @@ sixel_unhook(struct terminal *term) term_linefeed(term); term_carriage_return(term); + _sixel_overwrite_by_rectangle( + term, image.pos.row, image.pos.col, image.rows, image.cols); + sixel_insert(term, image); pixel_row_idx += height; From 468add5591699215bc2ce16a0db1957d838b8fe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 5 Oct 2020 18:33:50 +0200 Subject: [PATCH 32/33] =?UTF-8?q?sixel:=20reflow:=20drop=20sixels=20that?= =?UTF-8?q?=20crosses=20the=20scrollback=20history=E2=80=99s=20end?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes various crashes that occurred after a reflow due to the sixel image list invariants no longer being true. --- sixel.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/sixel.c b/sixel.c index 0b971a90..898b8713 100644 --- a/sixel.c +++ b/sixel.c @@ -643,6 +643,29 @@ sixel_reflow(struct terminal *term) continue; } + /* Drop sixels that now cross the current scrollback end + * border. This is similar to a sixel that have been + * scrolled out */ + /* TODO: should be possible to optimize this */ + bool sixel_destroyed = false; + int last_row = -1; + + for (int j = 0; j < six->rows; j++) { + int row_no = rebase_row(term, six->pos.row + j); + if (last_row != -1 && last_row >= row_no) { + sixel_destroy(six); + sixel_destroyed = true; + break; + } + + last_row = row_no; + } + + if (sixel_destroyed) { + LOG_WARN("destroyed sixel that now crossed history"); + continue; + } + /* Sixels that didn’t overlap may now do so, which isn’t * allowed of course */ _sixel_overwrite_by_rectangle( From 606e901ceab75f3624f3fc1dde8307a647947607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 7 Oct 2020 18:46:11 +0200 Subject: [PATCH 33/33] =?UTF-8?q?changelog:=20we=E2=80=99ve=20fixed=20seve?= =?UTF-8?q?ral=20sixel+reflow=20related=20bugs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 668b2c01..1df82ec3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,8 @@ * Sixel image at non-zero column positions getting sheared at seemingly random occasions (https://codeberg.org/dnkl/foot/issues/151). +* Crash after either resizing a window or changing the font size if + there were sixels present in the scrollback while doing so. ### Security