From 4fd26c251cd66029b8aab739ee2e77a6904b5bed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 11 Apr 2024 15:32:15 +0200 Subject: [PATCH 1/9] changelog: add a new 'unreleased' section --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e85730a7..175403dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +* [Unreleased](#unreleased) * [1.17.1](#1-17-1) * [1.17.0](#1-17-0) * [1.16.2](#1-16-2) @@ -50,6 +51,16 @@ * [1.2.0](#1-2-0) +## Unreleased +### Added +### Changed +### Deprecated +### Removed +### Fixed +### Security +### Contributors + + ## 1.17.1 ### Added From e753bb953bdf2a30d79d637aafa64cf3fdd2f7d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 12 Apr 2024 15:35:25 +0200 Subject: [PATCH 2/9] wayland: remove has_wl_compositor_v6 We deviate slightly from the specification, in that we don't assume a preferred buffer scale of 1. Instead, we "guess" the scale *until we receive a surface_preferred_buffer_scale event. Because of this, we don't need the has_wl_compositor_v6 member, as it's enough to check if we have a non-zero 'preferred buffer scale'. --- terminal.c | 2 +- wayland.c | 1 - wayland.h | 2 -- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/terminal.c b/terminal.c index 3fb918a4..deb9ba81 100644 --- a/terminal.c +++ b/terminal.c @@ -2181,7 +2181,7 @@ term_fractional_scaling(const struct terminal *term) bool term_preferred_buffer_scale(const struct terminal *term) { - return term->wl->has_wl_compositor_v6 && term->window->preferred_buffer_scale > 0; + return term->window->preferred_buffer_scale > 0; } bool diff --git a/wayland.c b/wayland.c index 1d9d3cdf..4add34e3 100644 --- a/wayland.c +++ b/wayland.c @@ -1096,7 +1096,6 @@ handle_global(void *data, struct wl_registry *registry, #if defined (WL_SURFACE_PREFERRED_BUFFER_SCALE_SINCE_VERSION) const uint32_t preferred = WL_SURFACE_PREFERRED_BUFFER_SCALE_SINCE_VERSION; - wayl->has_wl_compositor_v6 = version >= WL_SURFACE_PREFERRED_BUFFER_SCALE_SINCE_VERSION; #else const uint32_t preferred = required; #endif diff --git a/wayland.h b/wayland.h index 84fcbe48..575af1bb 100644 --- a/wayland.h +++ b/wayland.h @@ -430,8 +430,6 @@ struct wayland { struct wl_subcompositor *sub_compositor; struct wl_shm *shm; - bool has_wl_compositor_v6; - struct zxdg_output_manager_v1 *xdg_output_manager; struct xdg_wm_base *shell; From 23ada09d149140b7a38d376dd4fd7e2c3da589aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 15 Apr 2024 16:02:54 +0200 Subject: [PATCH 3/9] osc: reject notifications with invalid UTF-8 strings --- CHANGELOG.md | 4 ++++ osc.c | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 175403dc..0f3a0179 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,10 @@ ## Unreleased ### Added ### Changed + +* Notifications with invalid UTF-8 strings are now ignored. + + ### Deprecated ### Removed ### Fixed diff --git a/osc.c b/osc.c index 06cc7db6..446461a2 100644 --- a/osc.c +++ b/osc.c @@ -524,6 +524,19 @@ osc_notify(struct terminal *term, char *string) const char *title = strtok_r(string, ";", &ctx); const char *msg = strtok_r(NULL, "\x00", &ctx); + if (title == NULL) + return; + + if (mbsntoc32(NULL, title, strlen(title), 0) == (char32_t)-1) { + LOG_WARN("%s: notification title is not valid UTF-8, ignoring", title); + return; + } + + if (msg != NULL && mbsntoc32(NULL, msg, strlen(msg), 0) == (char32_t)-1) { + LOG_WARN("%s: notification message is not valid UTF-8, ignoring", msg); + return; + } + notify_notify(term, title, msg != NULL ? msg : ""); } From 71ce17d97795419c3fe5d28132e387f27e2ca958 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 15 Apr 2024 16:03:30 +0200 Subject: [PATCH 4/9] term: don't print outside grid when printing multi-column characters When auto-wrap is disabled, a multi-column character may be printed on a line that doesn't fit the entire character. That is, the "spacers" we print, as place holders in the columns after the first one, may reach outside the grid. We did (try to) check for this, but the check was off by one. Meaning, we could, in some cases, print outside the grid. --- CHANGELOG.md | 3 +++ terminal.c | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f3a0179..68b15356 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,9 @@ ### Deprecated ### Removed ### Fixed + +* Crash when printing double-width (or longer) characters to, or near, + the last column, when auto-wrap (private mode 7) has been disabled. ### Security ### Contributors diff --git a/terminal.c b/terminal.c index deb9ba81..e747009a 100644 --- a/terminal.c +++ b/terminal.c @@ -3680,11 +3680,13 @@ term_print(struct terminal *term, char32_t wc, int width) grid_row_uri_range_erase(row, col, col + width - 1); /* Advance cursor the 'additional' columns while dirty:ing the cells */ - for (int i = 1; i < width && col < term->cols; i++) { + for (int i = 1; i < width && (col + 1) < term->cols; i++) { col++; print_spacer(term, col, width - i); } + xassert(col < term->cols); + /* Advance cursor */ if (unlikely(++col >= term->cols)) { grid->cursor.lcf = true; @@ -3726,6 +3728,7 @@ ascii_printer_fast(struct terminal *term, char32_t wc) /* Advance cursor */ if (unlikely(++col >= term->cols)) { + xassert(col == term->cols); grid->cursor.lcf = true; col--; } else From 0ab05f480789c0ccd452ed4d7f678d3e4fa36874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 15 Apr 2024 16:05:56 +0200 Subject: [PATCH 5/9] sixel: also set 'alloc_height', when short-cutting a resize operation In some cases, a sixel may be resized vertically, while still having a zero-width. In this case, the resize operations are short-cutted, and no actual allocations are done. However, we forgot to set 'alloc_height' when doing so. As a result, the trimming code (when the sixel is "done"), trimmed away the entire sixel. --- CHANGELOG.md | 3 +++ sixel.c | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68b15356..cad31efd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,9 @@ * Crash when printing double-width (or longer) characters to, or near, the last column, when auto-wrap (private mode 7) has been disabled. +* Dynamically sized sixel being trimmed to nothing. + + ### Security ### Contributors diff --git a/sixel.c b/sixel.c index a7cbbe18..85be3608 100644 --- a/sixel.c +++ b/sixel.c @@ -1400,7 +1400,7 @@ resize_horizontally(struct terminal *term, int new_width_mutable) new_width_mutable = term->sixel.max_width; } - if (unlikely(term->sixel.image.width == new_width_mutable)) + if (unlikely(term->sixel.image.width >= new_width_mutable)) return; const int sixel_row_height = 6 * term->sixel.pan; @@ -1414,6 +1414,7 @@ resize_horizontally(struct terminal *term, int new_width_mutable) /* Lazy initialize height on first printed sixel */ xassert(old_width == 0); term->sixel.image.height = height = sixel_row_height; + term->sixel.image.alloc_height = sixel_row_height; } else height = term->sixel.image.height; @@ -1423,6 +1424,7 @@ resize_horizontally(struct terminal *term, int new_width_mutable) int alloc_height = (height + sixel_row_height - 1) / sixel_row_height * sixel_row_height; + xassert(new_width >= old_width); xassert(new_width > 0); xassert(alloc_height > 0); @@ -1474,6 +1476,7 @@ resize_vertically(struct terminal *term, const int new_height) if (unlikely(width == 0)) { xassert(term->sixel.image.data == NULL); term->sixel.image.height = new_height; + term->sixel.image.alloc_height = alloc_height; return true; } @@ -1508,7 +1511,7 @@ resize(struct terminal *term, int new_width_mutable, int new_height_mutable) { LOG_DBG("resizing image: %dx%d -> %dx%d", term->sixel.image.width, term->sixel.image.height, - new_width, new_height); + new_width_mutable, new_height_mutable); if (unlikely(new_width_mutable > term->sixel.max_width)) { LOG_WARN("maximum image width exceeded, truncating"); From 3d2588edf8a90a7ba74d75089193a52b69905a4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 15 Apr 2024 16:07:47 +0200 Subject: [PATCH 6/9] sixel: don't allow pan/pad changes after sixel data has been emitted Changing pan/pad changes the sixel's aspect ratio. While I don't know for certain what a real VT340 would do, I suspect it would change the aspect ratio of all subsequent sixels, but not those already emitted. The way we implement sixels in foot, makes this behavior hard to implement. We currently don't resize the image properly if the aspect ratio is changed, but not the RA area. We have code that assumes all sixel lines have the same aspect ratio, etc. Since no "normal" applications change the aspect ratio in the middle of a sixel, simply disallow it, and print a warning. This also fixes a crash, when writing sixels after having modified the aspect ratio. --- CHANGELOG.md | 3 +++ sixel.c | 26 +++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cad31efd..f4276f0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,9 @@ ### Removed ### Fixed +* Crash when changing aspect ratio of a sixel, in the middle of the + sixel data (this is unsupported in foot, but should of course not + result in a crash). * Crash when printing double-width (or longer) characters to, or near, the last column, when auto-wrap (private mode 7) has been disabled. * Dynamically sized sixel being trimmed to nothing. diff --git a/sixel.c b/sixel.c index 85be3608..d6bfa3a4 100644 --- a/sixel.c +++ b/sixel.c @@ -1852,12 +1852,32 @@ decgra(struct terminal *term, uint8_t c) pan = pan > 0 ? pan : 1; pad = pad > 0 ? pad : 1; + if (likely(term->sixel.image.width == 0 && + term->sixel.image.height == 0)) + { + term->sixel.pan = pan; + term->sixel.pad = pad; + } else { + /* + * Unsure what the VT340 does... + * + * We currently do *not* handle changing pan/pad in the + * middle of a sixel, since that means resizing/stretching + * the existing image. + * + * I'm *guessing* the VT340 simply changes the aspect + * ratio of all subsequent sixels. But, given the design + * of our implementation (the entire sixel is written to a + * single pixman image), we can't easily do that. + */ + LOG_WARN("sixel: unsupported: pan/pad changed after printing sixels"); + pan = term->sixel.pan; + pad = term->sixel.pad; + } + pv *= pan; ph *= pad; - term->sixel.pan = pan; - term->sixel.pad = pad; - LOG_DBG("pan=%u, pad=%u (aspect ratio = %d:%d), size=%ux%u", pan, pad, pan, pad, ph, pv); From 3507c724921c5a886336fa481eca30f1bb824064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 17 Apr 2024 08:52:31 +0200 Subject: [PATCH 7/9] ci: explicitly install openssl Let's see if this fixes the missing SSL module in Python... --- .woodpecker.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.woodpecker.yaml b/.woodpecker.yaml index 063390be..c078584b 100644 --- a/.woodpecker.yaml +++ b/.woodpecker.yaml @@ -1,3 +1,5 @@ +# -*- yaml -*- + steps: - name: codespell when: @@ -6,6 +8,7 @@ steps: - releases/* image: alpine:edge commands: + - apk add openssl - apk add python3 - apk add py3-pip - python3 -m venv codespell-venv From a5b369ede4fbb74f4e3ef8352a52974deeeeba86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 17 Apr 2024 08:59:27 +0200 Subject: [PATCH 8/9] ci: set explicit 'event' filters an all 'when'-statements This should fix the CI warnings we've started to see lately: [bad_habit] Please set an event filter on all when branches --- .woodpecker.yaml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.woodpecker.yaml b/.woodpecker.yaml index c078584b..9b121f2a 100644 --- a/.woodpecker.yaml +++ b/.woodpecker.yaml @@ -3,9 +3,9 @@ steps: - name: codespell when: - branch: - - master - - releases/* + - event: [manual, pull_request] + - event: [push, tag] + branch: [master, releases/*] image: alpine:edge commands: - apk add openssl @@ -19,9 +19,9 @@ steps: - name: subprojects when: - branch: - - master - - releases/* + - event: [manual, pull_request] + - event: [push, tag] + branch: [master, releases/*] image: alpine:edge commands: - apk add git @@ -32,9 +32,9 @@ steps: - name: x64 when: - branch: - - master - - releases/* + - event: [manual, pull_request] + - event: [push, tag] + branch: [master, releases/*] depends_on: [subprojects] image: alpine:edge commands: @@ -89,9 +89,9 @@ steps: - name: x86 when: - branch: - - master - - releases/* + - event: [manual, pull_request] + - event: [push, tag] + branch: [master, releases/*] depends_on: [subprojects] image: i386/alpine:edge commands: From 7c20fb247c977fe545b1ef0ce9d8935b0424bf70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 17 Apr 2024 08:35:58 +0200 Subject: [PATCH 9/9] term: stash last known DPI, and use after a unmapped/mapped sequence A compositor may unmap, and then remap the window, for example when the window is minimized, or if the user switches workspace. With DPI aware rendering, we *need* to know on which output we're mapped, in order to use the correct DPI. This means the first frame we render, before being mapped, always guesses the DPI. In an unmap/map sequence, guessing the wrong DPI means the window will flicker. Fix by stashing the last used DPI value, and use that instead of guessing. This means the *only* time we _actually_ guess the DPI, is the very first frame, when starting up foot. --- CHANGELOG.md | 3 +++ terminal.c | 24 +++++++++++++++++++++--- terminal.h | 1 + 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4276f0a..b4384f0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,9 @@ * Crash when printing double-width (or longer) characters to, or near, the last column, when auto-wrap (private mode 7) has been disabled. * Dynamically sized sixel being trimmed to nothing. +* Flickering with `dpi-aware=yes` and window is unmapped/remapped + (some compositors do this when window is minimized), in a + multi-monitor setup with different monitor DPIs. ### Security diff --git a/terminal.c b/terminal.c index e747009a..c027cfac 100644 --- a/terminal.c +++ b/terminal.c @@ -858,9 +858,25 @@ get_font_dpi(const struct terminal *term) xassert(tll_length(term->wl->monitors) > 0); const struct wl_window *win = term->window; - const struct monitor *mon = tll_length(win->on_outputs) > 0 - ? tll_back(win->on_outputs) - : &tll_front(term->wl->monitors); + const struct monitor *mon = NULL; + + if (tll_length(win->on_outputs) > 0) + mon = tll_back(win->on_outputs); + else { + if (term->font_dpi_before_unmap > 0.) { + /* + * Use last known "good" DPI + * + * This avoids flickering when window is unmapped/mapped + * (some compositors do this when a window is minimized), + * on a multi-monitor setup with different monitor DPIs. + */ + return term->font_dpi_before_unmap; + } + + if (tll_length(term->wl->monitors) > 0) + mon = &tll_front(term->wl->monitors); + } if (term_fractional_scaling(term)) return mon != NULL ? mon->dpi.physical : 96.; @@ -1182,6 +1198,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, xmalloc(sizeof(term->font_sizes[3][0]) * conf->fonts[3].count), }, .font_dpi = 0., + .font_dpi_before_unmap = -1., .font_subpixel = (conf->colors.alpha == 0xffff /* Can't do subpixel rendering on transparent background */ ? FCFT_SUBPIXEL_DEFAULT : FCFT_SUBPIXEL_NONE), @@ -2250,6 +2267,7 @@ term_font_dpi_changed(struct terminal *term, float old_scale) } term->font_dpi = dpi; + term->font_dpi_before_unmap = dpi; term->font_is_sized_by_dpi = will_scale_using_dpi; if (!need_font_reload) diff --git a/terminal.h b/terminal.h index fa80e693..2a0845ef 100644 --- a/terminal.h +++ b/terminal.h @@ -406,6 +406,7 @@ struct terminal { struct config_font *font_sizes[4]; struct pt_or_px font_line_height; float font_dpi; + float font_dpi_before_unmap; bool font_is_sized_by_dpi; int16_t font_x_ofs; int16_t font_y_ofs;