From 535c82d628909b4f54774885e7de366c07fa9779 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 15 Jun 2021 17:27:50 +0200 Subject: [PATCH 1/2] render: use a timer instead of relying on the frame callback for title update throttling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using the frame callback works most of the time, but e.g. Sway doesn’t call it while the window is hidden, and thus prevents us from updating the title in e.g. stacked views. This patch uses a timer FD instead. We store a timestamp from when the title was last updated. When the application wants to update the title, we first check if we already have a timer running, and if so, does nothing. If no timer is running, check the timestamp. If enough time has passed, update the title immediately. If not, instantiate a timer and wait for it to trigger. Set the minimum time between two updates to ~8ms (twice per frame, for a 60Hz output, and ~once per frame on a 120Hz output). Closes #591 --- CHANGELOG.md | 2 ++ render.c | 63 +++++++++++++++++++++++++++++++++++++++++----------- terminal.c | 10 ++++++--- terminal.h | 7 ++++-- 4 files changed, 64 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7e2f51a..6ea8eec3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -184,6 +184,8 @@ (https://codeberg.org/dnkl/foot/issues/586). * Crash in scrollback search when current XKB layout is missing _compose_ definitions. +* Window title not being updated while window is hidden + (https://codeberg.org/dnkl/foot/issues/591). ### Security diff --git a/render.c b/render.c index bebb479a..20af86aa 100644 --- a/render.c +++ b/render.c @@ -2992,13 +2992,11 @@ frame_callback(void *data, struct wl_callback *wl_callback, uint32_t callback_da bool grid = term->render.pending.grid; bool csd = term->render.pending.csd; bool search = term->is_searching && term->render.pending.search; - bool title = term->render.pending.title; bool urls = urls_mode_is_active(term) > 0 && term->render.pending.urls; term->render.pending.grid = false; term->render.pending.csd = false; term->render.pending.search = false; - term->render.pending.title = false; term->render.pending.urls = false; struct grid *original_grid = term->grid; @@ -3013,9 +3011,6 @@ frame_callback(void *data, struct wl_callback *wl_callback, uint32_t callback_da quirk_weston_csd_off(term); } - if (title) - render_update_title(term); - if (search) render_search_box(term); @@ -3447,19 +3442,17 @@ fdm_hook_refresh_pending_terminals(struct fdm *fdm, void *data) bool grid = term->render.refresh.grid; bool csd = term->render.refresh.csd; bool search = term->is_searching && term->render.refresh.search; - bool title = term->render.refresh.title; bool urls = urls_mode_is_active(term) && term->render.refresh.urls; - if (!(grid | csd | search | title | urls)) + if (!(grid | csd | search | urls)) continue; - if (term->render.app_sync_updates.enabled && !(csd | search | title | urls)) + if (term->render.app_sync_updates.enabled && !(csd | search | urls)) continue; term->render.refresh.grid = false; term->render.refresh.csd = false; term->render.refresh.search = false; - term->render.refresh.title = false; term->render.refresh.urls = false; if (term->window->frame_callback == NULL) { @@ -3474,8 +3467,6 @@ fdm_hook_refresh_pending_terminals(struct fdm *fdm, void *data) render_csd(term); quirk_weston_csd_off(term); } - if (title) - render_update_title(term); if (search) render_search_box(term); if (urls) @@ -3494,7 +3485,6 @@ fdm_hook_refresh_pending_terminals(struct fdm *fdm, void *data) term->render.pending.grid |= grid; term->render.pending.csd |= csd; term->render.pending.search |= search; - term->render.pending.title |= title; term->render.pending.urls |= urls; } } @@ -3511,10 +3501,57 @@ fdm_hook_refresh_pending_terminals(struct fdm *fdm, void *data) } } +static bool +fdm_title_update(struct fdm *fdm, int fd, int events, void *data) +{ + struct terminal *term = data; + fdm_del(fdm, fd); + + term->render.title.timer_fd = -1; + render_update_title(term); + + struct timeval now; + if (gettimeofday(&now, NULL) == 0) + term->render.title.last_update = now; + + return true; +} + void render_refresh_title(struct terminal *term) { - term->render.refresh.title = true; + if (term->render.title.timer_fd >= 0) + return; + + struct timeval now; + if (gettimeofday(&now, NULL) < 0) + return; + + struct timeval diff; + timersub(&now, &term->render.title.last_update, &diff); + + if (diff.tv_sec == 0 && diff.tv_usec < 8333) { + int fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK); + if (fd < 0) + return; + + const struct itimerspec timeout = { + .it_value = {.tv_nsec = 8333 * 1000 - diff.tv_usec * 1000}, + }; + + if (timerfd_settime(fd, 0, &timeout, NULL) < 0) { + close(fd); + return; + } + + if (!fdm_add(term->fdm, fd, EPOLLIN, &fdm_title_update, term)) + close(fd); + else + term->render.title.timer_fd = fd; + } else { + term->render.title.last_update = now; + render_update_title(term); + } } void diff --git a/terminal.c b/terminal.c index e689f4f6..aef21d72 100644 --- a/terminal.c +++ b/terminal.c @@ -1113,6 +1113,9 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .render = { .scrollback_lines = conf->scrollback.lines, .app_sync_updates.timer_fd = app_sync_updates_fd, + .title = { + .timer_fd = -1, + }, .workers = { .count = conf->render_worker_count, .queue = tll_init(), @@ -1289,6 +1292,7 @@ term_shutdown(struct terminal *term) fdm_del(term->fdm, term->selection.auto_scroll.fd); fdm_del(term->fdm, term->render.app_sync_updates.timer_fd); + fdm_del(term->fdm, term->render.title.timer_fd); fdm_del(term->fdm, term->delayed_render_timer.lower_fd); fdm_del(term->fdm, term->delayed_render_timer.upper_fd); fdm_del(term->fdm, term->blink.fd); @@ -1304,6 +1308,7 @@ term_shutdown(struct terminal *term) term->selection.auto_scroll.fd = -1; term->render.app_sync_updates.timer_fd = -1; + term->render.title.timer_fd = -1; term->delayed_render_timer.lower_fd = -1; term->delayed_render_timer.upper_fd = -1; term->blink.fd = -1; @@ -1354,6 +1359,7 @@ term_destroy(struct terminal *term) fdm_del(term->fdm, term->selection.auto_scroll.fd); fdm_del(term->fdm, term->render.app_sync_updates.timer_fd); + fdm_del(term->fdm, term->render.title.timer_fd); fdm_del(term->fdm, term->delayed_render_timer.lower_fd); fdm_del(term->fdm, term->delayed_render_timer.upper_fd); fdm_del(term->fdm, term->cursor_blink.fd); @@ -2670,9 +2676,7 @@ term_enable_app_sync_updates(struct terminal *term) /* Disable pending refresh *iff* the grid is the *only* thing * scheduled to be re-rendered */ if (!term->render.refresh.csd && !term->render.refresh.search && - !term->render.refresh.title && - !term->render.pending.csd && !term->render.pending.search && - !term->render.pending.title) + !term->render.pending.csd && !term->render.pending.search) { term->render.refresh.grid = false; term->render.pending.grid = false; diff --git a/terminal.h b/terminal.h index b96621fd..c810c0f0 100644 --- a/terminal.h +++ b/terminal.h @@ -474,7 +474,6 @@ struct terminal { bool grid; bool csd; bool search; - bool title; bool urls; } refresh; @@ -483,13 +482,17 @@ struct terminal { bool grid; bool csd; bool search; - bool title; bool urls; } pending; bool margins; /* Someone explicitly requested a refresh of the margins */ bool urgency; /* Signal 'urgency' (paint borders red) */ + struct { + struct timeval last_update; + int timer_fd; + } title; + int scrollback_lines; /* Number of scrollback lines, from conf (TODO: move out from render struct?) */ struct { From 07b455e882f1925930963b643011a53906cadb9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 18 Jun 2021 15:53:47 +0200 Subject: [PATCH 2/2] =?UTF-8?q?render:=20don=E2=80=99t=20create/destroy=20?= =?UTF-8?q?the=20title=20update=20timer=20each=20time?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create it once when instantiating the terminal. Add a boolean to track whether the timer is running or not. --- render.c | 32 ++------------------------------ terminal.c | 49 ++++++++++++++++++++++++++++++++++++++++++------- terminal.h | 1 + 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/render.c b/render.c index 20af86aa..0bf09bd8 100644 --- a/render.c +++ b/render.c @@ -3501,26 +3501,10 @@ fdm_hook_refresh_pending_terminals(struct fdm *fdm, void *data) } } -static bool -fdm_title_update(struct fdm *fdm, int fd, int events, void *data) -{ - struct terminal *term = data; - fdm_del(fdm, fd); - - term->render.title.timer_fd = -1; - render_update_title(term); - - struct timeval now; - if (gettimeofday(&now, NULL) == 0) - term->render.title.last_update = now; - - return true; -} - void render_refresh_title(struct terminal *term) { - if (term->render.title.timer_fd >= 0) + if (term->render.title.is_armed) return; struct timeval now; @@ -3531,23 +3515,11 @@ render_refresh_title(struct terminal *term) timersub(&now, &term->render.title.last_update, &diff); if (diff.tv_sec == 0 && diff.tv_usec < 8333) { - int fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK); - if (fd < 0) - return; - const struct itimerspec timeout = { .it_value = {.tv_nsec = 8333 * 1000 - diff.tv_usec * 1000}, }; - if (timerfd_settime(fd, 0, &timeout, NULL) < 0) { - close(fd); - return; - } - - if (!fdm_add(term->fdm, fd, EPOLLIN, &fdm_title_update, term)) - close(fd); - else - term->render.title.timer_fd = fd; + timerfd_settime(term->render.title.timer_fd, 0, &timeout, NULL); } else { term->render.title.last_update = now; render_update_title(term); diff --git a/terminal.c b/terminal.c index aef21d72..1e1a7f78 100644 --- a/terminal.c +++ b/terminal.c @@ -552,6 +552,31 @@ fdm_app_sync_updates_timeout( return true; } +static bool +fdm_title_update_timeout(struct fdm *fdm, int fd, int events, void *data) +{ + if (events & EPOLLHUP) + return false; + + struct terminal *term = data; + uint64_t unused; + ssize_t ret = read(term->render.title.timer_fd, &unused, sizeof(unused)); + + if (ret < 0) { + if (errno == EAGAIN) + return true; + LOG_ERRNO("failed to read title update throttle timer"); + return false; + } + + struct itimerspec reset = {{0}}; + timerfd_settime(term->render.title.timer_fd, 0, &reset, NULL); + term->render.title.is_armed = false; + + render_refresh_title(term); + return true; +} + static bool initialize_render_workers(struct terminal *term) { @@ -980,6 +1005,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, int delay_lower_fd = -1; int delay_upper_fd = -1; int app_sync_updates_fd = -1; + int title_update_fd = -1; struct terminal *term = malloc(sizeof(*term)); if (unlikely(term == NULL)) { @@ -987,27 +1013,33 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, return NULL; } - if ((ptmx = posix_openpt(O_RDWR | O_NOCTTY)) == -1) { + if ((ptmx = posix_openpt(O_RDWR | O_NOCTTY)) < 0) { LOG_ERRNO("failed to open PTY"); goto close_fds; } - if ((flash_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK)) == -1) { + if ((flash_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK)) < 0) { LOG_ERRNO("failed to create flash timer FD"); goto close_fds; } - if ((delay_lower_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK)) == -1 || - (delay_upper_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK)) == -1) + if ((delay_lower_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK)) < 0 || + (delay_upper_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK)) < 0) { LOG_ERRNO("failed to create delayed rendering timer FDs"); goto close_fds; } - if ((app_sync_updates_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK)) == -1) + if ((app_sync_updates_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK)) < 0) { LOG_ERRNO("failed to create application synchronized updates timer FD"); goto close_fds; } + if ((title_update_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK)) < 0) + { + LOG_ERRNO("failed to create title update throttle timer FD"); + goto close_fds; + } + if (ioctl(ptmx, (unsigned int)TIOCSWINSZ, &(struct winsize){.ws_row = 24, .ws_col = 80}) < 0) { @@ -1032,7 +1064,8 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, if (!fdm_add(fdm, flash_fd, EPOLLIN, &fdm_flash, term) || !fdm_add(fdm, delay_lower_fd, EPOLLIN, &fdm_delayed_render, term) || !fdm_add(fdm, delay_upper_fd, EPOLLIN, &fdm_delayed_render, term) || - !fdm_add(fdm, app_sync_updates_fd, EPOLLIN, &fdm_app_sync_updates_timeout, term)) + !fdm_add(fdm, app_sync_updates_fd, EPOLLIN, &fdm_app_sync_updates_timeout, term) || + !fdm_add(fdm, title_update_fd, EPOLLIN, &fdm_title_update_timeout, term)) { goto err; } @@ -1114,7 +1147,8 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .scrollback_lines = conf->scrollback.lines, .app_sync_updates.timer_fd = app_sync_updates_fd, .title = { - .timer_fd = -1, + .is_armed = false, + .timer_fd = title_update_fd, }, .workers = { .count = conf->render_worker_count, @@ -1220,6 +1254,7 @@ close_fds: fdm_del(fdm, delay_lower_fd); fdm_del(fdm, delay_upper_fd); fdm_del(fdm, app_sync_updates_fd); + fdm_del(fdm, title_update_fd); free(term); return NULL; diff --git a/terminal.h b/terminal.h index c810c0f0..e6bcfa40 100644 --- a/terminal.h +++ b/terminal.h @@ -490,6 +490,7 @@ struct terminal { struct { struct timeval last_update; + bool is_armed; int timer_fd; } title;