auto-sync: draw on idle to avoid flicker/tearing

st could easily tear/flicker with animation or other unattended
output. This commit eliminates most of the tear/flicker.

Before this commit, the display timing had two "modes":

- Interactively, st was waiting fixed `1000/xfps` ms after forwarding
  the kb/mouse event to the application and before drawing.

- Unattended, and specifically with animations, the draw frequency was
  throttled to `actionfps`. Animation at a higher rate would throttle
  and likely tear, and at lower rates it was tearing big frames
  (specifically, when one `read` didn't get a full "frame").

The interactive behavior was decent, but it was impossible to get good
unattended-draw behavior even with carefully chosen configuration.

This commit changes the behavior such that it draws on idle instead of
using fixed latency/frequency. This means that it tries to draw only
when it's very likely that the application has completed its output
(or after some duration without idle), so it mostly succeeds to avoid
tear, flicker, and partial drawing.

The config values minlatency/maxlatency replace xfps/actionfps and
define the range which the algorithm is allowed to wait from the
initial draw-trigger until the actual draw. The range enables the
flexibility to choose when to draw - when least likely to flicker.

It also unifies the interactive and unattended behavior and config
values, which makes the code simpler as well - without sacrificing
latency during interactive use, because typically interactively idle
arrives very quickly, so the wait is typically minlatency.

While it only slighly improves interactive behavior, for animations
and other unattended-drawing it improves greatly, as it effectively
adapts to any [animation] output rate without tearing, throttling,
redundant drawing, or unnecessary delays (sounds impossible, but it
works).

README.md

@@ -1,4 +1,4 @@
-Similar to [dwm-flexipatch](https://github.com/bakkeby/dwm-flexipatch) this st 0.8.3 (371878, 2020-04-30) project has a different take on st patching. It uses preprocessor directives to decide whether or not to include a patch during build time. Essentially this means that this build, for better or worse, contains both the patched _and_ the original code. The aim being that you can select which patches to include and the build will contain that code and nothing more.
+Similar to [dwm-flexipatch](https://github.com/bakkeby/dwm-flexipatch) this st 0.8.3 (4e90c0, 2020-05-09) project has a different take on st patching. It uses preprocessor directives to decide whether or not to include a patch during build time. Essentially this means that this build, for better or worse, contains both the patched _and_ the original code. The aim being that you can select which patches to include and the build will contain that code and nothing more.
 
 For example to include the `alpha` patch then you would only need to flip this setting from 0 to 1 in [patches.h](https://github.com/bakkeby/st-flexipatch/blob/master/patches.def.h):
 ```c
@@ -15,6 +15,8 @@ Refer to [https://st.suckless.org/](https://st.suckless.org/) for details on the
 
 ### Changelog:
 
+2020-05-20 - Upgrade to 4e90c0, 2020-05-09, removed visualbell 1, 2, 3 patches and force redraw after keypress due to incompatibility
+
 2020-04-20 - Upgrade to c279f5, 2020-04-19, and added the force redraw on pselect after key is pressed patch and the externalpipein patch
 
 2020-03-29 - Added invert and workingdir patches
@@ -75,8 +77,8 @@ Refer to [https://st.suckless.org/](https://st.suckless.org/) for details on the
    - [font2](https://st.suckless.org/patches/font2/)
       - allows you to add a spare font besides the default
 
-   - [force-redraw-after-keypress](https://lists.suckless.org/hackers/2004/17221.html)
+   - [~force-redraw-after-keypress~](https://lists.suckless.org/hackers/2004/17221.html)
-      - this patch forces the terminal to check for new data on the tty on keypress with the aim of reducing input latency
+      - ~this patch forces the terminal to check for new data on the tty on keypress with the aim of reducing input latency~
 
    - [hidecursor](https://st.suckless.org/patches/hidecursor/)
       - hides the X cursor whenever a key is pressed and show it back when the mouse is moved in the terminal window
@@ -120,8 +122,8 @@ Refer to [https://st.suckless.org/](https://st.suckless.org/) for details on the
    - [vertcenter](https://st.suckless.org/patches/vertcenter/)
       - vertically center lines in the space available if you have set a larger chscale in config.h
 
-   - [visualbell](https://st.suckless.org/patches/visualbell/)
+   - [~visualbell~](https://st.suckless.org/patches/visualbell/)
-      - adds visual indicators for the terminal bell event
+      - ~adds visual indicators for the terminal bell event~
 
    - [workingdir](https://st.suckless.org/patches/workingdir/)
       - allows user to specify the initial path st should use as the working directory

config.def.h

@@ -57,9 +57,14 @@ static unsigned int tripleclicktimeout = 600;
 /* alt screens */
 int allowaltscreen = 1;
 
-/* frames per second st should at maximum draw to the screen */
+/*
-static unsigned int xfps = 120;
+ * draw latency range in ms - from new content/keypress/etc until drawing.
-static unsigned int actionfps = 30;
+ * within this range, st draws when content stops arriving (idle). mostly it's
+ * near minlatency, but it waits longer for slow updates to avoid partial draw.
+ * low minlatency will tear/flicker more, as it can "detect" idle too early.
+ */
+static double minlatency = 8;
+static double maxlatency = 33;
 
 /*
  * blinking timeout (set to 0 to disable blinking) for the terminal blinking
@@ -92,33 +97,6 @@ const int boxdraw_braille = 0;
  */
 static int bellvolume = 0;
 
-#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
-/*
- * visual-bell timeout (set to 0 to disable visual-bell).
- */
-static int vbelltimeout = 0;
-/*
- * visual bell mode when enabled:
- *   1: Inverse whole screen
- *   2: Inverse outer (border) cells
- *   3: Draw a filled circle (VISUALBELL_3_PATCH only)
- */
-static int vbellmode = 1;
-#if VISUALBELL_3_PATCH
-/*
- * for vbellmode == 3 (circle) the following parameters apply:
- * - base and outline colors (colorname index - see below).
- * - radius: relative to window width, or if negative: relative to cell-width.
- * - position: relative to window width/height (0 and 1 are at the edges).
- */
-static int vbellcolor = 3;
-static int vbellcolor_outline = 1;
-static float vbellradius = 0.03;
-static float vbellx = 0.5;
-static float vbelly = 0.5;
-#endif // VISUALBELL_3_PATCH
-#endif // VISUALBELL_2_PATCH
-
 /* default TERM value */
 char *termname = "st-256color";
 
@@ -253,8 +231,8 @@ ResourcePref resources[] = {
 		{ "cursorColor",  STRING,  &colorname[258] },
 		{ "termname",     STRING,  &termname },
 		{ "shell",        STRING,  &shell },
-		{ "xfps",         INTEGER, &xfps },
+		{ "minlatency",   INTEGER, &minlatency },
-		{ "actionfps",    INTEGER, &actionfps },
+		{ "maxlatency",   INTEGER, &minlatency },
 		{ "blinktimeout", INTEGER, &blinktimeout },
 		{ "bellvolume",   INTEGER, &bellvolume },
 		{ "tabspaces",    INTEGER, &tabspaces },

patch/visualbell.c

@@ -1,45 +0,0 @@
-int
-isvbellcell(int x, int y)
-{
-	if (vbellmode == 1)
-		return 1;
-	if (vbellmode == 2)
-		return y == 0 || y == win.th / win.ch - 1 ||
-		       x == 0 || x == win.tw / win.cw - 1;
-	return 0;
-}
-
-void
-vbellbegin()
-{
-	clock_gettime(CLOCK_MONOTONIC, &win.lastvbell);
-	if (win.vbellset) /* already visible, just extend win.lastvbell */
-		return;
-	win.vbellset = 1;
-	#if VISUALBELL_3_PATCH
-	if (vbellmode != 3) /* 3 is an overlay, no need to re-render cells */
-		tfulldirt();
-	#else
-	tfulldirt();
-	#endif // VISUALBELL_3_PATCH
-	draw();
-	XFlush(xw.dpy);
-}
-
-#if VISUALBELL_3_PATCH
-void
-xfillcircle(int x, int y, int r, uint color_ix)
-{
-	XSetForeground(xw.dpy, dc.gc, dc.col[color_ix].pixel);
-	XFillArc(xw.dpy, xw.buf, dc.gc, x - r, y - r, r * 2, r * 2, 0, 360*64);
-}
-
-void
-xdrawvbell() {
-	int r = round(vbellradius * (vbellradius > 0 ? win.w : -win.cw));
-	int x = borderpx + r + vbellx * (win.tw - 2 * r);
-	int y = borderpx + r + vbelly * (win.th - 2 * r);
-	xfillcircle(x, y, r, vbellcolor_outline);
-	xfillcircle(x, y, r / 1.2, vbellcolor); /* 1.2 - an artistic choice */
-}
-#endif // VISUALBELL_3_PATCH

patch/visualbell.h

@@ -1,7 +0,0 @@
-static void vbellbegin();
-static int isvbellcell(int x, int y);
-
-#if VISUALBELL_3_PATCH
-static void xdrawvbell();
-static void xfillcircle(int x, int y, int r, uint color_ix);
-#endif // VISUALBELL_3_PATCH

patch/x_include.c

@@ -23,9 +23,6 @@
 #if ST_EMBEDDER_PATCH
 #include "st_embedder_x.c"
 #endif
-#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
-#include "visualbell.c"
-#endif
 #if XRESOURCES_PATCH
 #include "xresources.c"
 #endif

patch/x_include.h

@@ -20,9 +20,6 @@
 #if ST_EMBEDDER_PATCH
 #include "st_embedder_x.h"
 #endif
-#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
-#include "visualbell.h"
-#endif
 #if XRESOURCES_PATCH
 #include "xresources.h"
 #endif

patches.def.h

@@ -102,15 +102,6 @@
  */
 #define FONT2_PATCH 0
 
-/* This patch creates a global flag which is set when a keypress is sent
- * from X which forces the terminal to check for new data on the tty fd on
- * every return from pselect(). When new data read from the tty results in
- * a line being redrawn, the flag is reset. This results in a less input lag
- * when typing on the terminal.
- * https://lists.suckless.org/hackers/2004/17221.html
- */
-#define FORCE_REDRAW_AFTER_KEYPRESS 0
-
 /* Hide the X cursor whenever a key is pressed and show it back when the mouse is moved in
  * the terminal window.
  * https://st.suckless.org/patches/hidecursor/
@@ -203,30 +194,6 @@
  */
 #define VERTCENTER_PATCH 0
 
-/* On receiving a terminal bell event this patch briefly inverts the window content colors.
- * You may need to reduce the xfps value in config.h to less or equal to that of the refresh
- * rate of your monitor for this to be noticeble.
- * The visualbell 2 and 3 patches takes precedence over this patch.
- * https://st.suckless.org/patches/visualbell/
- */
-#define VISUALBELL_1_PATCH 0
-
-/* On receiving a terminal bell event this patch either:
- *    - briefly inverts the window content colors across the whole terminal or
- *    - briefly inverts the window content colors across the window border
- * The visualbell 3 patch takes precedence over this patch.
- * https://st.suckless.org/patches/visualbell/
- */
-#define VISUALBELL_2_PATCH 0
-
-/* On receiving a terminal bell event this patch either:
- *    - briefly inverts the window content colors across the whole terminal or
- *    - briefly inverts the window content colors across the window border or
- *    - draws a (configurable) circle as a visual bell indicator
- * https://st.suckless.org/patches/visualbell/
- */
-#define VISUALBELL_3_PATCH 0
-
 /* This patch allows user to specify the initial path st should use as the working directory.
  * https://st.suckless.org/patches/workingdir/
  */

st.c

@@ -212,9 +212,6 @@ static void tsetscroll(int, int);
 static void tswapscreen(void);
 static void tsetmode(int, int, int *, int);
 static int twrite(const char *, int, int);
-#if !VISUALBELL_2_PATCH && !VISUALBELL_3_PATCH
-static void tfulldirt(void);
-#endif // VISUALBELL_2_PATCH
 static void tcontrolcode(uchar );
 static void tdectest(char );
 static void tdefutf8(char);

st.h

@@ -94,9 +94,6 @@ int tattrset(int);
 void tnew(int, int);
 void tresize(int, int);
 void tsetdirtattr(int);
-#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
-void tfulldirt();
-#endif // VISUALBELL_2_PATCH
 void ttyhangup(void);
 int ttynew(char *, char *, char *, char **);
 size_t ttyread(void);

x.c

@@ -98,10 +98,6 @@ typedef struct {
 	#endif // VERTCENTER_PATCH
 	int mode; /* window state/mode flags */
 	int cursor; /* cursor style */
-	#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
-	int vbellset; /* 1 during visual bell, 0 otherwise */
-	struct timespec lastvbell;
-	#endif // VISUALBELL_2_PATCH
 } TermWindow;
 
 typedef struct {
@@ -251,9 +247,6 @@ static DC dc;
 static XWindow xw;
 static XSelection xsel;
 static TermWindow win;
-#if FORCE_REDRAW_AFTER_KEYPRESS
-static int pendingkpress = 0;
-#endif // FORCE_REDRAW_AFTER_KEYPRESS
 
 /* Font Ring Cache */
 enum {
@@ -293,9 +286,6 @@ static char *opt_dir   = NULL;
 #endif // WORKINGDIR_PATCH
 
 static int oldbutton = 3; /* button event on startup: 3 = release */
-#if VISUALBELL_1_PATCH && !VISUALBELL_2_PATCH && !VISUALBELL_3_PATCH
-static int bellon = 0;    /* visual bell status */
-#endif // VISUALBELL_1_PATCH
 
 #include "patch/x_include.c"
 
@@ -1946,10 +1936,6 @@ xdrawline(Line line, int x1, int y1, int x2)
 	Glyph base, new;
 	XftGlyphFontSpec *specs = xw.specbuf;
 
-	#if FORCE_REDRAW_AFTER_KEYPRESS
-	pendingkpress = 0;
-	#endif // FORCE_REDRAW_AFTER_KEYPRESS
-
 	numspecs = xmakeglyphfontspecs(specs, &line[x1], x2 - x1, x1, y1);
 	i = ox = 0;
 	for (x = x1; x < x2 && i < numspecs; x++) {
@@ -1958,10 +1944,6 @@ xdrawline(Line line, int x1, int y1, int x2)
 			continue;
 		if (selected(x, y1))
 			new.mode ^= ATTR_REVERSE;
-		#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
-		if (win.vbellset && isvbellcell(x, y1))
-			new.mode ^= ATTR_REVERSE;
-		#endif // VISUALBELL_2_PATCH
 		if (i > 0 && ATTRCMP(base, new)) {
 			xdrawglyphfontspecs(specs, base, i, ox, y1);
 			specs += i;
@@ -2078,20 +2060,6 @@ xbell(void)
 		xseturgency(1);
 	if (bellvolume)
 		XkbBell(xw.dpy, xw.win, bellvolume, (Atom)NULL);
-
-	#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
-	if (vbelltimeout)
-		vbellbegin();
-	#elif VISUALBELL_1_PATCH
-	/* visual bell*/
-	if (!bellon) {
-		bellon = 1;
-		MODBIT(win.mode, !IS_SET(MODE_REVERSE), MODE_REVERSE);
-		redraw();
-		XFlush(xw.dpy);
-		MODBIT(win.mode, !IS_SET(MODE_REVERSE), MODE_REVERSE);
-	}
-	#endif // VISUALBELL_1_PATCH / VISUALBELL_2_PATCH
 }
 
 void
@@ -2181,10 +2149,6 @@ kpress(XEvent *ev)
 	Status status;
 	Shortcut *bp;
 
-	#if FORCE_REDRAW_AFTER_KEYPRESS
-	pendingkpress = 1;
-	#endif // FORCE_REDRAW_AFTER_KEYPRESS
-
 	#if HIDECURSOR_PATCH
 	if (xw.pointerisvisible) {
 		XDefineCursor(xw.dpy, xw.win, xw.bpointer);
@@ -2289,13 +2253,9 @@ run(void)
 	XEvent ev;
 	int w = win.w, h = win.h;
 	fd_set rfd;
-	int xfd = XConnectionNumber(xw.dpy), xev, blinkset = 0, dodraw = 0;
+	int xfd = XConnectionNumber(xw.dpy), ttyfd, xev, drawing;
-	int ttyfd;
+	struct timespec seltv, *tv, now, lastblink, trigger;
-	struct timespec drawtimeout, *tv = NULL, now, last, lastblink;
+	double timeout;
-	long deltatime;
-	#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
-	long to_ms, remain;
-	#endif // VISUALBELL_2_PATCH
 
 	/* Waiting for window mapping */
 	do {
@@ -2316,82 +2276,31 @@ run(void)
 	ttyfd = ttynew(opt_line, shell, opt_io, opt_cmd);
 	cresize(w, h);
 
-	clock_gettime(CLOCK_MONOTONIC, &last);
+	for (timeout = -1, drawing = 0, lastblink = (struct timespec){0};;) {
-	lastblink = last;
-
-	for (xev = actionfps;;) {
 		FD_ZERO(&rfd);
 		FD_SET(ttyfd, &rfd);
 		FD_SET(xfd, &rfd);
 
+		if (XPending(xw.dpy))
+			timeout = 0;  /* existing events might not set xfd */
+
+		seltv.tv_sec = timeout / 1E3;
+		seltv.tv_nsec = 1E6 * (timeout - 1E3 * seltv.tv_sec);
+		tv = timeout >= 0 ? &seltv : NULL;
+
 		if (pselect(MAX(xfd, ttyfd)+1, &rfd, NULL, NULL, tv, NULL) < 0) {
 			if (errno == EINTR)
 				continue;
 			die("select failed: %s\n", strerror(errno));
 		}
-		if (FD_ISSET(ttyfd, &rfd)) {
-			ttyread();
-			if (blinktimeout) {
-				blinkset = tattrset(ATTR_BLINK);
-				if (!blinkset)
-					MODBIT(win.mode, 0, MODE_BLINK);
-			}
-		}
-
-		if (FD_ISSET(xfd, &rfd))
-			xev = actionfps;
-
 		clock_gettime(CLOCK_MONOTONIC, &now);
-		drawtimeout.tv_sec = 0;
-		drawtimeout.tv_nsec =  (1000 * 1E6)/ xfps;
-		tv = &drawtimeout;
 
-		dodraw = 0;
+		if (FD_ISSET(ttyfd, &rfd))
-		#if FORCE_REDRAW_AFTER_KEYPRESS
+			ttyread();
-		if (pendingkpress)
-			dodraw = 1;
-		#endif // FORCE_REDRAW_AFTER_KEYPRESS
-		#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
-		to_ms = -1; /* timeout in ms, indefinite if negative */
-		if (blinkset) {
-			remain = blinktimeout - TIMEDIFF(now, lastblink);
-			if (remain <= 0) {
-				dodraw = 1;
-				remain = 1; /* draw, wait 1ms, and re-calc */
-				tsetdirtattr(ATTR_BLINK);
-				win.mode ^= MODE_BLINK;
-				lastblink = now;
-			}
-			to_ms = remain;
-		}
-		if (win.vbellset) {
-			remain = vbelltimeout - TIMEDIFF(now, win.lastvbell);
-			if (remain <= 0) {
-				dodraw = 1;
-				remain = -1; /* draw (clear), and that's it */
-				tfulldirt();
-				win.vbellset = 0;
-			}
-			if (remain >= 0 && (to_ms < 0 || remain < to_ms))
-				to_ms = remain;
-		}
 
-		#else
+		xev = 0;
-		if (blinktimeout && TIMEDIFF(now, lastblink) > blinktimeout) {
-			tsetdirtattr(ATTR_BLINK);
-			win.mode ^= MODE_BLINK;
-			lastblink = now;
-			dodraw = 1;
-		}
-		#endif // VISUALBELL_2_PATCH
-		deltatime = TIMEDIFF(now, last);
-		if (deltatime > 1000 / (xev ? xfps : actionfps)) {
-			dodraw = 1;
-			last = now;
-		}
-
-		if (dodraw) {
 		while (XPending(xw.dpy)) {
+			xev = 1;
 			XNextEvent(xw.dpy, &ev);
 			if (XFilterEvent(&ev, None))
 				continue;
@@ -2399,49 +2308,45 @@ run(void)
 				(handler[ev.type])(&ev);
 		}
 
-			#if VISUALBELL_1_PATCH && !VISUALBELL_2_PATCH && !VISUALBELL_3_PATCH
+		/*
-			if (bellon) {
+		 * To reduce flicker and tearing, when new content or event
-				bellon = 0;
+		 * triggers drawing, we first wait a bit to ensure we got
-				redraw();
+		 * everything, and if nothing new arrives - we draw.
+		 * We start with trying to wait minlatency ms. If more content
+		 * arrives sooner, we retry with shorter and shorter preiods,
+		 * and eventually draw even without idle after maxlatency ms.
+		 * Typically this results in low latency while interacting,
+		 * maximum latency intervals during `cat huge.txt`, and perfect
+		 * sync with periodic updates from animations/key-repeats/etc.
+		 */
+		if (FD_ISSET(ttyfd, &rfd) || xev) {
+			if (!drawing) {
+				trigger = now;
+				drawing = 1;
+			}
+			timeout = (maxlatency - TIMEDIFF(now, trigger)) \
+			          / maxlatency * minlatency;
+			if (timeout > 0)
+				continue;  /* we have time, try to find idle */
 		}
-			else draw();
- 			XFlush(xw.dpy);
-			#else
-			draw();
-			#endif // VISUALBELL_1_PATCH
-			XFlush(xw.dpy);
 
-			if (xev && !FD_ISSET(xfd, &rfd))
+		/* idle detected or maxlatency exhausted -> draw */
-				xev--;
+		timeout = -1;
-			if (!FD_ISSET(ttyfd, &rfd) && !FD_ISSET(xfd, &rfd)) {
+		if (blinktimeout && tattrset(ATTR_BLINK)) {
-				#if VISUALBELL_2_PATCH || VISUALBELL_3_PATCH
+			timeout = blinktimeout - TIMEDIFF(now, lastblink);
-				if (to_ms >= 0) {
+			if (timeout <= 0) {
-					static const long k = 1E3, m = 1E6;
+				if (-timeout > blinktimeout) /* start visible */
-					drawtimeout.tv_sec = to_ms / k;
+					win.mode |= MODE_BLINK;
-					drawtimeout.tv_nsec = (to_ms % k) * m;
+				win.mode ^= MODE_BLINK;
-				} else {
+				tsetdirtattr(ATTR_BLINK);
-					tv = NULL;
+				lastblink = now;
-				}
+				timeout = blinktimeout;
-				#else
-				if (blinkset) {
-					if (TIMEDIFF(now, lastblink) \
-							> blinktimeout) {
-						drawtimeout.tv_nsec = 1000;
-					} else {
-						drawtimeout.tv_nsec = (1E6 * \
-							(blinktimeout - \
-							TIMEDIFF(now,
-								lastblink)));
-					}
-					drawtimeout.tv_sec = \
-					    drawtimeout.tv_nsec / 1E9;
-					drawtimeout.tv_nsec %= (long)1E9;
-				} else {
-					tv = NULL;
-				}
-				#endif // VISUALBELL_2_PATCH
 			}
 		}
+
+		draw();
+		XFlush(xw.dpy);
+		drawing = 0;
 	}
 }