From 157b64098a0832d66b5f8d206a14aa2c1342334b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 7 Aug 2022 09:38:25 +0200 Subject: [PATCH 01/17] =?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 ca337533..da7f779f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +* [Unreleased](#unreleased) * [1.13.0](#1-13-0) * [1.12.1](#1-12-1) * [1.12.0](#1-12-0) @@ -39,6 +40,16 @@ * [1.2.0](#1-2-0) +## Unreleased +### Added +### Changed +### Deprecated +### Removed +### Fixed +### Security +### Contributors + + ## 1.13.0 ### Added From 1cf22846a0e333c61d5db50c260fd897143c6a83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 8 Aug 2022 16:31:28 +0200 Subject: [PATCH 02/17] render: apply a dimmed overlay while in Unicode input mode --- CHANGELOG.md | 4 ++++ render.c | 33 ++++++++++++++++++++++++++++----- terminal.h | 7 ++++--- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da7f779f..e914e4d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,10 @@ ## Unreleased ### Added ### Changed + +* Window is now dimmed while in Unicode input mode. + + ### Deprecated ### Removed ### Fixed diff --git a/render.c b/render.c index ff82802e..82c9bd13 100644 --- a/render.c +++ b/render.c @@ -1466,10 +1466,21 @@ static void render_overlay(struct terminal *term) { struct wl_surf_subsurf *overlay = &term->window->overlay; + bool unicode_mode_active = false; + + /* Check if unicode mode is active on at least one seat focusing + * this terminal instance */ + tll_foreach(term->wl->seats, it) { + if (it->item.unicode_mode.active) { + unicode_mode_active = true; + break; + } + } const enum overlay_style style = term->is_searching ? OVERLAY_SEARCH : term->flash.active ? OVERLAY_FLASH : + unicode_mode_active ? OVERLAY_UNICODE_MODE : OVERLAY_NONE; if (likely(style == OVERLAY_NONE)) { @@ -1488,9 +1499,21 @@ render_overlay(struct terminal *term) pixman_image_set_clip_region32(buf->pix[0], NULL); - pixman_color_t color = style == OVERLAY_SEARCH - ? (pixman_color_t){0, 0, 0, 0x7fff} - : (pixman_color_t){.red=0x7fff, .green=0x7fff, .blue=0, .alpha=0x7fff}; + pixman_color_t color; + + switch (style) { + case OVERLAY_NONE: + break; + + case OVERLAY_SEARCH: + case OVERLAY_UNICODE_MODE: + color = (pixman_color_t){0, 0, 0, 0x7fff}; + break; + + case OVERLAY_FLASH: + color = (pixman_color_t){.red=0x7fff, .green=0x7fff, .blue=0, .alpha=0x7fff}; + break; + } /* Bounding rectangle of damaged areas - for wl_surface_damage_buffer() */ pixman_box32_t damage_bounds; @@ -1517,7 +1540,7 @@ render_overlay(struct terminal *term) * region that needs to be *cleared* in this frame. * * Finally, the union of the two “diff” regions above, gives - * us the total region affecte by a change, in either way. We + * us the total region affected by a change, in either way. We * use this as the bounding box for the * wl_surface_damage_buffer() call. */ @@ -1605,7 +1628,7 @@ render_overlay(struct terminal *term) else if (buf == term->render.last_overlay_buf && style == term->render.last_overlay_style) { - xassert(style == OVERLAY_FLASH); + xassert(style == OVERLAY_FLASH || style == OVERLAY_UNICODE_MODE); shm_did_not_use_buf(buf); return; } else { diff --git a/terminal.h b/terminal.h index bf6e74fe..0dde6330 100644 --- a/terminal.h +++ b/terminal.h @@ -289,9 +289,10 @@ enum term_surface { }; enum overlay_style { - OVERLAY_NONE = 0, - OVERLAY_SEARCH = 1, - OVERLAY_FLASH = 2, + OVERLAY_NONE, + OVERLAY_SEARCH, + OVERLAY_FLASH, + OVERLAY_UNICODE_MODE, }; typedef tll(struct ptmx_buffer) ptmx_buffer_list_t; From 5697348b461f4dec4daf67c62336bb357149baf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 12 Aug 2022 16:12:36 +0200 Subject: [PATCH 03/17] wayland: #ifdef on XDG_TOPLEVEL_CONFIGURE_BOUNDS_SINCE_VERSION This enables us to compile against wayland-protocols < 1.25 --- CHANGELOG.md | 4 ++++ wayland.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e914e4d3..4c10be50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,10 @@ ### Deprecated ### Removed ### Fixed + +* Compiling against wayland-protocols < 1.25 + + ### Security ### Contributors diff --git a/wayland.c b/wayland.c index fb41cf7d..05de79aa 100644 --- a/wayland.c +++ b/wayland.c @@ -706,6 +706,7 @@ xdg_toplevel_close(void *data, struct xdg_toplevel *xdg_toplevel) term_shutdown(term); } +#if defined(XDG_TOPLEVEL_CONFIGURE_BOUNDS_SINCE_VERSION) static void xdg_toplevel_configure_bounds(void *data, struct xdg_toplevel *xdg_toplevel, @@ -713,6 +714,7 @@ xdg_toplevel_configure_bounds(void *data, { /* TODO: ensure we don't pick a bigger size */ } +#endif #if defined(XDG_TOPLEVEL_WM_CAPABILITIES_SINCE_VERSION) static void @@ -742,7 +744,9 @@ xdg_toplevel_wm_capabilities(void *data, static const struct xdg_toplevel_listener xdg_toplevel_listener = { .configure = &xdg_toplevel_configure, /*.close = */&xdg_toplevel_close, /* epoll-shim defines a macro ‘close’... */ +#if defined(XDG_TOPLEVEL_CONFIGURE_BOUNDS_SINCE_VERSION) .configure_bounds = &xdg_toplevel_configure_bounds, +#endif #if defined(XDG_TOPLEVEL_WM_CAPABILITIES_SINCE_VERSION) .wm_capabilities = xdg_toplevel_wm_capabilities, #endif From 20b8ca1601a8a80770110860ae70d4487c95081b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 12 Aug 2022 16:13:25 +0200 Subject: [PATCH 04/17] input: ignore pointer motion events on unknown surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some cases, the compositor sends a pointer enter event with a NULL surface. It’s unclear if this is a compositor bug, or a race (where the compositor sends an enter event on a CSD surface at the same time foot unmaps the CSDs). Regardless, this causes seat->mouse_focus to be unset, which triggers a crash in foot on the next pointer motion event. This patch does two things: a) log a warning when we receive a pointer event with a NULL surface b) ignore motion events where seat->mouse_focus is NULL --- CHANGELOG.md | 3 +++ input.c | 16 ++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c10be50..5fa35a63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ ### Fixed * Compiling against wayland-protocols < 1.25 +* Crash on buggy compositors (GNOME) that sometimes send pointer-enter + events with a NULL surface. Foot now ignores these events, and the + subsequent motion and leave events. ### Security diff --git a/input.c b/input.c index 847ff6af..50336679 100644 --- a/input.c +++ b/input.c @@ -1709,11 +1709,9 @@ wl_pointer_enter(void *data, struct wl_pointer *wl_pointer, uint32_t serial, struct wl_surface *surface, wl_fixed_t surface_x, wl_fixed_t surface_y) { - xassert(surface != NULL); - xassert(serial != 0); - - if (surface == NULL) { + if (unlikely(surface == NULL)) { /* Seen on mutter-3.38 */ + LOG_WARN("compositor sent pointer_enter event with a NULL surface"); return; } @@ -1866,6 +1864,16 @@ wl_pointer_motion(void *data, struct wl_pointer *wl_pointer, struct seat *seat = data; struct wayland *wayl = seat->wayl; struct terminal *term = seat->mouse_focus; + + if (unlikely(term == NULL)) { + /* Typically happens when the compositor sent a pointer enter + * event with a NULL surface - see wl_pointer_enter(). + * + * In this case, we never set seat->mouse_focus (since we + * can’t map the enter event to a specific window). */ + return; + } + struct wl_window *win = term->window; LOG_DBG("pointer_motion: pointer=%p, x=%d, y=%d", (void *)wl_pointer, From ccef435736a5364c63cfe6c1d64e7c4f5a66a0b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 20 Aug 2022 18:25:05 +0200 Subject: [PATCH 05/17] =?UTF-8?q?ci:=20codespell:=20ignore=20=E2=80=98zar?= =?UTF-8?q?=E2=80=99=20(user=20who=20contributed)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .builds/alpine-x64.yml | 2 +- .gitlab-ci.yml | 2 +- .woodpecker.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.builds/alpine-x64.yml b/.builds/alpine-x64.yml index 8f341f3d..933c7121 100644 --- a/.builds/alpine-x64.yml +++ b/.builds/alpine-x64.yml @@ -49,4 +49,4 @@ tasks: - codespell: | pip install codespell cd foot - ~/.local/bin/codespell -Lser,doas README.md INSTALL.md CHANGELOG.md *.c *.h doc/*.scd + ~/.local/bin/codespell -Lser,doas,zar README.md INSTALL.md CHANGELOG.md *.c *.h doc/*.scd diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b2a459dc..28df1ccb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -109,4 +109,4 @@ codespell: - apk add python3 - apk add py3-pip - pip install codespell - - codespell -Lser,doas README.md INSTALL.md CHANGELOG.md *.c *.h doc/*.scd + - codespell -Lser,doas,zar README.md INSTALL.md CHANGELOG.md *.c *.h doc/*.scd diff --git a/.woodpecker.yml b/.woodpecker.yml index 8493aa47..284da761 100644 --- a/.woodpecker.yml +++ b/.woodpecker.yml @@ -9,7 +9,7 @@ pipeline: - apk add python3 - apk add py3-pip - pip install codespell - - codespell -Lser,doas README.md INSTALL.md CHANGELOG.md *.c *.h doc/*.scd + - codespell -Lser,doas,zar README.md INSTALL.md CHANGELOG.md *.c *.h doc/*.scd subprojects: when: From 736babcdbc10a20e9d620638adc2fec230a81e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 17 Aug 2022 17:36:10 +0200 Subject: [PATCH 06/17] selection: never highlight selected, empty cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes a regression, where empty cells "between" non-empty cells (i.e. non-trailing empty cells) sometimes were incorrectly highlighted. The idea has always been to highlight exactly those cells that will get extracted when they’re copied. This means we’ve not highlighted trailing empty cells, but we _have_ highlighted other empty cells, since they are converted to spaces when copied (whereas trailing empty cells are skipped). fa2d9f86996467ba33cc381f810ea966a4323381 changed how a selection is updated. That is, which cells gets marked as selected, and which ones gets unmarked. Since we no longer walk all the cells, but instead work with pixman regions representing selection diffs, we can no longer determine (with certainty) which empty cells should be selected and which shouldn’t. Before this patch (but after fa2d9f86996467ba33cc381f810ea966a4323381), we sometimes incorrectly highlighted empty cells that should not have been highlighted. This happened when we’ve first (correctly) highlighted a region of empty cells, but then shrink the selection such that all those empty cells should be de-selected. This patch changes the selection behavior to *never* highlight empty cells. This fixes the regression, but also means slightly different behavior, compared to pre-fa2d9f86996467ba33cc381f810ea966a4323381. The other alternative is to always highlight all empty cells. But, since I personally like the fact that we’re skipping trailing empty cells, I prefer the approach taken by this patch. --- CHANGELOG.md | 4 ++++ selection.c | 23 +++++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fa35a63..be617671 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ ### Changed * Window is now dimmed while in Unicode input mode. +* Selected empty cells are **never** highlighted as being + selected. They used to be, when followed by non-empty cells. ### Deprecated @@ -55,6 +57,8 @@ * Crash on buggy compositors (GNOME) that sometimes send pointer-enter events with a NULL surface. Foot now ignores these events, and the subsequent motion and leave events. +* Regression: “random” selected empty cells being highlighted as + selected when they should not. ### Security diff --git a/selection.c b/selection.c index 8c3a8357..2fd62f8a 100644 --- a/selection.c +++ b/selection.c @@ -737,8 +737,27 @@ mark_selected_region(struct terminal *term, pixman_box32_t *boxes, row->dirty = true; for (int c = box->x1, empty_count = 0; c < box->x2; c++) { - if (selected && row->cells[c].wc == 0) { - empty_count++; + if (row->cells[c].wc == 0) { + /* + * We used to highlight empty cells *if* they were + * followed by non-empty cell(s), since this + * corresponds to what gets extracted when the + * selection is copied (that is, empty cells + * “between” non-empty cells are converted to + * spaces). + * + * However, they way we handle selection updates + * (diffing the “old” selection area against the + * “new” one, using pixman regions), means we + * can’t correctly update the state of empty + * cells. The result is “random” empty cells being + * rendered as selected when they shouldn’t. + * + * “Fix” by *never* highlighting selected empty + * cells (they still get converted to spaces when + * copied, if followed by non-empty cells). + */ + /* empty_count++; */ continue; } From 6f1cf6fc56b906fb050009f587a5440188a69b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 22 Aug 2022 20:14:06 +0200 Subject: [PATCH 07/17] selection: once again highlight non-trailing empty cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Foot 1.13.0 introduced a regression where non-trailing empty cells were highlighted inconsistently (cells that shouldn’t be highlighted, were, seemingly at random). 86663522d5915c0fdb2b8b132bacc247fcfd1fb8 “fixed” this by never highlighting *any* empty cells. This meant the behavior, compared to foot 1.12 and earlier, changed. In foot 1.12 and older versions, non-trailing empty cells were highlighted, as long as the selection covered at least one of the trailing non-empty cells. This patch restores that behavior. To understand how this works, lets first take a look at how selection works: When a selection is made, and updated (i.e. the mouse is dragged, or the selection is extended through RMB etc), we need to (un)tag and dirty the cells that are a) newly selected, or b) newly deselected. That is, we look at the diff between the “old” and the “new” selection, and only update those cells. This is for performance reasons: iterating the entire selection is not feasible with large selections. However, it also means we cannot reason about empty cells; we simply don’t know if an empty cells is a trailing empty cell, or a non-trailing one. Then, when we render a frame, we iterate all the *visible* and *selected* cells, once again tagging them as selected (this is needed since a selected cell might have lost its selected tag if the cell was written to, by the client application, after the selection was made). At this point, we *can* reason about empty cells. So, to restore the highlighting behavior to that of foot 1.12, we do this: When working with the selection diffs when a selection is updated, we don’t special case empty cells at all. Thus, all empty cells covered by the selection is highlighted, and dirtied. But, when rendering the frame, we _do_ special case them. The only difference (compared to foot 1.12) is that we *must* explicitly *clear* the selection tag, and dirty the empty cells. This is to ensure the empty cells that were incorrectly highlighted by the selection update algorithm, isn’t rendered as that. This does have a slight performance impact, as empty cells are now always re-rendered. The impact should however be small. --- CHANGELOG.md | 2 -- selection.c | 44 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be617671..0ba3aa6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,8 +45,6 @@ ### Changed * Window is now dimmed while in Unicode input mode. -* Selected empty cells are **never** highlighted as being - selected. They used to be, when followed by non-empty cells. ### Deprecated diff --git a/selection.c b/selection.c index 2fd62f8a..5b65310c 100644 --- a/selection.c +++ b/selection.c @@ -713,7 +713,8 @@ pixman_region_for_coords(const struct terminal *term, static void mark_selected_region(struct terminal *term, pixman_box32_t *boxes, - size_t count, bool selected, bool dirty_cells) + size_t count, bool selected, bool dirty_cells, + bool highlight_empty) { for (size_t i = 0; i < count; i++) { const pixman_box32_t *box = &boxes[i]; @@ -737,7 +738,9 @@ mark_selected_region(struct terminal *term, pixman_box32_t *boxes, row->dirty = true; for (int c = box->x1, empty_count = 0; c < box->x2; c++) { - if (row->cells[c].wc == 0) { + struct cell *cell = &row->cells[c]; + + if (cell->wc == 0 && !highlight_empty) { /* * We used to highlight empty cells *if* they were * followed by non-empty cell(s), since this @@ -757,7 +760,36 @@ mark_selected_region(struct terminal *term, pixman_box32_t *boxes, * cells (they still get converted to spaces when * copied, if followed by non-empty cells). */ - /* empty_count++; */ + empty_count++; + + /* + * When the selection is *modified*, empty cells + * are treated just like non-empty cells; they are + * marked as selected, and dirtied. + * + * This is due to how the algorithm for updating + * the selection works; it uses regions to + * calculate the difference between the “old” and + * the “new” selection. This makes it impossible + * to tell if an empty cell is a *trailing* empty + * cell (that should not be highlighted), or an + * empty cells between non-empty cells (that + * *should* be highlighted). + * + * Then, when a frame is rendered, we loop the + * *visibible* cells that belong to the + * selection. At this point, we *can* tell if an + * empty cell is trailing or not. + * + * So, what we need to do is check if a + * ‘selected’, and empty cell has been marked as + * selected, temporarily unmark (forcing it dirty, + * to ensure it gets re-rendered). If it is *not* + * a trailing empty cell, it will get re-tagged as + * selected in the for-loop below. + */ + cell->attrs.clean = false; + cell->attrs.selected = false; continue; } @@ -810,10 +842,10 @@ selection_modify(struct terminal *term, struct coord start, struct coord end) pixman_box32_t *boxes = NULL; boxes = pixman_region32_rectangles(&no_longer_selected, &n_rects); - mark_selected_region(term, boxes, n_rects, false, true); + mark_selected_region(term, boxes, n_rects, false, true, true); boxes = pixman_region32_rectangles(&newly_selected, &n_rects); - mark_selected_region(term, boxes, n_rects, true, true); + mark_selected_region(term, boxes, n_rects, true, true, true); pixman_region32_fini(&newly_selected); pixman_region32_fini(&no_longer_selected); @@ -1078,7 +1110,7 @@ selection_dirty_cells(struct terminal *term) int n_rects = -1; pixman_box32_t *boxes = pixman_region32_rectangles(&visible_and_selected, &n_rects); - mark_selected_region(term, boxes, n_rects, true, false); + mark_selected_region(term, boxes, n_rects, true, false, false); pixman_region32_fini(&visible_and_selected); pixman_region32_fini(&view); From 03d3887e6bb964e9599ce84f5e1548df224371fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Aug 2022 16:38:39 +0200 Subject: [PATCH 08/17] ci (sr.ht): pull directly from git.sr.ht --- .builds/alpine-x64.yml | 2 +- .builds/alpine-x86.yml.disabled | 2 +- .builds/freebsd-x64.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.builds/alpine-x64.yml b/.builds/alpine-x64.yml index 933c7121..2e2ec2c4 100644 --- a/.builds/alpine-x64.yml +++ b/.builds/alpine-x64.yml @@ -24,7 +24,7 @@ packages: - font-noto-emoji sources: - - https://codeberg.org/dnkl/foot + - https://git.sr.ht/~dnkl/foot # triggers: # - action: email diff --git a/.builds/alpine-x86.yml.disabled b/.builds/alpine-x86.yml.disabled index 22a9e637..6d790227 100644 --- a/.builds/alpine-x86.yml.disabled +++ b/.builds/alpine-x86.yml.disabled @@ -23,7 +23,7 @@ packages: - font-noto-emoji sources: - - https://codeberg.org/dnkl/foot + - https://git.sr.ht/~dnkl/foot # triggers: # - action: email diff --git a/.builds/freebsd-x64.yml b/.builds/freebsd-x64.yml index 89803a6e..9642f96d 100644 --- a/.builds/freebsd-x64.yml +++ b/.builds/freebsd-x64.yml @@ -19,7 +19,7 @@ packages: - noto-emoji sources: - - https://codeberg.org/dnkl/foot + - https://git.sr.ht/~dnkl/foot # triggers: # - action: email From 2354298ecac8e08ecce8ff8b208cda0f0c6f5257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Aug 2022 17:48:00 +0200 Subject: [PATCH 09/17] selection: restore <= 1.12 behavior in block selection wrt empty cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That is, highlight empty cells, regardless of whether they’re trailing or not. --- selection.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/selection.c b/selection.c index 5b65310c..5bf72e62 100644 --- a/selection.c +++ b/selection.c @@ -1110,7 +1110,9 @@ selection_dirty_cells(struct terminal *term) int n_rects = -1; pixman_box32_t *boxes = pixman_region32_rectangles(&visible_and_selected, &n_rects); - mark_selected_region(term, boxes, n_rects, true, false, false); + + const bool highlight_empty = term->selection.kind == SELECTION_BLOCK; + mark_selected_region(term, boxes, n_rects, true, false, highlight_empty); pixman_region32_fini(&visible_and_selected); pixman_region32_fini(&view); From d810e4fc8eb3123e427f1453b269ced9608222c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Aug 2022 21:07:20 +0200 Subject: [PATCH 10/17] selection: mark_selected_region(): use an enum to encode how the cells are to be updated --- selection.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/selection.c b/selection.c index 5bf72e62..c94686e1 100644 --- a/selection.c +++ b/selection.c @@ -711,11 +711,26 @@ pixman_region_for_coords(const struct terminal *term, } } +enum mark_selection_variant { + MARK_SELECTION_MARK_AND_DIRTY, + MARK_SELECTION_UNMARK_AND_DIRTY, + MARK_SELECTION_MARK_FOR_RENDER, +}; + static void mark_selected_region(struct terminal *term, pixman_box32_t *boxes, - size_t count, bool selected, bool dirty_cells, - bool highlight_empty) + size_t count, enum mark_selection_variant mark_variant) { + const bool selected = + mark_variant == MARK_SELECTION_MARK_AND_DIRTY || + mark_variant == MARK_SELECTION_MARK_FOR_RENDER; + const bool dirty_cells = + mark_variant == MARK_SELECTION_MARK_AND_DIRTY || + mark_variant == MARK_SELECTION_UNMARK_AND_DIRTY; + const bool highlight_empty = + mark_variant != MARK_SELECTION_MARK_FOR_RENDER || + term->selection.kind == SELECTION_BLOCK; + for (size_t i = 0; i < count; i++) { const pixman_box32_t *box = &boxes[i]; @@ -842,10 +857,10 @@ selection_modify(struct terminal *term, struct coord start, struct coord end) pixman_box32_t *boxes = NULL; boxes = pixman_region32_rectangles(&no_longer_selected, &n_rects); - mark_selected_region(term, boxes, n_rects, false, true, true); + mark_selected_region(term, boxes, n_rects, MARK_SELECTION_UNMARK_AND_DIRTY); boxes = pixman_region32_rectangles(&newly_selected, &n_rects); - mark_selected_region(term, boxes, n_rects, true, true, true); + mark_selected_region(term, boxes, n_rects, MARK_SELECTION_MARK_AND_DIRTY); pixman_region32_fini(&newly_selected); pixman_region32_fini(&no_longer_selected); @@ -1110,9 +1125,7 @@ selection_dirty_cells(struct terminal *term) int n_rects = -1; pixman_box32_t *boxes = pixman_region32_rectangles(&visible_and_selected, &n_rects); - - const bool highlight_empty = term->selection.kind == SELECTION_BLOCK; - mark_selected_region(term, boxes, n_rects, true, false, highlight_empty); + mark_selected_region(term, boxes, n_rects, MARK_SELECTION_MARK_FOR_RENDER); pixman_region32_fini(&visible_and_selected); pixman_region32_fini(&view); From 649d56a4e5142c9ab8ea24f4d1acbd4a49d534f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 29 Aug 2022 20:46:19 +0200 Subject: [PATCH 11/17] =?UTF-8?q?grid:=20when=20setting=20the=20new=20view?= =?UTF-8?q?port,=20ensure=20it=E2=80=99s=20correctly=20bounded?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do this by using scrollback relative coordinates, and ensure the new viewport is not larger than (grid_rows - screen_rows), as that would mean the viewport crosses the scrollback wrap-around. --- grid.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/grid.c b/grid.c index f40803e1..f229effa 100644 --- a/grid.c +++ b/grid.c @@ -1016,23 +1016,6 @@ grid_resize_and_reflow( new_grid[idx] = grid_row_alloc(new_cols, true); } - grid->view = view_follows ? grid->offset : viewport.row; - - /* If enlarging the window, the old viewport may be too far down, - * with unallocated rows. Make sure this cannot happen */ - while (true) { - int idx = (grid->view + new_screen_rows - 1) & (new_rows - 1); - if (new_grid[idx] != NULL) - break; - grid->view--; - if (grid->view < 0) - grid->view += new_rows; - } - for (size_t r = 0; r < new_screen_rows; r++) { - int UNUSED idx = (grid->view + r) & (new_rows - 1); - xassert(new_grid[idx] != NULL); - } - /* Free old grid (rows already free:d) */ free(grid->rows); @@ -1040,6 +1023,17 @@ grid_resize_and_reflow( grid->num_rows = new_rows; grid->num_cols = new_cols; + /* + * Set new viewport, making sure it’s not too far down. + * + * This is done by using scrollback-start relative cooardinates, + * and bounding the new viewport to (grid_rows - screen_rows). + */ + int sb_view = grid_row_abs_to_sb( + grid, new_screen_rows, view_follows ? grid->offset : viewport.row); + grid->view = grid_row_sb_to_abs( + grid, new_screen_rows, min(sb_view, new_rows - new_screen_rows)); + /* Convert absolute coordinates to screen relative */ cursor.row -= grid->offset; while (cursor.row < 0) From 22989ef9b3c604f5ccc30a6ba3cb11263f7e1a80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 29 Aug 2022 20:47:33 +0200 Subject: [PATCH 12/17] =?UTF-8?q?grid:=20reflow:=20don=E2=80=99t=20line-wr?= =?UTF-8?q?ap=20the=20last=20row?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this patch, we would line-wrap the last row, just like any other row, and then afterwards try to reverse this, by adjusting the offset and free:ing and NULL:ing the "last row". The problem with this is if the scrollback is full. In this case, the row we’re freeing is the first row in the scrollback history. This means we’ll crash as soon as the viewport is moved to the top of the scrollback. The fix is fairly, simple. Skip the post-processing logic, and instead detect when we’re line-wrapping the last row, and skip the call to line_wrap(). This way, the last row in the new grid corresponds to the last row in the old grid. --- grid.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/grid.c b/grid.c index f229effa..bbac3395 100644 --- a/grid.c +++ b/grid.c @@ -935,13 +935,14 @@ grid_resize_and_reflow( start += cols; } - if (old_row->linebreak) { /* Erase the remaining cells */ memset(&new_row->cells[new_col_idx], 0, (new_cols - new_col_idx) * sizeof(new_row->cells[0])); new_row->linebreak = true; - line_wrap(); + + if (r + 1 < old_rows) + line_wrap(); } grid_row_free(old_grid[old_row_idx]); @@ -985,25 +986,6 @@ grid_resize_and_reflow( /* Set offset such that the last reflowed row is at the bottom */ grid->offset = new_row_idx - new_screen_rows + 1; - if (new_col_idx == 0) { - int next_to_last_new_row_idx = new_row_idx - 1; - next_to_last_new_row_idx += new_rows; - next_to_last_new_row_idx &= new_rows - 1; - - const struct row *next_to_last_row = new_grid[next_to_last_new_row_idx]; - if (next_to_last_row != NULL && next_to_last_row->linebreak) { - /* - * The next to last row is actually the *last* row. But we - * ended the reflow with a line-break, causing an empty - * row to be inserted at the bottom. Undo this. - */ - /* TODO: can we detect this in the reflow loop above instead? */ - grid->offset--; - grid_row_free(new_grid[new_row_idx]); - new_grid[new_row_idx] = NULL; - } - } - while (grid->offset < 0) grid->offset += new_rows; while (new_grid[grid->offset] == NULL) From 23871d4db58d15e33e0693010373c426fc50688e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 29 Aug 2022 21:03:21 +0200 Subject: [PATCH 13/17] =?UTF-8?q?grid:=20reflow:=20assert=20there=20aren?= =?UTF-8?q?=E2=80=99t=20any=20open=20URIs=20on=20the=20last=20row?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- grid.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/grid.c b/grid.c index bbac3395..7bfef5cb 100644 --- a/grid.c +++ b/grid.c @@ -943,6 +943,21 @@ grid_resize_and_reflow( if (r + 1 < old_rows) line_wrap(); + else if (new_row->extra != NULL && + new_row->extra->uri_ranges.count > 0) + { + /* + * line_wrap() "closes" still-open URIs. Since this is + * the *last* row, and since we’re line-breaking due + * to a hard line-break (rather than running out of + * cells in the "new_row"), there shouldn’t be an open + * URI (it would have been closed when we reached the + * end of the URI while reflowing the last "old" + * row). + */ + uint32_t last_idx = new_row->extra->uri_ranges.count - 1; + xassert(new_row->extra->uri_ranges.v[last_idx].end >= 0); + } } grid_row_free(old_grid[old_row_idx]); From 688bf1bd59df3c692b0891aaf46b6844ea415e65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 29 Aug 2022 21:04:56 +0200 Subject: [PATCH 14/17] changelog: crash when resizing window, or scrolling --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ba3aa6b..22267556 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,10 @@ subsequent motion and leave events. * Regression: “random” selected empty cells being highlighted as selected when they should not. +* Crash when either resizing the terminal window, or scrolling in the + scrollback history ([#1074][1074]) + +[1074]: https://codeberg.org/dnkl/foot/pulls/1074 ### Security From 5706141d0a5250af761f29989dad8161ed1cf2b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 30 Aug 2022 17:48:04 +0200 Subject: [PATCH 15/17] url-mode: connect osc-8 links only when both ID and URI matches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this, two OSC-8 links with a matching ID would be connected even if their URIs weren’t the same. This is against the spec: The same id is only used for connecting character cells whose URIs is also the same. Character cells pointing to different URIs should never be underlined together when hovering over. https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda#hover-underlining-and-the-id-parameter --- CHANGELOG.md | 2 ++ url-mode.c | 16 ++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22267556..fe6e7fcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,8 @@ selected when they should not. * Crash when either resizing the terminal window, or scrolling in the scrollback history ([#1074][1074]) +* OSC-8 URLs with matching IDs, but mismatching URIs being incorrectly + connected. [1074]: https://codeberg.org/dnkl/foot/pulls/1074 diff --git a/url-mode.c b/url-mode.c index 538b60f0..6fa16623 100644 --- a/url-mode.c +++ b/url-mode.c @@ -677,18 +677,23 @@ urls_assign_key_combos(const struct config *conf, url_list_t *urls) if (count == 0) return; - uint64_t seen_ids[count]; char32_t *combos[count]; generate_key_combos(conf, count, combos); size_t combo_idx = 0; - size_t id_idx = 0; tll_foreach(*urls, it) { bool id_already_seen = false; - for (size_t i = 0; i < id_idx; i++) { - if (it->item.id == seen_ids[i]) { + /* Look for already processed URLs where both the URI and the + * ID matches */ + tll_foreach(*urls, it2) { + if (&it->item == &it2->item) + break; + + if (it->item.id == it2->item.id && + strcmp(it->item.url, it2->item.url) == 0) + { id_already_seen = true; break; } @@ -696,7 +701,6 @@ urls_assign_key_combos(const struct config *conf, url_list_t *urls) if (id_already_seen) continue; - seen_ids[id_idx++] = it->item.id; /* * Scan previous URLs, and check if *this* URL matches any of @@ -730,7 +734,7 @@ urls_assign_key_combos(const struct config *conf, url_list_t *urls) char *key = ac32tombs(it->item.key); xassert(key != NULL); - LOG_DBG("URL: %s (%s)", it->item.url, key); + LOG_DBG("URL: %s (key=%s, id=%"PRIu64")", it->item.url, key, it->item.id); free(key); } #endif From 864de72b171ea6e6e002ec119f017ed57fecc0b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 31 Aug 2022 19:17:38 +0200 Subject: [PATCH 16/17] changelog: prepare for 1.13.1 --- CHANGELOG.md | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe6e7fcb..ff203e04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -* [Unreleased](#unreleased) +* [1.13.1](#1-13-1) * [1.13.0](#1-13-0) * [1.12.1](#1-12-1) * [1.12.0](#1-12-0) @@ -40,15 +40,13 @@ * [1.2.0](#1-2-0) -## Unreleased -### Added +## 1.13.1 + ### Changed * Window is now dimmed while in Unicode input mode. -### Deprecated -### Removed ### Fixed * Compiling against wayland-protocols < 1.25 @@ -65,10 +63,6 @@ [1074]: https://codeberg.org/dnkl/foot/pulls/1074 -### Security -### Contributors - - ## 1.13.0 ### Added From cd1933baf12eeef82e04a926f9150ca815d54768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 31 Aug 2022 19:19:15 +0200 Subject: [PATCH 17/17] meson: bump version to 1.13.1 --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index c5597113..360db8fa 100644 --- a/meson.build +++ b/meson.build @@ -1,5 +1,5 @@ project('foot', 'c', - version: '1.13.0', + version: '1.13.1', license: 'MIT', meson_version: '>=0.58.0', default_options: [