improve timing in scrot --delay

Upgrade scrotSleep() to scrotWaitUntil() which sleeps until a given time
on the clock rather than sleeping for a given period of time. In all
cases, this improves the timing by compensating for the time passed
between entering main() and taking the screenshot. When used with
--count, it fixes the drift caused by calling nanosleep() in a
loop.

Also remove an incorrect comment and rename the delay_selection variable
to delaySelection to match our style.

configure.ac

@@ -68,9 +68,10 @@ AC_SUBST([CPPFLAGS], ["$X11_CFLAGS $XCOMPOSITE_CFLAGS $XEXT_CFLAGS \
 # Checks for header files.
 AC_CHECK_HEADERS([stdint.h sys/time.h unistd.h])
 
-# Required: Checks for library functions.
+# Checks for library functions.
 AC_CHECK_FUNCS([getopt_long getsubopt gethostname strdup strerror strndup strtol],,
     AC_MSG_ERROR([required functions are not present.]))
+AC_CHECK_FUNCS([clock_nanosleep])
 
 AC_CONFIG_FILES([Makefile src/Makefile])
 AC_OUTPUT

src/options.c

@@ -380,8 +380,9 @@ void optionsParse(int argc, char *argv[])
             opt.display = optarg;
             break;
         case 'd':
-            opt.delay_selection = *optarg == 'b';
-            if (opt.delay_selection)
+
+            opt.delaySelection = *optarg == 'b';
+            if (opt.delaySelection)
                 ++optarg;
             opt.delay = optionsParseNum(optarg, 0, INT_MAX, &errmsg);
             if (errmsg) {

src/options.h

@@ -38,6 +38,8 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #pragma once
 
+#include <time.h>
+
 #include "scrot_selection.h"
 
 // General purpose enum
@@ -49,6 +51,7 @@ enum Direction {
 
 struct ScrotOptions {
     int delay;
+    struct timespec delayStart;
     int quality;
     enum thumb { THUMB_DISABLED, THUMB_PERCENT, THUMB_RES } thumb;
     int thumbPercent;
@@ -75,7 +78,7 @@ struct ScrotOptions {
     int autoselectW;
     SelectionMode selection;
     int monitor;
-    bool delay_selection;
+    bool delaySelection;
     bool countdown;
     bool border;
     bool silent;

src/scrot.c

@@ -62,6 +62,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include <X11/extensions/Xfixes.h>
 #include <X11/extensions/Xinerama.h>
 
+#include "config.h"
 #include "imlib.h"
 #include "note.h"
 #include "options.h"
@@ -75,6 +76,7 @@ static Imlib_Image scrotGrabFocused(void);
 static void applyFilterIfRequired(void);
 static Bool scrotXEventVisibility(Display *, XEvent *, XPointer);
 static Imlib_Image scrotGrabAutoselect(void);
+static void scrotWaitUntil(const struct timespec *);
 static Imlib_Image scrotGrabShotMulti(void);
 static Imlib_Image scrotGrabShotMonitor(void);
 static Imlib_Image scrotGrabStackWindows(void);
@@ -102,6 +104,9 @@ int main(int argc, char *argv[])
     struct timespec timeStamp;
     struct tm *tm;
 
+    /* Get the time ASAP to reduce the timing error in case --delay is used. */
+    clock_gettime(CONTINUOUS_CLOCK, &opt.delayStart);
+
     atexit(uninitXAndImlib);
 
     optionsParse(argc, argv);
@@ -285,33 +290,51 @@ static Imlib_Image scrotGrabAutoselect(void)
     return im;
 }
 
-/* similar to sleep, but deals with EINTR.
- * ideally, we'd want to sleep against an absolute time to prevent any drift
- * but clock_nanosleep doesn't seem to be widely implemented.
- */
-static void scrotSleep(int sec)
+void scrotDoDelay(void)
 {
-    assert(sec > 0);
-    struct timespec delay = { .tv_sec = sec };
-    while (nanosleep(&delay, &delay) < 0 && errno == EINTR);
+    if (!opt.delay)
+        return;
+    if (opt.countdown) {
+        dprintf(STDERR_FILENO, "Taking shot in ");
+        for (int i = opt.delay; i > 0; i--) {
+            dprintf(STDERR_FILENO, "%d.. ", i);
+            opt.delayStart.tv_sec++;
+            scrotWaitUntil(&opt.delayStart);
+        }
+        dprintf(STDERR_FILENO, "0.\n");
+    } else {
+        opt.delayStart.tv_sec += opt.delay;
+        scrotWaitUntil(&opt.delayStart);
+    }
 }
 
-void scrotDoDelay(void)
+/* scrotWaitUntil: clock_nanosleep with a simpler interface and no EINTR nagging
+ */
+static void scrotWaitUntil(const struct timespec *time)
 {
-    if (opt.delay) {
-        if (opt.countdown) {
-            int i;
-
-            dprintf(STDERR_FILENO, "Taking shot in %d.. ", opt.delay);
-            scrotSleep(1);
-            for (i = opt.delay - 1; i > 0; i--) {
-                dprintf(STDERR_FILENO, "%d.. ", i);
-                scrotSleep(1);
-            }
-            dprintf(STDERR_FILENO, "0.\n");
-        } else
-            scrotSleep(opt.delay);
-    }
+#if defined(HAVE_CLOCK_NANOSLEEP)
+    while (clock_nanosleep(CONTINUOUS_CLOCK, TIMER_ABSTIME, time, NULL)
+        == -1 && errno == EINTR);
+#else
+    /* OpenBSD and OS X lack clock_nanosleep(), so we use a trivial algorithm to
+     * correct for drift and call nanosleep().
+     */
+    struct timespec tmp;
+    do {
+        clock_gettime(CONTINUOUS_CLOCK, &tmp);
+
+        /* XXX: Use timespecsub(). OS X doesn't have that BSD macro, and libbsd
+         * doesn't support OS X save for an unmaintained fork. libobsd supports
+         * OS X but doesn't have the macro yet.
+         */
+        tmp.tv_sec = time->tv_sec - tmp.tv_sec;
+        tmp.tv_nsec = time->tv_nsec - tmp.tv_nsec;
+        if (tmp.tv_nsec < 0) {
+            tmp.tv_sec--;
+            tmp.tv_nsec += 1000000000L;
+        }
+    } while (nanosleep(&tmp, &tmp) == -1 && errno == EINTR);
+#endif
 }
 
 /* Clip rectangle nicely */

src/scrot_selection.c

@@ -428,8 +428,7 @@ Imlib_Image scrotSelectionSelectMode(void)
 
     opt.selection.mode = SELECTION_MODE_CAPTURE;
 
-    /* if --delay-select is active, then do the delay before selection */
-    if (opt.delay_selection)
+    if (opt.delaySelection)
         scrotDoDelay();
 
     if (!scrotSelectionGetUserSel(&rect0))
@@ -441,8 +440,10 @@ Imlib_Image scrotSelectionSelectMode(void)
         if (!scrotSelectionGetUserSel(&rect1))
             return NULL;
 
-    if (!opt.delay_selection)
+    if (!opt.delaySelection) {
+        clock_gettime(CONTINUOUS_CLOCK, &opt.delayStart);
         scrotDoDelay();
+    }
 
     Imlib_Image capture = imlib_create_image_from_drawable(0, rect0.x, rect0.y,
         rect0.w, rect0.h, 1);

src/util.h

@@ -1,6 +1,6 @@
 /* util.h
 
-Copyright 2021 Guilherme Janczak <[email protected]>
+Copyright 2021,2023 Guilherme Janczak <[email protected]>
 Copyright 2023 NRK <[email protected]>
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -26,6 +26,19 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #pragma once
 
+#include <time.h>
+
+/* On Linux, CLOCK_MONOTONIC does not progress while the system is suspended,
+ * and an alternative non-standard clock which does not suffer from this problem
+ * called CLOCK_BOOTTIME is available. Scrot's CONTINUOUS_CLOCK has the exact
+ * same semantics as CLOCK_MONOTONIC, only it avoids this bug.
+ */
+#if defined(__linux__)
+    #define CONTINUOUS_CLOCK CLOCK_BOOTTIME
+#else
+    #define CONTINUOUS_CLOCK CLOCK_MONOTONIC
+#endif
+
 #define ARRAY_COUNT(X)   (sizeof(X) / sizeof(0[X]))
 #define MAX(A, B)        ((A) > (B) ? (A) : (B))