From ea39496f3083bac43f7da4adc0ae7e96d5329cf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 1 Jul 2021 20:19:28 +0200 Subject: [PATCH 01/52] =?UTF-8?q?changelog:=20add=20new=20=E2=80=98unrelea?= =?UTF-8?q?sed=E2=80=99=20section?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfa083ea..7fb6f75f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +* [Unreleased](#unreleased) * [1.8.1](#1-8-1) * [1.8.0](#1-8-0) * [1.7.2](#1-7-2) @@ -27,6 +28,16 @@ * [1.2.0](#1-2-0) +## Unreleased +### Added +### Changed +### Deprecated +### Removed +### Fixed +### Security +### Contributors + + ## 1.8.1 ### Added From 4e8db9d8b6d890ece807a8597fd94ee1da25d2d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 1 Jul 2021 20:44:53 +0200 Subject: [PATCH 02/52] changelog: s/from/in --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fb6f75f..f3908ca8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,7 @@ * Grapheme cluster width is now limited to two cells by default. This may cause cursor synchronization issues with many applications. You can set `[tweak].grapheme-width-method=wcswidth` to revert to the - behavior from foot-1.8.0. + behavior in foot-1.8.0. ### Fixed From b18d3aef17b59c97fd0dcdb881bfd88c36341571 Mon Sep 17 00:00:00 2001 From: Craig Barnes Date: Fri, 2 Jul 2021 08:36:39 +0100 Subject: [PATCH 03/52] vt: add some unit tests for action_collect() --- vt.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/vt.c b/vt.c index b9a4f4b0..b34f8fbe 100644 --- a/vt.c +++ b/vt.c @@ -412,6 +412,29 @@ action_collect(struct terminal *term, uint8_t c) LOG_WARN("only four private/intermediate characters supported"); } +UNITTEST +{ + struct terminal term = {.vt = {.private = 0}}; + uint32_t expected = ' '; + action_collect(&term, ' '); + xassert(term.vt.private == expected); + + expected |= '/' << 8; + action_collect(&term, '/'); + xassert(term.vt.private == expected); + + expected |= '<' << 16; + action_collect(&term, '<'); + xassert(term.vt.private == expected); + + expected |= '?' << 24; + action_collect(&term, '?'); + xassert(term.vt.private == expected); + + action_collect(&term, '?'); + xassert(term.vt.private == expected); +} + static void action_esc_dispatch(struct terminal *term, uint8_t final) { From 562096a21a9e0338a27eb920852683fca6956795 Mon Sep 17 00:00:00 2001 From: Craig Barnes Date: Fri, 2 Jul 2021 09:12:57 +0100 Subject: [PATCH 04/52] foot.ini: mention Control+c in default "cancel" key bindings This should have been done as part of commit 6eb6668c3ccb1f8a5921d938bd41722b11c82a7e. --- foot.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/foot.ini b/foot.ini index c10ed143..8a200d4b 100644 --- a/foot.ini +++ b/foot.ini @@ -122,7 +122,7 @@ # show-urls-copy=none [search-bindings] -# cancel=Control+g Escape +# cancel=Control+g Control+c Escape # commit=Return # find-prev=Control+r # find-next=Control+s @@ -142,7 +142,7 @@ # primary-paste=Shift+Insert [url-bindings] -# cancel=Control+g Control+d Escape +# cancel=Control+g Control+c Control+d Escape # toggle-url-visible=t [mouse-bindings] From 396a5ff79b6d59e1f187ba8d0a99d3aa58870fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 2 Jul 2021 16:29:08 +0200 Subject: [PATCH 05/52] allow-overflowing-double-width-glyphs: take glyph offset into account A narrow, but offset:ed glyph should still be considered double width. This patch also fixes a crash, when the maybe-double width glyph is in the last column. This is a regression. --- CHANGELOG.md | 7 +++++++ render.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3908ca8..a7dd2a15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,13 @@ ### Deprecated ### Removed ### Fixed + +* Glyph offset not being taken into account when applying + `tweak.allow-overflowing-double-width-glyphs`. +* Regression: crash when a single-char, double-width glyph is in the + last column, and `tweak.allow-overflowing-double-width-glyphs=yes`. + + ### Security ### Contributors diff --git a/render.c b/render.c index 499303f0..dbca64aa 100644 --- a/render.c +++ b/render.c @@ -618,13 +618,13 @@ render_cell(struct terminal *term, pixman_image_t *pix, if (term->conf->tweak.allow_overflowing_double_width_glyphs && ((glyph_count > 0 && cell_cols == 1 && - glyphs[0]->width >= term->cell_width * 15 / 10 && - glyphs[0]->width < 3 * term->cell_width && - col < term->cols - 1) || + glyphs[0]->x + glyphs[0]->width >= term->cell_width * 15 / 10 && + glyphs[0]->x + glyphs[0]->width < 3 * term->cell_width) || (term->conf->tweak.pua_double_width && ((base >= 0x00e000 && base <= 0x00f8ff) || (base >= 0x0f0000 && base <= 0x0ffffd) || (base >= 0x100000 && base <= 0x10fffd)))) && + col < term->cols - 1 && (row->cells[col + 1].wc == 0 || row->cells[col + 1].wc == L' ')) { cell_cols = 2; From 85b63b5e4a774b2c6f04859fd2a202ad362c6a5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 2 Jul 2021 16:48:46 +0200 Subject: [PATCH 06/52] render: require glyph count > 0 and cell-cols also for forced double-width PUAs --- render.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/render.c b/render.c index dbca64aa..80d5efb5 100644 --- a/render.c +++ b/render.c @@ -616,15 +616,15 @@ render_cell(struct terminal *term, pixman_image_t *pix, * - *this* cells is followed by an empty cell, or a space */ if (term->conf->tweak.allow_overflowing_double_width_glyphs && - ((glyph_count > 0 && - cell_cols == 1 && - glyphs[0]->x + glyphs[0]->width >= term->cell_width * 15 / 10 && + glyph_count > 0 && + cell_cols == 1 && + col < term->cols - 1 && + ((glyphs[0]->x + glyphs[0]->width >= term->cell_width * 15 / 10 && glyphs[0]->x + glyphs[0]->width < 3 * term->cell_width) || (term->conf->tweak.pua_double_width && ((base >= 0x00e000 && base <= 0x00f8ff) || (base >= 0x0f0000 && base <= 0x0ffffd) || (base >= 0x100000 && base <= 0x10fffd)))) && - col < term->cols - 1 && (row->cells[col + 1].wc == 0 || row->cells[col + 1].wc == L' ')) { cell_cols = 2; From d10132588530dd411efce2006d711f39203cd913 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 2 Jul 2021 16:53:49 +0200 Subject: [PATCH 07/52] doc: foot.ini.5: typo: relay-size-ms -> resize-delay-ms --- doc/foot.ini.5.scd | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/foot.ini.5.scd b/doc/foot.ini.5.scd index 684497ca..3ca08e84 100644 --- a/doc/foot.ini.5.scd +++ b/doc/foot.ini.5.scd @@ -188,8 +188,8 @@ in this order: In other words, while you are fiddling with the window size, foot does not send the updated dimensions to the client. Only when you - pause the fiddling for *relay-size-ms* milliseconds is the client - updated. + pause the fiddling for *resize-delay-ms* milliseconds is the + client updated. Emphasis is on _while_ here; as soon as the interactive resize ends (i.e. when you let go of the window border), the final From 07652d3b9e58dd99f5fedd6776092ed815ccb7fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 2 Jul 2021 16:31:46 +0200 Subject: [PATCH 08/52] term: rows printed to now defaults to having a hard linebreak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That is, instead of requiring a ‘\n’ to be printed, non-empty lines are now treated as having a hard linebreak by default. The linebreak is cleared on an explicit wrap. --- CHANGELOG.md | 5 +++++ terminal.c | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7dd2a15..dc969826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,11 @@ ## Unreleased ### Added ### Changed + +* Non-empty lines are now considered to have a hard linebreak, + _unless_ an actual word-wrap is inserted. + + ### Deprecated ### Removed ### Fixed diff --git a/terminal.c b/terminal.c index 6c175147..bede6811 100644 --- a/terminal.c +++ b/terminal.c @@ -2756,6 +2756,7 @@ print_linewrap(struct terminal *term) return; } + term->grid->cur_row->linebreak = false; term->grid->cursor.lcf = false; const int row = term->grid->cursor.point.row; @@ -2848,7 +2849,7 @@ term_print(struct terminal *term, wchar_t wc, int width) cell->attrs = term->vt.attrs; row->dirty = true; - row->linebreak = false; + row->linebreak = true; /* Advance cursor the 'additional' columns while dirty:ing the cells */ for (int i = 1; i < width && term->grid->cursor.point.col < term->cols - 1; i++) { @@ -2887,7 +2888,7 @@ ascii_printer_fast(struct terminal *term, wchar_t wc) cell->attrs = term->vt.attrs; row->dirty = true; - row->linebreak = false; + row->linebreak = true; /* Advance cursor */ if (unlikely(++term->grid->cursor.point.col >= term->cols)) { From fcb60abc132b12ccf8fb8ff215acc924a44773d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 4 Jul 2021 17:59:40 +0200 Subject: [PATCH 09/52] config: add locked-title=no|yes Closes #386 --- CHANGELOG.md | 5 +++++ config.c | 3 +++ config.h | 1 + doc/foot.ini.5.scd | 4 ++++ foot.ini | 4 ++++ terminal.c | 4 ++++ terminal.h | 1 + 7 files changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc969826..db325da7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,11 @@ ## Unreleased ### Added + +* `locked-title=no|yes` to `foot.ini` + (https://codeberg.org/dnkl/foot/issues/386). + + ### Changed * Non-empty lines are now considered to have a hard linebreak, diff --git a/config.c b/config.c index e02ca3de..92582f2c 100644 --- a/config.c +++ b/config.c @@ -643,6 +643,9 @@ parse_section_main(const char *key, const char *value, struct config *conf, conf->title = xstrdup(value); } + else if (strcmp(key, "locked-title") == 0) + conf->locked_title = str_to_bool(value); + else if (strcmp(key, "app-id") == 0) { free(conf->app_id); conf->app_id = xstrdup(value); diff --git a/config.h b/config.h index 2300a0ad..190ea427 100644 --- a/config.h +++ b/config.h @@ -77,6 +77,7 @@ struct config { wchar_t *word_delimiters; bool login_shell; bool no_wait; + bool locked_title; struct { enum conf_size_type type; diff --git a/doc/foot.ini.5.scd b/doc/foot.ini.5.scd index 3ca08e84..6ceaf3b9 100644 --- a/doc/foot.ini.5.scd +++ b/doc/foot.ini.5.scd @@ -224,6 +224,10 @@ in this order: *title* Initial window title. Default: _foot_. +*locked-title* + Boolean. If enabled, applications are not allowed to change the + title at run-time. Default: _no_. + *app-id* Value to set the *app-id* property on the Wayland window to. The compositor can use this value to e.g. group multiple windows, or diff --git a/foot.ini b/foot.ini index 8a200d4b..31167cd5 100644 --- a/foot.ini +++ b/foot.ini @@ -4,6 +4,10 @@ # term=foot (or xterm-256color if built with -Dterminfo=disabled) # login-shell=no +# app-id=foot +# title=foot +# locked-title=no + # font=monospace:size=8 # font-bold= # font-italic= diff --git a/terminal.c b/terminal.c index bede6811..488ac316 100644 --- a/terminal.c +++ b/terminal.c @@ -2634,9 +2634,13 @@ term_xcursor_update(struct terminal *term) void term_set_window_title(struct terminal *term, const char *title) { + if (term->conf->locked_title && term->window_title_has_been_set) + return; + free(term->window_title); term->window_title = xstrdup(title); render_refresh_title(term); + term->window_title_has_been_set = true; } void diff --git a/terminal.h b/terminal.h index 385b36e0..94c1cf75 100644 --- a/terminal.h +++ b/terminal.h @@ -382,6 +382,7 @@ struct terminal { bool sixel_cursor_right_of_graphics:1; } xtsave; + bool window_title_has_been_set; char *window_title; tll(char *) window_title_stack; From 9a14e5d8182ac18b24eeaa80c73c0fdc71c43092 Mon Sep 17 00:00:00 2001 From: Ben Brown Date: Thu, 8 Jul 2021 10:43:51 +0100 Subject: [PATCH 10/52] doc: fix typo in foot man page --- doc/foot.1.scd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/foot.1.scd b/doc/foot.1.scd index ab34a2dd..afa6cf54 100644 --- a/doc/foot.1.scd +++ b/doc/foot.1.scd @@ -204,7 +204,7 @@ Note that these are just the defaults; they can be changed in characters are whitespace characters. *ctrl*+*v*, *ctrl*+*y* - Paste from clipboard into the searh buffer. + Paste from clipboard into the search buffer. *shift*+*insert* Paste from primary selection into the search buffer. From 0a6e7e6167e04e1ee38cfaba4d38c2776cf474b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 11 Jul 2021 09:54:04 +0200 Subject: [PATCH 11/52] url-mode: purge SHM pixmaps when destroying URLs Unlike other surface types, the SHM cookie depends on the address of each URL instance. This means if we enable, disable, and then enable URL mode again (thus showing exactly the same URLs as the first time), the URLs will have new addresses, and thus the old SHM pixmaps will not get purged automatically. So, manually purge them when destroying the URLs. --- CHANGELOG.md | 2 ++ url-mode.c | 3 +++ wayland.c | 5 +++++ 3 files changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index db325da7..44093dc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,8 @@ `tweak.allow-overflowing-double-width-glyphs`. * Regression: crash when a single-char, double-width glyph is in the last column, and `tweak.allow-overflowing-double-width-glyphs=yes`. +* FD exhaustion when repeatedly entering/exiting URL mode with many + URLs. ### Security diff --git a/url-mode.c b/url-mode.c index 5f56a2f6..8ac5631e 100644 --- a/url-mode.c +++ b/url-mode.c @@ -14,6 +14,7 @@ #include "grid.h" #include "render.h" #include "selection.h" +#include "shm.h" #include "spawn.h" #include "terminal.h" #include "uri.h" @@ -728,7 +729,9 @@ urls_reset(struct terminal *term) if (term->window != NULL) { tll_foreach(term->window->urls, it) { + const struct url *url = it->item.url; wayl_win_subsurface_destroy(&it->item.surf); + shm_purge(term->wl->shm, shm_cookie_url(url)); tll_remove(term->window->urls, it); } } diff --git a/wayland.c b/wayland.c index 80fef88a..ecc4f31e 100644 --- a/wayland.c +++ b/wayland.c @@ -26,6 +26,7 @@ #include "input.h" #include "render.h" #include "selection.h" +#include "shm.h" #include "util.h" #include "xmalloc.h" @@ -1410,6 +1411,9 @@ wayl_win_destroy(struct wl_window *win) if (win == NULL) return; + struct terminal *term = win->term; + struct wl_shm *shm = term->wl->shm; + if (win->csd.move_timeout_fd != -1) close(win->csd.move_timeout_fd); @@ -1457,6 +1461,7 @@ wayl_win_destroy(struct wl_window *win) tll_foreach(win->urls, it) { wayl_win_subsurface_destroy(&it->item.surf); + shm_purge(shm, shm_cookie_url(it->item.url)); tll_remove(win->urls, it); } From b5950d9b27f28d4b62f533b4df728e1170aacd62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 11 Jul 2021 09:59:25 +0200 Subject: [PATCH 12/52] wayland: purge CSD pixmaps when destroying the CSDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CSDs aren’t typically toggled on and off. Thus, when disabled, immediately purge their corresponding pixmap buffers, to free up some memory, and release file descriptors. --- wayland.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/wayland.c b/wayland.c index ecc4f31e..bea6fb52 100644 --- a/wayland.c +++ b/wayland.c @@ -51,8 +51,13 @@ csd_instantiate(struct wl_window *win) static void csd_destroy(struct wl_window *win) { - for (size_t i = 0; i < ALEN(win->csd.surface); i++) + struct terminal *term = win->term; + struct wl_shm *shm = term->wl->shm; + + for (size_t i = 0; i < ALEN(win->csd.surface); i++) { wayl_win_subsurface_destroy(&win->csd.surface[i]); + shm_purge(shm, shm_cookie_csd(term, i)); + } } static void From a9872aac5af98ac5a643304b062aa984c4642f97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 11 Jul 2021 10:01:22 +0200 Subject: [PATCH 13/52] wayland: explicitly purge all SHM pixmaps when destroying window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All SHM pixmap cookies depend on the terminal instance’s memory address. Thus, after a terminal instance has been destroyed, shm pixmaps that belonged to it will never be purged automatically. --- wayland.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/wayland.c b/wayland.c index bea6fb52..ed3ddffd 100644 --- a/wayland.c +++ b/wayland.c @@ -1475,6 +1475,14 @@ wayl_win_destroy(struct wl_window *win) wayl_win_subsurface_destroy(&win->scrollback_indicator); wayl_win_subsurface_destroy(&win->render_timer); + shm_purge(shm, shm_cookie_search(term)); + shm_purge(shm, shm_cookie_scrollback_indicator(term)); + shm_purge(shm, shm_cookie_render_timer(term)); + shm_purge(shm, shm_cookie_grid(term)); + + for (size_t i = 0; i < ALEN(win->csd.surface); i++) + shm_purge(shm, shm_cookie_csd(term, i)); + #if defined(HAVE_XDG_ACTIVATION) if (win->xdg_activation_token != NULL) xdg_activation_token_v1_destroy(win->xdg_activation_token); From cf6d04f9f2fcb7a4f12d35edd0a6d4cd5591b59c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 11 Jul 2021 10:06:12 +0200 Subject: [PATCH 14/52] url-mode: fix crash when removing duplicate and/or overlapping URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing overlaping and duplicated URLs is done by running two nested loops, that both iterate the same URL list. When a duplicate is found, one of the URLs is destroyed and removed from the list. Deleting and removing an item *is* safe, but only as long as _no other_ iterator has references to it. In this case, if we remove an item from e.g. the inner iterator, we’ll crash if the outer iterator’s *next* item is the item being removed. Closes #627 --- CHANGELOG.md | 2 ++ terminal.h | 1 + url-mode.c | 18 +++++++++++------- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44093dc5..acea5f9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,8 @@ last column, and `tweak.allow-overflowing-double-width-glyphs=yes`. * FD exhaustion when repeatedly entering/exiting URL mode with many URLs. +* Double free of URL while removing duplicated and/or overlapping URLs + in URL mode (https://codeberg.org/dnkl/foot/issues/627). ### Security diff --git a/terminal.h b/terminal.h index 94c1cf75..309f7db6 100644 --- a/terminal.h +++ b/terminal.h @@ -264,6 +264,7 @@ struct url { enum url_action action; bool url_mode_dont_change_url_attr; /* Entering/exiting URL mode doesn’t touch the cells’ attr.url */ bool osc8; + bool duplicate; }; typedef tll(struct url) url_list_t; diff --git a/url-mode.c b/url-mode.c index 8ac5631e..94c05b3a 100644 --- a/url-mode.c +++ b/url-mode.c @@ -470,16 +470,20 @@ remove_overlapping(url_list_t *urls, int cols) */ xassert(in->osc8 || out->osc8); - if (in->osc8) { - url_destroy(&outer->item); - tll_remove(*urls, outer); - } else { - url_destroy(&inner->item); - tll_remove(*urls, inner); - } + if (in->osc8) + outer->item.duplicate = true; + else + inner->item.duplicate = true; } } } + + tll_foreach(*urls, it) { + if (it->item.duplicate) { + url_destroy(&it->item); + tll_remove(*urls, it); + } + } } void From 107cbb10157b165e19ed2f7fb747c60c670fe6c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 11 Jul 2021 10:22:47 +0200 Subject: [PATCH 15/52] =?UTF-8?q?shm:=20close=20pool=20FD=20if=20buffer=20?= =?UTF-8?q?doesn=E2=80=99t=20support=20SHM=20scrolling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only reason to keep the pool FD open is if we’re going to SHM scroll the buffer; we need the FD for fallocate(FALLOC_FL_PUNCH_HOLE). In all other cases, there’s absolutely no need to keep the FD open. Thus, close it as soon as we’ve instantiated the buffer. This frees up FDs, and help keep foot from FD ulimit. --- shm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/shm.c b/shm.c index ed20af3a..00804400 100644 --- a/shm.c +++ b/shm.c @@ -156,7 +156,6 @@ page_size(void) static bool instantiate_offset(struct wl_shm *shm, struct buffer *buf, off_t new_offset) { - xassert(buf->fd >= 0); xassert(buf->mmapped == NULL); xassert(buf->wl_buf == NULL); xassert(buf->pix == NULL); @@ -417,6 +416,12 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, } #endif + if (!shm_can_scroll(ret)) { + /* We only need to keep the pool FD open if we’re going to SHM + * scroll it */ + close(pool_fd); + } + return ret; err: From f030c87ee6b7f94e3704ae5c94667b7d46f30776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 11 Jul 2021 11:31:11 +0200 Subject: [PATCH 16/52] url-mode: abort when running into un-allocated scrollback memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When tagging URL cells (in preparation for rendering URL mode), we loop the URL’s entire range, setting the `url` attribute of all cells, and dirtying the rows. It is possible to create URLs that are invalid, and wrap around the scrollback, even though the scrollback hasn’t yet been filled. For example, by starting an OSC-8 URL, moving the cursor, and then closing the OSC-8 URL. These URLs are invalid, but are still rendered just fine. “Fine” being relative - they will typically fill the entire screen. But at least that’s a very clear indication for the user that’s something is wrong. The problem is when we hit un-allocated scrollback rows. We didn’t check for NULL rows, and crashed. This has now been fixed. --- CHANGELOG.md | 2 ++ url-mode.c | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index acea5f9c..f7bd40d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,8 @@ URLs. * Double free of URL while removing duplicated and/or overlapping URLs in URL mode (https://codeberg.org/dnkl/foot/issues/627). +* Crash when an unclosed OSC-8 URL ran into un-allocated scrollback + rows. ### Security diff --git a/url-mode.c b/url-mode.c index 94c05b3a..524b8d1b 100644 --- a/url-mode.c +++ b/url-mode.c @@ -654,6 +654,11 @@ tag_cells_for_url(struct terminal *term, const struct url *url, bool value) c = 0; row = term->grid->rows[r]; + if (row == NULL) { + /* Un-allocated scrollback. This most likely means a + * runaway OSC-8 URL. */ + break; + } row->dirty = true; } } From 8290cebc09c74ac0969ed2692a8de08ca134871c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 11 Jul 2021 11:54:08 +0200 Subject: [PATCH 17/52] server: the terminal now purges SHM pixmaps on its own --- server.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/server.c b/server.c index 9ab70f03..06f477db 100644 --- a/server.c +++ b/server.c @@ -132,13 +132,6 @@ term_shutdown_handler(void *data, int exit_code) { struct terminal_instance *instance = data; - struct wl_shm *shm = instance->server->wayl->shm; - - shm_purge(shm, shm_cookie_grid(instance->terminal)); - shm_purge(shm, shm_cookie_search(instance->terminal)); - for (enum csd_surface surf = 0; surf < CSD_SURF_COUNT; surf++) - shm_purge(shm, shm_cookie_csd(instance->terminal, surf)); - instance->terminal = NULL; instance_destroy(instance, exit_code); } From fcd989734235c25e64f8fb38ffcfdf381e933873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 14 Jul 2021 19:17:44 +0200 Subject: [PATCH 18/52] csi: invert the meaning of DECSDM There has been some confusion whether enabling DECSDM (private mode 80) enables or disables sixel scrolling. Foot currently enables scrolling when DECSDM is set, and this patch changes this, such that setting DECSDM now *disables* scrolling. The confusion is apparently due to a documentation error in the VT340 manual, as described in https://github.com/dankamongmen/notcurses/issues/1782#issuecomment-863603641. And that makes sense, in a way: the SDM in DECSDM stands for Sixel Display Mode. I.e. it stands to reason that enabling that disables scrolling. Anyway, this lead to https://github.com/hackerb9/lsix/issues/41, where it was eventually proven (by testing on a real VT340), that foot, and a large number of other terminals (including XTerm) has it wrong: https://github.com/hackerb9/lsix/issues/41#issuecomment-873269599. --- CHANGELOG.md | 2 ++ csi.c | 8 ++++---- terminal.h | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7bd40d8..20c52d1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ * Non-empty lines are now considered to have a hard linebreak, _unless_ an actual word-wrap is inserted. +* Setting `DECSDM` now _disables_ sixel scrolling, while resetting it + _enables_ scrolling (https://codeberg.org/dnkl/foot/issues/631). ### Deprecated diff --git a/csi.c b/csi.c index fcc2800f..80ee75cd 100644 --- a/csi.c +++ b/csi.c @@ -402,7 +402,7 @@ decset_decrst(struct terminal *term, unsigned param, bool enable) break; case 80: - term->sixel.scrolling = enable; + term->sixel.scrolling = !enable; break; case 1000: @@ -611,7 +611,7 @@ decrqm(const struct terminal *term, unsigned param, bool *enabled) case 12: *enabled = term->cursor_blink.decset; return true; case 25: *enabled = !term->hide_cursor; return true; case 45: *enabled = term->reverse_wrap; return true; - case 80: *enabled = term->sixel.scrolling; return true; + case 80: *enabled = !term->sixel.scrolling; return true; case 1000: *enabled = term->mouse_tracking == MOUSE_CLICK; return true; case 1001: *enabled = false; return true; case 1002: *enabled = term->mouse_tracking == MOUSE_DRAG; return true; @@ -654,7 +654,7 @@ xtsave(struct terminal *term, unsigned param) case 25: term->xtsave.show_cursor = !term->hide_cursor; break; case 45: term->xtsave.reverse_wrap = term->reverse_wrap; break; case 47: term->xtsave.alt_screen = term->grid == &term->alt; break; - case 80: term->xtsave.sixel_scrolling = term->sixel.scrolling; break; + case 80: term->xtsave.sixel_display_mode = !term->sixel.scrolling; break; case 1000: term->xtsave.mouse_click = term->mouse_tracking == MOUSE_CLICK; break; case 1001: break; case 1002: term->xtsave.mouse_drag = term->mouse_tracking == MOUSE_DRAG; break; @@ -696,7 +696,7 @@ xtrestore(struct terminal *term, unsigned param) case 25: enable = term->xtsave.show_cursor; break; case 45: enable = term->xtsave.reverse_wrap; break; case 47: enable = term->xtsave.alt_screen; break; - case 80: enable = term->xtsave.sixel_scrolling; break; + case 80: enable = term->xtsave.sixel_display_mode; break; case 1000: enable = term->xtsave.mouse_click; break; case 1001: return; case 1002: enable = term->xtsave.mouse_drag; break; diff --git a/terminal.h b/terminal.h index 309f7db6..12a637a0 100644 --- a/terminal.h +++ b/terminal.h @@ -378,7 +378,7 @@ struct terminal { bool ime:1; bool app_sync_updates:1; - bool sixel_scrolling:1; + bool sixel_display_mode:1; bool sixel_private_palette:1; bool sixel_cursor_right_of_graphics:1; } xtsave; From 9b4796e996a6b524718591df8d8ea1ebe3b9bbbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 13 Jul 2021 20:28:21 +0200 Subject: [PATCH 19/52] =?UTF-8?q?render:=20don=E2=80=99t=20assume=20PIXMAN?= =?UTF-8?q?=5Fa8r8g8b8=20for=20color=20glyphs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We may want to use PIXMAN_b8g8r8a8 on big-endian in the future. --- render.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/render.c b/render.c index 80d5efb5..81fe481d 100644 --- a/render.c +++ b/render.c @@ -682,7 +682,7 @@ render_cell(struct terminal *term, pixman_image_t *pix, if (i > 0 && glyph->x >= 0) g_x -= term->cell_width; - if (unlikely(pixman_image_get_format(glyph->pix) == PIXMAN_a8r8g8b8)) { + if (unlikely(PIXMAN_FORMAT_BPP(pixman_image_get_format(glyph->pix)) == 32)) { /* Glyph surface is a pre-rendered image (typically a color emoji...) */ if (!(cell->attrs.blink && term->blink.state == BLINK_OFF)) { pixman_image_composite32( @@ -2801,7 +2801,7 @@ render_search_box(struct terminal *term) continue; } - if (unlikely(pixman_image_get_format(glyph->pix) == PIXMAN_a8r8g8b8)) { + if (unlikely(PIXMAN_FORMAT_BPP(pixman_image_get_format(glyph->pix)) == 32)) { /* Glyph surface is a pre-rendered image (typically a color emoji...) */ pixman_image_composite32( PIXMAN_OP_OVER, glyph->pix, NULL, buf->pix[0], 0, 0, 0, 0, From 34f42b3dd638195bc35acdaa87762d0915757eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 13 Jul 2021 21:13:35 +0200 Subject: [PATCH 20/52] box-drawing: big-endian support when setting bits manually --- box-drawing.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/box-drawing.c b/box-drawing.c index 2eb92efd..b06fc322 100644 --- a/box-drawing.c +++ b/box-drawing.c @@ -1244,6 +1244,16 @@ draw_box_drawings_double_vertical_and_horizontal(struct buf *buf) vline(hmid + 2 * thick, buf->height, vmid + 2 * thick, thick); } +static inline void +set_a1_bit(uint8_t *data, size_t ofs, size_t bit_no) +{ +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ + data[ofs] |= 1 << bit_no; +#else + data[ofs] |= 1 << (7 - bit_no); +#endif +} + static void draw_box_drawings_light_arc(struct buf *buf, wchar_t wc) { @@ -1354,7 +1364,7 @@ draw_box_drawings_light_arc(struct buf *buf, wchar_t wc) if (fmt == PIXMAN_a1) { size_t idx = c / 8; size_t bit_no = c % 8; - data[r * stride + idx] |= 1 << bit_no; + set_a1_bit(data, r * stride + idx, bit_no); } else data[r * stride + c] = 0xff; } @@ -1376,7 +1386,7 @@ draw_box_drawings_light_arc(struct buf *buf, wchar_t wc) if (fmt == PIXMAN_a1) { size_t ofs = col / 8; size_t bit_no = col % 8; - data[row * stride + ofs] |= 1 << bit_no; + set_a1_bit(data, row * stride + ofs, bit_no); } else data[row * stride + col] = 0xff; } @@ -1392,7 +1402,7 @@ draw_box_drawings_light_arc(struct buf *buf, wchar_t wc) if (fmt == PIXMAN_a1) { size_t ofs = col / 8; size_t bit_no = col % 8; - data[row * stride + ofs] |= 1 << bit_no; + set_a1_bit(data, row * stride + ofs, bit_no); } else data[row * stride + col] = 0xff; } @@ -1767,7 +1777,7 @@ draw_light_shade(struct buf *buf) for (size_t col = 0; col < buf->width; col += 2) { size_t idx = col / 8; size_t bit_no = col % 8; - buf->data[row * buf->stride + idx] |= 1 << bit_no; + set_a1_bit(buf->data, row * buf->stride + idx, bit_no); } } } @@ -1790,7 +1800,7 @@ draw_medium_shade(struct buf *buf) for (size_t col = row % 2; col < buf->width; col += 2) { size_t idx = col / 8; size_t bit_no = col % 8; - buf->data[row * buf->stride + idx] |= 1 << bit_no; + set_a1_bit(buf->data, row * buf->stride + idx, bit_no); } } } @@ -1813,7 +1823,7 @@ draw_dark_shade(struct buf *buf) for (size_t col = 0; col < buf->width; col += 1 + row % 2) { size_t idx = col / 8; size_t bit_no = col % 8; - buf->data[row * buf->stride + idx] |= 1 << bit_no; + set_a1_bit(buf->data, row * buf->stride + idx, bit_no); } } } From 654e65631d4c1f2c25c9c750187348da088cf878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 14 Jul 2021 19:48:13 +0200 Subject: [PATCH 21/52] changelog: box-drawing fixes on big-endian --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7bd40d8..590c5aa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,8 @@ in URL mode (https://codeberg.org/dnkl/foot/issues/627). * Crash when an unclosed OSC-8 URL ran into un-allocated scrollback rows. +* Some box-drawing characters were rendered incorrectly on big-endian + architectures. ### Security From 0c777d825da92778233cbec1a9c659429ee1e20c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 14 Jul 2021 19:55:23 +0200 Subject: [PATCH 22/52] =?UTF-8?q?Revert=20"render:=20don=E2=80=99t=20assum?= =?UTF-8?q?e=20PIXMAN=5Fa8r8g8b8=20for=20color=20glyphs"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 9b4796e996a6b524718591df8d8ea1ebe3b9bbbd. Sub-pixel anti-aliasing also uses 32-bit glyphs (but x8r8g8b8, instead of a8r8g8b8)... --- render.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/render.c b/render.c index 81fe481d..80d5efb5 100644 --- a/render.c +++ b/render.c @@ -682,7 +682,7 @@ render_cell(struct terminal *term, pixman_image_t *pix, if (i > 0 && glyph->x >= 0) g_x -= term->cell_width; - if (unlikely(PIXMAN_FORMAT_BPP(pixman_image_get_format(glyph->pix)) == 32)) { + if (unlikely(pixman_image_get_format(glyph->pix) == PIXMAN_a8r8g8b8)) { /* Glyph surface is a pre-rendered image (typically a color emoji...) */ if (!(cell->attrs.blink && term->blink.state == BLINK_OFF)) { pixman_image_composite32( @@ -2801,7 +2801,7 @@ render_search_box(struct terminal *term) continue; } - if (unlikely(PIXMAN_FORMAT_BPP(pixman_image_get_format(glyph->pix)) == 32)) { + if (unlikely(pixman_image_get_format(glyph->pix) == PIXMAN_a8r8g8b8)) { /* Glyph surface is a pre-rendered image (typically a color emoji...) */ pixman_image_composite32( PIXMAN_OP_OVER, glyph->pix, NULL, buf->pix[0], 0, 0, 0, 0, From 9658e9cc18cd671d6016479280796510e42b23a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 14 Jul 2021 20:14:10 +0200 Subject: [PATCH 23/52] =?UTF-8?q?render:=20tiocswinsz:=20don=E2=80=99t=20r?= =?UTF-8?q?emove/close=20the=20fd=20passed=20as=20argument?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There’s a chance the resize timeout FD was closed, and *reused*, after epoll() told us the FD is readable, but before our callback runs. Thus, closing the FD provided as an argument is dangerous, as it may refer to something completely different. --- render.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/render.c b/render.c index 80d5efb5..c50988fd 100644 --- a/render.c +++ b/render.c @@ -3119,8 +3119,10 @@ fdm_tiocswinsz(struct fdm *fdm, int fd, int events, void *data) if (events & EPOLLIN) tiocswinsz(term); - fdm_del(fdm, fd); - term->window->resize_timeout_fd = -1; + if (term->window->resize_timeout_fd >= 0) { + fdm_del(fdm, term->window->resize_timeout_fd); + term->window->resize_timeout_fd = -1; + } return true; } From 22651ed2217632a34bb9045306fac6ab9a2b3414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 14 Jul 2021 20:51:42 +0200 Subject: [PATCH 24/52] shm: reset buffer pool FD when we close it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes an issue where we ended up "double closing" buffer FDs. In many cases (especially on compositors with SSDs) this was pretty rare. And even when it did happen, the FD was normally unused, and thus nothing bad happened. However, by quickly resizing the window while using CSDs, it was fairly easy to trigger this. We sometimes ended up closing the TIOCSWINCH timer FD while thinking it was a buffer FD, but most of the times we just ended up closing _another_ buffer’s pool FD, leading to an immediate disconnect by the compositor. --- shm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/shm.c b/shm.c index 00804400..cad60aae 100644 --- a/shm.c +++ b/shm.c @@ -420,6 +420,7 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, /* We only need to keep the pool FD open if we’re going to SHM * scroll it */ close(pool_fd); + ret->fd = -1; } return ret; From fcc2e62a7d9a38ab5818815e1f7c52267653f096 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 18:23:49 +0200 Subject: [PATCH 25/52] render: track alpha directly, rather whether to use it or not --- render.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/render.c b/render.c index c50988fd..7dcad297 100644 --- a/render.c +++ b/render.c @@ -458,7 +458,7 @@ render_cell(struct terminal *term, pixman_image_t *pix, uint32_t _fg = 0; uint32_t _bg = 0; - bool apply_alpha = false; + uint16_t alpha = 0xffff; if (is_selected && term->colors.use_custom_selection) { _fg = term->colors.selection_fg; @@ -472,14 +472,14 @@ render_cell(struct terminal *term, pixman_image_t *pix, uint32_t swap = _fg; _fg = _bg; _bg = swap; - } else - apply_alpha = !cell->attrs.have_bg; + } else if (!cell->attrs.have_bg) + alpha = term->colors.alpha; } if (unlikely(is_selected && _fg == _bg)) { /* Invert bg when selected/highlighted text has same fg/bg */ _bg = ~_bg; - apply_alpha = false; + alpha = 0xffff; } if (cell->attrs.dim) @@ -491,8 +491,7 @@ render_cell(struct terminal *term, pixman_image_t *pix, _fg = color_dim(_fg); pixman_color_t fg = color_hex_to_pixman(_fg); - pixman_color_t bg = color_hex_to_pixman_with_alpha( - _bg, apply_alpha ? term->colors.alpha : 0xffff); + pixman_color_t bg = color_hex_to_pixman_with_alpha(_bg, alpha); if (term->is_searching && !is_selected) { color_dim_for_search(&fg); From e259f460f3f39d67be2bd6a218d0348fa73a2e02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 18:24:27 +0200 Subject: [PATCH 26/52] render: margins: use correct background color * Sync actual color used, with how render_cell() selects the background color * Use color_dim_for_search(), not color_dim() --- render.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/render.c b/render.c index 7dcad297..b404f2f5 100644 --- a/render.c +++ b/render.c @@ -815,13 +815,10 @@ render_margin(struct terminal *term, struct buffer *buf, const int line_count = end_line - start_line; uint32_t _bg = !term->reverse ? term->colors.bg : term->colors.fg; - if (term->is_searching) - _bg = color_dim(_bg); + pixman_color_t bg = color_hex_to_pixman_with_alpha(_bg, term->colors.alpha); - pixman_color_t bg = color_hex_to_pixman_with_alpha( - _bg, - (_bg == (term->reverse ? term->colors.fg : term->colors.bg) - ? term->colors.alpha : 0xffff)); + if (term->is_searching) + color_dim_for_search(&bg); pixman_image_fill_rectangles( PIXMAN_OP_SRC, buf->pix[0], &bg, 4, From 628fd3909873433be34db5da80b0fedc714f69b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 18:26:26 +0200 Subject: [PATCH 27/52] render: scrollback indicator: improve handling of very small window sizes --- CHANGELOG.md | 2 ++ render.c | 27 +++++++++++++++++---------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 590c5aa5..a5aa84d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,8 @@ rows. * Some box-drawing characters were rendered incorrectly on big-endian architectures. +* Scrollback indicator being incorrectly rendered when window size is + very small. ### Security diff --git a/render.c b/render.c index b404f2f5..a8809513 100644 --- a/render.c +++ b/render.c @@ -2047,10 +2047,6 @@ render_scrollback_position(struct terminal *term) const int height = (2 * margin + term->cell_height + scale - 1) / scale * scale; - unsigned long cookie = shm_cookie_scrollback_indicator(term); - struct buffer *buf = shm_get_buffer( - term->wl->shm, width, height, cookie, false, 1); - /* *Where* to render - parent relative coordinates */ int surf_top = 0; switch (term->conf->scrollback.indicator.position) { @@ -2068,18 +2064,30 @@ render_scrollback_position(struct terminal *term) /* Make sure we don't collide with the scrollback search box */ lines--; } - xassert(lines > 0); - int pixels = lines * term->cell_height - height + 2 * margin; + lines = max(lines, 0); + + int pixels = max(lines * term->cell_height - height + 2 * margin, 0); surf_top = term->cell_height - margin + (int)(percent * pixels); break; } } + const int x = (term->width - margin - width) / scale * scale; + const int y = (term->margins.top + surf_top) / scale * scale; + + if (y + height > term->height) { + wl_surface_attach(win->scrollback_indicator.surf, NULL, 0, 0); + wl_surface_commit(win->scrollback_indicator.surf); + return; + } + + unsigned long cookie = shm_cookie_scrollback_indicator(term); + struct buffer *buf = shm_get_buffer( + term->wl->shm, width, height, cookie, false, 1); + wl_subsurface_set_position( - win->scrollback_indicator.sub, - (term->width - margin - width) / scale, - (term->margins.top + surf_top) / scale); + win->scrollback_indicator.sub, x / scale, y / scale); render_osd( term, @@ -2088,7 +2096,6 @@ render_scrollback_position(struct terminal *term) buf, text, term->colors.table[0], term->colors.table[8 + 4], width - margin - wcslen(text) * term->cell_width, margin); - } static void From 5e64c67c252bcc475fa4b155c0480f4da112dda4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 18:27:10 +0200 Subject: [PATCH 28/52] render: search: set clip region Fixes crash when the search box has been reduced in height, due to limited window space. --- CHANGELOG.md | 2 ++ render.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5aa84d9..67cbc32a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,8 @@ rows. * Some box-drawing characters were rendered incorrectly on big-endian architectures. +* Crash when resizing the window to the smallest possible size while + scrollback search is active. * Scrollback indicator being incorrectly rendered when window size is very small. diff --git a/render.c b/render.c index a8809513..7d365643 100644 --- a/render.c +++ b/render.c @@ -2626,6 +2626,12 @@ render_search_box(struct terminal *term) unsigned long cookie = shm_cookie_search(term); struct buffer *buf = shm_get_buffer(term->wl->shm, width, height, cookie, false, 1); + pixman_region32_t clip; + pixman_region32_init_rect(&clip, 0, 0, width, height); + pixman_image_set_clip_region32(buf->pix[0], &clip); + pixman_region32_fini(&clip); + + #define WINDOW_X(x) (margin + x) #define WINDOW_Y(y) (term->height - margin - height + y) From 7533684d8fa85c33756d18841f2bac5379f40f8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 18:32:19 +0200 Subject: [PATCH 29/52] shm: add shm_get_many() - allows buffers to share a single pool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit shm_get_many() always returns new buffers (i.e. never old, cached ones). The newly allocated buffers are also marked for immediate purging, meaning they’ll be destroyed on the next call to either shm_get_buffer(), or shm_get_many(). Furthermore, we add a new attribute, ‘locked’, to the buffer struct. When auto purging buffers, look at this instead of comparing cookies. Buffer consumers are expected to set ‘locked’ while they hold a reference to it, and don’t want it destroyed behind their back. --- render.c | 4 + shm.c | 338 +++++++++++++++++++++++++++++++++++-------------------- shm.h | 61 ++++++++-- 3 files changed, 274 insertions(+), 129 deletions(-) diff --git a/render.c b/render.c index 7d365643..37766232 100644 --- a/render.c +++ b/render.c @@ -2322,6 +2322,7 @@ grid_render(struct terminal *term) } if (term->render.last_buf != NULL) { + term->render.last_buf->locked = false; free(term->render.last_buf->scroll_damage); term->render.last_buf->scroll_damage = NULL; } @@ -2330,6 +2331,7 @@ grid_render(struct terminal *term) term->render.was_flashing = term->flash.active; term->render.was_searching = term->is_searching; + buf->locked = true; buf->age = 0; xassert(buf->scroll_damage == NULL); @@ -3416,6 +3418,8 @@ damage_view: tll_free(term->normal.scroll_damage); tll_free(term->alt.scroll_damage); + if (term->render.last_buf != NULL) + term->render.last_buf->locked = false; term->render.last_buf = NULL; term_damage_view(term); render_refresh_csd(term); diff --git a/shm.c b/shm.c index cad60aae..17b9c15d 100644 --- a/shm.c +++ b/shm.c @@ -88,20 +88,37 @@ buffer_destroy_dont_close(struct buffer *buf) buf->mmapped = NULL; } +static void +pool_unref(struct buffer_pool *pool) +{ + if (pool == NULL) + return; + + xassert(pool->ref_count > 0); + pool->ref_count--; + + if (pool->ref_count > 0) + return; + + if (pool->real_mmapped != MAP_FAILED) + munmap(pool->real_mmapped, pool->mmap_size); + if (pool->wl_pool != NULL) + wl_shm_pool_destroy(pool->wl_pool); + if (pool->fd >= 0) + close(pool->fd); + + pool->real_mmapped = MAP_FAILED; + pool->wl_pool = NULL; + pool->fd = -1; + free(pool); +} + static void buffer_destroy(struct buffer *buf) { buffer_destroy_dont_close(buf); - if (buf->real_mmapped != MAP_FAILED) - munmap(buf->real_mmapped, buf->mmap_size); - if (buf->pool != NULL) - wl_shm_pool_destroy(buf->pool); - if (buf->fd >= 0) - close(buf->fd); - - buf->real_mmapped = MAP_FAILED; + pool_unref(buf->pool); buf->pool = NULL; - buf->fd = -1; free(buf->scroll_damage); pixman_region32_fini(&buf->dirty); @@ -124,7 +141,10 @@ static void buffer_release(void *data, struct wl_buffer *wl_buffer) { struct buffer *buffer = data; - LOG_DBG("release: cookie=%lx (buf=%p)", buffer->cookie, (void *)buffer); + + LOG_DBG("release: cookie=%lx (buf=%p, total buffer count: %zu)", + buffer->cookie, (void *)buffer, tll_length(buffers)); + xassert(buffer->wl_buf == wl_buffer); xassert(buffer->busy); buffer->busy = false; @@ -159,15 +179,20 @@ instantiate_offset(struct wl_shm *shm, struct buffer *buf, off_t new_offset) xassert(buf->mmapped == NULL); xassert(buf->wl_buf == NULL); xassert(buf->pix == NULL); + xassert(buf->pool != NULL); + + const struct buffer_pool *pool = buf->pool; void *mmapped = MAP_FAILED; struct wl_buffer *wl_buf = NULL; pixman_image_t **pix = xcalloc(buf->pix_instances, sizeof(*pix)); - mmapped = (uint8_t *)buf->real_mmapped + new_offset; + mmapped = (uint8_t *)pool->real_mmapped + new_offset; wl_buf = wl_shm_pool_create_buffer( - buf->pool, new_offset, buf->width, buf->height, buf->stride, WL_SHM_FORMAT_ARGB8888); + pool->wl_pool, new_offset, buf->width, buf->height, buf->stride, + WL_SHM_FORMAT_ARGB8888); + if (wl_buf == NULL) { LOG_ERR("failed to create SHM buffer"); goto err; @@ -183,8 +208,8 @@ instantiate_offset(struct wl_shm *shm, struct buffer *buf, off_t new_offset) } } - buf->offset = new_offset; buf->mmapped = mmapped; + buf->offset = new_offset; buf->wl_buf = wl_buf; buf->pix = pix; @@ -205,18 +230,19 @@ err: return false; } -struct buffer * -shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, bool scrollable, size_t pix_instances) +static void NOINLINE +destroy_all_purgeables(void) { /* Purge buffers marked for purging */ tll_foreach(buffers, it) { - if (it->item.cookie != cookie) + if (it->item.locked) continue; if (!it->item.purge) continue; - xassert(!it->item.busy); + if (it->item.busy) + continue; LOG_DBG("cookie=%lx: purging buffer %p (width=%d, height=%d): %zu KB", cookie, (void *)&it->item, it->item.width, it->item.height, @@ -225,56 +251,15 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, buffer_destroy(&it->item); tll_remove(buffers, it); } +} - struct buffer *cached = NULL; - - tll_foreach(buffers, it) { - if (it->item.width != width) - continue; - if (it->item.height != height) - continue; - if (it->item.cookie != cookie) - continue; - - if (it->item.busy) - it->item.age++; - else -#if FORCED_DOUBLE_BUFFERING - if (it->item.age == 0) - it->item.age++; - else -#endif - { - LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)", - cookie, (void *)&it->item); - it->item.busy = true; - it->item.purge = false; - pixman_region32_clear(&it->item.dirty); - free(it->item.scroll_damage); - it->item.scroll_damage = NULL; - xassert(it->item.pix_instances == pix_instances); - cached = &it->item; - } - } - - if (cached != NULL) - return cached; - - /* Purge old buffers associated with this cookie */ - tll_foreach(buffers, it) { - if (it->item.cookie != cookie) - continue; - - if (it->item.busy) - continue; - - if (it->item.width == width && it->item.height == height) - continue; - - LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)&it->item); - it->item.purge = true; - } - +static void NOINLINE +get_new_buffers(struct wl_shm *shm, size_t count, + struct buffer_description info[static count], + struct buffer *bufs[static count], + size_t pix_instances, bool scrollable, bool immediate_purge) +{ + xassert(count == 1 || !scrollable); /* * No existing buffer available. Create a new one by: * @@ -285,14 +270,21 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, * The pixman image and the wayland buffer are now sharing memory. */ + int stride[count]; + int sizes[count]; + + size_t total_size = 0; + for (size_t i = 0; i < count; i++) { + stride[i] = stride_for_format_and_width(PIXMAN_a8r8g8b8, info[i].width); + sizes[i] = stride[i] * info[i].height; + total_size += sizes[i]; + } + int pool_fd = -1; - const int stride = stride_for_format_and_width(PIXMAN_a8r8g8b8, width); - const size_t size = stride * height; void *real_mmapped = MAP_FAILED; - struct wl_shm_pool *pool = NULL; - - LOG_DBG("cookie=%lx: allocating new buffer: %zu KB", cookie, size / 1024); + struct wl_shm_pool *wl_pool = NULL; + struct buffer_pool *pool = NULL; /* Backing memory for SHM */ #if defined(MEMFD_CREATE) @@ -311,14 +303,16 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, } #if __SIZEOF_POINTER__ == 8 - off_t initial_offset = scrollable && max_pool_size > 0 ? (max_pool_size / 4) & ~(page_size() - 1) : 0; - off_t memfd_size = scrollable && max_pool_size > 0 ? max_pool_size : size; + off_t offset = scrollable && max_pool_size > 0 ? (max_pool_size / 4) & ~(page_size() - 1) : 0; + off_t memfd_size = scrollable && max_pool_size > 0 ? max_pool_size : total_size; #else - off_t initial_offset = 0; - off_t memfd_size = size; + off_t offset = 0; + off_t memfd_size = total_size; #endif - LOG_DBG("memfd-size: %lu, initial offset: %lu", memfd_size, initial_offset); + xassert(scrollable || (offset == 0 && memfd_size == total_size)); + + LOG_DBG("memfd-size: %lu, initial offset: %lu", memfd_size, offset); if (ftruncate(pool_fd, memfd_size) == -1) { LOG_ERRNO("failed to set size of SHM backing memory file"); @@ -344,8 +338,8 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, } if (scrollable && !can_punch_hole) { - initial_offset = 0; - memfd_size = size; + offset = 0; + memfd_size = total_size; scrollable = false; if (ftruncate(pool_fd, memfd_size) < 0) { @@ -374,37 +368,55 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, } #endif - pool = wl_shm_create_pool(shm, pool_fd, memfd_size); - if (pool == NULL) { + wl_pool = wl_shm_create_pool(shm, pool_fd, memfd_size); + if (wl_pool == NULL) { LOG_ERR("failed to create SHM pool"); goto err; } - /* Push to list of available buffers, but marked as 'busy' */ - tll_push_back( - buffers, - ((struct buffer){ - .cookie = cookie, - .width = width, - .height = height, - .stride = stride, - .busy = true, - .size = size, - .pix_instances = pix_instances, - .fd = pool_fd, - .pool = pool, - .scrollable = scrollable, - .real_mmapped = real_mmapped, - .mmap_size = memfd_size, - .offset = 0, - .age = 1234, /* Force a full repaint */ - })); - - struct buffer *ret = &tll_back(buffers); - if (!instantiate_offset(shm, ret, initial_offset)) + pool = malloc(sizeof(*pool)); + if (pool == NULL) { + LOG_ERRNO("failed to allocate buffer pool"); goto err; + } - pixman_region32_init(&ret->dirty); + *pool = (struct buffer_pool){ + .fd = pool_fd, + .wl_pool = wl_pool, + .real_mmapped = real_mmapped, + .mmap_size = memfd_size, + .ref_count = 0, + }; + + for (size_t i = 0; i < count; i++) { + + /* Push to list of available buffers, but marked as 'busy' */ + tll_push_front( + buffers, + ((struct buffer){ + .cookie = info[i].cookie, + .width = info[i].width, + .height = info[i].height, + .stride = stride[i], + .busy = true, + .purge = immediate_purge, + .size = sizes[i], + .pix_instances = pix_instances, + .pool = pool, + .scrollable = scrollable, + .offset = 0, + .age = 1234, /* Force a full repaint */ + })); + + struct buffer *buf = &tll_front(buffers); + if (!instantiate_offset(shm, buf, offset)) + goto err; + + pixman_region32_init(&buf->dirty); + pool->ref_count++; + offset += buf->size; + bufs[i] = buf; + } #if defined(MEASURE_SHM_ALLOCS) && MEASURE_SHM_ALLOCS { @@ -416,18 +428,19 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, } #endif - if (!shm_can_scroll(ret)) { + if (!shm_can_scroll(bufs[0])) { /* We only need to keep the pool FD open if we’re going to SHM * scroll it */ close(pool_fd); - ret->fd = -1; + pool->fd = -1; } - return ret; + return; err: - if (pool != NULL) - wl_shm_pool_destroy(pool); + pool_unref(pool); + if (wl_pool != NULL) + wl_shm_pool_destroy(wl_pool); if (real_mmapped != MAP_FAILED) munmap(real_mmapped, memfd_size); if (pool_fd != -1) @@ -435,7 +448,78 @@ err: /* We don't handle this */ abort(); - return NULL; +} + +void +shm_get_many(struct wl_shm *shm, size_t count, + struct buffer_description info[static count], + struct buffer *bufs[static count], + size_t pix_instances) +{ + destroy_all_purgeables(); + get_new_buffers(shm, count, info, bufs, pix_instances, false, true); +} + +struct buffer * +shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, + bool scrollable, size_t pix_instances) +{ + destroy_all_purgeables(); + + struct buffer *cached = NULL; + tll_foreach(buffers, it) { + if (it->item.width != width) + continue; + if (it->item.height != height) + continue; + if (it->item.cookie != cookie) + continue; + + if (it->item.busy) + it->item.age++; + else +#if FORCED_DOUBLE_BUFFERING + if (it->item.age == 0) + it->item.age++; + else +#endif + { + if (cached == NULL) { + LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)", + cookie, (void *)&it->item); + it->item.busy = true; + it->item.purge = false; + pixman_region32_clear(&it->item.dirty); + free(it->item.scroll_damage); + it->item.scroll_damage = NULL; + xassert(it->item.pix_instances == pix_instances); + cached = &it->item; + } + } + } + + if (cached != NULL) + return cached; + + /* Mark old buffers associated with this cookie for purging */ + tll_foreach(buffers, it) { + if (it->item.cookie != cookie) + continue; + + if (it->item.busy) + continue; + + if (it->item.width == width && it->item.height == height) + continue; + + LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)&it->item); + it->item.purge = true; + } + + struct buffer *ret; + get_new_buffers(shm, 1, &(struct buffer_description){width, height, cookie}, + &ret, pix_instances, scrollable, false); + return ret; } bool @@ -453,12 +537,15 @@ shm_can_scroll(const struct buffer *buf) static bool wrap_buffer(struct wl_shm *shm, struct buffer *buf, off_t new_offset) { + struct buffer_pool *pool = buf->pool; + xassert(pool->ref_count == 1); + /* We don't allow overlapping offsets */ off_t UNUSED diff = new_offset < buf->offset ? buf->offset - new_offset : new_offset - buf->offset; xassert(diff > buf->size); - memcpy((uint8_t *)buf->real_mmapped + new_offset, buf->mmapped, buf->size); + memcpy((uint8_t *)pool->real_mmapped + new_offset, buf->mmapped, buf->size); off_t trim_ofs, trim_len; if (new_offset > buf->offset) { @@ -468,11 +555,11 @@ wrap_buffer(struct wl_shm *shm, struct buffer *buf, off_t new_offset) } else { /* Trim everything *after* the new buffer location */ trim_ofs = new_offset + buf->size; - trim_len = buf->mmap_size - trim_ofs; + trim_len = pool->mmap_size - trim_ofs; } if (fallocate( - buf->fd, + pool->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, trim_ofs, trim_len) < 0) { @@ -490,11 +577,15 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { + struct buffer_pool *pool = buf->pool; + xassert(can_punch_hole); xassert(buf->busy); xassert(buf->pix); xassert(buf->wl_buf); - xassert(buf->fd >= 0); + xassert(pool != NULL); + xassert(pool->ref_count == 1); + xassert(pool->fd >= 0); LOG_DBG("scrolling %d rows (%d bytes)", rows, rows * buf->stride); @@ -541,7 +632,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, const off_t trim_len = new_offset; if (fallocate( - buf->fd, + pool->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, trim_ofs, trim_len) < 0) { @@ -596,6 +687,9 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, { xassert(rows > 0); + struct buffer_pool *pool = buf->pool; + xassert(pool->ref_count == 1); + const off_t diff = rows * buf->stride; if (diff > buf->offset) { LOG_DBG("memfd offset reverse wrap-around"); @@ -634,10 +728,10 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, /* Free unused memory - everything after the relocated buffer */ const off_t trim_ofs = new_offset + buf->size; - const off_t trim_len = buf->mmap_size - trim_ofs; + const off_t trim_len = pool->mmap_size - trim_ofs; if (fallocate( - buf->fd, + pool->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, trim_ofs, trim_len) < 0) { @@ -712,9 +806,13 @@ shm_purge(struct wl_shm *shm, unsigned long cookie) if (it->item.cookie != cookie) continue; - xassert(!it->item.busy); - - buffer_destroy(&it->item); - tll_remove(buffers, it); + if (it->item.busy) { + LOG_WARN("deferring purge of 'busy' buffer (width=%d, height=%d)", + it->item.width, it->item.height); + it->item.purge = true; + } else { + buffer_destroy(&it->item); + tll_remove(buffers, it); + } } } diff --git a/shm.h b/shm.h index 385a0e30..610f0311 100644 --- a/shm.h +++ b/shm.h @@ -9,6 +9,16 @@ #include "terminal.h" +struct buffer_pool { + int fd; /* memfd */ + struct wl_shm_pool *wl_pool; + + void *real_mmapped; /* Address returned from mmap */ + size_t mmap_size; /* Size of mmap (>= size) */ + + size_t ref_count; +}; + struct buffer { unsigned long cookie; @@ -16,7 +26,8 @@ struct buffer { int height; int stride; - bool busy; + bool locked; /* Caller owned, shm won’t destroy it */ + bool busy; /* Owned by compositor */ size_t size; /* Buffer size */ void *mmapped; /* Raw data (TODO: rename) */ @@ -25,11 +36,7 @@ struct buffer { size_t pix_instances; /* Internal */ - int fd; /* memfd */ - struct wl_shm_pool *pool; - - void *real_mmapped; /* Address returned from mmap */ - size_t mmap_size; /* Size of mmap (>= size) */ + struct buffer_pool *pool; off_t offset; /* Offset into memfd where data begins */ bool scrollable; @@ -41,11 +48,47 @@ struct buffer { pixman_region32_t dirty; }; -struct buffer *shm_get_buffer( - struct wl_shm *shm, int width, int height, unsigned long cookie, bool scrollable, size_t pix_instances); -void shm_fini(void); +struct buffer_description { + int width; + int height; + unsigned long cookie; +}; +void shm_fini(void); void shm_set_max_pool_size(off_t max_pool_size); + +/* + * Returns a single buffer. + * + * May returned a cached buffer. If so, the buffer’s age indicates how + * many shm_get_buffer() calls have been made for the same + * width/height/cookie while the buffer was still busy. + * + * A newly allocated buffer has an age of 1234. + */ +struct buffer *shm_get_buffer( + struct wl_shm *shm, int width, int height, unsigned long cookie, + bool scrollable, size_t pix_instances); + +/* + * Returns many buffers, described by ‘info’, all sharing the same SHM + * buffer pool. + * + * Never returns cached buffers. However, the newly created buffers + * are all inserted into the regular buffer cache, and are treated + * just like buffers created by shm_get_buffer(). + * + * This function is useful when allocating many small buffers, with + * (roughly) the same life time. + * + * Buffers are tagged for immediate purging, and will be destroyed as + * soon as the compositor releases them. + */ +void shm_get_many( + struct wl_shm *shm, size_t count, + struct buffer_description info[static count], + struct buffer *bufs[static count], size_t pix_instances); + bool shm_can_scroll(const struct buffer *buf); bool shm_scroll(struct wl_shm *shm, struct buffer *buf, int rows, int top_margin, int top_keep_rows, From 31fad5846516ee1e6135bd35f0478b93928a3759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 18:39:41 +0200 Subject: [PATCH 30/52] url-mode: use shm_get_many() If we have lots of URLs, we use up a *lot* of SHM buffers (and thus pools). Each and every one is a single mmap(), of at least 4K. Since all URL labels are created and destroyed at the same time, it makes sense to use a single pool for all buffers. To do this, we must now do two passes; first one to generate the actual string (label content), and to calculate the label sizes. Then we use this information to allocate all SHM buffers at once, from the same pool. Finally, we iterate the URLs again, this time to actually render them. --- CHANGELOG.md | 1 + render.c | 92 ++++++++++++++++++++++++++++++++++++++++++++-------- url-mode.c | 3 -- 3 files changed, 79 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67cbc32a..b66079fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ scrollback search is active. * Scrollback indicator being incorrectly rendered when window size is very small. +* Reduced memory usage in URL mode. ### Security diff --git a/render.c b/render.c index 37766232..8e66bf00 100644 --- a/render.c +++ b/render.c @@ -2886,6 +2886,10 @@ render_urls(struct terminal *term) struct wl_window *win = term->window; xassert(tll_length(win->urls) > 0); + const int scale = term->scale; + const int x_margin = 2 * scale; + const int y_margin = 1 * scale; + /* Calculate view start, counted from the *current* scrollback start */ const int scrollback_end = (term->grid->offset + term->rows) & (term->grid->num_rows - 1); @@ -2897,6 +2901,44 @@ render_urls(struct terminal *term) const bool show_url = term->urls_show_uri_on_jump_label; + /* + * There can potentially be a lot of URLs. + * + * Since each URL is a separate sub-surface, and requires its own + * SHM buffer, we may be allocating a lot of buffers. + * + * SHM buffers normally have their own, private SHM buffer + * pool. Each pool is mmapped, and thus allocates *at least* + * 4K. Since URL labels are typically small, we end up using an + * excessive amount of both virtual and physical memory. + * + * For this reason, we instead use shm_get_many(), which uses a + * single, shared pool for all buffers. + * + * To be able to use it, we need to have all the *all* the buffer + * dimensions up front. + * + * Thus, the first iteration through the URLs do the heavy + * lifting: builds the label contents and calculates both its + * position and size. But instead of rendering the label + * immediately, we store the calculated data, and then do a second + * pass, where we first get all our buffers, and then render to + * them. + */ + + /* Positioning data + label contents */ + struct { + const struct wl_url *url; + wchar_t *text; + int x; + int y; + } info[tll_length(win->urls)]; + + /* For shm_get_many() */ + struct buffer_description shm_desc[tll_length(win->urls)]; + + size_t render_count = 0; + tll_foreach(win->urls, it) { const struct url *url = it->item.url; const wchar_t *key = url->key; @@ -2953,6 +2995,7 @@ render_urls(struct terminal *term) /* Maximum width of label, in pixels */ const int max_width = term->width - term->margins.left - term->margins.right - x; + const int max_cols = max_width / term->cell_width; const size_t key_len = wcslen(key); @@ -2990,10 +3033,6 @@ render_urls(struct terminal *term) * Do it in a way such that we don’t cut the label in the * middle of a double-width character. */ - const int scale = term->scale; - const int x_margin = 2 * scale; - const int y_margin = 1 * scale; - const int max_cols = max_width / term->cell_width; int cols = 0; @@ -3021,23 +3060,48 @@ render_urls(struct terminal *term) const int height = (2 * y_margin + term->cell_height + scale - 1) / scale * scale; - struct buffer *buf = shm_get_buffer( - term->wl->shm, width, height, shm_cookie_url(url), false, 1); + info[render_count].url = &it->item; + info[render_count].text = xwcsdup(label); + info[render_count].x = x; + info[render_count].y = y; + + shm_desc[render_count].width = width; + shm_desc[render_count].height = height; + shm_desc[render_count].cookie = shm_cookie_url(url); + + render_count++; + } + + struct buffer *bufs[render_count]; + shm_get_many(term->wl->shm, render_count, shm_desc, bufs, 1); + + uint32_t fg = term->conf->colors.use_custom.jump_label + ? term->conf->colors.jump_label.fg + : term->colors.table[0]; + uint32_t bg = term->conf->colors.use_custom.jump_label + ? term->conf->colors.jump_label.bg + : term->colors.table[3]; + + for (size_t i = 0; i < render_count; i++) { + struct wl_surface *surf = info[i].url->surf.surf; + struct wl_subsurface *sub_surf = info[i].url->surf.sub; + + const wchar_t *label = info[i].text; + const int x = info[i].x; + const int y = info[i].y; + + xassert(surf != NULL); + xassert(sub_surf != NULL); wl_subsurface_set_position( sub_surf, (term->margins.left + x) / term->scale, (term->margins.top + y) / term->scale); - uint32_t fg = term->conf->colors.use_custom.jump_label - ? term->conf->colors.jump_label.fg - : term->colors.table[0]; - uint32_t bg = term->conf->colors.use_custom.jump_label - ? term->conf->colors.jump_label.bg - : term->colors.table[3]; - render_osd( - term, surf, sub_surf, buf, label, fg, bg, x_margin, y_margin); + term, surf, sub_surf, bufs[i], label, fg, bg, x_margin, y_margin); + + free(info[i].text); } } diff --git a/url-mode.c b/url-mode.c index 524b8d1b..a802cc6c 100644 --- a/url-mode.c +++ b/url-mode.c @@ -14,7 +14,6 @@ #include "grid.h" #include "render.h" #include "selection.h" -#include "shm.h" #include "spawn.h" #include "terminal.h" #include "uri.h" @@ -738,9 +737,7 @@ urls_reset(struct terminal *term) if (term->window != NULL) { tll_foreach(term->window->urls, it) { - const struct url *url = it->item.url; wayl_win_subsurface_destroy(&it->item.surf); - shm_purge(term->wl->shm, shm_cookie_url(url)); tll_remove(term->window->urls, it); } } From 5d7b729ac51ac80984da6d40f91a3a12bff0888a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 18:45:25 +0200 Subject: [PATCH 31/52] wayland: win_destroy: unmap URL labels --- wayland.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/wayland.c b/wayland.c index ed3ddffd..880a4cf3 100644 --- a/wayland.c +++ b/wayland.c @@ -1447,6 +1447,12 @@ wayl_win_destroy(struct wl_window *win) wl_surface_commit(win->search.surf); } + /* URLs */ + tll_foreach(win->urls, it) { + wl_surface_attach(it->item.surf.surf, NULL, 0, 0); + wl_surface_commit(it->item.surf.surf); + } + /* CSD */ for (size_t i = 0; i < ALEN(win->csd.surface); i++) { if (win->csd.surface[i].surf != NULL) { From a486851bdd6bbf2fee9a5009c30770f22ca102ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 19:19:31 +0200 Subject: [PATCH 32/52] shm: auto-purge when we have multiple buffers eligible for re-use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It may happen that we end up with multiple non-busy, same-sized buffers for the same cookie (context), and thus eligible for re-use. Before this patch, we would keep all those buffers around. This is completely unnecessary. Under normal circumstances, we’ll either be re-using a single buffer, or swap between two. In the second case, the “other” buffer is always busy, and thus not eligible for re-use. So, if we _do_ detect multiple, re-usable buffers, pick the one with the lowest “age” (increasing the chance of applying damage tracking, instead of re-drawing everything), and mark the other one for purging. --- shm.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/shm.c b/shm.c index 17b9c15d..064d52ec 100644 --- a/shm.c +++ b/shm.c @@ -494,6 +494,15 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, it->item.scroll_damage = NULL; xassert(it->item.pix_instances == pix_instances); cached = &it->item; + } else { + /* We have multiple buffers eligable for + * re-use. Pick the “youngest” one, and mark the + * other one for purging */ + if (it->item.age < cached->age) { + cached->purge = true; + cached = &it->item; + } else + it->item.purge = true; } } } From 931595bda573588d8529565869aafa3574179011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 19:40:15 +0200 Subject: [PATCH 33/52] shm: codespell --- shm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shm.c b/shm.c index 064d52ec..14d14d02 100644 --- a/shm.c +++ b/shm.c @@ -495,7 +495,7 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, xassert(it->item.pix_instances == pix_instances); cached = &it->item; } else { - /* We have multiple buffers eligable for + /* We have multiple buffers eligible for * re-use. Pick the “youngest” one, and mark the * other one for purging */ if (it->item.age < cached->age) { From 91801ae55da017c2dd0f44c04b8d316a9a2090ef Mon Sep 17 00:00:00 2001 From: Timur Celik Date: Tue, 15 Jun 2021 11:45:27 +0200 Subject: [PATCH 34/52] render: Allow cells to bleed into their neighbor This patch adds a `confined` flag to each cell to track if the last rendered glyph bled into it's right neighbor. To keep things simple, bleeding into any other neighbor cell than the immediate right one is not allowed. This should cover most use cases. Before rendering a row we now do a prepass and mark all cells unclean that are affected by a bleeding neighbor. If there are consecutive bleeding cells, the whole group must be re-rendered even if only a single cell has changed. The patch also deprecates both old overflowing glyph options *allow-overflowing-double-width-glyphs* and *pua-double-width* in favor of a single new one named *overflowing-glyphs*. --- CHANGELOG.md | 11 +++++++ config.c | 17 +++------- config.h | 3 +- doc/foot.ini.5.scd | 34 +++++++------------- render.c | 78 ++++++++++++++++++++++++---------------------- terminal.h | 3 +- 6 files changed, 71 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c2ed336..f12a8b53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,9 @@ * `locked-title=no|yes` to `foot.ini` (https://codeberg.org/dnkl/foot/issues/386). +* `tweak.overflowing-glyphs` option, which can be enabled to fix rendering + issues with glyphs of any width that appear cut-off + (https://codeberg.org/dnkl/foot/issues/592). ### Changed @@ -45,6 +48,12 @@ ### Deprecated ### Removed + +* The `tweak.allow-overflowing-double-width-glyphs` and + `tweak.pua-double-width` options (which have been superseded by + `tweak.overflowing-glyphs`). + + ### Fixed * Glyph offset not being taken into account when applying @@ -69,6 +78,8 @@ ### Security ### Contributors +* [clktmr](https://codeberg.org/clktmr) + ## 1.8.1 diff --git a/config.c b/config.c index 92582f2c..a9acb58e 100644 --- a/config.c +++ b/config.c @@ -2213,16 +2213,10 @@ parse_section_tweak( return false; } - else if (strcmp(key, "allow-overflowing-double-width-glyphs") == 0) { - conf->tweak.allow_overflowing_double_width_glyphs = str_to_bool(value); - if (!conf->tweak.allow_overflowing_double_width_glyphs) - LOG_WARN("tweak: disabled overflowing double-width glyphs"); - } - - else if (strcmp(key, "pua-double-width") == 0) { - conf->tweak.pua_double_width = str_to_bool(value); - if (conf->tweak.pua_double_width) - LOG_WARN("tweak: PUA double width glyphs enabled"); + else if (strcmp(key, "overflowing-glyphs") == 0) { + conf->tweak.overflowing_glyphs = str_to_bool(value); + if (!conf->tweak.overflowing_glyphs) + LOG_WARN("tweak: disabled overflowing glyphs"); } else if (strcmp(key, "damage-whole-window") == 0) { @@ -2833,7 +2827,7 @@ config_load(struct config *conf, const char *conf_path, .tweak = { .fcft_filter = FCFT_SCALING_FILTER_LANCZOS3, - .allow_overflowing_double_width_glyphs = true, + .overflowing_glyphs = true, .grapheme_shaping = false, .grapheme_width_method = GRAPHEME_WIDTH_DOUBLE, .delayed_render_lower_ns = 500000, /* 0.5ms */ @@ -2844,7 +2838,6 @@ config_load(struct config *conf, const char *conf_path, .damage_whole_window = false, .box_drawing_base_thickness = 0.04, .box_drawing_solid_shades = true, - .pua_double_width = false, }, .notifications = tll_init(), diff --git a/config.h b/config.h index 190ea427..729eba3e 100644 --- a/config.h +++ b/config.h @@ -245,7 +245,7 @@ struct config { struct { enum fcft_scaling_filter fcft_filter; - bool allow_overflowing_double_width_glyphs; + bool overflowing_glyphs; bool grapheme_shaping; enum {GRAPHEME_WIDTH_WCSWIDTH, GRAPHEME_WIDTH_DOUBLE} grapheme_width_method; bool render_timer_osd; @@ -256,7 +256,6 @@ struct config { off_t max_shm_pool_size; float box_drawing_base_thickness; bool box_drawing_solid_shades; - bool pua_double_width; } tweak; user_notifications_t notifications; diff --git a/doc/foot.ini.5.scd b/doc/foot.ini.5.scd index 6ceaf3b9..8b2f1dc8 100644 --- a/doc/foot.ini.5.scd +++ b/doc/foot.ini.5.scd @@ -847,34 +847,24 @@ any of these options. Default: _lanczos3_. -*allow-overflowing-double-width-glyphs* - Boolean. When enabled, double width glyphs with a character width - of 1 are allowed to overflow into the neighbouring cell. +*overflowing-glyphs* + Boolean. When enabled, glyphs wider than their cell(s) are allowed + to render into one additional neighbouring cell. - One use case for this is fonts "icon" characters in the Unicode - private usage area, e.g. Nerd Fonts, or Powerline Fonts. Without - this option, such glyphs will appear "cut off". + One use case for this are fonts with wide italic characters that + "bend" into the next cell. Without this option, such glyphs will + appear "cut off". - Another use case are legacy emoji characters like *WHITE FROWNING - FACE*. + Another use case are fonts with "icon" characters in the Unicode + private usage area, e.g. Nerd Fonts, or Powerline Fonts and legacy + emoji characters like *WHITE FROWNING FACE*. - Note: this feature uses _heuristics_ to determine *which* glyphs - should be allowed to overflow. - - See also: *pua-double-width* + Note: might impact performance depending on the font used. + Especially small font sizes can cause many overflowing glyphs + because of subpixel rendering. Default: _yes_. -*pua-double-width* - Boolean. When enabled, Unicode code points from the private usage - area (PUA) are always considered to be double width, regardless of - the actual glyph width. - - Ignored if *allow-overflowing-double-width-glyphs* has been - disabled. - - Default: _no_. - *render-timer* Enables a frame rendering timer, that prints the time it takes to render each frame, in microseconds, either on-screen, to stderr, diff --git a/render.c b/render.c index 8e66bf00..41a20226 100644 --- a/render.c +++ b/render.c @@ -437,6 +437,20 @@ draw_cursor(const struct terminal *term, const struct cell *cell, } } +static inline void +render_cell_prepass(struct terminal *term, struct row *row, int col) +{ + for (; col < term->cols - 1; col++) { + if (row->cells[col].attrs.confined || + (row->cells[col].attrs.clean == row->cells[col + 1].attrs.clean)) { + break; + } + + row->cells[col].attrs.clean = 0; + row->cells[col + 1].attrs.clean = 0; + } +} + static int render_cell(struct terminal *term, pixman_image_t *pix, struct row *row, int col, int row_no, bool has_cursor) @@ -446,6 +460,7 @@ render_cell(struct terminal *term, pixman_image_t *pix, return 0; cell->attrs.clean = 1; + cell->attrs.confined = true; int width = term->cell_width; int height = term->cell_height; @@ -597,51 +612,32 @@ render_cell(struct terminal *term, pixman_image_t *pix, cell_cols = max(1, min(cell_cols, cols_left)); /* - * Hack! - * - * Deal with double-width glyphs for which wcwidth() returns - * 1. Typically Unicode private usage area characters, - * e.g. powerline, or nerd hack fonts. - * - * Users can enable a tweak option that lets this glyphs - * overflow/bleed into the neighbouring cell. - * - * We only apply this workaround if: - * - the user has explicitly enabled this feature - * - the *character* width is 1 - * - the *glyph* width is at least 1.5 cells - * - the *glyph* width is less than 3 cells - * - *this* column isn’t the last column - * - *this* cells is followed by an empty cell, or a space + * Determine cells that will bleed into their right neighbor and remember + * them for cleanup in the next frame. */ - if (term->conf->tweak.allow_overflowing_double_width_glyphs && - glyph_count > 0 && - cell_cols == 1 && - col < term->cols - 1 && - ((glyphs[0]->x + glyphs[0]->width >= term->cell_width * 15 / 10 && - glyphs[0]->x + glyphs[0]->width < 3 * term->cell_width) || - (term->conf->tweak.pua_double_width && - ((base >= 0x00e000 && base <= 0x00f8ff) || - (base >= 0x0f0000 && base <= 0x0ffffd) || - (base >= 0x100000 && base <= 0x10fffd)))) && - (row->cells[col + 1].wc == 0 || row->cells[col + 1].wc == L' ')) + int render_width = cell_cols * width; + if (term->conf->tweak.overflowing_glyphs && + glyph_count > 0) { - cell_cols = 2; + int glyph_width = 0, advance = 0; + for (size_t i = 0; i < glyph_count; i++) { + glyph_width = max(glyph_width, + advance + glyphs[i]->x + glyphs[i]->width); + advance += glyphs[i]->advance.x; + } - /* - * Ensure the cell we’re overflowing into gets re-rendered, to - * ensure it is erased if *this* cell is erased. Note that we - * do *not* mark the row as dirty - we don’t need to re-render - * the cell if nothing else on the row has changed. - */ - row->cells[col].attrs.clean = 0; - row->cells[col + 1].attrs.clean = 0; + if (glyph_width > render_width) { + render_width = min(glyph_width, render_width + width); + + for (int i = 0; i < cell_cols; i++) + row->cells[col + i].attrs.confined = false; + } } pixman_region32_t clip; pixman_region32_init_rect( &clip, x, y, - cell_cols * term->cell_width, term->cell_height); + render_width, term->cell_height); pixman_image_set_clip_region32(pix, &clip); /* Background */ @@ -771,6 +767,10 @@ static void render_row(struct terminal *term, pixman_image_t *pix, struct row *row, int row_no, int cursor_col) { + if (term->conf->tweak.overflowing_glyphs) + for (int col = term->cols - 1; col >= 0; col--) + render_cell_prepass(term, row, col); + for (int col = term->cols - 1; col >= 0; col--) render_cell(term, pix, row, col, row_no, cursor_col == col); } @@ -1192,8 +1192,10 @@ render_sixel(struct terminal *term, pixman_image_t *pix, (last_col_needs_erase && last_col)) { render_cell(term, pix, row, col, term_row_no, cursor_col == col); - } else + } else { cell->attrs.clean = 1; + cell->attrs.confined = 1; + } } } } diff --git a/terminal.h b/terminal.h index 12a637a0..5ffa6154 100644 --- a/terminal.h +++ b/terminal.h @@ -42,11 +42,12 @@ struct attributes { uint32_t fg:24; bool clean:1; + bool confined:1; bool have_fg:1; bool have_bg:1; uint32_t selected:2; bool url:1; - uint32_t reserved:2; + uint32_t reserved:1; uint32_t bg:24; }; static_assert(sizeof(struct attributes) == 8, "VT attribute struct too large"); From 75f7f21a48f3c4054cae024ff769b781481786a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 22:17:12 +0200 Subject: [PATCH 35/52] shm: split up buffer struct into internal/private and public parts --- shm.c | 205 ++++++++++++++++++++++++++++++++++++---------------------- shm.h | 21 +----- 2 files changed, 127 insertions(+), 99 deletions(-) diff --git a/shm.c b/shm.c index 14d14d02..12688445 100644 --- a/shm.c +++ b/shm.c @@ -55,11 +55,34 @@ */ static off_t max_pool_size = 512 * 1024 * 1024; -static tll(struct buffer) buffers; - static bool can_punch_hole = false; static bool can_punch_hole_initialized = false; +struct buffer_pool { + int fd; /* memfd */ + struct wl_shm_pool *wl_pool; + + void *real_mmapped; /* Address returned from mmap */ + size_t mmap_size; /* Size of mmap (>= size) */ + + size_t ref_count; +}; + +struct buffer_private { + struct buffer public; + + bool busy; /* Owned by compositor */ + + unsigned long cookie; + struct buffer_pool *pool; + off_t offset; /* Offset into memfd where data begins */ + + bool scrollable; + bool purge; /* True if this buffer should be destroyed */ +}; + +static tll(struct buffer_private) buffers; + #undef MEASURE_SHM_ALLOCS #if defined(MEASURE_SHM_ALLOCS) static size_t max_alloced = 0; @@ -114,14 +137,14 @@ pool_unref(struct buffer_pool *pool) } static void -buffer_destroy(struct buffer *buf) +buffer_destroy(struct buffer_private *buf) { - buffer_destroy_dont_close(buf); + buffer_destroy_dont_close(&buf->public); pool_unref(buf->pool); buf->pool = NULL; - free(buf->scroll_damage); - pixman_region32_fini(&buf->dirty); + free(buf->public.scroll_damage); + pixman_region32_fini(&buf->public.dirty); } void @@ -140,12 +163,12 @@ shm_fini(void) static void buffer_release(void *data, struct wl_buffer *wl_buffer) { - struct buffer *buffer = data; + struct buffer_private *buffer = data; LOG_DBG("release: cookie=%lx (buf=%p, total buffer count: %zu)", buffer->cookie, (void *)buffer, tll_length(buffers)); - xassert(buffer->wl_buf == wl_buffer); + xassert(buffer->public.wl_buf == wl_buffer); xassert(buffer->busy); buffer->busy = false; } @@ -174,23 +197,24 @@ page_size(void) #endif static bool -instantiate_offset(struct wl_shm *shm, struct buffer *buf, off_t new_offset) +instantiate_offset(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) { - xassert(buf->mmapped == NULL); - xassert(buf->wl_buf == NULL); - xassert(buf->pix == NULL); + xassert(buf->public.mmapped == NULL); + xassert(buf->public.pix == NULL); + xassert(buf->public.wl_buf == NULL); xassert(buf->pool != NULL); const struct buffer_pool *pool = buf->pool; void *mmapped = MAP_FAILED; struct wl_buffer *wl_buf = NULL; - pixman_image_t **pix = xcalloc(buf->pix_instances, sizeof(*pix)); + pixman_image_t **pix = xcalloc(buf->public.pix_instances, sizeof(*pix)); mmapped = (uint8_t *)pool->real_mmapped + new_offset; wl_buf = wl_shm_pool_create_buffer( - pool->wl_pool, new_offset, buf->width, buf->height, buf->stride, + pool->wl_pool, new_offset, + buf->public.width, buf->public.height, buf->public.stride, WL_SHM_FORMAT_ARGB8888); if (wl_buf == NULL) { @@ -199,26 +223,27 @@ instantiate_offset(struct wl_shm *shm, struct buffer *buf, off_t new_offset) } /* One pixman image for each worker thread (do we really need multiple?) */ - for (size_t i = 0; i < buf->pix_instances; i++) { + for (size_t i = 0; i < buf->public.pix_instances; i++) { pix[i] = pixman_image_create_bits_no_clear( - PIXMAN_a8r8g8b8, buf->width, buf->height, (uint32_t *)mmapped, buf->stride); + PIXMAN_a8r8g8b8, buf->public.width, buf->public.height, + (uint32_t *)mmapped, buf->public.stride); if (pix[i] == NULL) { LOG_ERR("failed to create pixman image"); goto err; } } - buf->mmapped = mmapped; + buf->public.mmapped = mmapped; + buf->public.wl_buf = wl_buf; + buf->public.pix = pix; buf->offset = new_offset; - buf->wl_buf = wl_buf; - buf->pix = pix; wl_buffer_add_listener(wl_buf, &buffer_listener, buf); return true; err: if (pix != NULL) { - for (size_t i = 0; i < buf->pix_instances; i++) + for (size_t i = 0; i < buf->public.pix_instances; i++) if (pix[i] != NULL) pixman_image_unref(pix[i]); } @@ -235,7 +260,7 @@ destroy_all_purgeables(void) { /* Purge buffers marked for purging */ tll_foreach(buffers, it) { - if (it->item.locked) + if (it->item.public.locked) continue; if (!it->item.purge) @@ -393,29 +418,31 @@ get_new_buffers(struct wl_shm *shm, size_t count, /* Push to list of available buffers, but marked as 'busy' */ tll_push_front( buffers, - ((struct buffer){ + ((struct buffer_private){ + .public = { + .width = info[i].width, + .height = info[i].height, + .stride = stride[i], + .size = sizes[i], + .pix_instances = pix_instances, + .age = 1234, /* Force a full repaint */ + }, .cookie = info[i].cookie, - .width = info[i].width, - .height = info[i].height, - .stride = stride[i], .busy = true, .purge = immediate_purge, - .size = sizes[i], - .pix_instances = pix_instances, .pool = pool, .scrollable = scrollable, .offset = 0, - .age = 1234, /* Force a full repaint */ })); - struct buffer *buf = &tll_front(buffers); + struct buffer_private *buf = &tll_front(buffers); if (!instantiate_offset(shm, buf, offset)) goto err; - pixman_region32_init(&buf->dirty); + pixman_region32_init(&buf->public.dirty); pool->ref_count++; - offset += buf->size; - bufs[i] = buf; + offset += buf->public.size; + bufs[i] = &buf->public; } #if defined(MEASURE_SHM_ALLOCS) && MEASURE_SHM_ALLOCS @@ -466,17 +493,17 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, { destroy_all_purgeables(); - struct buffer *cached = NULL; + struct buffer_private *cached = NULL; tll_foreach(buffers, it) { - if (it->item.width != width) + if (it->item.public.width != width) continue; - if (it->item.height != height) + if (it->item.public.height != height) continue; if (it->item.cookie != cookie) continue; if (it->item.busy) - it->item.age++; + it->item.public.age++; else #if FORCED_DOUBLE_BUFFERING if (it->item.age == 0) @@ -489,16 +516,16 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, cookie, (void *)&it->item); it->item.busy = true; it->item.purge = false; - pixman_region32_clear(&it->item.dirty); - free(it->item.scroll_damage); - it->item.scroll_damage = NULL; - xassert(it->item.pix_instances == pix_instances); + pixman_region32_clear(&it->item.public.dirty); + free(it->item.public.scroll_damage); + it->item.public.scroll_damage = NULL; + xassert(it->item.public.pix_instances == pix_instances); cached = &it->item; } else { /* We have multiple buffers eligible for * re-use. Pick the “youngest” one, and mark the * other one for purging */ - if (it->item.age < cached->age) { + if (it->item.public.age < cached->public.age) { cached->purge = true; cached = &it->item; } else @@ -508,7 +535,7 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, } if (cached != NULL) - return cached; + return &cached->public; /* Mark old buffers associated with this cookie for purging */ tll_foreach(buffers, it) { @@ -518,7 +545,7 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, if (it->item.busy) continue; - if (it->item.width == width && it->item.height == height) + if (it->item.public.width == width && it->item.public.height == height) continue; LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)&it->item); @@ -532,9 +559,10 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, } bool -shm_can_scroll(const struct buffer *buf) +shm_can_scroll(const struct buffer *_buf) { #if __SIZEOF_POINTER__ == 8 + const struct buffer_private *buf = (const struct buffer_private *)_buf; return can_punch_hole && max_pool_size > 0 && buf->scrollable; #else /* Not enough virtual address space in 32-bit */ @@ -544,17 +572,20 @@ shm_can_scroll(const struct buffer *buf) #if __SIZEOF_POINTER__ == 8 && defined(FALLOC_FL_PUNCH_HOLE) static bool -wrap_buffer(struct wl_shm *shm, struct buffer *buf, off_t new_offset) +wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) { struct buffer_pool *pool = buf->pool; xassert(pool->ref_count == 1); /* We don't allow overlapping offsets */ - off_t UNUSED diff = - new_offset < buf->offset ? buf->offset - new_offset : new_offset - buf->offset; - xassert(diff > buf->size); + off_t UNUSED diff = new_offset < buf->offset + ? buf->offset - new_offset + : new_offset - buf->offset; + xassert(diff > buf->public.size); - memcpy((uint8_t *)pool->real_mmapped + new_offset, buf->mmapped, buf->size); + memcpy((uint8_t *)pool->real_mmapped + new_offset, + buf->public.mmapped, + buf->public.size); off_t trim_ofs, trim_len; if (new_offset > buf->offset) { @@ -563,7 +594,7 @@ wrap_buffer(struct wl_shm *shm, struct buffer *buf, off_t new_offset) trim_len = new_offset; } else { /* Trim everything *after* the new buffer location */ - trim_ofs = new_offset + buf->size; + trim_ofs = new_offset + buf->public.size; trim_len = pool->mmap_size - trim_ofs; } @@ -577,12 +608,12 @@ wrap_buffer(struct wl_shm *shm, struct buffer *buf, off_t new_offset) } /* Re-instantiate pixman+wl_buffer+raw pointersw */ - buffer_destroy_dont_close(buf); + buffer_destroy_dont_close(&buf->public); return instantiate_offset(shm, buf, new_offset); } static bool -shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, +shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -590,19 +621,19 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, xassert(can_punch_hole); xassert(buf->busy); - xassert(buf->pix); - xassert(buf->wl_buf); + xassert(buf->public.pix != NULL); + xassert(buf->public.wl_buf != NULL); xassert(pool != NULL); xassert(pool->ref_count == 1); xassert(pool->fd >= 0); LOG_DBG("scrolling %d rows (%d bytes)", rows, rows * buf->stride); - const off_t diff = rows * buf->stride; + const off_t diff = rows * buf->public.stride; xassert(rows > 0); - xassert(diff < buf->size); + xassert(diff < buf->public.size); - if (buf->offset + diff + buf->size > max_pool_size) { + if (buf->offset + diff + buf->public.size > max_pool_size) { LOG_DBG("memfd offset wrap around"); if (!wrap_buffer(shm, buf, 0)) goto err; @@ -610,7 +641,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, off_t new_offset = buf->offset + diff; xassert(new_offset > buf->offset); - xassert(new_offset + buf->size <= max_pool_size); + xassert(new_offset + buf->public.size <= max_pool_size); #if TIME_SCROLL struct timeval time1; @@ -621,10 +652,13 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, if (top_keep_rows > 0) { /* Copy current 'top' region to its new location */ + const int stride = buf->public.stride; + uint8_t *base = buf->public.mmapped; + memmove( - (uint8_t *)buf->mmapped + (top_margin + rows) * buf->stride, - (uint8_t *)buf->mmapped + (top_margin + 0) * buf->stride, - top_keep_rows * buf->stride); + base + (top_margin + rows) * stride, + base + (top_margin + 0) * stride, + top_keep_rows * stride); #if TIME_SCROLL gettimeofday(&time2, NULL); @@ -634,7 +668,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, } /* Destroy old objects (they point to the old offset) */ - buffer_destroy_dont_close(buf); + buffer_destroy_dont_close(&buf->public); /* Free unused memory - everything up until the new offset */ const off_t trim_ofs = 0; @@ -668,10 +702,14 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, if (ret && bottom_keep_rows > 0) { /* Copy 'bottom' region to its new location */ + const size_t size = buf->public.size; + const int stride = buf->public.stride; + uint8_t *base = buf->public.mmapped; + memmove( - (uint8_t *)buf->mmapped + buf->size - (bottom_margin + bottom_keep_rows) * buf->stride, - (uint8_t *)buf->mmapped + buf->size - (bottom_margin + rows + bottom_keep_rows) * buf->stride, - bottom_keep_rows * buf->stride); + base + size - (bottom_margin + bottom_keep_rows) * stride, + base + size - (bottom_margin + rows + bottom_keep_rows) * stride, + bottom_keep_rows * stride); #if TIME_SCROLL struct timeval time5; @@ -690,7 +728,7 @@ err: } static bool -shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, +shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -699,10 +737,10 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, struct buffer_pool *pool = buf->pool; xassert(pool->ref_count == 1); - const off_t diff = rows * buf->stride; + const off_t diff = rows * buf->public.stride; if (diff > buf->offset) { LOG_DBG("memfd offset reverse wrap-around"); - if (!wrap_buffer(shm, buf, (max_pool_size - buf->size) & ~(page_size() - 1))) + if (!wrap_buffer(shm, buf, (max_pool_size - buf->public.size) & ~(page_size() - 1))) goto err; } @@ -720,10 +758,14 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, if (bottom_keep_rows > 0) { /* Copy 'bottom' region to its new location */ + const size_t size = buf->public.size; + const int stride = buf->public.stride; + uint8_t *base = buf->public.mmapped; + memmove( - (uint8_t *)buf->mmapped + buf->size - (bottom_margin + rows + bottom_keep_rows) * buf->stride, - (uint8_t *)buf->mmapped + buf->size - (bottom_margin + bottom_keep_rows) * buf->stride, - bottom_keep_rows * buf->stride); + base + size - (bottom_margin + rows + bottom_keep_rows) * stride, + base + size - (bottom_margin + bottom_keep_rows) * stride, + bottom_keep_rows * stride); #if TIME_SCROLL gettimeofday(&time1, NULL); @@ -733,10 +775,10 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, } /* Destroy old objects (they point to the old offset) */ - buffer_destroy_dont_close(buf); + buffer_destroy_dont_close(&buf->public); /* Free unused memory - everything after the relocated buffer */ - const off_t trim_ofs = new_offset + buf->size; + const off_t trim_ofs = new_offset + buf->public.size; const off_t trim_len = pool->mmap_size - trim_ofs; if (fallocate( @@ -766,10 +808,13 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, if (ret && top_keep_rows > 0) { /* Copy current 'top' region to its new location */ + const int stride = buf->public.stride; + uint8_t *base = buf->public.mmapped; + memmove( - (uint8_t *)buf->mmapped + (top_margin + 0) * buf->stride, - (uint8_t *)buf->mmapped + (top_margin + rows) * buf->stride, - top_keep_rows * buf->stride); + base + (top_margin + 0) * stride, + base + (top_margin + rows) * stride, + top_keep_rows * stride); #if TIME_SCROLL struct timeval time4; @@ -788,14 +833,16 @@ err: #endif /* FALLOC_FL_PUNCH_HOLE */ bool -shm_scroll(struct wl_shm *shm, struct buffer *buf, int rows, +shm_scroll(struct wl_shm *shm, struct buffer *_buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { #if __SIZEOF_POINTER__ == 8 && defined(FALLOC_FL_PUNCH_HOLE) - if (!shm_can_scroll(buf)) + if (!shm_can_scroll(_buf)) return false; + struct buffer_private *buf = (struct buffer_private *)_buf; + xassert(rows != 0); return rows > 0 ? shm_scroll_forward(shm, buf, rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows) @@ -817,7 +864,7 @@ shm_purge(struct wl_shm *shm, unsigned long cookie) if (it->item.busy) { LOG_WARN("deferring purge of 'busy' buffer (width=%d, height=%d)", - it->item.width, it->item.height); + it->item.public.width, it->item.public.height); it->item.purge = true; } else { buffer_destroy(&it->item); diff --git a/shm.h b/shm.h index 610f0311..01f1a711 100644 --- a/shm.h +++ b/shm.h @@ -9,25 +9,13 @@ #include "terminal.h" -struct buffer_pool { - int fd; /* memfd */ - struct wl_shm_pool *wl_pool; - - void *real_mmapped; /* Address returned from mmap */ - size_t mmap_size; /* Size of mmap (>= size) */ - - size_t ref_count; -}; - struct buffer { - unsigned long cookie; + bool locked; /* Caller owned, shm won’t destroy it */ int width; int height; int stride; - bool locked; /* Caller owned, shm won’t destroy it */ - bool busy; /* Owned by compositor */ size_t size; /* Buffer size */ void *mmapped; /* Raw data (TODO: rename) */ @@ -35,13 +23,6 @@ struct buffer { pixman_image_t **pix; size_t pix_instances; - /* Internal */ - struct buffer_pool *pool; - off_t offset; /* Offset into memfd where data begins */ - - bool scrollable; - bool purge; /* True if this buffer should be destroyed */ - unsigned age; struct damage *scroll_damage; size_t scroll_damage_count; From 9b6cee825beb927ec24ef28f0f162623db218fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 22:18:09 +0200 Subject: [PATCH 36/52] shm: rename buffer.mmapped to buffer.data --- render.c | 6 +++--- shm.c | 18 ++++++++++-------- shm.h | 2 +- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/render.c b/render.c index 41a20226..076a8d54 100644 --- a/render.c +++ b/render.c @@ -954,7 +954,7 @@ grid_render_scroll(struct terminal *term, struct buffer *buf, term, buf, dmg->region.end - dmg->lines, term->rows, false); } else { /* Fallback for when we either cannot do SHM scrolling, or it failed */ - uint8_t *raw = buf->mmapped; + uint8_t *raw = buf->data; memmove(raw + dst_y * buf->stride, raw + src_y * buf->stride, height * buf->stride); @@ -1019,7 +1019,7 @@ grid_render_scroll_reverse(struct terminal *term, struct buffer *buf, term, buf, dmg->region.start, dmg->region.start + dmg->lines, false); } else { /* Fallback for when we either cannot do SHM scrolling, or it failed */ - uint8_t *raw = buf->mmapped; + uint8_t *raw = buf->data; memmove(raw + dst_y * buf->stride, raw + src_y * buf->stride, height * buf->stride); @@ -2162,7 +2162,7 @@ reapply_old_damage(struct terminal *term, struct buffer *new, struct buffer *old } if (new->age > 1) { - memcpy(new->mmapped, old->mmapped, new->size); + memcpy(new->data, old->data, new->size); return; } diff --git a/shm.c b/shm.c index 12688445..26d1b839 100644 --- a/shm.c +++ b/shm.c @@ -108,7 +108,7 @@ buffer_destroy_dont_close(struct buffer *buf) free(buf->pix); buf->pix = NULL; buf->wl_buf = NULL; - buf->mmapped = NULL; + buf->data = NULL; } static void @@ -150,6 +150,8 @@ buffer_destroy(struct buffer_private *buf) void shm_fini(void) { + xassert(tll_length(buffers) == 0); + tll_foreach(buffers, it) { buffer_destroy(&it->item); tll_remove(buffers, it); @@ -199,7 +201,7 @@ page_size(void) static bool instantiate_offset(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) { - xassert(buf->public.mmapped == NULL); + xassert(buf->public.data == NULL); xassert(buf->public.pix == NULL); xassert(buf->public.wl_buf == NULL); xassert(buf->pool != NULL); @@ -233,7 +235,7 @@ instantiate_offset(struct wl_shm *shm, struct buffer_private *buf, off_t new_off } } - buf->public.mmapped = mmapped; + buf->public.data = mmapped; buf->public.wl_buf = wl_buf; buf->public.pix = pix; buf->offset = new_offset; @@ -584,7 +586,7 @@ wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) xassert(diff > buf->public.size); memcpy((uint8_t *)pool->real_mmapped + new_offset, - buf->public.mmapped, + buf->public.data, buf->public.size); off_t trim_ofs, trim_len; @@ -653,7 +655,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, if (top_keep_rows > 0) { /* Copy current 'top' region to its new location */ const int stride = buf->public.stride; - uint8_t *base = buf->public.mmapped; + uint8_t *base = buf->public.data; memmove( base + (top_margin + rows) * stride, @@ -704,7 +706,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, /* Copy 'bottom' region to its new location */ const size_t size = buf->public.size; const int stride = buf->public.stride; - uint8_t *base = buf->public.mmapped; + uint8_t *base = buf->public.data; memmove( base + size - (bottom_margin + bottom_keep_rows) * stride, @@ -760,7 +762,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, /* Copy 'bottom' region to its new location */ const size_t size = buf->public.size; const int stride = buf->public.stride; - uint8_t *base = buf->public.mmapped; + uint8_t *base = buf->public.data; memmove( base + size - (bottom_margin + rows + bottom_keep_rows) * stride, @@ -809,7 +811,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, if (ret && top_keep_rows > 0) { /* Copy current 'top' region to its new location */ const int stride = buf->public.stride; - uint8_t *base = buf->public.mmapped; + uint8_t *base = buf->public.data; memmove( base + (top_margin + 0) * stride, diff --git a/shm.h b/shm.h index 01f1a711..43398cc9 100644 --- a/shm.h +++ b/shm.h @@ -17,7 +17,7 @@ struct buffer { int stride; size_t size; /* Buffer size */ - void *mmapped; /* Raw data (TODO: rename) */ + void *data; /* Raw data (TODO: rename) */ struct wl_buffer *wl_buf; pixman_image_t **pix; From f5da62c462430551869d830ef6c4302d4c3b4fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 22:21:37 +0200 Subject: [PATCH 37/52] shm: replace inclusion of terminal.h with a forward declaration of damage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s the only thing we “need” from terminal.h --- shm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shm.h b/shm.h index 43398cc9..dc37f1b7 100644 --- a/shm.h +++ b/shm.h @@ -7,7 +7,7 @@ #include #include -#include "terminal.h" +struct damage; struct buffer { bool locked; /* Caller owned, shm won’t destroy it */ From 99ea47c97ac6884e469d1d2cdb76220f3398a737 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 22:30:08 +0200 Subject: [PATCH 38/52] =?UTF-8?q?shm:=20move=20=E2=80=98size=E2=80=99=20to?= =?UTF-8?q?=20the=20private=20buffer=20struct?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- render.c | 2 +- shm.c | 31 ++++++++++++++++--------------- shm.h | 3 +-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/render.c b/render.c index 076a8d54..c48c515a 100644 --- a/render.c +++ b/render.c @@ -2162,7 +2162,7 @@ reapply_old_damage(struct terminal *term, struct buffer *new, struct buffer *old } if (new->age > 1) { - memcpy(new->data, old->data, new->size); + memcpy(new->data, old->data, new->height * new->stride); return; } diff --git a/shm.c b/shm.c index 26d1b839..cd26c62e 100644 --- a/shm.c +++ b/shm.c @@ -71,14 +71,15 @@ struct buffer_pool { struct buffer_private { struct buffer public; - bool busy; /* Owned by compositor */ + size_t size; + bool busy; /* Owned by compositor */ unsigned long cookie; struct buffer_pool *pool; - off_t offset; /* Offset into memfd where data begins */ + off_t offset; /* Offset into memfd where data begins */ bool scrollable; - bool purge; /* True if this buffer should be destroyed */ + bool purge; /* True if this buffer should be destroyed */ }; static tll(struct buffer_private) buffers; @@ -425,11 +426,11 @@ get_new_buffers(struct wl_shm *shm, size_t count, .width = info[i].width, .height = info[i].height, .stride = stride[i], - .size = sizes[i], .pix_instances = pix_instances, .age = 1234, /* Force a full repaint */ }, .cookie = info[i].cookie, + .size = sizes[i], .busy = true, .purge = immediate_purge, .pool = pool, @@ -443,7 +444,7 @@ get_new_buffers(struct wl_shm *shm, size_t count, pixman_region32_init(&buf->public.dirty); pool->ref_count++; - offset += buf->public.size; + offset += buf->size; bufs[i] = &buf->public; } @@ -583,11 +584,11 @@ wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) off_t UNUSED diff = new_offset < buf->offset ? buf->offset - new_offset : new_offset - buf->offset; - xassert(diff > buf->public.size); + xassert(diff > buf->size); memcpy((uint8_t *)pool->real_mmapped + new_offset, buf->public.data, - buf->public.size); + buf->size); off_t trim_ofs, trim_len; if (new_offset > buf->offset) { @@ -596,7 +597,7 @@ wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) trim_len = new_offset; } else { /* Trim everything *after* the new buffer location */ - trim_ofs = new_offset + buf->public.size; + trim_ofs = new_offset + buf->size; trim_len = pool->mmap_size - trim_ofs; } @@ -633,9 +634,9 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, const off_t diff = rows * buf->public.stride; xassert(rows > 0); - xassert(diff < buf->public.size); + xassert(diff < buf->size); - if (buf->offset + diff + buf->public.size > max_pool_size) { + if (buf->offset + diff + buf->size > max_pool_size) { LOG_DBG("memfd offset wrap around"); if (!wrap_buffer(shm, buf, 0)) goto err; @@ -643,7 +644,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, off_t new_offset = buf->offset + diff; xassert(new_offset > buf->offset); - xassert(new_offset + buf->public.size <= max_pool_size); + xassert(new_offset + buf->size <= max_pool_size); #if TIME_SCROLL struct timeval time1; @@ -704,7 +705,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, if (ret && bottom_keep_rows > 0) { /* Copy 'bottom' region to its new location */ - const size_t size = buf->public.size; + const size_t size = buf->size; const int stride = buf->public.stride; uint8_t *base = buf->public.data; @@ -742,7 +743,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, const off_t diff = rows * buf->public.stride; if (diff > buf->offset) { LOG_DBG("memfd offset reverse wrap-around"); - if (!wrap_buffer(shm, buf, (max_pool_size - buf->public.size) & ~(page_size() - 1))) + if (!wrap_buffer(shm, buf, (max_pool_size - buf->size) & ~(page_size() - 1))) goto err; } @@ -760,7 +761,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, if (bottom_keep_rows > 0) { /* Copy 'bottom' region to its new location */ - const size_t size = buf->public.size; + const size_t size = buf->size; const int stride = buf->public.stride; uint8_t *base = buf->public.data; @@ -780,7 +781,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, buffer_destroy_dont_close(&buf->public); /* Free unused memory - everything after the relocated buffer */ - const off_t trim_ofs = new_offset + buf->public.size; + const off_t trim_ofs = new_offset + buf->size; const off_t trim_len = pool->mmap_size - trim_ofs; if (fallocate( diff --git a/shm.h b/shm.h index dc37f1b7..435f9de4 100644 --- a/shm.h +++ b/shm.h @@ -16,8 +16,7 @@ struct buffer { int height; int stride; - size_t size; /* Buffer size */ - void *data; /* Raw data (TODO: rename) */ + void *data; struct wl_buffer *wl_buf; pixman_image_t **pix; From 69260dd9604f39f5e7270ac44502c7e60618500b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:47:15 +0200 Subject: [PATCH 39/52] shm: we may exit with busy buffers remaining (seen on KDE) --- render.c | 2 +- shm.c | 28 +++++++++++++++++++++------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/render.c b/render.c index c48c515a..90fdae7b 100644 --- a/render.c +++ b/render.c @@ -2311,7 +2311,7 @@ grid_render(struct terminal *term) } else if (buf->age > 0) { - LOG_DBG("buffer age: %u", buf->age); + LOG_DBG("buffer age: %u (%p)", buf->age, (void *)buf); xassert(term->render.last_buf != NULL); xassert(term->render.last_buf != buf); diff --git a/shm.c b/shm.c index cd26c62e..87900f01 100644 --- a/shm.c +++ b/shm.c @@ -151,13 +151,26 @@ buffer_destroy(struct buffer_private *buf) void shm_fini(void) { - xassert(tll_length(buffers) == 0); + size_t busy_count UNUSED = 0; + size_t non_busy_count UNUSED = 0; tll_foreach(buffers, it) { + if (it->item.busy) + busy_count++; + else + non_busy_count++; + buffer_destroy(&it->item); tll_remove(buffers, it); } + LOG_DBG("buffers left: busy=%zu, non-busy=%zu", busy_count, non_busy_count); + + if (non_busy_count > 0) { + BUG("%zu non-busy buffers remaining (%zu buffers in total)", + non_busy_count, busy_count + non_busy_count); + } + #if defined(MEASURE_SHM_ALLOCS) && MEASURE_SHM_ALLOCS LOG_INFO("max total allocations was: %zu MB", max_alloced / 1024 / 1024); #endif @@ -272,8 +285,9 @@ destroy_all_purgeables(void) if (it->item.busy) continue; - LOG_DBG("cookie=%lx: purging buffer %p (width=%d, height=%d): %zu KB", - cookie, (void *)&it->item, it->item.width, it->item.height, + LOG_DBG("purging buffer %p (width=%d, height=%d): %zu KB", + (void *)&it->item, + it->item.public.width, it->item.public.height, it->item.size / 1024); buffer_destroy(&it->item); @@ -630,7 +644,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, xassert(pool->ref_count == 1); xassert(pool->fd >= 0); - LOG_DBG("scrolling %d rows (%d bytes)", rows, rows * buf->stride); + LOG_DBG("scrolling %d rows (%d bytes)", rows, rows * buf->public.stride); const off_t diff = rows * buf->public.stride; xassert(rows > 0); @@ -865,9 +879,9 @@ shm_purge(struct wl_shm *shm, unsigned long cookie) if (it->item.cookie != cookie) continue; - if (it->item.busy) { - LOG_WARN("deferring purge of 'busy' buffer (width=%d, height=%d)", - it->item.public.width, it->item.public.height); + if (it->item.busy) { + LOG_DBG("deferring purge of 'busy' buffer (width=%d, height=%d)", + it->item.public.width, it->item.public.height); it->item.purge = true; } else { buffer_destroy(&it->item); From 232fb20269b68157874f57d676c1b0b95c2386aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:47:57 +0200 Subject: [PATCH 40/52] shm: replace 'locked' attribute with a ref-counter The initial ref-count is either 1 or 0, depending on whether the buffer is supposed to be released "immeidately" (meaning, as soon as the compositor releases it). Two new user facing functions have been added: shm_addref() and shm_unref(). Our renderer now uses these two functions instead of manually setting and clearing the 'locked' attribute. shm_unref() will decrement the ref-counter, and destroy the buffer when the counter reaches zero. Except if the buffer is currently "busy" (compositor owned), in which case destruction is deferred to the release event. The buffer is still removed from the list though. --- render.c | 12 +++---- shm.c | 94 +++++++++++++++++++++++++++++------------------------- shm.h | 5 +-- terminal.c | 2 ++ 4 files changed, 60 insertions(+), 53 deletions(-) diff --git a/render.c b/render.c index 90fdae7b..c6835664 100644 --- a/render.c +++ b/render.c @@ -2324,19 +2324,18 @@ grid_render(struct terminal *term) } if (term->render.last_buf != NULL) { - term->render.last_buf->locked = false; - free(term->render.last_buf->scroll_damage); - term->render.last_buf->scroll_damage = NULL; + shm_unref(term->render.last_buf); + term->render.last_buf = NULL; } term->render.last_buf = buf; term->render.was_flashing = term->flash.active; term->render.was_searching = term->is_searching; - buf->locked = true; + shm_addref(buf); buf->age = 0; - xassert(buf->scroll_damage == NULL); + free(term->render.last_buf->scroll_damage); buf->scroll_damage_count = tll_length(term->grid->scroll_damage); buf->scroll_damage = xmalloc( buf->scroll_damage_count * sizeof(buf->scroll_damage[0])); @@ -3484,8 +3483,7 @@ damage_view: tll_free(term->normal.scroll_damage); tll_free(term->alt.scroll_damage); - if (term->render.last_buf != NULL) - term->render.last_buf->locked = false; + shm_unref(term->render.last_buf); term->render.last_buf = NULL; term_damage_view(term); render_refresh_csd(term); diff --git a/shm.c b/shm.c index 87900f01..a2f8cffe 100644 --- a/shm.c +++ b/shm.c @@ -71,6 +71,7 @@ struct buffer_pool { struct buffer_private { struct buffer public; + size_t ref_count; size_t size; bool busy; /* Owned by compositor */ @@ -79,7 +80,6 @@ struct buffer_private { off_t offset; /* Offset into memfd where data begins */ bool scrollable; - bool purge; /* True if this buffer should be destroyed */ }; static tll(struct buffer_private) buffers; @@ -148,6 +148,19 @@ buffer_destroy(struct buffer_private *buf) pixman_region32_fini(&buf->public.dirty); } +static bool +buffer_unref_no_remove_from_cache(struct buffer_private *buf) +{ + if (buf->ref_count > 0) + buf->ref_count--; + + if (buf->ref_count > 0 || buf->busy) + return false; + + buffer_destroy(buf); + return true; +} + void shm_fini(void) { @@ -187,6 +200,9 @@ buffer_release(void *data, struct wl_buffer *wl_buffer) xassert(buffer->public.wl_buf == wl_buffer); xassert(buffer->busy); buffer->busy = false; + + if (buffer->ref_count == 0) + shm_unref(&buffer->public); } static const struct wl_buffer_listener buffer_listener = { @@ -271,30 +287,6 @@ err: return false; } -static void NOINLINE -destroy_all_purgeables(void) -{ - /* Purge buffers marked for purging */ - tll_foreach(buffers, it) { - if (it->item.public.locked) - continue; - - if (!it->item.purge) - continue; - - if (it->item.busy) - continue; - - LOG_DBG("purging buffer %p (width=%d, height=%d): %zu KB", - (void *)&it->item, - it->item.public.width, it->item.public.height, - it->item.size / 1024); - - buffer_destroy(&it->item); - tll_remove(buffers, it); - } -} - static void NOINLINE get_new_buffers(struct wl_shm *shm, size_t count, struct buffer_description info[static count], @@ -444,9 +436,9 @@ get_new_buffers(struct wl_shm *shm, size_t count, .age = 1234, /* Force a full repaint */ }, .cookie = info[i].cookie, + .ref_count = immediate_purge ? 0 : 1, .size = sizes[i], .busy = true, - .purge = immediate_purge, .pool = pool, .scrollable = scrollable, .offset = 0, @@ -500,7 +492,6 @@ shm_get_many(struct wl_shm *shm, size_t count, struct buffer *bufs[static count], size_t pix_instances) { - destroy_all_purgeables(); get_new_buffers(shm, count, info, bufs, pix_instances, false, true); } @@ -508,8 +499,6 @@ struct buffer * shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, bool scrollable, size_t pix_instances) { - destroy_all_purgeables(); - struct buffer_private *cached = NULL; tll_foreach(buffers, it) { if (it->item.public.width != width) @@ -532,7 +521,6 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)", cookie, (void *)&it->item); it->item.busy = true; - it->item.purge = false; pixman_region32_clear(&it->item.public.dirty); free(it->item.public.scroll_damage); it->item.public.scroll_damage = NULL; @@ -543,10 +531,12 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, * re-use. Pick the “youngest” one, and mark the * other one for purging */ if (it->item.public.age < cached->public.age) { - cached->purge = true; + shm_unref(&cached->public); cached = &it->item; - } else - it->item.purge = true; + } else { + if (buffer_unref_no_remove_from_cache(&it->item)) + tll_remove(buffers, it); + } } } } @@ -559,14 +549,12 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, if (it->item.cookie != cookie) continue; - if (it->item.busy) - continue; - if (it->item.public.width == width && it->item.public.height == height) continue; LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)&it->item); - it->item.purge = true; + if (buffer_unref_no_remove_from_cache(&it->item)) + tll_remove(buffers, it); } struct buffer *ret; @@ -879,13 +867,31 @@ shm_purge(struct wl_shm *shm, unsigned long cookie) if (it->item.cookie != cookie) continue; - if (it->item.busy) { - LOG_DBG("deferring purge of 'busy' buffer (width=%d, height=%d)", - it->item.public.width, it->item.public.height); - it->item.purge = true; - } else { - buffer_destroy(&it->item); + if (buffer_unref_no_remove_from_cache(&it->item)) tll_remove(buffers, it); - } + } +} + +void +shm_addref(struct buffer *_buf) +{ + struct buffer_private *buf = (struct buffer_private *)_buf; + buf->ref_count++; +} + +void +shm_unref(struct buffer *_buf) +{ + if (_buf == NULL) + return; + + struct buffer_private *buf = (struct buffer_private *)_buf; + tll_foreach(buffers, it) { + if (&it->item != buf) + continue; + + if (buffer_unref_no_remove_from_cache(buf)) + tll_remove(buffers, it); + break; } } diff --git a/shm.h b/shm.h index 435f9de4..18941588 100644 --- a/shm.h +++ b/shm.h @@ -10,8 +10,6 @@ struct damage; struct buffer { - bool locked; /* Caller owned, shm won’t destroy it */ - int width; int height; int stride; @@ -23,6 +21,7 @@ struct buffer { size_t pix_instances; unsigned age; + struct damage *scroll_damage; size_t scroll_damage_count; pixman_region32_t dirty; @@ -74,6 +73,8 @@ bool shm_scroll(struct wl_shm *shm, struct buffer *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows); +void shm_addref(struct buffer *buf); +void shm_unref(struct buffer *buf); void shm_purge(struct wl_shm *shm, unsigned long cookie); struct terminal; diff --git a/terminal.c b/terminal.c index 488ac316..c7776f28 100644 --- a/terminal.c +++ b/terminal.c @@ -35,6 +35,7 @@ #include "selection.h" #include "sixel.h" #include "slave.h" +#include "shm.h" #include "spawn.h" #include "url-mode.h" #include "util.h" @@ -1457,6 +1458,7 @@ term_destroy(struct terminal *term) sem_destroy(&term->render.workers.done); xassert(tll_length(term->render.workers.queue) == 0); tll_free(term->render.workers.queue); + shm_unref(term->render.last_buf); tll_free(term->tab_stops); From 4efb34927eb46d8b12fc0d5638653cf23839d7dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:48:18 +0200 Subject: [PATCH 41/52] shm: remove deferred buffers from main list immediately When unref:ing a "busy" buffer, destruction is (still) deferred to the buffer release event. However, we now move the buffer off the buffer list immediately, and instead push it to a 'deferred' list. This prevents buffer re-use of buffers scheduled for destruction. It also means less buffers to iterate through when trying to find a re-usable buffer in shm_get_buffer(), since we no longer have to wade through a potentially long list of to-be-deleted buffers. --- shm.c | 143 ++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 90 insertions(+), 53 deletions(-) diff --git a/shm.c b/shm.c index a2f8cffe..d5c94c8b 100644 --- a/shm.c +++ b/shm.c @@ -82,7 +82,8 @@ struct buffer_private { bool scrollable; }; -static tll(struct buffer_private) buffers; +static tll(struct buffer_private *) buffers; +static tll(struct buffer_private *) deferred; #undef MEASURE_SHM_ALLOCS #if defined(MEASURE_SHM_ALLOCS) @@ -146,18 +147,22 @@ buffer_destroy(struct buffer_private *buf) free(buf->public.scroll_damage); pixman_region32_fini(&buf->public.dirty); + free(buf); } static bool buffer_unref_no_remove_from_cache(struct buffer_private *buf) { - if (buf->ref_count > 0) - buf->ref_count--; + xassert(buf->ref_count > 0); + buf->ref_count--; - if (buf->ref_count > 0 || buf->busy) + if (buf->ref_count > 0) return false; - buffer_destroy(buf); + if (buf->busy) + tll_push_back(deferred, buf); + else + buffer_destroy(buf); return true; } @@ -168,15 +173,23 @@ shm_fini(void) size_t non_busy_count UNUSED = 0; tll_foreach(buffers, it) { - if (it->item.busy) + if (it->item->busy) busy_count++; else non_busy_count++; - buffer_destroy(&it->item); + buffer_destroy(it->item); tll_remove(buffers, it); } + xassert(busy_count == 0); + busy_count = tll_length(deferred); + + tll_foreach(deferred, it) { + buffer_destroy(it->item); + tll_remove(deferred, it); + } + LOG_DBG("buffers left: busy=%zu, non-busy=%zu", busy_count, non_busy_count); if (non_busy_count > 0) { @@ -201,8 +214,21 @@ buffer_release(void *data, struct wl_buffer *wl_buffer) xassert(buffer->busy); buffer->busy = false; - if (buffer->ref_count == 0) - shm_unref(&buffer->public); + if (buffer->ref_count == 0) { + LOG_WARN("deferred delete"); + + bool found = false; + tll_foreach(deferred, it) { + if (it->item == buffer) { + found = true; + buffer_destroy(buffer); + tll_remove(deferred, it); + break; + } + } + + xassert(found); + } } static const struct wl_buffer_listener buffer_listener = { @@ -425,28 +451,33 @@ get_new_buffers(struct wl_shm *shm, size_t count, for (size_t i = 0; i < count; i++) { /* Push to list of available buffers, but marked as 'busy' */ - tll_push_front( - buffers, - ((struct buffer_private){ - .public = { - .width = info[i].width, - .height = info[i].height, - .stride = stride[i], - .pix_instances = pix_instances, - .age = 1234, /* Force a full repaint */ - }, - .cookie = info[i].cookie, - .ref_count = immediate_purge ? 0 : 1, - .size = sizes[i], - .busy = true, - .pool = pool, - .scrollable = scrollable, - .offset = 0, - })); + struct buffer_private *buf = xmalloc(sizeof(*buf)); + *buf = (struct buffer_private){ + .public = { + .width = info[i].width, + .height = info[i].height, + .stride = stride[i], + .pix_instances = pix_instances, + .age = 1234, /* Force a full repaint */ + }, + .cookie = info[i].cookie, + .ref_count = immediate_purge ? 0 : 1, + .size = sizes[i], + .busy = true, + .pool = pool, + .scrollable = scrollable, + .offset = 0, + }; - struct buffer_private *buf = &tll_front(buffers); - if (!instantiate_offset(shm, buf, offset)) + if (!instantiate_offset(shm, buf, offset)) { + free(buf); goto err; + } + + if (immediate_purge) + tll_push_front(deferred, buf); + else + tll_push_front(buffers, buf); pixman_region32_init(&buf->public.dirty); pool->ref_count++; @@ -501,40 +532,42 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, { struct buffer_private *cached = NULL; tll_foreach(buffers, it) { - if (it->item.public.width != width) + struct buffer_private *buf = it->item; + + if (buf->public.width != width) continue; - if (it->item.public.height != height) + if (buf->public.height != height) continue; - if (it->item.cookie != cookie) + if (buf->cookie != cookie) continue; - if (it->item.busy) - it->item.public.age++; + if (buf->busy) + buf->public.age++; else #if FORCED_DOUBLE_BUFFERING - if (it->item.age == 0) - it->item.age++; + if (buf->age == 0) + buf->age++; else #endif { if (cached == NULL) { LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)", - cookie, (void *)&it->item); - it->item.busy = true; - pixman_region32_clear(&it->item.public.dirty); - free(it->item.public.scroll_damage); - it->item.public.scroll_damage = NULL; - xassert(it->item.public.pix_instances == pix_instances); - cached = &it->item; + cookie, (void *)buf); + buf->busy = true; + pixman_region32_clear(&buf->public.dirty); + free(buf->public.scroll_damage); + buf->public.scroll_damage = NULL; + xassert(buf->public.pix_instances == pix_instances); + cached = it->item; } else { /* We have multiple buffers eligible for * re-use. Pick the “youngest” one, and mark the * other one for purging */ - if (it->item.public.age < cached->public.age) { + if (buf->public.age < cached->public.age) { shm_unref(&cached->public); - cached = &it->item; + cached = buf; } else { - if (buffer_unref_no_remove_from_cache(&it->item)) + if (buffer_unref_no_remove_from_cache(buf)) tll_remove(buffers, it); } } @@ -546,14 +579,16 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, /* Mark old buffers associated with this cookie for purging */ tll_foreach(buffers, it) { - if (it->item.cookie != cookie) + struct buffer_private *buf = it->item; + + if (buf->cookie != cookie) continue; - if (it->item.public.width == width && it->item.public.height == height) + if (buf->public.width == width && buf->public.height == height) continue; - LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)&it->item); - if (buffer_unref_no_remove_from_cache(&it->item)) + LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)buf); + if (buffer_unref_no_remove_from_cache(buf)) tll_remove(buffers, it); } @@ -864,10 +899,12 @@ shm_purge(struct wl_shm *shm, unsigned long cookie) /* Purge old buffers associated with this cookie */ tll_foreach(buffers, it) { - if (it->item.cookie != cookie) + struct buffer_private *buf = it->item; + + if (buf->cookie != cookie) continue; - if (buffer_unref_no_remove_from_cache(&it->item)) + if (buffer_unref_no_remove_from_cache(buf)) tll_remove(buffers, it); } } @@ -887,7 +924,7 @@ shm_unref(struct buffer *_buf) struct buffer_private *buf = (struct buffer_private *)_buf; tll_foreach(buffers, it) { - if (&it->item != buf) + if (it->item != buf) continue; if (buffer_unref_no_remove_from_cache(buf)) From 53851e13ec976001cdec2077d7963c292bcd6d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:48:49 +0200 Subject: [PATCH 42/52] shm: refactor: move away from a single, global, buffer list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Up until now, *all* buffers have been tracked in a single, global buffer list. We've used 'cookies' to separate buffers from different contexts (so that shm_get_buffer() doesn't try to re-use e.g. a search-box buffer for the main grid). This patch refactors this, and completely removes the global list. Instead of cookies, we now use 'chains'. A chain tracks both the properties to apply to newly created buffers (scrollable, number of pixman instances to instantiate etc), as well as the instantiated buffers themselves. This means there's strictly speaking not much use for shm_fini() anymore, since its up to the chain owner to call shm_chain_free(), which will also purge all buffers. However, since purging a buffer may be deferred, if the buffer is owned by the compositor at the time of the call to shm_purge() or shm_chain_free(), we still keep a global 'deferred' list, on to which deferred buffers are pushed. shm_fini() iterates this list and destroys the buffers _even_ if they are still owned by the compositor. This only happens at program termination, and not when destroying a terminal instance. I.e. closing a window in a “foot --server” does *not* trigger this. Each terminal instatiates a number of chains, and these chains are destroyed when the terminal instance is destroyed. Note that some buffers may be put on the deferred list, as mentioned above. --- render.c | 49 ++++++------ shm.c | 215 ++++++++++++++++++++++++++++------------------------- shm.h | 39 ++++------ terminal.c | 25 +++++++ terminal.h | 10 +++ wayland.c | 16 ++-- 6 files changed, 190 insertions(+), 164 deletions(-) diff --git a/render.c b/render.c index c6835664..7bcad519 100644 --- a/render.c +++ b/render.c @@ -943,7 +943,7 @@ grid_render_scroll(struct terminal *term, struct buffer *buf, if (try_shm_scroll) { did_shm_scroll = shm_scroll( - term->wl->shm, buf, dmg->lines * term->cell_height, + buf, dmg->lines * term->cell_height, term->margins.top, dmg->region.start * term->cell_height, term->margins.bottom, (term->rows - dmg->region.end) * term->cell_height); } @@ -1008,7 +1008,7 @@ grid_render_scroll_reverse(struct terminal *term, struct buffer *buf, if (try_shm_scroll) { did_shm_scroll = shm_scroll( - term->wl->shm, buf, -dmg->lines * term->cell_height, + buf, -dmg->lines * term->cell_height, term->margins.top, dmg->region.start * term->cell_height, term->margins.bottom, (term->rows - dmg->region.end) * term->cell_height); } @@ -1587,9 +1587,8 @@ render_csd_title(struct terminal *term) xassert(info.width % term->scale == 0); xassert(info.height % term->scale == 0); - unsigned long cookie = shm_cookie_csd(term, CSD_SURF_TITLE); - struct buffer *buf = shm_get_buffer( - term->wl->shm, info.width, info.height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.csd[CSD_SURF_TITLE]; + struct buffer *buf = shm_get_buffer(chain, info.width, info.height); uint32_t _color = term->conf->colors.fg; uint16_t alpha = 0xffff; @@ -1622,9 +1621,8 @@ render_csd_border(struct terminal *term, enum csd_surface surf_idx) xassert(info.width % term->scale == 0); xassert(info.height % term->scale == 0); - unsigned long cookie = shm_cookie_csd(term, surf_idx); - struct buffer *buf = shm_get_buffer( - term->wl->shm, info.width, info.height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.csd[surf_idx]; + struct buffer *buf = shm_get_buffer(chain, info.width, info.height); pixman_color_t color = color_hex_to_pixman_with_alpha(0, 0); render_csd_part(term, surf, buf, info.width, info.height, &color); @@ -1808,9 +1806,8 @@ render_csd_button(struct terminal *term, enum csd_surface surf_idx) xassert(info.width % term->scale == 0); xassert(info.height % term->scale == 0); - unsigned long cookie = shm_cookie_csd(term, surf_idx); - struct buffer *buf = shm_get_buffer( - term->wl->shm, info.width, info.height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.csd[surf_idx]; + struct buffer *buf = shm_get_buffer(chain, info.width, info.height); uint32_t _color; uint16_t alpha = 0xffff; @@ -2084,9 +2081,8 @@ render_scrollback_position(struct terminal *term) return; } - unsigned long cookie = shm_cookie_scrollback_indicator(term); - struct buffer *buf = shm_get_buffer( - term->wl->shm, width, height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.scrollback_indicator; + struct buffer *buf = shm_get_buffer(chain, width, height); wl_subsurface_set_position( win->scrollback_indicator.sub, x / scale, y / scale); @@ -2117,9 +2113,8 @@ render_render_timer(struct terminal *term, struct timeval render_time) const int height = (2 * margin + term->cell_height + scale - 1) / scale * scale; - unsigned long cookie = shm_cookie_render_timer(term); - struct buffer *buf = shm_get_buffer( - term->wl->shm, width, height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.render_timer; + struct buffer *buf = shm_get_buffer(chain, width, height); wl_subsurface_set_position( win->render_timer.sub, @@ -2292,9 +2287,8 @@ grid_render(struct terminal *term) xassert(term->width > 0); xassert(term->height > 0); - unsigned long cookie = shm_cookie_grid(term); - struct buffer *buf = shm_get_buffer( - term->wl->shm, term->width, term->height, cookie, true, 1 + term->render.workers.count); + struct buffer_chain *chain = term->render.chains.grid; + struct buffer *buf = shm_get_buffer(chain, term->width, term->height); /* Dirty old and current cursor cell, to ensure they’re repainted */ dirty_old_cursor(term); @@ -2626,8 +2620,8 @@ render_search_box(struct terminal *term) const size_t visible_cells = (visible_width - 2 * margin) / term->cell_width; size_t glyph_offset = term->render.search_glyph_offset; - unsigned long cookie = shm_cookie_search(term); - struct buffer *buf = shm_get_buffer(term->wl->shm, width, height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.search; + struct buffer *buf = shm_get_buffer(chain, width, height); pixman_region32_t clip; pixman_region32_init_rect(&clip, 0, 0, width, height); @@ -2936,7 +2930,8 @@ render_urls(struct terminal *term) } info[tll_length(win->urls)]; /* For shm_get_many() */ - struct buffer_description shm_desc[tll_length(win->urls)]; + int widths[tll_length(win->urls)]; + int heights[tll_length(win->urls)]; size_t render_count = 0; @@ -3066,15 +3061,15 @@ render_urls(struct terminal *term) info[render_count].x = x; info[render_count].y = y; - shm_desc[render_count].width = width; - shm_desc[render_count].height = height; - shm_desc[render_count].cookie = shm_cookie_url(url); + widths[render_count] = width; + heights[render_count] = height; render_count++; } + struct buffer_chain *chain = term->render.chains.url; struct buffer *bufs[render_count]; - shm_get_many(term->wl->shm, render_count, shm_desc, bufs, 1); + shm_get_many(chain, render_count, widths, heights, bufs); uint32_t fg = term->conf->colors.use_custom.jump_label ? term->conf->colors.jump_label.fg diff --git a/shm.c b/shm.c index d5c94c8b..acc14a68 100644 --- a/shm.c +++ b/shm.c @@ -68,21 +68,28 @@ struct buffer_pool { size_t ref_count; }; +struct buffer_chain; struct buffer_private { struct buffer public; + struct buffer_chain *chain; size_t ref_count; - size_t size; bool busy; /* Owned by compositor */ - unsigned long cookie; struct buffer_pool *pool; off_t offset; /* Offset into memfd where data begins */ + size_t size; bool scrollable; }; -static tll(struct buffer_private *) buffers; +struct buffer_chain { + tll(struct buffer_private *) bufs; + struct wl_shm *shm; + size_t pix_instances; + bool scrollable; +}; + static tll(struct buffer_private *) deferred; #undef MEASURE_SHM_ALLOCS @@ -151,7 +158,7 @@ buffer_destroy(struct buffer_private *buf) } static bool -buffer_unref_no_remove_from_cache(struct buffer_private *buf) +buffer_unref_no_remove_from_chain(struct buffer_private *buf) { xassert(buf->ref_count > 0); buf->ref_count--; @@ -169,34 +176,13 @@ buffer_unref_no_remove_from_cache(struct buffer_private *buf) void shm_fini(void) { - size_t busy_count UNUSED = 0; - size_t non_busy_count UNUSED = 0; - - tll_foreach(buffers, it) { - if (it->item->busy) - busy_count++; - else - non_busy_count++; - - buffer_destroy(it->item); - tll_remove(buffers, it); - } - - xassert(busy_count == 0); - busy_count = tll_length(deferred); + LOG_DBG("deferred buffers: %zu", tll_length(deferred)); tll_foreach(deferred, it) { buffer_destroy(it->item); tll_remove(deferred, it); } - LOG_DBG("buffers left: busy=%zu, non-busy=%zu", busy_count, non_busy_count); - - if (non_busy_count > 0) { - BUG("%zu non-busy buffers remaining (%zu buffers in total)", - non_busy_count, busy_count + non_busy_count); - } - #if defined(MEASURE_SHM_ALLOCS) && MEASURE_SHM_ALLOCS LOG_INFO("max total allocations was: %zu MB", max_alloced / 1024 / 1024); #endif @@ -207,27 +193,25 @@ buffer_release(void *data, struct wl_buffer *wl_buffer) { struct buffer_private *buffer = data; - LOG_DBG("release: cookie=%lx (buf=%p, total buffer count: %zu)", - buffer->cookie, (void *)buffer, tll_length(buffers)); - xassert(buffer->public.wl_buf == wl_buffer); xassert(buffer->busy); buffer->busy = false; if (buffer->ref_count == 0) { - LOG_WARN("deferred delete"); - bool found = false; tll_foreach(deferred, it) { if (it->item == buffer) { found = true; - buffer_destroy(buffer); tll_remove(deferred, it); break; } } + buffer_destroy(buffer); + xassert(found); + if (!found) + LOG_WARN("deferred delete: buffer not on the 'deferred' list"); } } @@ -255,7 +239,7 @@ page_size(void) #endif static bool -instantiate_offset(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) +instantiate_offset(struct buffer_private *buf, off_t new_offset) { xassert(buf->public.data == NULL); xassert(buf->public.pix == NULL); @@ -314,12 +298,11 @@ err: } static void NOINLINE -get_new_buffers(struct wl_shm *shm, size_t count, - struct buffer_description info[static count], - struct buffer *bufs[static count], - size_t pix_instances, bool scrollable, bool immediate_purge) +get_new_buffers(struct buffer_chain *chain, size_t count, + int widths[static count], int heights[static count], + struct buffer *bufs[static count], bool immediate_purge) { - xassert(count == 1 || !scrollable); + xassert(count == 1 || !chain->scrollable); /* * No existing buffer available. Create a new one by: * @@ -335,8 +318,8 @@ get_new_buffers(struct wl_shm *shm, size_t count, size_t total_size = 0; for (size_t i = 0; i < count; i++) { - stride[i] = stride_for_format_and_width(PIXMAN_a8r8g8b8, info[i].width); - sizes[i] = stride[i] * info[i].height; + stride[i] = stride_for_format_and_width(PIXMAN_a8r8g8b8, widths[i]); + sizes[i] = stride[i] * heights[i]; total_size += sizes[i]; } @@ -363,14 +346,18 @@ get_new_buffers(struct wl_shm *shm, size_t count, } #if __SIZEOF_POINTER__ == 8 - off_t offset = scrollable && max_pool_size > 0 ? (max_pool_size / 4) & ~(page_size() - 1) : 0; - off_t memfd_size = scrollable && max_pool_size > 0 ? max_pool_size : total_size; + off_t offset = chain->scrollable && max_pool_size > 0 + ? (max_pool_size / 4) & ~(page_size() - 1) + : 0; + off_t memfd_size = chain->scrollable && max_pool_size > 0 + ? max_pool_size + : total_size; #else off_t offset = 0; off_t memfd_size = total_size; #endif - xassert(scrollable || (offset == 0 && memfd_size == total_size)); + xassert(chain->scrollable || (offset == 0 && memfd_size == total_size)); LOG_DBG("memfd-size: %lu, initial offset: %lu", memfd_size, offset); @@ -397,10 +384,10 @@ get_new_buffers(struct wl_shm *shm, size_t count, #endif } - if (scrollable && !can_punch_hole) { + if (chain->scrollable && !can_punch_hole) { offset = 0; memfd_size = total_size; - scrollable = false; + chain->scrollable = false; if (ftruncate(pool_fd, memfd_size) < 0) { LOG_ERRNO("failed to set size of SHM backing memory file"); @@ -428,13 +415,13 @@ get_new_buffers(struct wl_shm *shm, size_t count, } #endif - wl_pool = wl_shm_create_pool(shm, pool_fd, memfd_size); + wl_pool = wl_shm_create_pool(chain->shm, pool_fd, memfd_size); if (wl_pool == NULL) { LOG_ERR("failed to create SHM pool"); goto err; } - pool = malloc(sizeof(*pool)); + pool = xmalloc(sizeof(*pool)); if (pool == NULL) { LOG_ERRNO("failed to allocate buffer pool"); goto err; @@ -454,22 +441,22 @@ get_new_buffers(struct wl_shm *shm, size_t count, struct buffer_private *buf = xmalloc(sizeof(*buf)); *buf = (struct buffer_private){ .public = { - .width = info[i].width, - .height = info[i].height, + .width = widths[i], + .height = heights[i], .stride = stride[i], - .pix_instances = pix_instances, + .pix_instances = chain->pix_instances, .age = 1234, /* Force a full repaint */ }, - .cookie = info[i].cookie, + .chain = chain, .ref_count = immediate_purge ? 0 : 1, - .size = sizes[i], .busy = true, .pool = pool, - .scrollable = scrollable, .offset = 0, + .size = sizes[i], + .scrollable = chain->scrollable, }; - if (!instantiate_offset(shm, buf, offset)) { + if (!instantiate_offset(buf, offset)) { free(buf); goto err; } @@ -477,7 +464,7 @@ get_new_buffers(struct wl_shm *shm, size_t count, if (immediate_purge) tll_push_front(deferred, buf); else - tll_push_front(buffers, buf); + tll_push_front(chain->bufs, buf); pixman_region32_init(&buf->public.dirty); pool->ref_count++; @@ -518,28 +505,24 @@ err: } void -shm_get_many(struct wl_shm *shm, size_t count, - struct buffer_description info[static count], - struct buffer *bufs[static count], - size_t pix_instances) +shm_get_many(struct buffer_chain *chain, size_t count, + int widths[static count], int heights[static count], + struct buffer *bufs[static count]) { - get_new_buffers(shm, count, info, bufs, pix_instances, false, true); + get_new_buffers(chain, count, widths, heights, bufs, true); } struct buffer * -shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, - bool scrollable, size_t pix_instances) +shm_get_buffer(struct buffer_chain *chain, int width, int height) { struct buffer_private *cached = NULL; - tll_foreach(buffers, it) { + tll_foreach(chain->bufs, it) { struct buffer_private *buf = it->item; if (buf->public.width != width) continue; if (buf->public.height != height) continue; - if (buf->cookie != cookie) - continue; if (buf->busy) buf->public.age++; @@ -551,13 +534,12 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, #endif { if (cached == NULL) { - LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)", - cookie, (void *)buf); + LOG_DBG("re-using buffer %p from cache", (void *)buf); buf->busy = true; pixman_region32_clear(&buf->public.dirty); free(buf->public.scroll_damage); buf->public.scroll_damage = NULL; - xassert(buf->public.pix_instances == pix_instances); + xassert(buf->public.pix_instances == chain->pix_instances); cached = it->item; } else { /* We have multiple buffers eligible for @@ -567,8 +549,8 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, shm_unref(&cached->public); cached = buf; } else { - if (buffer_unref_no_remove_from_cache(buf)) - tll_remove(buffers, it); + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); } } } @@ -578,23 +560,19 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, return &cached->public; /* Mark old buffers associated with this cookie for purging */ - tll_foreach(buffers, it) { + tll_foreach(chain->bufs, it) { struct buffer_private *buf = it->item; - if (buf->cookie != cookie) - continue; - if (buf->public.width == width && buf->public.height == height) continue; - LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)buf); - if (buffer_unref_no_remove_from_cache(buf)) - tll_remove(buffers, it); + LOG_DBG("marking buffer %p for purging", (void *)buf); + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); } struct buffer *ret; - get_new_buffers(shm, 1, &(struct buffer_description){width, height, cookie}, - &ret, pix_instances, scrollable, false); + get_new_buffers(chain, 1, &width, &height, &ret, false); return ret; } @@ -612,7 +590,7 @@ shm_can_scroll(const struct buffer *_buf) #if __SIZEOF_POINTER__ == 8 && defined(FALLOC_FL_PUNCH_HOLE) static bool -wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) +wrap_buffer(struct buffer_private *buf, off_t new_offset) { struct buffer_pool *pool = buf->pool; xassert(pool->ref_count == 1); @@ -649,11 +627,11 @@ wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) /* Re-instantiate pixman+wl_buffer+raw pointersw */ buffer_destroy_dont_close(&buf->public); - return instantiate_offset(shm, buf, new_offset); + return instantiate_offset(buf, new_offset); } static bool -shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, +shm_scroll_forward(struct buffer_private *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -675,7 +653,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, if (buf->offset + diff + buf->size > max_pool_size) { LOG_DBG("memfd offset wrap around"); - if (!wrap_buffer(shm, buf, 0)) + if (!wrap_buffer(buf, 0)) goto err; } @@ -684,6 +662,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, xassert(new_offset + buf->size <= max_pool_size); #if TIME_SCROLL + struct timeval tot; struct timeval time1; gettimeofday(&time1, NULL); @@ -731,7 +710,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, #endif /* Re-instantiate pixman+wl_buffer+raw pointersw */ - bool ret = instantiate_offset(shm, buf, new_offset); + bool ret = instantiate_offset(buf, new_offset); #if TIME_SCROLL struct timeval time4; @@ -768,7 +747,7 @@ err: } static bool -shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, +shm_scroll_reverse(struct buffer_private *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -780,7 +759,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, const off_t diff = rows * buf->public.stride; if (diff > buf->offset) { LOG_DBG("memfd offset reverse wrap-around"); - if (!wrap_buffer(shm, buf, (max_pool_size - buf->size) & ~(page_size() - 1))) + if (!wrap_buffer(buf, (max_pool_size - buf->size) & ~(page_size() - 1))) goto err; } @@ -837,7 +816,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, #endif /* Re-instantiate pixman+wl_buffer+raw pointers */ - bool ret = instantiate_offset(shm, buf, new_offset); + bool ret = instantiate_offset(buf, new_offset); #if TIME_SCROLL struct timeval time3; @@ -873,7 +852,7 @@ err: #endif /* FALLOC_FL_PUNCH_HOLE */ bool -shm_scroll(struct wl_shm *shm, struct buffer *_buf, int rows, +shm_scroll(struct buffer *_buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -885,27 +864,22 @@ shm_scroll(struct wl_shm *shm, struct buffer *_buf, int rows, xassert(rows != 0); return rows > 0 - ? shm_scroll_forward(shm, buf, rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows) - : shm_scroll_reverse(shm, buf, -rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows); + ? shm_scroll_forward(buf, rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows) + : shm_scroll_reverse(buf, -rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows); #else return false; #endif } void -shm_purge(struct wl_shm *shm, unsigned long cookie) +shm_purge(struct buffer_chain *chain) { - LOG_DBG("cookie=%lx: purging all buffers", cookie); + LOG_DBG("chain: %p: purging all buffers", (void *)chain); /* Purge old buffers associated with this cookie */ - tll_foreach(buffers, it) { - struct buffer_private *buf = it->item; - - if (buf->cookie != cookie) - continue; - - if (buffer_unref_no_remove_from_cache(buf)) - tll_remove(buffers, it); + tll_foreach(chain->bufs, it) { + if (buffer_unref_no_remove_from_chain(it->item)) + tll_remove(chain->bufs, it); } } @@ -923,12 +897,47 @@ shm_unref(struct buffer *_buf) return; struct buffer_private *buf = (struct buffer_private *)_buf; - tll_foreach(buffers, it) { + struct buffer_chain *chain = buf->chain; + + tll_foreach(chain->bufs, it) { if (it->item != buf) continue; - if (buffer_unref_no_remove_from_cache(buf)) - tll_remove(buffers, it); + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); break; } } + +void +shm_chain_purge(struct buffer_chain *chain) +{ + tll_foreach(chain->bufs, it) { + struct buffer_private *buf = it->item; + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); + } +} + +struct buffer_chain * +shm_chain_new(struct wl_shm *shm, bool scrollable, size_t pix_instances) +{ + struct buffer_chain *chain = xmalloc(sizeof(*chain)); + *chain = (struct buffer_chain){ + .bufs = tll_init(), + .shm = shm, + .pix_instances = pix_instances, + .scrollable = scrollable, + }; + return chain; +} + +void +shm_chain_free(struct buffer_chain *chain) +{ + if (chain == NULL) + return; + + shm_chain_purge(chain); + free(chain); +} diff --git a/shm.h b/shm.h index 18941588..180c8525 100644 --- a/shm.h +++ b/shm.h @@ -7,6 +7,8 @@ #include #include +#include + struct damage; struct buffer { @@ -27,28 +29,24 @@ struct buffer { pixman_region32_t dirty; }; -struct buffer_description { - int width; - int height; - unsigned long cookie; -}; - void shm_fini(void); void shm_set_max_pool_size(off_t max_pool_size); +struct buffer_chain; +struct buffer_chain *shm_chain_new( + struct wl_shm *shm, bool scrollable, size_t pix_instances); +void shm_chain_free(struct buffer_chain *chain); + /* * Returns a single buffer. * * May returned a cached buffer. If so, the buffer’s age indicates how * many shm_get_buffer() calls have been made for the same - * width/height/cookie while the buffer was still busy. + * width/height while the buffer was still busy. * * A newly allocated buffer has an age of 1234. */ -struct buffer *shm_get_buffer( - struct wl_shm *shm, int width, int height, unsigned long cookie, - bool scrollable, size_t pix_instances); - +struct buffer *shm_get_buffer(struct buffer_chain *chain, int width, int height); /* * Returns many buffers, described by ‘info’, all sharing the same SHM * buffer pool. @@ -64,25 +62,16 @@ struct buffer *shm_get_buffer( * soon as the compositor releases them. */ void shm_get_many( - struct wl_shm *shm, size_t count, - struct buffer_description info[static count], - struct buffer *bufs[static count], size_t pix_instances); + struct buffer_chain *chain, size_t count, + int widths[static count], int heights[static count], + struct buffer *bufs[static count]); bool shm_can_scroll(const struct buffer *buf); -bool shm_scroll(struct wl_shm *shm, struct buffer *buf, int rows, +bool shm_scroll(struct buffer *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows); void shm_addref(struct buffer *buf); void shm_unref(struct buffer *buf); -void shm_purge(struct wl_shm *shm, unsigned long cookie); -struct terminal; -static inline unsigned long shm_cookie_grid(const struct terminal *term) { return (unsigned long)((uintptr_t)term + 0); } -static inline unsigned long shm_cookie_search(const struct terminal *term) { return (unsigned long)((uintptr_t)term + 1); } -static inline unsigned long shm_cookie_scrollback_indicator(const struct terminal *term) { return (unsigned long)(uintptr_t)term + 2; } -static inline unsigned long shm_cookie_render_timer(const struct terminal *term) { return (unsigned long)(uintptr_t)term + 3; } -static inline unsigned long shm_cookie_csd(const struct terminal *term, int n) { return (unsigned long)((uintptr_t)term + 4 + (n)); } - -struct url; -static inline unsigned long shm_cookie_url(const struct url *url) { return (unsigned long)(uintptr_t)url; } +void shm_purge(struct buffer_chain *chain); diff --git a/terminal.c b/terminal.c index c7776f28..72ab1f2b 100644 --- a/terminal.c +++ b/terminal.c @@ -1144,6 +1144,23 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .tab_stops = tll_init(), .wl = wayl, .render = { + .chains = { + .grid = shm_chain_new(wayl->shm, true, 1 + conf->render_worker_count), + .search = shm_chain_new(wayl->shm, false, 1), + .scrollback_indicator = shm_chain_new(wayl->shm, false, 1), + .render_timer = shm_chain_new(wayl->shm, false, 1), + .url = shm_chain_new(wayl->shm, false, 1), + .csd = { + [CSD_SURF_TITLE] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_LEFT] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_RIGHT] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_TOP] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_BOTTOM] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_MINIMIZE] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_MAXIMIZE] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_CLOSE] = shm_chain_new(wayl->shm, false, 1), + }, + }, .scrollback_lines = conf->scrollback.lines, .app_sync_updates.timer_fd = app_sync_updates_fd, .title = { @@ -1458,7 +1475,15 @@ term_destroy(struct terminal *term) sem_destroy(&term->render.workers.done); xassert(tll_length(term->render.workers.queue) == 0); tll_free(term->render.workers.queue); + shm_unref(term->render.last_buf); + shm_chain_free(term->render.chains.grid); + shm_chain_free(term->render.chains.search); + shm_chain_free(term->render.chains.scrollback_indicator); + shm_chain_free(term->render.chains.render_timer); + shm_chain_free(term->render.chains.url); + for (size_t i = 0; i < CSD_SURF_COUNT; i++) + shm_chain_free(term->render.chains.csd[i]); tll_free(term->tab_stops); diff --git a/terminal.h b/terminal.h index 5ffa6154..b02036a2 100644 --- a/terminal.h +++ b/terminal.h @@ -21,6 +21,7 @@ #include "fdm.h" #include "macros.h" #include "reaper.h" +#include "shm.h" #include "wayland.h" /* @@ -475,6 +476,15 @@ struct terminal { enum term_surface active_surface; struct { + struct { + struct buffer_chain *grid; + struct buffer_chain *search; + struct buffer_chain *scrollback_indicator; + struct buffer_chain *render_timer; + struct buffer_chain *url; + struct buffer_chain *csd[CSD_SURF_COUNT]; + } chains; + /* Scheduled for rendering, as soon-as-possible */ struct { bool grid; diff --git a/wayland.c b/wayland.c index 880a4cf3..38c80999 100644 --- a/wayland.c +++ b/wayland.c @@ -52,11 +52,10 @@ static void csd_destroy(struct wl_window *win) { struct terminal *term = win->term; - struct wl_shm *shm = term->wl->shm; for (size_t i = 0; i < ALEN(win->csd.surface); i++) { wayl_win_subsurface_destroy(&win->csd.surface[i]); - shm_purge(shm, shm_cookie_csd(term, i)); + shm_purge(term->render.chains.csd[i]); } } @@ -1417,7 +1416,6 @@ wayl_win_destroy(struct wl_window *win) return; struct terminal *term = win->term; - struct wl_shm *shm = term->wl->shm; if (win->csd.move_timeout_fd != -1) close(win->csd.move_timeout_fd); @@ -1472,7 +1470,7 @@ wayl_win_destroy(struct wl_window *win) tll_foreach(win->urls, it) { wayl_win_subsurface_destroy(&it->item.surf); - shm_purge(shm, shm_cookie_url(it->item.url)); + shm_purge(term->render.chains.url); tll_remove(win->urls, it); } @@ -1481,13 +1479,13 @@ wayl_win_destroy(struct wl_window *win) wayl_win_subsurface_destroy(&win->scrollback_indicator); wayl_win_subsurface_destroy(&win->render_timer); - shm_purge(shm, shm_cookie_search(term)); - shm_purge(shm, shm_cookie_scrollback_indicator(term)); - shm_purge(shm, shm_cookie_render_timer(term)); - shm_purge(shm, shm_cookie_grid(term)); + shm_purge(term->render.chains.search); + shm_purge(term->render.chains.scrollback_indicator); + shm_purge(term->render.chains.render_timer); + shm_purge(term->render.chains.grid); for (size_t i = 0; i < ALEN(win->csd.surface); i++) - shm_purge(shm, shm_cookie_csd(term, i)); + shm_purge(term->render.chains.csd[i]); #if defined(HAVE_XDG_ACTIVATION) if (win->xdg_activation_token != NULL) From 0751172b922e32a9c6d54b52e721c934cb88fe9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:49:27 +0200 Subject: [PATCH 43/52] shm: get_buffer: purge mismatching buffers in first buffer iteration There's no longer any need to defer purging of mismatching buffer (i.e. buffers whose width/height doesn't match the requested ones) to after the cache lookup loop. --- shm.c | 32 ++++++-------------------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/shm.c b/shm.c index acc14a68..f7e7a4c5 100644 --- a/shm.c +++ b/shm.c @@ -519,10 +519,12 @@ shm_get_buffer(struct buffer_chain *chain, int width, int height) tll_foreach(chain->bufs, it) { struct buffer_private *buf = it->item; - if (buf->public.width != width) - continue; - if (buf->public.height != height) + if (buf->public.width != width || buf->public.height != height) { + LOG_DBG("purging mismatching buffer %p", (void *)buf); + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); continue; + } if (buf->busy) buf->public.age++; @@ -559,18 +561,6 @@ shm_get_buffer(struct buffer_chain *chain, int width, int height) if (cached != NULL) return &cached->public; - /* Mark old buffers associated with this cookie for purging */ - tll_foreach(chain->bufs, it) { - struct buffer_private *buf = it->item; - - if (buf->public.width == width && buf->public.height == height) - continue; - - LOG_DBG("marking buffer %p for purging", (void *)buf); - if (buffer_unref_no_remove_from_chain(buf)) - tll_remove(chain->bufs, it); - } - struct buffer *ret; get_new_buffers(chain, 1, &width, &height, &ret, false); return ret; @@ -909,16 +899,6 @@ shm_unref(struct buffer *_buf) } } -void -shm_chain_purge(struct buffer_chain *chain) -{ - tll_foreach(chain->bufs, it) { - struct buffer_private *buf = it->item; - if (buffer_unref_no_remove_from_chain(buf)) - tll_remove(chain->bufs, it); - } -} - struct buffer_chain * shm_chain_new(struct wl_shm *shm, bool scrollable, size_t pix_instances) { @@ -938,6 +918,6 @@ shm_chain_free(struct buffer_chain *chain) if (chain == NULL) return; - shm_chain_purge(chain); + shm_purge(chain); free(chain); } From 6657146a207834ac44425c01703935dee6d61d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:49:52 +0200 Subject: [PATCH 44/52] shm: chain_free: BUG() if there are buffers remaining after purge There may be buffers left, if their destruction has been deferred. However, they should be on the 'deferred' list, not the chain's buffer list. If there are buffers left on the chain's list, that means someone forgot to call shm_unref(). --- shm.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/shm.c b/shm.c index f7e7a4c5..0e55011d 100644 --- a/shm.c +++ b/shm.c @@ -515,6 +515,11 @@ shm_get_many(struct buffer_chain *chain, size_t count, struct buffer * shm_get_buffer(struct buffer_chain *chain, int width, int height) { + LOG_DBG( + "chain=%p: looking for a re-usable %dx%d buffer " + "among %zu potential buffers", + (void *)chain, width, height, tll_length(chain->bufs)); + struct buffer_private *cached = NULL; tll_foreach(chain->bufs, it) { struct buffer_private *buf = it->item; @@ -919,5 +924,11 @@ shm_chain_free(struct buffer_chain *chain) return; shm_purge(chain); + + if (tll_length(chain->bufs) > 0) { + BUG("chain=%p: there are buffers remaining; " + "is there a missing call to shm_unref()?", (void *)chain); + } + free(chain); } From 5f0ceb72f176caf7673ea1d4bc5195f26f4b4df7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:45:36 +0200 Subject: [PATCH 45/52] csi: erase scrollback: cancel selection if it touches the scrollback This breaks out the scrollback erasing logic for \E[3J from csi.c, and moves it to the new function term_erase_scrollback(), and changes the logic to calculate the start and end row (absolute) numbers of the scrollback, and only iterate those, instead of iterating *all* rows, filtering out those that are on-screen. It also adds an intersection range check of the selection range, and cancels the selection if it touches any of the deleted scrollback rows. This fixes a crash when trying to render the next frame, since the selection now references rows that have been freed. Closes #633 --- csi.c | 21 +----------------- selection.c | 4 +--- terminal.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++ terminal.h | 1 + 4 files changed, 66 insertions(+), 23 deletions(-) diff --git a/csi.c b/csi.c index 80ee75cd..dc5a018a 100644 --- a/csi.c +++ b/csi.c @@ -917,26 +917,7 @@ csi_dispatch(struct terminal *term, uint8_t final) case 3: { /* Erase scrollback */ - int end = (term->grid->offset + term->rows - 1) % term->grid->num_rows; - for (size_t i = 0; i < term->grid->num_rows; i++) { - if (end >= term->grid->offset) { - /* Not wrapped */ - if (i >= term->grid->offset && i <= end) - continue; - } else { - /* Wrapped */ - if (i >= term->grid->offset || i <= end) - continue; - } - - if (term->render.last_cursor.row == term->grid->rows[i]) - term->render.last_cursor.row = NULL; - - grid_row_free(term->grid->rows[i]); - term->grid->rows[i] = NULL; - } - term->grid->view = term->grid->offset; - term_damage_view(term); + term_erase_scrollback(term); break; } diff --git a/selection.c b/selection.c index 57ab8bf6..537614f6 100644 --- a/selection.c +++ b/selection.c @@ -69,10 +69,8 @@ selection_on_rows(const struct terminal *term, int row_start, int row_end) end = tmp; } - if (row_start >= start->row && row_end <= end->row) { - LOG_INFO("ON ROWS"); + if (row_start >= start->row && row_end <= end->row) return true; - } return false; } diff --git a/terminal.c b/terminal.c index 72ab1f2b..df6ddbed 100644 --- a/terminal.c +++ b/terminal.c @@ -2011,6 +2011,69 @@ term_erase(struct terminal *term, const struct coord *start, const struct coord sixel_overwrite_by_row(term, end->row, 0, end->col + 1); } +void +term_erase_scrollback(struct terminal *term) +{ + const int mask = term->grid->num_rows - 1; + const int start = (term->grid->offset + term->rows) & mask; + const int end = (term->grid->offset - 1) & mask; + const int sel_start = term->selection.start.row; + const int sel_end = term->selection.end.row; + + if (sel_end >= 0) { + /* + * Cancel selection if it touches any of the rows in the + * scrollback, since we can’t have the selection reference + * soon-to-be deleted rows. + * + * This is done by range checking the selection range against + * the scrollback range. + * + * To make this comparison simpler, the start/end absolute row + * numbers are “rebased” against the scrollback start, where + * row 0 is the *first* row in the scrollback. A high number + * thus means the row is further *down* in the scrollback, + * closer to the screen bottom. + */ + int scrollback_start = term->grid->offset + term->rows; + + int rel_sel_start = sel_start - scrollback_start + term->grid->num_rows; + int rel_sel_end = sel_end - scrollback_start + term->grid->num_rows; + + int rel_start = start - scrollback_start + term->grid->num_rows; + int rel_end = end - scrollback_start + term->grid->num_rows; + + rel_sel_start &= mask; + rel_sel_end &= mask; + rel_start &= mask; + rel_end &= mask; + + if ((rel_sel_start <= rel_start && rel_sel_end >= rel_start) || + (rel_sel_start <= rel_end && rel_sel_end >= rel_end) || + (rel_sel_start >= rel_start && rel_sel_end <= rel_end)) + { + selection_cancel(term); + } + } + + for (int i = start;; i = (i + 1) & mask) { + struct row *row = term->grid->rows[i]; + if (row != NULL) { + if (term->render.last_cursor.row == row) + term->render.last_cursor.row = NULL; + + grid_row_free(row); + term->grid->rows[i] = NULL; + } + + if (i == end) + break; + } + + term->grid->view = term->grid->offset; + term_damage_view(term); +} + int term_row_rel_to_abs(const struct terminal *term, int row) { diff --git a/terminal.h b/terminal.h index b02036a2..d699d644 100644 --- a/terminal.h +++ b/terminal.h @@ -662,6 +662,7 @@ void term_damage_scroll( void term_erase( struct terminal *term, const struct coord *start, const struct coord *end); +void term_erase_scrollback(struct terminal *term); int term_row_rel_to_abs(const struct terminal *term, int row); void term_cursor_home(struct terminal *term); From d0a7f999c662759d1704d97d5217736e84f508c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 18 Jul 2021 15:34:19 +0200 Subject: [PATCH 46/52] term: add unit test for term_erase_scrollback() --- terminal.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/terminal.c b/terminal.c index df6ddbed..b16cf19a 100644 --- a/terminal.c +++ b/terminal.c @@ -2074,6 +2074,103 @@ term_erase_scrollback(struct terminal *term) term_damage_view(term); } +UNITTEST +{ + const int scrollback_rows = 16; + const int term_rows = 5; + const int cols = 5; + + struct fdm *fdm = fdm_init(); + xassert(fdm != NULL); + + struct terminal term = { + .fdm = fdm, + .rows = term_rows, + .cols = cols, + .normal = { + .rows = xcalloc(scrollback_rows, sizeof(term.normal.rows[0])), + .num_rows = scrollback_rows, + .num_cols = cols, + }, + .grid = &term.normal, + .selection = { + .start = {-1, -1}, + .end = {-1, -1}, + .kind = SELECTION_NONE, + .auto_scroll = { + .fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK), + }, + }, + }; + + xassert(term.selection.auto_scroll.fd >= 0); + +#define populate_scrollback() do { \ + for (int i = 0; i < scrollback_rows; i++) { \ + if (term.normal.rows[i] == NULL) { \ + struct row *r = xcalloc(1, sizeof(*term.normal.rows[i])); \ + r->cells = xcalloc(cols, sizeof(r->cells[0])); \ + term.normal.rows[i] = r; \ + } \ + } \ + } while (0) + + /* + * Test case 1 - no selection, just verify all rows except those + * on screen have been deleted. + */ + + populate_scrollback(); + term.normal.offset = 11; + term_erase_scrollback(&term); + for (int i = 0; i < scrollback_rows; i++) { + if (i >= term.normal.offset && i < term.normal.offset + term_rows) + xassert(term.normal.rows[i] != NULL); + else + xassert(term.normal.rows[i] == NULL); + } + + /* + * Test case 2 - selection that touches the scrollback. Verify the + * selection is cancelled. + */ + + term.normal.offset = 14; /* Screen covers rows 14,15,0,1,2 */ + + /* Selection covers rows 15,0,1,2,3 */ + term.selection.start = (struct coord){.row = 15}; + term.selection.end = (struct coord){.row = 19}; + term.selection.kind = SELECTION_CHAR_WISE; + + populate_scrollback(); + term_erase_scrollback(&term); + xassert(term.selection.start.row < 0); + xassert(term.selection.end.row < 0); + xassert(term.selection.kind == SELECTION_NONE); + + /* + * Test case 3 - selection that does *not* touch the + * scrollback. Verify the selection is *not* cancelled. + */ + + /* Selection covers rows 15,0 */ + term.selection.start = (struct coord){.row = 15}; + term.selection.end = (struct coord){.row = 16}; + term.selection.kind = SELECTION_CHAR_WISE; + + populate_scrollback(); + term_erase_scrollback(&term); + xassert(term.selection.start.row == 15); + xassert(term.selection.end.row == 16); + xassert(term.selection.kind == SELECTION_CHAR_WISE); + + close(term.selection.auto_scroll.fd); + for (int i = 0; i < scrollback_rows; i++) + grid_row_free(term.normal.rows[i]); + free(term.normal.rows); + fdm_destroy(fdm); +} + int term_row_rel_to_abs(const struct terminal *term, int row) { From 7d8884fec4fac371430cb2f0ba85a9d09e3cd171 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 18 Jul 2021 15:38:46 +0200 Subject: [PATCH 47/52] changelog: crash when \E[3J was executed with a selection in the scrollback --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f12a8b53..11dce7f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,9 @@ * Scrollback indicator being incorrectly rendered when window size is very small. * Reduced memory usage in URL mode. +* Crash when the `E3` escape (`\E[3J`) was executed, and there was a + selection in the scrollback + (https://codeberg.org/dnkl/foot/issues/633). ### Security From f65e062ce4defaa647b43927e462eaf2d683942b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 18 Jul 2021 16:30:42 +0200 Subject: [PATCH 48/52] =?UTF-8?q?sixel:=20destroy():=20don=E2=80=99t=20unr?= =?UTF-8?q?ef=20a=20NULL=20pixman=20image?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sixel.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sixel.c b/sixel.c index 86878978..18a9c146 100644 --- a/sixel.c +++ b/sixel.c @@ -88,9 +88,10 @@ sixel_init(struct terminal *term, int p1, int p2, int p3) void sixel_destroy(struct sixel *sixel) { - pixman_image_unref(sixel->pix); - free(sixel->data); + if (sixel->pix != NULL) + pixman_image_unref(sixel->pix); + free(sixel->data); sixel->pix = NULL; sixel->data = NULL; } From e8e9cd559529b91968af9f776df8b87751df7b97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 18 Jul 2021 16:31:33 +0200 Subject: [PATCH 49/52] term: erase_scrollback(): destroy sixels that touch the scrollback --- CHANGELOG.md | 2 +- terminal.c | 66 ++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11dce7f7..7e44d8a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,7 +74,7 @@ very small. * Reduced memory usage in URL mode. * Crash when the `E3` escape (`\E[3J`) was executed, and there was a - selection in the scrollback + selection, or sixel image, in the scrollback (https://codeberg.org/dnkl/foot/issues/633). diff --git a/terminal.c b/terminal.c index b16cf19a..af5172bb 100644 --- a/terminal.c +++ b/terminal.c @@ -2014,9 +2014,16 @@ term_erase(struct terminal *term, const struct coord *start, const struct coord void term_erase_scrollback(struct terminal *term) { - const int mask = term->grid->num_rows - 1; + const int num_rows = term->grid->num_rows; + const int mask = num_rows - 1; + const int start = (term->grid->offset + term->rows) & mask; const int end = (term->grid->offset - 1) & mask; + + const int scrollback_start = term->grid->offset + term->rows; + const int rel_start = (start - scrollback_start + num_rows) & mask; + const int rel_end = (end - scrollback_start + num_rows) & mask; + const int sel_start = term->selection.start.row; const int sel_end = term->selection.end.row; @@ -2035,18 +2042,9 @@ term_erase_scrollback(struct terminal *term) * thus means the row is further *down* in the scrollback, * closer to the screen bottom. */ - int scrollback_start = term->grid->offset + term->rows; - int rel_sel_start = sel_start - scrollback_start + term->grid->num_rows; - int rel_sel_end = sel_end - scrollback_start + term->grid->num_rows; - - int rel_start = start - scrollback_start + term->grid->num_rows; - int rel_end = end - scrollback_start + term->grid->num_rows; - - rel_sel_start &= mask; - rel_sel_end &= mask; - rel_start &= mask; - rel_end &= mask; + const int rel_sel_start = (sel_start - scrollback_start + num_rows) & mask; + const int rel_sel_end = (sel_end - scrollback_start + num_rows) & mask; if ((rel_sel_start <= rel_start && rel_sel_end >= rel_start) || (rel_sel_start <= rel_end && rel_sel_end >= rel_end) || @@ -2056,6 +2054,20 @@ term_erase_scrollback(struct terminal *term) } } + tll_foreach(term->grid->sixel_images, it) { + struct sixel *six = &it->item; + const int six_start = (six->pos.row - scrollback_start + num_rows) & mask; + const int six_end = (six->pos.row + six->rows - 1 - scrollback_start + num_rows) & mask; + + if ((six_start <= rel_start && six_end >= rel_start) || + (six_start <= rel_end && six_end >= rel_end) || + (six_start >= rel_start && six_end <= rel_end)) + { + sixel_destroy(six); + tll_remove(term->grid->sixel_images, it); + } + } + for (int i = start;; i = (i + 1) & mask) { struct row *row = term->grid->rows[i]; if (row != NULL) { @@ -2164,6 +2176,36 @@ UNITTEST xassert(term.selection.end.row == 16); xassert(term.selection.kind == SELECTION_CHAR_WISE); + term.selection.start = (struct coord){-1, -1}; + term.selection.end = (struct coord){-1, -1}; + term.selection.kind = SELECTION_NONE; + + /* + * Test case 4 - sixel that touch the scrollback + */ + + struct sixel six = { + .rows = 5, + .pos = { + .row = 15, + }, + }; + tll_push_back(term.normal.sixel_images, six); + populate_scrollback(); + term_erase_scrollback(&term); + xassert(tll_length(term.normal.sixel_images) == 0); + + /* + * Test case 5 - sixel that does *not* touch the scrollback + */ + six.rows = 3; + tll_push_back(term.normal.sixel_images, six); + populate_scrollback(); + term_erase_scrollback(&term); + xassert(tll_length(term.normal.sixel_images) == 1); + + /* Cleanup */ + tll_free(term.normal.sixel_images); close(term.selection.auto_scroll.fd); for (int i = 0; i < scrollback_rows; i++) grid_row_free(term.normal.rows[i]); From 5b6a2b0eaff1ffef84d728a6c76c044c7f01a1d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 18 Jul 2021 16:44:49 +0200 Subject: [PATCH 50/52] =?UTF-8?q?shm:=20get=5Fmany():=20allow=20=E2=80=9CN?= =?UTF-8?q?ULL=E2=80=9D=20buffers=20-=20buffers=20where=20width=20or=20hei?= =?UTF-8?q?ght=20is=200?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a zero-sized buffer is requested, simply return a NULL buffer, instead of crashing with a Wayland protocol error. This makes it easier to request many buffers, where some may be zero-sized, without having to pack the width/height and bufs arrays. --- shm.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shm.c b/shm.c index 0e55011d..60e45d4d 100644 --- a/shm.c +++ b/shm.c @@ -436,6 +436,10 @@ get_new_buffers(struct buffer_chain *chain, size_t count, }; for (size_t i = 0; i < count; i++) { + if (sizes[i] == 0) { + bufs[i] = NULL; + continue; + } /* Push to list of available buffers, but marked as 'busy' */ struct buffer_private *buf = xmalloc(sizeof(*buf)); From 20fc80e57e365f2e002eead7de110cb23ffa7adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 18 Jul 2021 16:46:43 +0200 Subject: [PATCH 51/52] render: use a single backing SHM pool for CSD surface buffers --- render.c | 73 +++++++++++++++++++++++++++--------------------------- terminal.c | 14 ++--------- terminal.h | 2 +- wayland.c | 11 +++----- 4 files changed, 44 insertions(+), 56 deletions(-) diff --git a/render.c b/render.c index 7bcad519..7750dd19 100644 --- a/render.c +++ b/render.c @@ -1575,20 +1575,16 @@ render_csd_part(struct terminal *term, } static void -render_csd_title(struct terminal *term) +render_csd_title(struct terminal *term, const struct csd_data *info, + struct buffer *buf) { xassert(term->window->csd_mode == CSD_YES); - struct csd_data info = get_csd_data(term, CSD_SURF_TITLE); struct wl_surface *surf = term->window->csd.surface[CSD_SURF_TITLE].surf; + xassert(info->width > 0 && info->height > 0); - xassert(info.width > 0 && info.height > 0); - - xassert(info.width % term->scale == 0); - xassert(info.height % term->scale == 0); - - struct buffer_chain *chain = term->render.chains.csd[CSD_SURF_TITLE]; - struct buffer *buf = shm_get_buffer(chain, info.width, info.height); + xassert(info->width % term->scale == 0); + xassert(info->height % term->scale == 0); uint32_t _color = term->conf->colors.fg; uint16_t alpha = 0xffff; @@ -1602,30 +1598,27 @@ render_csd_title(struct terminal *term) _color = color_dim(_color); pixman_color_t color = color_hex_to_pixman_with_alpha(_color, alpha); - render_csd_part(term, surf, buf, info.width, info.height, &color); + render_csd_part(term, surf, buf, info->width, info->height, &color); csd_commit(term, surf, buf); } static void -render_csd_border(struct terminal *term, enum csd_surface surf_idx) +render_csd_border(struct terminal *term, enum csd_surface surf_idx, + const struct csd_data *info, struct buffer *buf) { xassert(term->window->csd_mode == CSD_YES); xassert(surf_idx >= CSD_SURF_LEFT && surf_idx <= CSD_SURF_BOTTOM); - struct csd_data info = get_csd_data(term, surf_idx); struct wl_surface *surf = term->window->csd.surface[surf_idx].surf; - if (info.width == 0 || info.height == 0) + if (info->width == 0 || info->height == 0) return; - xassert(info.width % term->scale == 0); - xassert(info.height % term->scale == 0); - - struct buffer_chain *chain = term->render.chains.csd[surf_idx]; - struct buffer *buf = shm_get_buffer(chain, info.width, info.height); + xassert(info->width % term->scale == 0); + xassert(info->height % term->scale == 0); pixman_color_t color = color_hex_to_pixman_with_alpha(0, 0); - render_csd_part(term, surf, buf, info.width, info.height, &color); + render_csd_part(term, surf, buf, info->width, info->height, &color); csd_commit(term, surf, buf); } @@ -1792,22 +1785,19 @@ render_csd_button_close(struct terminal *term, struct buffer *buf) } static void -render_csd_button(struct terminal *term, enum csd_surface surf_idx) +render_csd_button(struct terminal *term, enum csd_surface surf_idx, + const struct csd_data *info, struct buffer *buf) { xassert(term->window->csd_mode == CSD_YES); xassert(surf_idx >= CSD_SURF_MINIMIZE && surf_idx <= CSD_SURF_CLOSE); - struct csd_data info = get_csd_data(term, surf_idx); struct wl_surface *surf = term->window->csd.surface[surf_idx].surf; - if (info.width == 0 || info.height == 0) + if (info->width == 0 || info->height == 0) return; - xassert(info.width % term->scale == 0); - xassert(info.height % term->scale == 0); - - struct buffer_chain *chain = term->render.chains.csd[surf_idx]; - struct buffer *buf = shm_get_buffer(chain, info.width, info.height); + xassert(info->width % term->scale == 0); + xassert(info->height % term->scale == 0); uint32_t _color; uint16_t alpha = 0xffff; @@ -1856,7 +1846,7 @@ render_csd_button(struct terminal *term, enum csd_surface surf_idx) _color = color_dim(_color); pixman_color_t color = color_hex_to_pixman_with_alpha(_color, alpha); - render_csd_part(term, surf, buf, info.width, info.height, &color); + render_csd_part(term, surf, buf, info->width, info->height, &color); switch (surf_idx) { case CSD_SURF_MINIMIZE: render_csd_button_minimize(term, buf); break; @@ -1880,12 +1870,16 @@ render_csd(struct terminal *term) if (term->window->is_fullscreen) return; + struct csd_data infos[CSD_SURF_COUNT]; + int widths[CSD_SURF_COUNT]; + int heights[CSD_SURF_COUNT]; + for (size_t i = 0; i < CSD_SURF_COUNT; i++) { - struct csd_data info = get_csd_data(term, i); - const int x = info.x; - const int y = info.y; - const int width = info.width; - const int height = info.height; + infos[i] = get_csd_data(term, i); + const int x = infos[i].x; + const int y = infos[i].y; + const int width = infos[i].width; + const int height = infos[i].height; struct wl_surface *surf = term->window->csd.surface[i].surf; struct wl_subsurface *sub = term->window->csd.surface[i].sub; @@ -1894,20 +1888,27 @@ render_csd(struct terminal *term) xassert(sub != NULL); if (width == 0 || height == 0) { + widths[i] = heights[i] = 0; wl_subsurface_set_position(sub, 0, 0); wl_surface_attach(surf, NULL, 0, 0); wl_surface_commit(surf); continue; } + widths[i] = width; + heights[i] = height; + wl_subsurface_set_position(sub, x / term->scale, y / term->scale); } + struct buffer *bufs[CSD_SURF_COUNT]; + shm_get_many(term->render.chains.csd, CSD_SURF_COUNT, widths, heights, bufs); + for (size_t i = CSD_SURF_LEFT; i <= CSD_SURF_BOTTOM; i++) - render_csd_border(term, i); + render_csd_border(term, i, &infos[i], bufs[i]); for (size_t i = CSD_SURF_MINIMIZE; i <= CSD_SURF_CLOSE; i++) - render_csd_button(term, i); - render_csd_title(term); + render_csd_button(term, i, &infos[i], bufs[i]); + render_csd_title(term, &infos[CSD_SURF_TITLE], bufs[CSD_SURF_TITLE]); } static void diff --git a/terminal.c b/terminal.c index af5172bb..46e7fc01 100644 --- a/terminal.c +++ b/terminal.c @@ -1150,16 +1150,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .scrollback_indicator = shm_chain_new(wayl->shm, false, 1), .render_timer = shm_chain_new(wayl->shm, false, 1), .url = shm_chain_new(wayl->shm, false, 1), - .csd = { - [CSD_SURF_TITLE] = shm_chain_new(wayl->shm, false, 1), - [CSD_SURF_LEFT] = shm_chain_new(wayl->shm, false, 1), - [CSD_SURF_RIGHT] = shm_chain_new(wayl->shm, false, 1), - [CSD_SURF_TOP] = shm_chain_new(wayl->shm, false, 1), - [CSD_SURF_BOTTOM] = shm_chain_new(wayl->shm, false, 1), - [CSD_SURF_MINIMIZE] = shm_chain_new(wayl->shm, false, 1), - [CSD_SURF_MAXIMIZE] = shm_chain_new(wayl->shm, false, 1), - [CSD_SURF_CLOSE] = shm_chain_new(wayl->shm, false, 1), - }, + .csd = shm_chain_new(wayl->shm, false, 1), }, .scrollback_lines = conf->scrollback.lines, .app_sync_updates.timer_fd = app_sync_updates_fd, @@ -1482,8 +1473,7 @@ term_destroy(struct terminal *term) shm_chain_free(term->render.chains.scrollback_indicator); shm_chain_free(term->render.chains.render_timer); shm_chain_free(term->render.chains.url); - for (size_t i = 0; i < CSD_SURF_COUNT; i++) - shm_chain_free(term->render.chains.csd[i]); + shm_chain_free(term->render.chains.csd); tll_free(term->tab_stops); diff --git a/terminal.h b/terminal.h index d699d644..688e42cc 100644 --- a/terminal.h +++ b/terminal.h @@ -482,7 +482,7 @@ struct terminal { struct buffer_chain *scrollback_indicator; struct buffer_chain *render_timer; struct buffer_chain *url; - struct buffer_chain *csd[CSD_SURF_COUNT]; + struct buffer_chain *csd; } chains; /* Scheduled for rendering, as soon-as-possible */ diff --git a/wayland.c b/wayland.c index 38c80999..0679dee3 100644 --- a/wayland.c +++ b/wayland.c @@ -53,10 +53,9 @@ csd_destroy(struct wl_window *win) { struct terminal *term = win->term; - for (size_t i = 0; i < ALEN(win->csd.surface); i++) { + for (size_t i = 0; i < ALEN(win->csd.surface); i++) wayl_win_subsurface_destroy(&win->csd.surface[i]); - shm_purge(term->render.chains.csd[i]); - } + shm_purge(term->render.chains.csd); } static void @@ -1470,7 +1469,6 @@ wayl_win_destroy(struct wl_window *win) tll_foreach(win->urls, it) { wayl_win_subsurface_destroy(&it->item.surf); - shm_purge(term->render.chains.url); tll_remove(win->urls, it); } @@ -1483,9 +1481,8 @@ wayl_win_destroy(struct wl_window *win) shm_purge(term->render.chains.scrollback_indicator); shm_purge(term->render.chains.render_timer); shm_purge(term->render.chains.grid); - - for (size_t i = 0; i < ALEN(win->csd.surface); i++) - shm_purge(term->render.chains.csd[i]); + shm_purge(term->render.chains.url); + shm_purge(term->render.chains.csd); #if defined(HAVE_XDG_ACTIVATION) if (win->xdg_activation_token != NULL) From 318385040c4af386e944e009bec839a9161defb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 18 Jul 2021 17:16:17 +0200 Subject: [PATCH 52/52] changelog: remove entries that are no longer relevant The tweak.allow-overflowing-double-width-glyphs option have been superseded by tweak.overflowing-glyphs. --- CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e44d8a7..a25599fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,10 +56,6 @@ ### Fixed -* Glyph offset not being taken into account when applying - `tweak.allow-overflowing-double-width-glyphs`. -* Regression: crash when a single-char, double-width glyph is in the - last column, and `tweak.allow-overflowing-double-width-glyphs=yes`. * FD exhaustion when repeatedly entering/exiting URL mode with many URLs. * Double free of URL while removing duplicated and/or overlapping URLs