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

diff --git a/include/scratchpad.h b/include/scratchpad.h
index 4fb7523a0bf4df581c25c2831d23380a774ce4e2..4d5533273983b1a8a11b8bc6d357fcdbd569fac2 100644 (file)
--- a/include/scratchpad.h
+++ b/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

diff --git a/src/handlers.c b/src/handlers.c
index 2a767eb22a03f3e2b19975a955ae017d9b6d1fba..3a8d2344de9e186778ca6fc28025b635c3dfa488 100644 (file)
--- a/src/handlers.c
+++ b/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;

diff --git a/src/main.c b/src/main.c
index 6bd08b1e2ac04138b5659d41a837c4accc6bf305..e75e7fbfe7b4d8988b47955944e8daf49bca7365 100644 (file)
--- a/src/main.c
+++ b/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))) {

diff --git a/src/scratchpad.c b/src/scratchpad.c
index acc3c55772c4ae100910ba593014c86775e7a5b8..da50cee76442c5d8c9d712065adc7896ee0625d0 100644 (file)
--- a/src/scratchpad.c
+++ b/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;
+}

diff --git a/src/tree.c b/src/tree.c
index d6e7ecd91a9f5aa2d28b23f0e1db27b3918ce5f3..cee61b9dcee4ba85607331628e3e82a31a6df1e9 100644 (file)
--- a/src/tree.c
+++ b/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;
 

diff --git a/testcases/t/505-scratchpad-resolution.t b/testcases/t/505-scratchpad-resolution.t
new file mode 100644 (file)
index 0000000..3d0bcfa
--- /dev/null
+++ b/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;