From 3fbbf5f81f8f044e332dae662998086f77702e53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 20 Mar 2021 14:14:58 +0100 Subject: [PATCH 01/32] =?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 50e87711..46e3a5b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +* [Unreleased](#unreleased) * [1.7.0](#1-7-0) * [1.6.4](#1-6-4) * [1.6.3](#1-6-3) @@ -23,6 +24,16 @@ * [1.2.0](#1-2-0) +## Unreleased +### Added +### Changed +### Deprecated +### Removed +### Fixed +### Security +### Contributors + + ## 1.7.0 ### Added From 5f1d56bccc51cb7d6f0de2c0d34e78c665f4793e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 20 Mar 2021 23:58:22 +0100 Subject: [PATCH 02/32] =?UTF-8?q?doc:=20benchmark:=20updated=20=E2=80=9Cla?= =?UTF-8?q?ptop=E2=80=9D=20benchmark=20with=201.7.0=20results?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- doc/benchmark.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/doc/benchmark.md b/doc/benchmark.md index a5cc416a..16cc63ae 100644 --- a/doc/benchmark.md +++ b/doc/benchmark.md @@ -50,7 +50,7 @@ Scrollback: 10000 lines | unicode-random | 0.224s ±0.001 | 0.144s ±0.001 | 0.092s ±0.004 [^1] | 21.294s ±1.580 | 26.594s ±3.801 | -## 2020-12-21 +## 2021-03-20 ### System @@ -73,13 +73,13 @@ Scrollback=10000 lines ### Results -| Benchmark | Foot (GCC+PGO) 1.6.0.r30 | Foot (no PGO) 1.6.0.r30 | Alacritty 0.6.0 | URxvt 9.22 | St 0.8.4 | XTerm 362 | -|------------------------|-------------------------:|------------------------:|-------------------:|-----------------:|--------------:|----------------:| -| alt-random | 0.734s ±0.051 | 1.186s ±0.101 | 1.580s ±0.083 | 1.709s ±0.090 | 1.953s ±0.038 | 38.693s ±0.298 | -| alt-random-colors | 0.728s ±0.047 | 1.267s ±0.090 | 1.579s ±0.073 | 2.108s ±0.121 | 2.185s ±0.099 | 34.123s ±0.194 | -| scrolling | 1.639s ±0.040 | 1.641s ±0.053 | 1.397s ±0.048 | 1.389s ±0.046 | 3.599s ±0.124 | 136.514s ±0.534 | -| scrolling-filled-lines | 1.328s ±0.050 | 1.640s ±0.052 | 2.108s ±0.068 | 2.032s ±0.121 | 2.718s ±0.088 | 21.383s ±0.072 | -| unicode-random | 0.304s ±0.018 | 0.271s ±0.017 | 0.143s ±0.002 [^1] | 20.543s ±0.098 | crashed | 16.013s ±0.253 | +| Benchmark | Foot (GCC+PGO) 1.7.0.r2 | Foot (no PGO) 1.7.0.r2 | Alacritty 0.7.2 | URxvt 9.22 | St 0.8.4 | XTerm 366 | +|------------------------|------------------------:|-----------------------:|-------------------:|-----------------:|--------------:|----------------:| +| alt-random | 0.714s ±0.047 | 0.900s ±0.041 | 1.586s ±0.045 | 1.684s ±0.034 | 2.054s ±0.121 | 37.205s ±0.252 | +| alt-random-colors | 0.736s ±0.054 | 0.950s ±0.082 | 1.565s ±0.043 | 2.150s ±0.137 | 2.195s ±0.154 | 33.112s ±0.167 | +| scrolling | 1.593s ±0.070 | 1.559s ±0.055 | 1.517s ±0.079 | 1.462s ±0.052 | 3.308s ±0.133 | 134.432s ±0.436 | +| scrolling-filled-lines | 1.178s ±0.044 | 1.309s ±0.045 | 2.281s ±0.086 | 2.044s ±0.060 | 2.732s ±0.056 | 20.753s ±0.067 | +| unicode-random | 0.349s ±0.009 | 0.352s ±0.007 | 0.148s ±0.010 [^1] | 19.090s ±0.363 | crashed | 15.579s ±0.093 | [^1]: [Alacritty and "unicode-random"](#alacritty-and-unicode-random) From eccf2b674e5d98fa298806f8a0ec45fdac2b13df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 21 Mar 2021 00:00:49 +0100 Subject: [PATCH 03/32] doc: benchmark: update "workstation" benchmarks with 1.7.0 results --- doc/benchmark.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/doc/benchmark.md b/doc/benchmark.md index 16cc63ae..244f25b9 100644 --- a/doc/benchmark.md +++ b/doc/benchmark.md @@ -18,7 +18,7 @@ terminal. This is done **20** times for each test. Then it calculates the _mean_ and _standard deviation_ for each test. -## 2020-10-09 +## 2021-03-20 ### System @@ -41,13 +41,13 @@ Scrollback: 10000 lines ### Results -| Benchmark | Foot (GCC+PGO) 1.5.0.r90 | Foot 1.5.0.r90 | Alacritty 0.5.0 | URxvt 9.22 | XTerm 360 | -|------------------------|-------------------------:|---------------:|-------------------:|---------------:|---------------:| -| alt-random | 0.353s ±0.007 | 0.685s ±0.005 | 0.903s ±0.011 | 1.102s ±0.004 | 12.886s ±0.064 | -| alt-random-colors | 0.354s ±0.019 | 0.665s ±0.004 | 0.933s ±0.004 | 1.149s ±0.013 | 11.739s ±0.093 | -| scrolling | 1.387s ±0.077 | 1.257s ±0.032 | 1.048s ±0.011 | 1.001s ±0.023 | 38.187s ±0.192 | -| scrolling-filled-lines | 0.607s ±0.008 | 0.834s ±0.038 | 1.246s ±0.020 | 1.224s ±0.008 | 6.619s ±0.166 | -| unicode-random | 0.224s ±0.001 | 0.144s ±0.001 | 0.092s ±0.004 [^1] | 21.294s ±1.580 | 26.594s ±3.801 | +| Benchmark | Foot (GCC+PGO) 1.7.0.r2 | Foot 1.7.0.r2 | Alacritty 0.7.2 | URxvt 9.22 | XTerm 366 | +|------------------------|------------------------:|--------------:|-------------------:|---------------:|---------------:| +| alt-random | 0.382s ±0.003 | 0.550s ±0.007 | 0.995s ±0.010 | 1.201s ±0.006 | 12.756s ±0.045 | +| alt-random-colors | 0.380s ±0.002 | 0.543s ±0.003 | 1.017s ±0.013 | 1.399s ±0.018 | 11.591s ±0.141 | +| scrolling | 1.302s ±0.019 | 1.284s ±0.052 | 1.107s ±0.028 | 1.097s ±0.015 | 37.537s ±0.121 | +| scrolling-filled-lines | 0.646s ±0.016 | 0.610s ±0.003 | 1.290s ±0.012 | 1.325s ±0.037 | 6.817s ±0.084 | +| unicode-random | 0.167s ±0.001 | 0.276s ±0.445 | 0.097s ±0.002 [^1] | 18.032s ±0.334 | 29.731s ±3.746 | ## 2021-03-20 From 0c85905972fdc739fb140e32c3389c08e769bfff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 21 Mar 2021 11:55:03 +0100 Subject: [PATCH 04/32] input: must have all required modifiers to un-shift a symbol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When detecting, and repairing, “broken” key bindings (where the key binding itself explicitly lists a modifier that is consumed by the final symbol - e.g “Shift+W”), don’t just look for an intersection between the set of modifiers needed to produce the final symbol, and the modifiers listed in the key combo. Instead, check if the key combo has *all* the required modifiers. Example: Shift+AltGr+w produces Page_Down. I.e. Page_Down is the _shifted_ symbol, ‘w’ is the un-shifted symbol, and Shift+AltGr are the modifiers required to shift ‘w’ to Page_Down. If we have the key combo Shift+Page_Down, foot would, correctly, determine that Page_Down is a shifted symbol. It would find the Shift+AltGr modifier set, and since the intersection of “Shift+AltGr” and “Shift” (from our key combo) is non-empty, foot would (incorrectly) determine that we can, and should, replace Page_Down with its un-shifted symbol ‘w’. This is completely wrong, since Shift+w does _not_ produce Page_Down. Closes #407 --- CHANGELOG.md | 5 +++++ input.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46e3a5b8..266d9b77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,11 @@ ### Deprecated ### Removed ### Fixed + +* Logic that repairs invalid key bindings ended up breaking valid key + bindings instead (https://codeberg.org/dnkl/foot/issues/407). + + ### Security ### Contributors diff --git a/input.c b/input.c index bc28bbf2..9a26e778 100644 --- a/input.c +++ b/input.c @@ -450,7 +450,7 @@ maybe_repair_key_combo(const struct seat *seat, /* Check if key combo’s modifier set intersects */ for (size_t j = 0; j < mod_mask_count; j++) { - if (!(mod_masks[j] & mods)) + if ((mod_masks[j] & mods) != mod_masks[j]) continue; char combo[64] = {0}; From 7609fbba472a767849a34ca68b21ceaeccee9565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 20 Mar 2021 15:32:31 +0100 Subject: [PATCH 05/32] term: increase/decrease custom line-height with font size changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user has set a custom line-height, we now adjust it when increasing/decreasing (“zooming”) the font size at run-time. Previously, the line-height was fixed at the size specified in foot.ini. --- CHANGELOG.md | 2 ++ config.h | 6 ------ terminal.c | 15 +++++++++++++-- terminal.h | 7 +++++++ 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 266d9b77..0e065995 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,8 @@ * Logic that repairs invalid key bindings ended up breaking valid key bindings instead (https://codeberg.org/dnkl/foot/issues/407). +* Custom `line-height` settings now scale when increasing or + decreasing the font size at run-time. ### Security diff --git a/config.h b/config.h index 1fe82257..f8c7aaf0 100644 --- a/config.h +++ b/config.h @@ -53,12 +53,6 @@ struct config_mouse_binding { struct config_binding_pipe pipe; }; -/* If px != 0 then px is valid, otherwise pt is valid */ -struct pt_or_px { - int16_t px; - float pt; -}; - struct config_spawn_template { char *raw_cmd; char **argv; diff --git a/terminal.c b/terminal.c index 68a7a138..cf0e1d13 100644 --- a/terminal.c +++ b/terminal.c @@ -649,8 +649,8 @@ term_set_fonts(struct terminal *term, struct fcft_font *fonts[static 4]) : term->fonts[0]->max_advance.x) + pt_or_px_as_pixels(term, &conf->letter_spacing); - term->cell_height = conf->line_height.px >= 0 - ? pt_or_px_as_pixels(term, &conf->line_height) + term->cell_height = term->font_line_height.px >= 0 + ? pt_or_px_as_pixels(term, &term->font_line_height) : max(term->fonts[0]->height, term->fonts[0]->ascent + term->fonts[0]->descent); @@ -979,6 +979,7 @@ load_fonts_from_conf(struct terminal *term) } } + term->font_line_height = term->conf->line_height; return reload_fonts(term); } @@ -1196,6 +1197,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .pt_size = it->item.pt_size, .px_size = it->item.px_size}; } } + term->font_line_height = conf->line_height; /* Start the slave/client */ if ((term->slave = slave_spawn( @@ -1780,6 +1782,15 @@ term_font_size_adjust(struct terminal *term, double amount) } } + if (term->font_line_height.px >= 0) { + double old_pt_size = term->font_line_height.px > 0 + ? term->font_line_height.px * 72. / term->font_dpi + : term->font_line_height.pt; + + term->font_line_height.px = 0; + term->font_line_height.pt = fmax(old_pt_size + amount, 0); + } + return reload_fonts(term); } diff --git a/terminal.h b/terminal.h index 044fc581..6cf52522 100644 --- a/terminal.h +++ b/terminal.h @@ -261,6 +261,12 @@ struct url { }; typedef tll(struct url) url_list_t; +/* If px != 0 then px is valid, otherwise pt is valid */ +struct pt_or_px { + int16_t px; + float pt; +}; + struct terminal { struct fdm *fdm; struct reaper *reaper; @@ -312,6 +318,7 @@ struct terminal { struct fcft_font *fonts[4]; struct config_font *font_sizes[4]; + struct pt_or_px font_line_height; float font_dpi; int font_scale; int16_t font_x_ofs; From ee75e10e71ca4f942fbbc6cdac6b20fdb57ab67f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Mar 2021 11:02:51 +0100 Subject: [PATCH 06/32] term: term_print(): clear line break flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The line break flag is used by the text reflow and text extraction (i.e. copy-paste) logic, to determine whether or not to insert a newline between two lines. There’s some amount of heuristics involved in this. For example, the client application could emit a newline, move the cursor back up, and print text. What does that mean for us? Foot’s behavior up until now has been this: The line break flag is set on the row, when the application emits an explicit linefeed. The flag is cleared when the line is erased. But otherwise not. This meant that emitting a linefeed and then moving the cursor back up and printing text, did not clear the line break flag. This in turn meant that text copied always had newlines inserted, even though that was not the client applications intention. By clearing the line break flag whenever _anything_ is printed to a row, the new behavior is, in practice, that the line break flag is only set on a row if a linefeed was that *last* thing printed to that row. Closes #410 --- terminal.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/terminal.c b/terminal.c index cf0e1d13..cc423395 100644 --- a/terminal.c +++ b/terminal.c @@ -2870,6 +2870,7 @@ term_print(struct terminal *term, wchar_t wc, int width) cell->attrs = term->vt.attrs; row->dirty = true; + row->linebreak = false; cell->attrs.clean = 0; /* Advance cursor the 'additional' columns while dirty:ing the cells */ @@ -2909,6 +2910,7 @@ ascii_printer_fast(struct terminal *term, wchar_t wc) cell->attrs = term->vt.attrs; row->dirty = true; + row->linebreak = false; cell->attrs.clean = 0; /* Advance cursor */ From c5d57d23eaf1871b6a6ea74ba1385f39c64e26a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Mar 2021 14:18:04 +0100 Subject: [PATCH 07/32] changelog: newlines incorrectly inserted into copied text --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e065995..ff203ed7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ bindings instead (https://codeberg.org/dnkl/foot/issues/407). * Custom `line-height` settings now scale when increasing or decreasing the font size at run-time. +* Newlines sometimes incorrectly inserted into copied text + (https://codeberg.org/dnkl/foot/issues/410). ### Security From e8ffb05bc7fb3b996752f345b77af4b757f69776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Mar 2021 13:03:07 +0100 Subject: [PATCH 08/32] ime: move preedit state from terminal struct to the seat struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This ensures different seat’s don’t step on each others IME pre-edit state. It also removes most dependencies on having a valid term pointer for many IME operations. We’re still not all the way, since we support disabling IME with a private mode, which is per terminal, not seat. Thus, we still require the seat to have keyboard focus on one of our windows. Closes #324. But note that *rendering* of multiple seat’s IME pre-edit strings is still broken. --- ime.c | 148 ++++++++++++++++++++++++++++++----------------------- ime.h | 5 +- pgo/pgo.c | 1 + render.c | 92 ++++++++++++++++++++------------- terminal.c | 40 +++++++-------- terminal.h | 17 +----- wayland.c | 2 +- wayland.h | 9 ++++ 8 files changed, 178 insertions(+), 136 deletions(-) diff --git a/ime.c b/ime.c index ec38be18..4137a949 100644 --- a/ime.c +++ b/ime.c @@ -37,10 +37,6 @@ leave(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, struct seat *seat = data; LOG_DBG("leave: seat=%s", seat->name); - struct terminal *term = seat->kbd_focus; - if (term != NULL) - term_ime_reset(term); - ime_disable(seat); seat->ime.focused = false; } @@ -53,7 +49,7 @@ preedit_string(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, struct seat *seat = data; - ime_reset_preedit(seat); + ime_reset_pending_preedit(seat); if (text != NULL) { seat->ime.preedit.pending.text = xstrdup(text); @@ -70,7 +66,7 @@ commit_string(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, struct seat *seat = data; - ime_reset_commit(seat); + ime_reset_pending_commit(seat); if (text != NULL) seat->ime.commit.pending.text = xstrdup(text); @@ -107,6 +103,7 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, LOG_DBG("done: serial=%u", serial); struct seat *seat = data; + struct terminal *term = seat->kbd_focus; if (seat->ime.serial != serial) { LOG_DBG("IME serial mismatch: expected=0x%08x, got 0x%08x", @@ -114,16 +111,16 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, return; } - xassert(seat->kbd_focus); - struct terminal *term = seat->kbd_focus; - /* 1. Delete existing pre-edit text */ - if (term->ime.preedit.cells != NULL) { - term_ime_reset(term); - if (term->is_searching) - render_refresh_search(term); - else - render_refresh(term); + if (seat->ime.preedit.cells != NULL) { + ime_reset_preedit(seat); + + if (term != NULL) { + if (term->is_searching) + render_refresh_search(term); + else + render_refresh(term); + } } /* @@ -139,12 +136,14 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, const char *text = seat->ime.commit.pending.text; size_t len = strlen(text); - if (term->is_searching) { - search_add_chars(term, text, len); - render_refresh_search(term); - } else - term_to_slave(term, text, len); - ime_reset_commit(seat); + if (term != NULL) { + if (term->is_searching) { + search_add_chars(term, text, len); + render_refresh_search(term); + } else + term_to_slave(term, text, len); + } + ime_reset_pending_commit(seat); } /* 4. Calculate surrounding text to send - not supported */ @@ -155,41 +154,41 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, : 0; if (wchars == 0 || wchars == (size_t)-1) { - ime_reset_preedit(seat); + ime_reset_pending_preedit(seat); return; } /* First, convert to unicode */ - term->ime.preedit.text = xmalloc((wchars + 1) * sizeof(wchar_t)); - mbstowcs(term->ime.preedit.text, seat->ime.preedit.pending.text, wchars); - term->ime.preedit.text[wchars] = L'\0'; + seat->ime.preedit.text = xmalloc((wchars + 1) * sizeof(wchar_t)); + mbstowcs(seat->ime.preedit.text, seat->ime.preedit.pending.text, wchars); + seat->ime.preedit.text[wchars] = L'\0'; /* Next, count number of cells needed */ size_t cell_count = 0; size_t widths[wchars + 1]; for (size_t i = 0; i < wchars; i++) { - int width = max(wcwidth(term->ime.preedit.text[i]), 1); + int width = max(wcwidth(seat->ime.preedit.text[i]), 1); widths[i] = width; cell_count += width; } /* Allocate cells */ - term->ime.preedit.cells = xmalloc( - cell_count * sizeof(term->ime.preedit.cells[0])); - term->ime.preedit.count = cell_count; + seat->ime.preedit.cells = xmalloc( + cell_count * sizeof(seat->ime.preedit.cells[0])); + seat->ime.preedit.count = cell_count; /* Populate cells */ for (size_t i = 0, cell_idx = 0; i < wchars; i++) { - struct cell *cell = &term->ime.preedit.cells[cell_idx]; + struct cell *cell = &seat->ime.preedit.cells[cell_idx]; int width = widths[i]; - cell->wc = term->ime.preedit.text[i]; + cell->wc = seat->ime.preedit.text[i]; cell->attrs = (struct attributes){.clean = 0}; for (int j = 1; j < width; j++) { - cell = &term->ime.preedit.cells[cell_idx + j]; + cell = &seat->ime.preedit.cells[cell_idx + j]; cell->wc = CELL_MULT_COL_SPACER; cell->attrs = (struct attributes){.clean = 1}; } @@ -206,18 +205,18 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, /* Note: docs says *both* begin and end should be -1, * but what else can we do if only one is -1? */ LOG_DBG("pre-edit cursor is hidden"); - term->ime.preedit.cursor.hidden = true; - term->ime.preedit.cursor.start = -1; - term->ime.preedit.cursor.end = -1; + seat->ime.preedit.cursor.hidden = true; + seat->ime.preedit.cursor.start = -1; + seat->ime.preedit.cursor.end = -1; } else if (seat->ime.preedit.pending.cursor_begin == byte_len && seat->ime.preedit.pending.cursor_end == byte_len) { /* Cursor is *after* the entire pre-edit string */ - term->ime.preedit.cursor.hidden = false; - term->ime.preedit.cursor.start = cell_count; - term->ime.preedit.cursor.end = cell_count; + seat->ime.preedit.cursor.hidden = false; + seat->ime.preedit.cursor.start = cell_count; + seat->ime.preedit.cursor.end = cell_count; } else { @@ -271,7 +270,7 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, /* Expand cursor end to end of glyph */ while (cell_end > cell_begin && cell_end < cell_count && - term->ime.preedit.cells[cell_end].wc == CELL_MULT_COL_SPACER) + seat->ime.preedit.cells[cell_end].wc == CELL_MULT_COL_SPACER) { cell_end++; } @@ -284,50 +283,65 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, xassert(cell_end >= 0); xassert(cell_end <= cell_count); - term->ime.preedit.cursor.hidden = false; - term->ime.preedit.cursor.start = cell_begin; - term->ime.preedit.cursor.end = cell_end; + seat->ime.preedit.cursor.hidden = false; + seat->ime.preedit.cursor.start = cell_begin; + seat->ime.preedit.cursor.end = cell_end; } /* Underline pre-edit string that is *not* covered by the cursor */ - bool hidden = term->ime.preedit.cursor.hidden; - int start = term->ime.preedit.cursor.start; - int end = term->ime.preedit.cursor.end; + bool hidden = seat->ime.preedit.cursor.hidden; + int start = seat->ime.preedit.cursor.start; + int end = seat->ime.preedit.cursor.end; for (size_t i = 0, cell_idx = 0; i < wchars; cell_idx += widths[i], i++) { if (hidden || start == end || cell_idx < start || cell_idx >= end) { - struct cell *cell = &term->ime.preedit.cells[cell_idx]; + struct cell *cell = &seat->ime.preedit.cells[cell_idx]; cell->attrs.underline = true; } } - ime_reset_preedit(seat); + ime_reset_pending_preedit(seat); - if (term->is_searching) - render_refresh_search(term); - else - render_refresh(term); + if (term != NULL) { + if (term->is_searching) + render_refresh_search(term); + else + render_refresh(term); + } } void -ime_reset_preedit(struct seat *seat) +ime_reset_pending_preedit(struct seat *seat) { free(seat->ime.preedit.pending.text); seat->ime.preedit.pending.text = NULL; } void -ime_reset_commit(struct seat *seat) +ime_reset_pending_commit(struct seat *seat) { free(seat->ime.commit.pending.text); seat->ime.commit.pending.text = NULL; } void -ime_reset(struct seat *seat) +ime_reset_pending(struct seat *seat) { - ime_reset_preedit(seat); - ime_reset_commit(seat); + ime_reset_pending_preedit(seat); + ime_reset_pending_commit(seat); +} + +void +ime_reset_preedit(struct seat *seat) +{ + if (seat->ime.preedit.cells == NULL) + return; + + free(seat->ime.preedit.text); + free(seat->ime.preedit.cells); + seat->ime.preedit.text = NULL; + seat->ime.preedit.cells = NULL; + seat->ime.preedit.count = 0; } void @@ -339,7 +353,7 @@ ime_send_cursor_rect(struct seat *seat, struct terminal *term) if (!seat->ime.focused) return; - if (!term->ime.enabled) + if (!term->ime_enabled) return; if (seat->ime.cursor_rect.pending.x == seat->ime.cursor_rect.sent.x && @@ -370,15 +384,21 @@ ime_enable(struct seat *seat) return; struct terminal *term = seat->kbd_focus; + + /* TODO: we’ve actaully seen text-input::enter without first + * seeing keyboard::enter... so perhaps we should check for this, + * and... do what? Ignore IME completely, or do we need to call + * ime_enable() from keyboard::enter too? */ xassert(term != NULL); if (!seat->ime.focused) return; - if (!term->ime.enabled) + if (!term->ime_enabled) return; - ime_reset(seat); + ime_reset_pending(seat); + ime_reset_preedit(seat); zwp_text_input_v3_enable(seat->wl_text_input); zwp_text_input_v3_set_content_type( @@ -408,7 +428,8 @@ ime_disable(struct seat *seat) if (!seat->ime.focused) return; - ime_reset(seat); + ime_reset_pending(seat); + ime_reset_preedit(seat); zwp_text_input_v3_disable(seat->wl_text_input); zwp_text_input_v3_commit(seat->wl_text_input); @@ -419,7 +440,7 @@ void ime_update_cursor_rect(struct seat *seat, struct terminal *term) { /* Set in render_ime_preedit() */ - if (term->ime.preedit.cells != NULL) + if (seat->ime.preedit.cells != NULL) goto update; /* Set in render_search_box() */ @@ -466,9 +487,10 @@ void ime_enable(struct seat *seat) {} void ime_disable(struct seat *seat) {} void ime_update_cursor_rect(struct seat *seat, struct terminal *term) {} +void ime_reset_pending_preedit(struct seat *seat) {} +void ime_reset_pending_commit(struct seat *seat) {} +void ime_reset_pending(struct seat *seat) {} void ime_reset_preedit(struct seat *seat) {} -void ime_reset_commit(struct seat *seat) {} -void ime_reset(struct seat *seat) {} void ime_send_cursor_rect(struct seat *seat, struct terminal *term) {} #endif diff --git a/ime.h b/ime.h index ed2fe507..01438a5a 100644 --- a/ime.h +++ b/ime.h @@ -15,7 +15,8 @@ void ime_enable(struct seat *seat); void ime_disable(struct seat *seat); void ime_update_cursor_rect(struct seat *seat, struct terminal *term); +void ime_reset_pending_preedit(struct seat *seat); +void ime_reset_pending_commit(struct seat *seat); +void ime_reset_pending(struct seat *seat); void ime_reset_preedit(struct seat *seat); -void ime_reset_commit(struct seat *seat); -void ime_reset(struct seat *seat); void ime_send_cursor_rect(struct seat *seat, struct terminal *term); diff --git a/pgo/pgo.c b/pgo/pgo.c index 933d100c..ec8a7883 100644 --- a/pgo/pgo.c +++ b/pgo/pgo.c @@ -129,6 +129,7 @@ void cmd_scrollback_down(struct terminal *term, int rows) {} void ime_enable(struct seat *seat) {} void ime_disable(struct seat *seat) {} +void ime_reset_preedit(struct seat *seat) {} void notify_notify(const struct terminal *term, const char *title, const char *body) diff --git a/render.c b/render.c index fcd07f56..79f90cc7 100644 --- a/render.c +++ b/render.c @@ -1114,12 +1114,12 @@ render_sixel_images(struct terminal *term, pixman_image_t *pix, } } -static void -render_ime_preedit(struct terminal *term, struct buffer *buf) -{ #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - - if (likely(term->ime.preedit.cells == NULL)) +static void +render_ime_preedit_for_seat(struct terminal *term, struct seat *seat, + struct buffer *buf) +{ + if (likely(seat->ime.preedit.cells == NULL)) return; if (unlikely(term->is_searching)) @@ -1135,10 +1135,10 @@ render_ime_preedit(struct terminal *term, struct buffer *buf) if (cursor.row < 0 || cursor.row >= term->rows) return; - int cells_needed = term->ime.preedit.count; + int cells_needed = seat->ime.preedit.count; - if (term->ime.preedit.cursor.start == cells_needed && - term->ime.preedit.cursor.end == cells_needed) + if (seat->ime.preedit.cursor.start == cells_needed && + seat->ime.preedit.cursor.end == cells_needed) { /* Cursor will be drawn *after* the pre-edit string, i.e. in * the cell *after*. This means we need to copy, and dirty, @@ -1160,8 +1160,8 @@ render_ime_preedit(struct terminal *term, struct buffer *buf) col_idx -= cells_used - cells_left; if (cells_needed > cells_used) { - int start = term->ime.preedit.cursor.start; - int end = term->ime.preedit.cursor.end; + int start = seat->ime.preedit.cursor.start; + int end = seat->ime.preedit.cursor.end; if (start == end) { /* Ensure *end* of pre-edit string is visible */ @@ -1177,7 +1177,7 @@ render_ime_preedit(struct terminal *term, struct buffer *buf) /* Make sure we don't start in the middle of a character */ while (ime_ofs < cells_needed && - term->ime.preedit.cells[ime_ofs].wc == CELL_MULT_COL_SPACER) + seat->ime.preedit.cells[ime_ofs].wc == CELL_MULT_COL_SPACER) { ime_ofs++; } @@ -1208,9 +1208,9 @@ render_ime_preedit(struct terminal *term, struct buffer *buf) row->dirty = true; /* Render pre-edit text */ - xassert(term->ime.preedit.cells[ime_ofs].wc != CELL_MULT_COL_SPACER); - for (int i = 0, idx = ime_ofs; idx < term->ime.preedit.count; i++, idx++) { - const struct cell *cell = &term->ime.preedit.cells[idx]; + xassert(seat->ime.preedit.cells[ime_ofs].wc != CELL_MULT_COL_SPACER); + for (int i = 0, idx = ime_ofs; idx < seat->ime.preedit.count; i++, idx++) { + const struct cell *cell = &seat->ime.preedit.cells[idx]; if (cell->wc == CELL_MULT_COL_SPACER) continue; @@ -1223,11 +1223,11 @@ render_ime_preedit(struct terminal *term, struct buffer *buf) render_cell(term, buf->pix[0], row, col_idx + i, row_idx, false); } - int start = term->ime.preedit.cursor.start - ime_ofs; - int end = term->ime.preedit.cursor.end - ime_ofs; + int start = seat->ime.preedit.cursor.start - ime_ofs; + int end = seat->ime.preedit.cursor.end - ime_ofs; - if (!term->ime.preedit.cursor.hidden) { - const struct cell *start_cell = &term->ime.preedit.cells[0]; + if (!seat->ime.preedit.cursor.hidden) { + const struct cell *start_cell = &seat->ime.preedit.cells[0]; pixman_color_t fg = color_hex_to_pixman(term->colors.fg); pixman_color_t bg = color_hex_to_pixman(term->colors.bg); @@ -1271,6 +1271,17 @@ render_ime_preedit(struct terminal *term, struct buffer *buf) term->margins.top + row_idx * term->cell_height, term->width - term->margins.left - term->margins.right, 1 * term->cell_height); +} +#endif + +static void +render_ime_preedit(struct terminal *term, struct buffer *buf) +{ +#if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED + tll_foreach(term->wl->seats, it) { + if (it->item.kbd_focus == term) + render_ime_preedit_for_seat(term, &it->item, buf); + } #endif } @@ -2238,9 +2249,18 @@ render_search_box(struct terminal *term) */ #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED + /* TODO: do we want to/need to handle multi-seat? */ + struct seat *ime_seat = NULL; + tll_foreach(term->wl->seats, it) { + if (it->item.kbd_focus == term) { + ime_seat = &it->item; + break; + } + } + size_t text_len = term->search.len; - if (term->ime.preedit.text != NULL) - text_len += wcslen(term->ime.preedit.text); + if (ime_seat != NULL && ime_seat->ime.preedit.text != NULL) + text_len += wcslen(ime_seat->ime.preedit.text); wchar_t *text = xmalloc((text_len + 1) * sizeof(wchar_t)); text[0] = L'\0'; @@ -2250,8 +2270,8 @@ render_search_box(struct terminal *term) text[term->search.cursor] = L'\0'; /* Insert pre-edit text at cursor */ - if (term->ime.preedit.text != NULL) - wcscat(text, term->ime.preedit.text); + if (ime_seat != NULL && ime_seat->ime.preedit.text != NULL) + wcscat(text, ime_seat->ime.preedit.text); /* And finally everything after the cursor */ wcsncat(text, &term->search.buf[term->search.cursor], @@ -2319,18 +2339,18 @@ render_search_box(struct terminal *term) continue; #if (FOOT_IME_ENABLED) && FOOT_IME_ENABLED - if (term->ime.preedit.cells != NULL) { - if (term->ime.preedit.cursor.start == term->ime.preedit.cursor.end) { + if (ime_seat != NULL && ime_seat->ime.preedit.cells != NULL) { + if (ime_seat->ime.preedit.cursor.start == ime_seat->ime.preedit.cursor.end) { /* All IME's I've seen so far keeps the cursor at * index 0, so ensure the *end* of the pre-edit string * is visible */ - cell_idx += term->ime.preedit.count; + cell_idx += ime_seat->ime.preedit.count; } else { /* Try to predict in which direction we'll shift the text */ - if (cell_idx + term->ime.preedit.cursor.start > glyph_offset) - cell_idx += term->ime.preedit.cursor.end; + if (cell_idx + ime_seat->ime.preedit.cursor.start > glyph_offset) + cell_idx += ime_seat->ime.preedit.cursor.end; else - cell_idx += term->ime.preedit.cursor.start; + cell_idx += ime_seat->ime.preedit.cursor.start; } } #endif @@ -2383,8 +2403,10 @@ render_search_box(struct terminal *term) /* Render cursor */ if (i == term->search.cursor) { #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - bool have_preedit = term->ime.preedit.cells != NULL; - bool hidden = term->ime.preedit.cursor.hidden; + bool have_preedit = + ime_seat != NULL && ime_seat->ime.preedit.cells != NULL; + bool hidden = + ime_seat != NULL && ime_seat->ime.preedit.cursor.hidden; if (have_preedit && !hidden) { /* Cursor may be outside the visible area: @@ -2394,13 +2416,13 @@ render_search_box(struct terminal *term) /* If cursor is outside the visible area, we need to * adjust our rectangle's position */ - int start = term->ime.preedit.cursor.start + int start = ime_seat->ime.preedit.cursor.start + min((ssize_t)(cell_idx - glyph_offset), 0); - int end = term->ime.preedit.cursor.end + int end = ime_seat->ime.preedit.cursor.end + min((ssize_t)(cell_idx - glyph_offset), 0); if (start == end) { - int count = min(term->ime.preedit.count, cells_left); + int count = min(ime_seat->ime.preedit.count, cells_left); /* Underline the entire (visible part of) pre-edit text */ draw_underline(term, buf->pix[0], font, &fg, x, y, count); @@ -2415,7 +2437,7 @@ render_search_box(struct terminal *term) /* Underline everything before and after the cursor */ int count1 = min(start, cells_left); int count2 = max( - min(term->ime.preedit.count - term->ime.preedit.cursor.end, + min(ime_seat->ime.preedit.count - ime_seat->ime.preedit.cursor.end, cells_left - end), 0); draw_underline(term, buf->pix[0], font, &fg, x, y, count1); @@ -2487,7 +2509,7 @@ render_search_box(struct terminal *term) } #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - if (term->ime.preedit.cells != NULL) + if (ime_seat != NULL && ime_seat->ime.preedit.cells != NULL) /* Already rendered */; else #endif diff --git a/terminal.c b/terminal.c index cc423395..1984562f 100644 --- a/terminal.c +++ b/terminal.c @@ -1182,9 +1182,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .foot_exe = xstrdup(foot_exe), .cwd = xstrdup(cwd), #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - .ime = { - .enabled = true, - }, + .ime_enabled = true, #endif }; @@ -2405,10 +2403,8 @@ term_kbd_focus_out(struct terminal *term) return; #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - if (term->ime.preedit.cells != NULL) { - term_ime_reset(term); + if (term_ime_reset(term)) render_refresh(term); - } #endif term->kbd_focus = false; @@ -3039,7 +3035,7 @@ bool term_ime_is_enabled(const struct terminal *term) { #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - return term->ime.enabled; + return term->ime_enabled; #else return false; #endif @@ -3049,13 +3045,12 @@ void term_ime_enable(struct terminal *term) { #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - if (term->ime.enabled) + if (term->ime_enabled) return; LOG_DBG("IME enabled"); - term->ime.enabled = true; - term_ime_reset(term); + term->ime_enabled = true; /* IME is per seat - enable on all seat currently focusing us */ tll_foreach(term->wl->seats, it) { @@ -3069,13 +3064,12 @@ void term_ime_disable(struct terminal *term) { #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - if (!term->ime.enabled) + if (!term->ime_enabled) return; LOG_DBG("IME disabled"); - term->ime.enabled = false; - term_ime_reset(term); + term->ime_enabled = false; /* IME is per seat - disable on all seat currently focusing us */ tll_foreach(term->wl->seats, it) { @@ -3085,18 +3079,24 @@ term_ime_disable(struct terminal *term) #endif } -void +bool term_ime_reset(struct terminal *term) { + bool at_least_one_seat_was_reset = false; + #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - if (term->ime.preedit.cells != NULL) { - free(term->ime.preedit.text); - free(term->ime.preedit.cells); - term->ime.preedit.text = NULL; - term->ime.preedit.cells = NULL; - term->ime.preedit.count = 0; + tll_foreach(term->wl->seats, it) { + struct seat *seat = &it->item; + + if (seat->kbd_focus != term) + continue; + + ime_reset_preedit(seat); + at_least_one_seat_was_reset = true; } #endif + + return at_least_one_seat_was_reset; } void diff --git a/terminal.h b/terminal.h index 6cf52522..ce808ce8 100644 --- a/terminal.h +++ b/terminal.h @@ -567,20 +567,7 @@ struct terminal { struct grid *url_grid_snapshot; #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED - struct { - bool enabled; - struct { - wchar_t *text; - struct cell *cells; - int count; - - struct { - bool hidden; - int start; /* Cell index, inclusive */ - int end; /* Cell index, exclusive */ - } cursor; - } preedit; - } ime; + bool ime_enabled; #endif bool is_shutting_down; @@ -713,7 +700,7 @@ bool term_view_to_text( bool term_ime_is_enabled(const struct terminal *term); void term_ime_enable(struct terminal *term); void term_ime_disable(struct terminal *term); -void term_ime_reset(struct terminal *term); +bool term_ime_reset(struct terminal *term); void term_ime_set_cursor_rect( struct terminal *term, int x, int y, int width, int height); diff --git a/wayland.c b/wayland.c index a092e054..edc9b482 100644 --- a/wayland.c +++ b/wayland.c @@ -189,7 +189,7 @@ seat_destroy(struct seat *seat) if (seat->wl_seat != NULL) wl_seat_release(seat->wl_seat); - ime_reset(seat); + ime_reset_pending(seat); free(seat->clipboard.text); free(seat->primary.text); free(seat->name); diff --git a/wayland.h b/wayland.h index 34314414..0b916ccc 100644 --- a/wayland.h +++ b/wayland.h @@ -256,6 +256,15 @@ struct seat { int32_t cursor_begin; int32_t cursor_end; } pending; + + wchar_t *text; + struct cell *cells; + int count; + struct { + bool hidden; + int start; /* Cell index, inclusive */ + int end; /* Cell index, exclusive */ + } cursor; } preedit; struct { From 1c355f7b7f61a85578144b967aa50aa9e8f2cf36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Mar 2021 13:37:45 +0100 Subject: [PATCH 09/32] ime: codespell: actaually -> actually --- ime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ime.c b/ime.c index 4137a949..6c65265d 100644 --- a/ime.c +++ b/ime.c @@ -385,7 +385,7 @@ ime_enable(struct seat *seat) struct terminal *term = seat->kbd_focus; - /* TODO: we’ve actaully seen text-input::enter without first + /* TODO: we’ve actually seen text-input::enter without first * seeing keyboard::enter... so perhaps we should check for this, * and... do what? Ignore IME completely, or do we need to call * ime_enable() from keyboard::enter too? */ From 13b45db13e134c81ea72d1d70bf782ecd2445900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Mar 2021 13:56:33 +0100 Subject: [PATCH 10/32] =?UTF-8?q?ime:=20don=E2=80=99t=20pass=20=E2=80=98te?= =?UTF-8?q?rm=E2=80=99=20to=20ime=5Fupdate=5Fcursor=5Frect()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In all instances where we call ime_update_cursor_rect(), the ‘term’ argument is the same as seat->kbd_focus. So, let ime_update_cursor_rect() use that directly instead. Also make ime_send_cursor_rect() static (i.e. local to ime.c). --- ime.c | 15 +++++++++------ ime.h | 3 +-- render.c | 4 ++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/ime.c b/ime.c index 6c65265d..a224d570 100644 --- a/ime.c +++ b/ime.c @@ -344,8 +344,8 @@ ime_reset_preedit(struct seat *seat) seat->ime.preedit.count = 0; } -void -ime_send_cursor_rect(struct seat *seat, struct terminal *term) +static void +ime_send_cursor_rect(struct seat *seat) { if (unlikely(seat->wayl->text_input_manager == NULL)) return; @@ -353,6 +353,8 @@ ime_send_cursor_rect(struct seat *seat, struct terminal *term) if (!seat->ime.focused) return; + struct terminal *term = seat->kbd_focus; + if (!term->ime_enabled) return; @@ -437,8 +439,10 @@ ime_disable(struct seat *seat) } void -ime_update_cursor_rect(struct seat *seat, struct terminal *term) +ime_update_cursor_rect(struct seat *seat) { + struct terminal *term = seat->kbd_focus; + /* Set in render_ime_preedit() */ if (seat->ime.preedit.cells != NULL) goto update; @@ -469,7 +473,7 @@ ime_update_cursor_rect(struct seat *seat, struct terminal *term) seat->ime.cursor_rect.pending.height = height; update: - ime_send_cursor_rect(seat, term); + ime_send_cursor_rect(seat); } const struct zwp_text_input_v3_listener text_input_listener = { @@ -485,12 +489,11 @@ const struct zwp_text_input_v3_listener text_input_listener = { void ime_enable(struct seat *seat) {} void ime_disable(struct seat *seat) {} -void ime_update_cursor_rect(struct seat *seat, struct terminal *term) {} +void ime_update_cursor_rect(struct seat *seat) {} void ime_reset_pending_preedit(struct seat *seat) {} void ime_reset_pending_commit(struct seat *seat) {} void ime_reset_pending(struct seat *seat) {} void ime_reset_preedit(struct seat *seat) {} -void ime_send_cursor_rect(struct seat *seat, struct terminal *term) {} #endif diff --git a/ime.h b/ime.h index 01438a5a..2aa4efe4 100644 --- a/ime.h +++ b/ime.h @@ -13,10 +13,9 @@ struct terminal; void ime_enable(struct seat *seat); void ime_disable(struct seat *seat); -void ime_update_cursor_rect(struct seat *seat, struct terminal *term); +void ime_update_cursor_rect(struct seat *seat); void ime_reset_pending_preedit(struct seat *seat); void ime_reset_pending_commit(struct seat *seat); void ime_reset_pending(struct seat *seat); void ime_reset_preedit(struct seat *seat); -void ime_send_cursor_rect(struct seat *seat, struct terminal *term); diff --git a/render.c b/render.c index 79f90cc7..dd8070e0 100644 --- a/render.c +++ b/render.c @@ -2739,7 +2739,7 @@ frame_callback(void *data, struct wl_callback *wl_callback, uint32_t callback_da tll_foreach(term->wl->seats, it) { if (it->item.kbd_focus == term) - ime_update_cursor_rect(&it->item, term); + ime_update_cursor_rect(&it->item); } term->grid = original_grid; @@ -3195,7 +3195,7 @@ fdm_hook_refresh_pending_terminals(struct fdm *fdm, void *data) tll_foreach(term->wl->seats, it) { if (it->item.kbd_focus == term) - ime_update_cursor_rect(&it->item, term); + ime_update_cursor_rect(&it->item); } term->grid = original_grid; From ed3e70a9c78413e85b2cb1c379d26d592c13f827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Mar 2021 14:01:46 +0100 Subject: [PATCH 11/32] =?UTF-8?q?ime:=20don=E2=80=99t=20enable=20IME=20if?= =?UTF-8?q?=20we=20don=E2=80=99t=20have=20keyboard=20focus?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #411 --- ime.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/ime.c b/ime.c index a224d570..deb6dc9c 100644 --- a/ime.c +++ b/ime.c @@ -385,17 +385,13 @@ ime_enable(struct seat *seat) if (unlikely(seat->wayl->text_input_manager == NULL)) return; - struct terminal *term = seat->kbd_focus; - - /* TODO: we’ve actually seen text-input::enter without first - * seeing keyboard::enter... so perhaps we should check for this, - * and... do what? Ignore IME completely, or do we need to call - * ime_enable() from keyboard::enter too? */ - xassert(term != NULL); - if (!seat->ime.focused) return; + struct terminal *term = seat->kbd_focus; + if (term == NULL) + return; + if (!term->ime_enabled) return; From adb5a344fe6b4a8789813f5a39cce7067a40034f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Mar 2021 14:02:05 +0100 Subject: [PATCH 12/32] ime: log warning on text-input::done() received without keyboard focus --- ime.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ime.c b/ime.c index deb6dc9c..58aecf7f 100644 --- a/ime.c +++ b/ime.c @@ -111,6 +111,15 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, return; } + if (term == NULL) { + static bool have_warned = false; + if (!have_warned) { + LOG_WARN( + "%s: text-input::done() received on seat that isn't " + "focusing a terminal window", seat->name); + } + } + /* 1. Delete existing pre-edit text */ if (seat->ime.preedit.cells != NULL) { ime_reset_preedit(seat); From 6ae8ed7e6be3614fd3b621d690d881a79ec789f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Mar 2021 14:06:10 +0100 Subject: [PATCH 13/32] =?UTF-8?q?changelog:=20don=E2=80=99t=20crash=20upon?= =?UTF-8?q?=20receiving=20text-input::enter=20without=20keyboard=20focus?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff203ed7..04b0fe7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,9 @@ decreasing the font size at run-time. * Newlines sometimes incorrectly inserted into copied text (https://codeberg.org/dnkl/foot/issues/410). +* Crash when compositor send `text-input-v3::enter` events without + first having sent a `keyboard::enter` event + (https://codeberg.org/dnkl/foot/issues/411). ### Security From ebb92a4db6e7c3484b918c07092aa381103bada7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Mar 2021 17:30:56 +0100 Subject: [PATCH 14/32] =?UTF-8?q?ime:=20set=20=E2=80=98have=5Fwarned=20=3D?= =?UTF-8?q?=20true=E2=80=99=20to=20avoid=20warning=20over=20and=20over=20a?= =?UTF-8?q?gain?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ime.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ime.c b/ime.c index 58aecf7f..91a746e9 100644 --- a/ime.c +++ b/ime.c @@ -117,6 +117,7 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, LOG_WARN( "%s: text-input::done() received on seat that isn't " "focusing a terminal window", seat->name); + have_warned = true; } } From 8111ff4be8335c73b93717ba6b948ee57395a740 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 24 Mar 2021 20:46:51 +0100 Subject: [PATCH 15/32] render: draw sixels before taking the render worker lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes a possible deadlock; render_sixels_images() may call render_cell(), which may need to take the worker lock (when rendering either a blinking cell, or a box drawing glyph that isn’t yet in the glyph cache). --- CHANGELOG.md | 1 + render.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04b0fe7f..7c7a6b1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ * Crash when compositor send `text-input-v3::enter` events without first having sent a `keyboard::enter` event (https://codeberg.org/dnkl/foot/issues/411). +* Deadlock when rendering sixel images. ### Security diff --git a/render.c b/render.c index dd8070e0..d8e1a009 100644 --- a/render.c +++ b/render.c @@ -2090,6 +2090,8 @@ grid_render(struct terminal *term) cursor.row &= term->grid->num_rows - 1; } + render_sixel_images(term, buf->pix[0], &cursor); + if (term->render.workers.count > 0) { mtx_lock(&term->render.workers.lock); term->render.workers.buf = buf; @@ -2099,8 +2101,6 @@ grid_render(struct terminal *term) xassert(tll_length(term->render.workers.queue) == 0); } - render_sixel_images(term, buf->pix[0], &cursor); - int first_dirty_row = -1; for (int r = 0; r < term->rows; r++) { struct row *row = grid_row_in_view(term->grid, r); From 5b8b3baa6537d56de5a4c78dfd505a851da25aa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 24 Mar 2021 20:51:18 +0100 Subject: [PATCH 16/32] render: render_osd(): no need to pass width/height as parameters All uses of render_osd() passes buf->width/buf->height as width/height. Thus, we can simply remove the width/height parameters and have render_osd() use buf->width and buf->height directly. --- render.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/render.c b/render.c index d8e1a009..f250560a 100644 --- a/render.c +++ b/render.c @@ -1751,12 +1751,12 @@ render_osd(struct terminal *term, struct wl_surface *surf, struct wl_subsurface *sub_surf, struct buffer *buf, const wchar_t *text, uint32_t _fg, uint32_t _bg, - unsigned width, unsigned height, unsigned x, unsigned y) + unsigned x, unsigned y) { pixman_color_t bg = color_hex_to_pixman(_bg); pixman_image_fill_rectangles( PIXMAN_OP_SRC, buf->pix[0], &bg, 1, - &(pixman_rectangle16_t){0, 0, width, height}); + &(pixman_rectangle16_t){0, 0, buf->width, buf->height}); struct fcft_font *font = term->fonts[0]; pixman_color_t fg = color_hex_to_pixman(_fg); @@ -1782,12 +1782,12 @@ render_osd(struct terminal *term, quirk_weston_subsurface_desync_on(sub_surf); wl_surface_attach(surf, buf->wl_buf, 0, 0); - wl_surface_damage_buffer(surf, 0, 0, width, height); + wl_surface_damage_buffer(surf, 0, 0, buf->width, buf->height); wl_surface_set_buffer_scale(surf, term->scale); struct wl_region *region = wl_compositor_create_region(term->wl->compositor); if (region != NULL) { - wl_region_add(region, 0, 0, width, height); + wl_region_add(region, 0, 0, buf->width, buf->height); wl_surface_set_opaque_region(surf, region); wl_region_destroy(region); } @@ -1916,7 +1916,7 @@ render_scrollback_position(struct terminal *term) win->scrollback_indicator.sub, buf, text, term->colors.table[0], term->colors.table[8 + 4], - width, height, width - margin - wcslen(text) * term->cell_width, margin); + width - margin - wcslen(text) * term->cell_width, margin); } @@ -1949,7 +1949,7 @@ render_render_timer(struct terminal *term, struct timeval render_time) win->render_timer.sub, buf, text, term->colors.table[0], term->colors.table[8 + 1], - width, height, margin, margin); + margin, margin); } static void frame_callback( @@ -2670,8 +2670,8 @@ render_urls(struct terminal *term) ? term->conf->colors.jump_label.bg : term->colors.table[3]; - render_osd(term, surf, sub_surf, buf, label, - fg, bg, width, height, x_margin, y_margin); + render_osd( + term, surf, sub_surf, buf, label, fg, bg, x_margin, y_margin); } } From 1a0f13640e6f007cd08220dc54fed8b6dd8eec3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 24 Mar 2021 20:52:58 +0100 Subject: [PATCH 17/32] render: make sure surface buffer sizes are a multiple of the scaling factor The buffer attached to a surface with wl_surface_attach() must have a width and height that both are a multiple of the scale configured for that buffer: The new size of the surface is calculated based on the buffer size transformed by the inverse buffer_transform and the inverse buffer_scale. This means that at commit time the supplied buffer size must be an integer multiple of the buffer_scale. If that's not the case, an invalid_size error is sent. Due to a libwayland bug[^1], this is currently *not* being reported as an error. However, recent versions of Sway have started enforcing this, and is e.g. dropping (not rendering) sub-surfaces that does not adhere to this. [^1]: https://gitlab.freedesktop.org/wayland/wayland/-/issues/194 Closes #409 --- render.c | 52 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/render.c b/render.c index f250560a..b857e360 100644 --- a/render.c +++ b/render.c @@ -1409,6 +1409,9 @@ get_csd_data(const struct terminal *term, enum csd_surface surf_idx) static void csd_commit(struct terminal *term, struct wl_surface *surf, struct buffer *buf) { + xassert(buf->width % term->scale == 0); + xassert(buf->height % term->scale == 0); + wl_surface_attach(surf, buf->wl_buf, 0, 0); wl_surface_damage_buffer(surf, 0, 0, buf->width, buf->height); wl_surface_set_buffer_scale(surf, term->scale); @@ -1440,6 +1443,9 @@ render_csd_title(struct terminal *term) xassert(info.width > 0 && info.height > 0); + 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); @@ -1472,6 +1478,9 @@ render_csd_border(struct terminal *term, enum csd_surface surf_idx) if (info.width == 0 || info.height == 0) return; + 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); @@ -1641,6 +1650,9 @@ render_csd_button(struct terminal *term, enum csd_surface surf_idx) if (info.width == 0 || info.height == 0) return; + 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); @@ -1780,6 +1792,9 @@ render_osd(struct terminal *term, x += term->cell_width; } + xassert(buf->width % term->scale == 0); + xassert(buf->height % term->scale == 0); + quirk_weston_subsurface_desync_on(sub_surf); wl_surface_attach(surf, buf->wl_buf, 0, 0); wl_surface_damage_buffer(surf, 0, 0, buf->width, buf->height); @@ -1873,8 +1888,11 @@ render_scrollback_position(struct terminal *term) const int scale = term->scale; const int margin = 3 * scale; - const int width = 2 * margin + cell_count * term->cell_width; - const int height = 2 * margin + term->cell_height; + + const int width = + (2 * margin + cell_count * term->cell_width + scale - 1) / scale * scale; + 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( @@ -1929,10 +1947,13 @@ render_render_timer(struct terminal *term, struct timeval render_time) double usecs = render_time.tv_sec * 1000000 + render_time.tv_usec; swprintf(text, sizeof(text) / sizeof(text[0]), L"%.2f µs", usecs); + const int scale = term->scale; const int cell_count = wcslen(text); - const int margin = 3 * term->scale; - const int width = 2 * margin + cell_count * term->cell_width; - const int height = 2 * margin + term->cell_height; + const int margin = 3 * scale; + const int width = + (2 * margin + cell_count * term->cell_width + scale - 1) / scale * scale; + 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( @@ -2227,6 +2248,9 @@ grid_render(struct terminal *term) term->window->surface, 0, 0, INT32_MAX, INT32_MAX); } + xassert(buf->width % term->scale == 0); + xassert(buf->height % term->scale == 0); + wl_surface_attach(term->window->surface, buf->wl_buf, 0, 0); quirk_kde_damage_before_attach(term->window->surface); wl_surface_commit(term->window->surface); @@ -2298,10 +2322,10 @@ render_search_box(struct terminal *term) const size_t width = term->width - 2 * margin; const size_t visible_width = min( term->width - 2 * margin, - 2 * margin + wanted_visible_cells * term->cell_width); + (2 * margin + wanted_visible_cells * term->cell_width + scale - 1) / scale * scale); const size_t height = min( term->height - 2 * margin, - 2 * margin + 1 * term->cell_height); + (2 * margin + 1 * term->cell_height + scale - 1) / scale * scale); const size_t visible_cells = (visible_width - 2 * margin) / term->cell_width; size_t glyph_offset = term->render.search_glyph_offset; @@ -2527,6 +2551,9 @@ render_search_box(struct terminal *term) margin / scale, max(0, (int32_t)term->height - height - margin) / scale); + xassert(buf->width % scale == 0); + xassert(buf->height % scale == 0); + wl_surface_attach(term->window->search.surf, buf->wl_buf, 0, 0); wl_surface_damage_buffer(term->window->search.surf, 0, 0, width, height); wl_surface_set_buffer_scale(term->window->search.surf, scale); @@ -2634,10 +2661,13 @@ render_urls(struct terminal *term) size_t len = wcslen(label); int cols = wcswidth(label, len); - const int x_margin = 2 * term->scale; - const int y_margin = 1 * term->scale; - int width = 2 * x_margin + cols * term->cell_width; - int height = 2 * y_margin + term->cell_height; + const int scale = term->scale; + const int x_margin = 2 * scale; + const int y_margin = 1 * scale; + const int width = + (2 * x_margin + cols * term->cell_width + scale - 1) / scale * scale; + 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); From a8de14c0bf3ddeaa65e098f20dfbdd4a41d76f5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 25 Mar 2021 09:42:15 +0100 Subject: [PATCH 18/32] changelog: sub-surface sizes not a multiple of the scaling factor --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c7a6b1f..53f95872 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,10 @@ first having sent a `keyboard::enter` event (https://codeberg.org/dnkl/foot/issues/411). * Deadlock when rendering sixel images. +* URL labels, scrollback search box or scrollback position indicator + sometimes not showing up, caused by invalidly sized surface buffers + when output scaling was enabled + (https://codeberg.org/dnkl/foot/issues/409). ### Security From 9b20764f35094963ecbfc40a972ef54443630447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:30:13 +0100 Subject: [PATCH 19/32] features: --version now logs +/-pgo That is, whether the binary was compiled with PGO or not. --- client.c | 6 ++++-- foot-features.h | 9 +++++++++ main.c | 6 ++++-- meson.build | 3 +++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/client.c b/client.c index 975269b1..93dcc666 100644 --- a/client.c +++ b/client.c @@ -35,8 +35,10 @@ static const char * version_and_features(void) { static char buf[256]; - snprintf(buf, sizeof(buf), "version: %s %cime", - FOOT_VERSION, feature_ime() ? '+' : '-'); + snprintf(buf, sizeof(buf), "version: %s %cime %cpgo", + FOOT_VERSION, + feature_ime() ? '+' : '-', + feature_pgo() ? '+' : '-'); return buf; } diff --git a/foot-features.h b/foot-features.h index 8da22f6c..ae00c564 100644 --- a/foot-features.h +++ b/foot-features.h @@ -10,3 +10,12 @@ static inline bool feature_ime(void) return false; #endif } + +static inline bool feature_pgo(void) +{ +#if defined(FOOT_PGO_ENABLED) && FOOT_PGO_ENABLED + return true; +#else + return false; +#endif +} diff --git a/main.c b/main.c index 0941d3db..2c14d6d0 100644 --- a/main.c +++ b/main.c @@ -45,8 +45,10 @@ static const char * version_and_features(void) { static char buf[256]; - snprintf(buf, sizeof(buf), "version: %s %cime", - FOOT_VERSION, feature_ime() ? '+' : '-'); + snprintf(buf, sizeof(buf), "version: %s %cime %cpgo", + FOOT_VERSION, + feature_ime() ? '+' : '-', + feature_pgo() ? '+' : '-'); return buf; } diff --git a/meson.build b/meson.build index 01f27be9..e1b23102 100644 --- a/meson.build +++ b/meson.build @@ -24,6 +24,9 @@ add_project_arguments( (get_option('ime') ? ['-DFOOT_IME_ENABLED=1'] : []) + + (get_option('b_pgo') == 'use' + ? ['-DFOOT_PGO_ENABLED=1'] + : []) + cc.get_supported_arguments( ['-pedantic', '-fstrict-aliasing', From c6fb10863d0f985c1b9eb6a859086e761d393547 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:31:09 +0100 Subject: [PATCH 20/32] meson: build source files common to both foot and footclient as libraries Source files used by both foot and footclient are now compiled as static libraries, which foot and footclient links against. --- meson.build | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/meson.build b/meson.build index e1b23102..237d99c6 100644 --- a/meson.build +++ b/meson.build @@ -117,16 +117,18 @@ version = custom_target( output: 'version.h', command: [generate_version_sh, meson.project_version(), '@SOURCE_ROOT@', '@OUTPUT@']) +log = static_library('log', 'log.c', 'log.h') +debug = static_library('debug', 'debug.c', 'debug.h') + +xmalloc = static_library( + 'xmalloc', 'xmalloc.c', 'xmalloc.h', 'xsnprintf.c', 'xsnprintf.h') + misc = static_library( 'misc', - 'debug.c', 'debug.h', 'hsl.c', 'hsl.h', - 'log.c', 'log.h', 'macros.h', 'misc.c', 'misc.h', - 'uri.c', 'uri.h', - 'xmalloc.c', 'xmalloc.h', - 'xsnprintf.c', 'xsnprintf.h', + 'uri.c', 'uri.h' ) vtlib = static_library( @@ -140,7 +142,7 @@ vtlib = static_library( wl_proto_src + wl_proto_headers, version, dependencies: [libepoll, pixman, fcft, tllist, wayland_client], - link_with: misc, + link_with: [log, debug, misc, xmalloc], ) pgolib = static_library( @@ -195,14 +197,11 @@ executable( executable( 'footclient', 'client.c', 'client-protocol.h', - 'debug.c', 'debug.h', 'foot-features.h', - 'log.c', 'log.h', 'macros.h', 'util.h', - 'xmalloc.c', 'xmalloc.h', - 'xsnprintf.c', 'xsnprintf.h', version, + link_with: [log, debug, xmalloc], install: true) if tic.found() From ae6a656f4963b8cc5bde13fa347cfd4b76a4d4a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:34:18 +0100 Subject: [PATCH 21/32] meson: only build the pgo helper binary when -Db_pgo=generate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the only time it’s needed. In non-PGO builds, it is completely unnecessary, and we’re only wasting CPU cycles building it. In full PGO builds, with -Db_pgo=use, we get link warnings and/or failures, depending on compiler, since the pgo binary hasn’t been executed. --- meson.build | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/meson.build b/meson.build index 237d99c6..54e5b315 100644 --- a/meson.build +++ b/meson.build @@ -155,13 +155,15 @@ pgolib = static_library( link_with: vtlib, ) -executable( - 'pgo', - 'pgo/pgo.c', - wl_proto_src + wl_proto_headers, - dependencies: [math, threads, libepoll, pixman, wayland_client, fcft, tllist], - link_with: pgolib, -) +if get_option('b_pgo') == 'generate' + executable( + 'pgo', + 'pgo/pgo.c', + wl_proto_src + wl_proto_headers, + dependencies: [math, threads, libepoll, pixman, wayland_client, fcft, tllist], + link_with: pgolib, + ) +endif executable( 'foot', From 628382bc90f0bed2574752b2317ab25415a38213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:36:34 +0100 Subject: [PATCH 22/32] PKGBUILD: do PGO with either gcc or clang, but nothing else MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this, we assumed gcc was being used. The build would fail if clang (or something else) was used. Now, we check whether we’re compiling with gcc or clang, and disable PGO if it’s neither of them. Furthermore, we’re now adding the necessary flags needed to do PGO with clang. Note that ‘llvm-profdata’, from the ‘llvm’ package is needed for PGO:ing with clang. PGO is disabled if it isn’t available. It would be nice to have ‘llvm’ as an optional make dependency, but PKGBUILDs doesn’t appear to support such a thing. Finally, the -Wno-missing-profile flag, and its friends, have been removed; instead we execute “footclient --version” (and “foot --version”, in partial PGO builds) to ensure all generated binaries have been executed before we do the final build (with -Db_pgo=use). This fixes build failures with clang >= 11.x. --- PKGBUILD | 77 ++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 22 deletions(-) diff --git a/PKGBUILD b/PKGBUILD index d5a99516..67894b80 100644 --- a/PKGBUILD +++ b/PKGBUILD @@ -14,37 +14,70 @@ pkgver() { } build() { + local compiler=other + local do_pgo=no + # makepkg uses -O2 by default, but we *really* want -O3 - # -Wno-missing-profile since we're not exercising everything when doing PGO builds - export CFLAGS+=" -O3 -Wno-missing-profile" + CFLAGS+=" -O3" + + # Figure out which compiler we're using, and whether or not to do PGO + case $(${CC-cc} --version) in + *GCC*) + compiler=gcc + do_pgo=yes + ;; + + *clang*) + compiler=clang + + # We need llvm to be able to manage the profiling data + if command -v llvm-profdata > /dev/null; then + do_pgo=yes + + # Meson adds -fprofile-correction, which Clang doesn't + # understand + CFLAGS+=" -Wno-ignored-optimization-argument" + fi + ;; + esac meson --prefix=/usr --buildtype=release --wrap-mode=nofallback -Db_lto=true .. - find -name "*.gcda" -delete - meson configure -Db_pgo=generate - ninja + if [[ ${do_pgo} == yes ]]; then + find -name "*.gcda" -delete + meson configure -Db_pgo=generate + ninja - script_options="--scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline --sixel" + local script_options="--scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline --sixel" - tmp_file=$(mktemp) + tmp_file=$(mktemp) - if [[ -v WAYLAND_DISPLAY ]]; then - ./foot \ - --config /dev/null \ - --term=xterm \ - sh -c "../scripts/generate-alt-random-writes.py ${script_options} ${tmp_file} && cat ${tmp_file}" - else - ../scripts/generate-alt-random-writes.py \ - --rows=67 \ - --cols=135 \ - ${script_options} \ - ${tmp_file} - ./pgo ${tmp_file} ${tmp_file} ${tmp_file} + if [[ -v WAYLAND_DISPLAY ]]; then + ./footclient --version + ./foot \ + --config /dev/null \ + --term=xterm \ + sh -c "../scripts/generate-alt-random-writes.py ${script_options} ${tmp_file} && cat ${tmp_file}" + else + ./footclient --version + ./foot --version + ../scripts/generate-alt-random-writes.py \ + --rows=67 \ + --cols=135 \ + ${script_options} \ + ${tmp_file} + ./pgo ${tmp_file} ${tmp_file} ${tmp_file} + fi + + rm "${tmp_file}" + + if [[ ${compiler} == clang ]]; then + llvm-profdata merge default_*profraw --output=default.profdata + fi + + meson configure -Db_pgo=use fi - rm "${tmp_file}" - - meson configure -Db_pgo=use ninja } From 243578d308e2ce27fed24250dac5b363be1c05c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:43:03 +0100 Subject: [PATCH 23/32] =?UTF-8?q?install:=20add=20=E2=80=98llvm=E2=80=99?= =?UTF-8?q?=20as=20an=20optional=20build=20dependency?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is needed for PGO builds with Clang. --- INSTALL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/INSTALL.md b/INSTALL.md index 1cb0d9a7..33f6aed7 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -65,6 +65,7 @@ In addition to the dev variant of the packages above, you need: * wayland protocols * ncurses (needed to generate terminfo) * scdoc (for man page generation) +* llvm (for PGO builds with Clang) * [tllist](https://codeberg.org/dnkl/tllist) [^1] A note on compilers; in general, foot runs **much** faster when From 3fd9256c0207cd7ff0b36b69282d70463be4dfb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:43:21 +0100 Subject: [PATCH 24/32] install: clang-11.x compatible PGO instructions LLVM/Clang 11.x throws errors for _files_ without any profiling data: error: ../../async.c: Function control flow change detected (hash mismatch) async_write Hash = 72057641800614222 [-Werror,-Wbackend-plugin] This caused multiple issues with the current PGO instructions: * The `pgo` binary failed to link with `meson configure -Db_pgo=use` when doing a full PGO build (since it isn't used in this case); * `footclient` fails to link for the same reason. * `foot` fails to link for the same reason in **partial** PGO builds. The solution is to make sure all binaries that are built in the final phase (`-Db_pgo=use`) have been executed in the _generate_ phase. Doing this also means we can drop `-Wno-missing-profile` and friends. Also add `--sixel` to the `generate-alt-random-writes` command line in the description for a full PGO build. Closes #418 --- INSTALL.md | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index 33f6aed7..9fa651e1 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -185,21 +185,15 @@ slower!) binary. First, configure the build directory: ```sh -export CFLAGS="$CFLAGS -O3 -Wno-missing-profile" +export CFLAGS="$CFLAGS -O3" meson --buildtype=release --prefix=/usr -Db_lto=true ../.. ``` It is **very** important `-O3` is being used here, as GCC-10.1.x and later have a regression where PGO with `-O2` is **much** slower. -If you are using Clang instead of GCC, use the following `CFLAGS` instead: - -```sh -export CFLAGS="$CFLAGS -O3 \ - -Wno-ignored-optimization-argument \ - -Wno-profile-instr-out-of-date \ - -Wno-profile-instr-unprofiled" -``` +Clang users **must** add `-Wno-ignored-optimization-argument` to +`CFLAGS`. Then, tell meson we want to _generate_ profiling data, and build: @@ -235,6 +229,8 @@ We will use the `pgo` binary along with input corpus generated by `scripts/generate-alt-random-writes.py`: ```sh +./footclient --version +./foot --version tmp_file=$(mktemp) ../../scripts/generate-alt-random-writes \ --rows=67 \ @@ -254,7 +250,12 @@ tmp_file=$(mktemp) rm ${tmp_file} ``` -The snippet above first creates an (empty) temporary file. Then, it +The first step, running `./foot --version` and `./footclient +--version` might seem unnecessary, but is needed to ensure we have +_some_ profiling data for functions not covered by the PGO helper +binary. Without this, the final link phase will fail. + +The snippet above then creates an (empty) temporary file. Then, it runs a script that generates random escape sequences (if you cat `${tmp_file}` in a terminal, you’ll see random colored characters all over the screen). Finally, we feed the randomly generated escape @@ -272,14 +273,19 @@ This method requires a running Wayland session. We will use the script `scripts/generate-alt-random-writes.py`: ```sh +./footclient --version foot_tmp_file=$(mktemp) -./foot --config=/dev/null --term=xterm sh -c " --scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline ${foot_tmp_file} && cat ${foot_tmp_file}" +./foot --config=/dev/null --term=xterm sh -c " --scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline --sixel ${foot_tmp_file} && cat ${foot_tmp_file}" rm ${foot_tmp_file} ``` You should see a foot window open up, with random colored text. The window should close after ~1-2s. +The first step, `./footclient --version` might seem unnecessary, but +is needed to ensure we have _some_ profiling data for +`footclient`. Without this, the final link phase will fail. + ##### Use the generated PGO data From 9786197d036e70ebe51c33a40f4bf2489f29e115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 21:00:41 +0100 Subject: [PATCH 25/32] changelog: updated PGO build instructions --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53f95872..0b10e083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,11 @@ ## Unreleased ### Added ### Changed + +* Update PGO build instructions in `INSTALL.md` + (https://codeberg.org/dnkl/foot/issues/418). + + ### Deprecated ### Removed ### Fixed From 65012609b8379cdbdab4ea39c66955476ed6722e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 27 Mar 2021 12:28:39 +0100 Subject: [PATCH 26/32] readme: dpi & fonts: focus on *current* behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The text was describing the original behavior, where output scaling was completely ignored, and tacked on a “btw, starting with foot-1.6...”. Now, simply describe the current behavior, and be more clear about _how_ dpi-aware can be changed. --- README.md | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 2c3281ad..644b2b55 100644 --- a/README.md +++ b/README.md @@ -345,22 +345,23 @@ This is not how it is meant to be. Fonts are measured in _point sizes_ **for a reason**; a given point size should have the same height on all mediums, be it printers or monitors, regardless of their DPI. -Foot will always use the monitor's physical DPI value. Scale factors -are irrelevant (well, they affect e.g. padding, but not the font -size). This means the glyphs rendered by foot should always have the -same physical height, regardless of monitor. +Foot’s default behavior is to use the monitor’s DPI to size fonts when +output scaling has been disabled. On monitors where output scaling has +been enabled, fonts will instead be sized using the scaling +factor. -Foot will re-size the fonts on-the-fly when the window is moved -between screens with different DPIs values. If the window covers +This can be changed to either **always** use the monitor’s DPI +(regardless of scaling factor), or to **never** use it. See the +`dpi-aware` option in `foot.ini`. See the man page, **foot.ini**(5) +for more information. + +When fonts are sized using the monitor’s DPI, glyphs should always +have the same physical height, regardless of monitor. + +Furthermore, foot will re-size the fonts on-the-fly when the window is +moved between screens with different DPIs values. If the window covers multiple screens, with different DPIs, the highest DPI will be used. -Starting with foot-1.6, the _default_ behavior is to use the monitor’s -DPI to size fonts when output scaling has been disabled. On monitors -where output scaling has been enabled, fonts will instead be sized -using the scaling factor. This can be changed with the `dpi-aware` -option in `foot.ini`. See the man page, **foot.ini**(5) for more -information. - _Note_: if you configure **pixelsize**, rather than **size**, then DPI changes will **not** change the font size. Pixels are always pixels. From b5ceed7b2bfa41584e17d156ecac231dde195798 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 27 Mar 2021 13:17:43 +0100 Subject: [PATCH 27/32] =?UTF-8?q?meson:=20replace=20log+debug+xmalloc=20st?= =?UTF-8?q?atic=20libraries=20with=20a=20single=20=E2=80=98common=E2=80=99?= =?UTF-8?q?=20library?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are cyclic dependencies between the log, debug and xmalloc libraries. While having them as separate static libraries work, as long as consumers of them link against all of them, having them in a single library feels slightly better. --- meson.build | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/meson.build b/meson.build index 54e5b315..9fbf99a6 100644 --- a/meson.build +++ b/meson.build @@ -117,11 +117,13 @@ version = custom_target( output: 'version.h', command: [generate_version_sh, meson.project_version(), '@SOURCE_ROOT@', '@OUTPUT@']) -log = static_library('log', 'log.c', 'log.h') -debug = static_library('debug', 'debug.c', 'debug.h') - -xmalloc = static_library( - 'xmalloc', 'xmalloc.c', 'xmalloc.h', 'xsnprintf.c', 'xsnprintf.h') +common = static_library( + 'common', + 'log.c', 'log.h', + 'debug.c', 'debug.h', + 'xmalloc.c', 'xmalloc.h', + 'xsnprintf.c', 'xsnprintf.h' +) misc = static_library( 'misc', @@ -142,7 +144,7 @@ vtlib = static_library( wl_proto_src + wl_proto_headers, version, dependencies: [libepoll, pixman, fcft, tllist, wayland_client], - link_with: [log, debug, misc, xmalloc], + link_with: [common, misc], ) pgolib = static_library( @@ -203,7 +205,7 @@ executable( 'macros.h', 'util.h', version, - link_with: [log, debug, xmalloc], + link_with: common, install: true) if tic.found() From 2b7c5db1880b54120841c441518108795cb85815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 27 Mar 2021 22:15:44 +0100 Subject: [PATCH 28/32] search: when matching cell content, treat empty cells as spaces --- search.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/search.c b/search.c index 30a50ae5..02cea9e0 100644 --- a/search.c +++ b/search.c @@ -241,6 +241,9 @@ matches_cell(const struct terminal *term, const struct cell *cell, size_t search base = composed->base; } + if (composed == NULL && base == 0 && term->search.buf[search_ofs] == L' ') + return 1; + if (wcsncasecmp(&base, &term->search.buf[search_ofs], 1) != 0) return -1; From 0a24d0e40fe9e9a9bfb1dd1f4715f91af8a6168e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 27 Mar 2021 23:02:56 +0100 Subject: [PATCH 29/32] changelog: spaces matches empty cells in scrollback search mode --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b10e083..dec9f961 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ * Update PGO build instructions in `INSTALL.md` (https://codeberg.org/dnkl/foot/issues/418). +* In scrollback search mode, empty cells can now be matched by spaces. ### Deprecated From 3566be591a971a118dca6edc263dd7eefaa5f648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 28 Mar 2021 12:55:09 +0200 Subject: [PATCH 30/32] sixel: initialize max_non_empty_row_no to -1, not 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0 is a perfectly valid row number, and if max_non_empty_row_no==0, that means we have *1* sixel row, and after trimming the image, the image will have a height of 6 pixels. If the sixel sequence is empty (or at least doesn’t emit any non-empty pixels), then trimming the image should result in an image height of 0. When max_non_empty_row_no is initialized to -1, it will still have that value in unhook(), which makes the final image height 0. --- sixel.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/sixel.c b/sixel.c index 3953f735..c856122e 100644 --- a/sixel.c +++ b/sixel.c @@ -48,7 +48,7 @@ sixel_init(struct terminal *term, int p1, int p2, int p3) term->sixel.state = SIXEL_DECSIXEL; term->sixel.pos = (struct coord){0, 0}; - term->sixel.max_non_empty_row_no = 0; + term->sixel.max_non_empty_row_no = -1; term->sixel.row_byte_ofs = 0; term->sixel.color_idx = 0; term->sixel.param = 0; @@ -1032,7 +1032,7 @@ sixel_add(struct terminal *term, int col, int width, uint32_t color, uint8_t six size_t ofs = term->sixel.row_byte_ofs + col; uint32_t *data = &term->sixel.image.data[ofs]; - int max_non_empty_row = 0; + int max_non_empty_row = -1; int row = term->sixel.pos.row; for (int i = 0; i < 6; i++, sixel >>= 1, data += width) { @@ -1175,7 +1175,11 @@ decgra(struct terminal *term, uint8_t c) ph <= term->sixel.max_height && pv <= term->sixel.max_width) { resize(term, ph, pv); - term->sixel.max_non_empty_row_no = pv - 1; + if (!term->sixel.transparent_bg) { + /* This ensures the sixel’s final image size is *at + * least* this large */ + term->sixel.max_non_empty_row_no = pv - 1; + } } term->sixel.state = SIXEL_DECSIXEL; From 19289bad5ef60f4c5ca2b45c0e495db60ed34267 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 28 Mar 2021 12:57:49 +0200 Subject: [PATCH 31/32] sixel: free backing buffer if final image size is zero --- sixel.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sixel.c b/sixel.c index c856122e..cac60493 100644 --- a/sixel.c +++ b/sixel.c @@ -726,6 +726,11 @@ sixel_unhook(struct terminal *term) term->sixel.image.height = term->sixel.max_non_empty_row_no + 1; } + if (term->sixel.image.height == 0 || term->sixel.image.width == 0) { + /* We won’t be emitting any sixels - free backing image */ + free(term->sixel.image.data); + } + int pixel_row_idx = 0; int pixel_rows_left = term->sixel.image.height; const int stride = term->sixel.image.width * sizeof(uint32_t); From efbbcf289f28f61b936b94a8daab84fd04158000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 28 Mar 2021 13:04:15 +0200 Subject: [PATCH 32/32] changelog: empty sixels resulted in non-empty images --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b10e083..38d2a395 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ sometimes not showing up, caused by invalidly sized surface buffers when output scaling was enabled (https://codeberg.org/dnkl/foot/issues/409). +* Empty sixels resulted in non-empty images. ### Security