scratchpad: fix moving scratchpad window

From the source:

    When starting i3 initially (and after each change to the connected
    outputs), this function fixes the resolution of the __i3
    pseudo-output. When that resolution is not set to a function which
    shares a common divisor with every active output’s resolution,
    floating point calculation errors will lead to the scratchpad window
    moving when shown repeatedly.

fixes #632

include/scratchpad.h

@@ -30,4 +30,14 @@ void scratchpad_move(Con *con);
  */
 void scratchpad_show(Con *con);
 
+/**
+ * When starting i3 initially (and after each change to the connected outputs),
+ * this function fixes the resolution of the __i3 pseudo-output. When that
+ * resolution is not set to a function which shares a common divisor with every
+ * active output’s resolution, floating point calculation errors will lead to
+ * the scratchpad window moving when shown repeatedly.
+ *
+ */
+void scratchpad_fix_resolution(void);
+
 #endif

src/handlers.c

@@ -212,6 +212,7 @@ static void handle_motion_notify(xcb_motion_notify_event_t *event) {
 
     Con *con;
     if ((con = con_by_frame_id(event->event)) == NULL) {
+        DLOG("MotionNotify for an unknown container, checking if it crosses screen boundaries.\n");
         check_crossing_screen_boundary(event->root_x, event->root_y);
         return;
     }
@@ -405,6 +406,8 @@ static void handle_screen_change(xcb_generic_event_t *e) {
 
     randr_query_outputs();
 
+    scratchpad_fix_resolution();
+
     ipc_send_event("output", I3_IPC_EVENT_OUTPUT, "{\"change\":\"unspecified\"}");
 
     return;

src/main.c

@@ -657,6 +657,8 @@ int main(int argc, char *argv[]) {
         randr_init(&randr_base);
     }
 
+    scratchpad_fix_resolution();
+
     xcb_query_pointer_reply_t *pointerreply;
     Output *output = NULL;
     if (!(pointerreply = xcb_query_pointer_reply(conn, pointercookie, NULL))) {

src/scratchpad.c

@@ -141,3 +141,67 @@ void scratchpad_show(Con *con) {
 
     con_focus(con_descend_focused(con));
 }
+
+/*
+ * Greatest common divisor, implemented only for the least common multiple
+ * below.
+ *
+ */
+static int _gcd(const int m, const int n) {
+    if (n == 0)
+        return m;
+    return _gcd(n, (m % n));
+}
+
+/*
+ * Least common multiple. We use it to determine the (ideally not too large)
+ * resolution for the __i3 pseudo-output on which the scratchpad is on (see
+ * below). We could just multiply the resolutions, but for some pathetic cases
+ * (many outputs), using the LCM will achieve better results.
+ *
+ * Man, when you were learning about these two algorithms for the first time,
+ * did you think you’d ever need them in a real-world software project of
+ * yours? I certainly didn’t until now. :-D
+ *
+ */
+static int _lcm(const int m, const int n) {
+    const int o = _gcd(m, n);
+    return ((m * n) / o);
+}
+
+/*
+ * When starting i3 initially (and after each change to the connected outputs),
+ * this function fixes the resolution of the __i3 pseudo-output. When that
+ * resolution is not set to a function which shares a common divisor with every
+ * active output’s resolution, floating point calculation errors will lead to
+ * the scratchpad window moving when shown repeatedly.
+ *
+ */
+void scratchpad_fix_resolution(void) {
+    Con *__i3_scratch = workspace_get("__i3_scratch", NULL);
+    Con *__i3_output = con_get_output(__i3_scratch);
+    DLOG("Current resolution: (%d, %d) %d x %d\n",
+         __i3_output->rect.x, __i3_output->rect.y,
+         __i3_output->rect.width, __i3_output->rect.height);
+    Con *output;
+    int new_width = -1,
+        new_height = -1;
+    TAILQ_FOREACH(output, &(croot->nodes_head), nodes) {
+        if (output == __i3_output)
+            continue;
+        DLOG("output %s's resolution: (%d, %d) %d x %d\n",
+             output->name, output->rect.x, output->rect.y,
+             output->rect.width, output->rect.height);
+        if (new_width == -1) {
+            new_width = output->rect.width;
+            new_height = output->rect.height;
+        } else {
+            new_width = _lcm(new_width, output->rect.width);
+            new_height = _lcm(new_height, output->rect.height);
+        }
+    }
+    DLOG("new width = %d, new height = %d\n",
+         new_width, new_height);
+    __i3_output->rect.width = new_width;
+    __i3_output->rect.height = new_height;
+}

src/tree.c

@@ -30,7 +30,9 @@ static Con *_create___i3(void) {
     x_set_name(__i3, "[i3 con] pseudo-output __i3");
     /* For retaining the correct position/size of a scratchpad window, the
      * dimensions of the real outputs should be multiples of the __i3
-     * pseudo-output. */
+     * pseudo-output. Ensuring that is the job of scratchpad_fix_resolution()
+     * which gets called after this function and after detecting all the
+     * outputs (or whenever an output changes). */
     __i3->rect.width = 1280;
     __i3->rect.height = 1024;
 

testcases/t/505-scratchpad-resolution.t

@@ -0,0 +1,76 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Verifies that scratchpad windows don’t move due to floating point caulcation
+# errors when repeatedly hiding/showing, no matter what display resolution.
+#
+use i3test i3_autostart => 0;
+
+my $config = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+fake-outputs 683x768+0+0,1024x768+683+0
+EOT
+my $pid = launch_with_config($config);
+
+my $i3 = i3(get_socket_path());
+
+$x->root->warp_pointer(0, 0);
+sync_with_i3;
+
+sub verify_scratchpad_doesnt_move {
+    my ($ws) = @_;
+
+    is(scalar @{get_ws($ws)->{nodes}}, 0, 'no nodes on this ws');
+
+    my $window = open_window;
+
+    is(scalar @{get_ws($ws)->{nodes}}, 1, 'one nodes on this ws');
+
+    cmd 'move scratchpad';
+
+    is(scalar @{get_ws($ws)->{nodes}}, 0, 'no nodes on this ws');
+
+    my $last_x = -1;
+    for (1 .. 20) {
+        cmd 'scratchpad show';
+        is(scalar @{get_ws($ws)->{floating_nodes}}, 1, 'one floating node on this ws');
+
+        # Verify that the coordinates are within bounds.
+        my $content = get_ws($ws);
+        my $srect = $content->{floating_nodes}->[0]->{rect};
+        if ($last_x > -1) {
+            is($srect->{x}, $last_x, 'scratchpad window did not move');
+        }
+        $last_x = $srect->{x};
+        cmd 'scratchpad show';
+    }
+
+    # We need to kill the scratchpad window, otherwise scratchpad show in
+    # subsequent calls of verify_scratchpad_doesnt_move will cycle between all
+    # the windows.
+    cmd 'scratchpad show';
+    cmd 'kill';
+}
+
+################################################################################
+# test it on the left output first (1366x768)
+################################################################################
+
+my $second = fresh_workspace(output => 0);
+verify_scratchpad_doesnt_move($second);
+
+################################################################################
+# now on the right output (1024x768)
+################################################################################
+
+$x->root->warp_pointer(683 + 10, 0);
+sync_with_i3;
+
+my $third = fresh_workspace(output => 1);
+verify_scratchpad_doesnt_move($third);
+
+exit_gracefully($pid);
+
+done_testing;