From ba0d0e8bbb970599c1c0a13a27cc8bae71f60185 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Mon, 23 Mar 2020 19:16:53 +0100
Subject: [PATCH 1/2] term: delayed rendering: read timers even though is_armed
 = false

There's a race/chance that we'll have disarmed the delayed rendering
timers and still get the call.

While it _shouldn't_ be anything to read from the timers, it doesn't
hurt to try. And, if the timers *are* readable, not reading them means
we'll end up in an infinite FDM loop.
---
 terminal.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/terminal.c b/terminal.c
index b39bb8d9..4554ae20 100644
--- a/terminal.c
+++ b/terminal.c
@@ -396,8 +396,6 @@ fdm_delayed_render(struct fdm *fdm, int fd, int events, void *data)
         return false;
 
     struct terminal *term = data;
-    if (!term->delayed_render_timer.is_armed)
-        return true;
 
     uint64_t unused;
     ssize_t ret1 = 0;
@@ -412,6 +410,9 @@ fdm_delayed_render(struct fdm *fdm, int fd, int events, void *data)
         if (errno == EAGAIN)
             return true;
 
+        if (!term->delayed_render_timer.is_armed)
+            return true;
+
         LOG_ERRNO("failed to read timeout timer");
         return false;
     }
@@ -425,13 +426,15 @@ fdm_delayed_render(struct fdm *fdm, int fd, int events, void *data)
     last = (struct timespec){0};
 #endif
 
-    render_refresh(term);
-
     /* Reset timers */
     struct itimerspec reset = {{0}};
     timerfd_settime(term->delayed_render_timer.lower_fd, 0, &reset, NULL);
     timerfd_settime(term->delayed_render_timer.upper_fd, 0, &reset, NULL);
-    term->delayed_render_timer.is_armed = false;
+
+    if (term->delayed_render_timer.is_armed) {
+        term->delayed_render_timer.is_armed = false;
+        render_refresh(term);
+    }
 
     return true;
 }

From 5a972cb98ed372005168c808c7414865704007bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Mon, 23 Mar 2020 19:21:41 +0100
Subject: [PATCH 2/2] delayed rendering: ignore frame callback if delayed
 rendering is active

Before, we applied delayed rendering (that is, we gave the client a
chance to do more writes before we scheduled a render refresh) only
when the renderer were idle.

However, with e.g. a high keyboard repeat rate, it is very much
possible to start the render loop and then never break out of it while
receiving keyboard input.

This causes screen flickering, as we're no longer even trying to
detect the clients transaction boundaries.

So, let's rewrite how this is done.

First, we give the user the ability to disable delayed rendering
altogether, by setting either the lower or upper timeout to 0.

Second, when delayed rendering is enabled, we ignore the frame
callback. That is, when receiving input, we *always* reschedule the
lower timeout timer, regardless of whether the render is idle or not.

The render's frame callback handler will *not* render the grid if the
delayed render timers are armed.

This means for longer client data bursts, we may now skip frames. That
is, we're trading screen flicker for the occasional frame hickup.

For short client data bursts we should behave roughly as before.

This greatly improves the behavior of fullscreen, or near fullscreen,
updates of large grids (example, scrolling in emacs in fullscreen,
with a vertical buffer split).
---
 render.c   |  5 ++++-
 terminal.c | 13 ++++++-------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/render.c b/render.c
index d8a26060..85879733 100644
--- a/render.c
+++ b/render.c
@@ -1474,7 +1474,10 @@ frame_callback(void *data, struct wl_callback *wl_callback, uint32_t callback_da
 
     if (term->render.pending.grid) {
         term->render.pending.grid = false;
-        grid_render(term);
+
+        /* TODO: need to check if this breaks GNOME/weston */
+        if (!term->delayed_render_timer.is_armed)
+            grid_render(term);
     }
 }
 
diff --git a/terminal.c b/terminal.c
index 4554ae20..bac6e232 100644
--- a/terminal.c
+++ b/terminal.c
@@ -32,6 +32,8 @@
 #define min(x, y) ((x) < (y) ? (x) : (y))
 #define max(x, y) ((x) > (y) ? (x) : (y))
 
+#define PTMX_TIMING 0
+
 static const char *const XCURSOR_LEFT_PTR = "left_ptr";
 static const char *const XCURSOR_TEXT = "text";
 static const char *const XCURSOR_HAND2 = "hand2";
@@ -125,8 +127,6 @@ fdm_ptmx_out(struct fdm *fdm, int fd, int events, void *data)
     return true;
 }
 
-#define PTMX_TIMING 0
-
 #if PTMX_TIMING
 static struct timespec last = {0};
 #endif
@@ -197,9 +197,10 @@ fdm_ptmx(struct fdm *fdm, int fd, int events, void *data)
          * compositor anyway. The delay we introduce here only
          * has any effect when the renderer is idle.
          */
-        if (term->window->frame_callback == NULL) {
-            /* First timeout - reset each time we receive input. */
+        uint64_t lower_ns = term->conf->tweak.delayed_render_lower_ns;
+        uint64_t upper_ns = term->conf->tweak.delayed_render_upper_ns;
 
+        if (lower_ns > 0 && upper_ns > 0) {
 #if PTMX_TIMING
             struct timespec now;
 
@@ -215,8 +216,6 @@ fdm_ptmx(struct fdm *fdm, int fd, int events, void *data)
             last = now;
 #endif
 
-            uint64_t lower_ns = term->conf->tweak.delayed_render_lower_ns;
-            uint64_t upper_ns = term->conf->tweak.delayed_render_upper_ns;
             assert(lower_ns < 1000000000);
             assert(upper_ns < 1000000000);
             assert(upper_ns > lower_ns);
@@ -236,7 +235,7 @@ fdm_ptmx(struct fdm *fdm, int fd, int events, void *data)
                 term->delayed_render_timer.is_armed = true;
             }
         } else
-            term->render.pending.grid = true;
+            render_refresh(term);
     }
 
     if (hup) {