]> git.sur5r.net Git - i3/i3/commitdiff
Make get_output_next() work with non-aligned RandR setups (+test) (Thanks Feh, swh...
authorMichael Stapelberg <michael@stapelberg.de>
Sat, 22 Sep 2012 14:49:37 +0000 (16:49 +0200)
committerMichael Stapelberg <michael@stapelberg.de>
Sat, 22 Sep 2012 14:54:59 +0000 (16:54 +0200)
A good visualization of the new algorithm is this:

           +--------+
           |        |
+--------+=|   S1   |========================
|        | |        |
|   S0   | +--------+
|        |         +--------+
+--------+=========|        |================
                   |   S2   | +--------+
                   |        | |        |
                   +--------+ |   S3   |
                              |        |
                              +--------+

When focus is on S0, 'focus output right' will first match S1 (the
closest output which overlaps in the highlighted area), then S2, but not
S3 (since S3 does not overlap into the highlighted area).

fixes #669
fixes #771

include/randr.h
src/commands.c
src/randr.c
src/tree.c
testcases/t/506-focus-right.t [new file with mode: 0644]

index 019e1f7440092985922c283453b4c90d3ffee991..b5c02144dc729986d3ecebb502ac2517af33706d 100644 (file)
 TAILQ_HEAD(outputs_head, xoutput);
 extern struct outputs_head outputs;
 
+typedef enum {
+    CLOSEST_OUTPUT = 0,
+    FARTHEST_OUTPUT = 1
+} output_close_far_t;
+
 /**
  * We have just established a connection to the X server and need the initial
  * XRandR information to setup workspaces for each screen.
@@ -96,6 +101,6 @@ Output *get_output_most(direction_t direction, Output *current);
  * Gets the output which is the next one in the given direction.
  *
  */
-Output *get_output_next(direction_t direction, Output *current);
+Output *get_output_next(direction_t direction, Output *current, output_close_far_t close_far);
 
 #endif
index 51ac28b1ca95872861d76bf14dff565d0e421a2e..4a8258e2c5c5e5209c4c12e844fc3ac29270cb29 100644 (file)
@@ -57,19 +57,19 @@ static Output *get_output_from_string(Output *current_output, const char *output
     Output *output;
 
     if (strcasecmp(output_str, "left") == 0) {
-        output = get_output_next(D_LEFT, current_output);
+        output = get_output_next(D_LEFT, current_output, CLOSEST_OUTPUT);
         if (!output)
             output = get_output_most(D_RIGHT, current_output);
     } else if (strcasecmp(output_str, "right") == 0) {
-        output = get_output_next(D_RIGHT, current_output);
+        output = get_output_next(D_RIGHT, current_output, CLOSEST_OUTPUT);
         if (!output)
             output = get_output_most(D_LEFT, current_output);
     } else if (strcasecmp(output_str, "up") == 0) {
-        output = get_output_next(D_UP, current_output);
+        output = get_output_next(D_UP, current_output, CLOSEST_OUTPUT);
         if (!output)
             output = get_output_most(D_DOWN, current_output);
     } else if (strcasecmp(output_str, "down") == 0) {
-        output = get_output_next(D_DOWN, current_output);
+        output = get_output_next(D_DOWN, current_output, CLOSEST_OUTPUT);
         if (!output)
             output = get_output_most(D_UP, current_output);
     } else output = get_output_by_name(output_str);
@@ -1006,13 +1006,13 @@ void cmd_move_con_to_output(I3_CMD, char *name) {
 
     // TODO: clean this up with commands.spec as soon as we switched away from the lex/yacc command parser
     if (strcasecmp(name, "up") == 0)
-        output = get_output_next(D_UP, current_output);
+        output = get_output_next(D_UP, current_output, CLOSEST_OUTPUT);
     else if (strcasecmp(name, "down") == 0)
-        output = get_output_next(D_DOWN, current_output);
+        output = get_output_next(D_DOWN, current_output, CLOSEST_OUTPUT);
     else if (strcasecmp(name, "left") == 0)
-        output = get_output_next(D_LEFT, current_output);
+        output = get_output_next(D_LEFT, current_output, CLOSEST_OUTPUT);
     else if (strcasecmp(name, "right") == 0)
-        output = get_output_next(D_RIGHT, current_output);
+        output = get_output_next(D_RIGHT, current_output, CLOSEST_OUTPUT);
     else
         output = get_output_by_name(name);
 
index 8b6ba1d9ac52a13a1e87423330fc50de038a0710..97915704e855276f41bbb4cf161b248bffe3abbb 100644 (file)
@@ -101,89 +101,76 @@ Output *get_output_containing(int x, int y) {
  *
  */
 Output *get_output_most(direction_t direction, Output *current) {
-    Output *output, *candidate = NULL;
-    int position = 0;
-    TAILQ_FOREACH(output, &outputs, outputs) {
-        if (!output->active)
-            continue;
-
-        /* Repeated calls of WIN determine the winner of the comparison */
-        #define WIN(variable, condition) \
-            if (variable condition) { \
-                candidate = output; \
-                position = variable; \
-            } \
-            break;
-
-        if (((direction == D_UP) || (direction == D_DOWN)) &&
-            (current->rect.x != output->rect.x))
-            continue;
-
-        if (((direction == D_LEFT) || (direction == D_RIGHT)) &&
-            (current->rect.y != output->rect.y))
-            continue;
-
-        switch (direction) {
-            case D_UP:
-                WIN(output->rect.y, <= position);
-            case D_DOWN:
-                WIN(output->rect.y, >= position);
-            case D_LEFT:
-                WIN(output->rect.x, <= position);
-            case D_RIGHT:
-                WIN(output->rect.x, >= position);
-        }
-    }
-
-    assert(candidate != NULL);
-
-    return candidate;
+    Output *best = get_output_next(direction, current, FARTHEST_OUTPUT);
+    if (!best)
+        best = current;
+    DLOG("current = %s, best = %s\n", current->name, best->name);
+    return best;
 }
 
 /*
  * Gets the output which is the next one in the given direction.
  *
  */
-Output *get_output_next(direction_t direction, Output *current) {
-    Output *output, *candidate = NULL;
-
+Output *get_output_next(direction_t direction, Output *current, output_close_far_t close_far) {
+    Rect *cur = &(current->rect),
+         *other;
+    Output *output,
+           *best = NULL;
     TAILQ_FOREACH(output, &outputs, outputs) {
         if (!output->active)
             continue;
 
-        if (((direction == D_UP) || (direction == D_DOWN)) &&
-            (current->rect.x != output->rect.x))
+        other = &(output->rect);
+
+        if ((direction == D_RIGHT && other->x > cur->x) ||
+            (direction == D_LEFT  && other->x < cur->x)) {
+            /* Skip the output when it doesn’t overlap the other one’s y
+             * coordinate at all. */
+            if ((other->y + other->height) < cur->y &&
+                (cur->y   + cur->height)   < other->y)
+                continue;
+        } else if ((direction == D_DOWN && other->y > cur->y) ||
+                   (direction == D_UP   && other->y < cur->y)) {
+            /* Skip the output when it doesn’t overlap the other one’s x
+             * coordinate at all. */
+            if ((other->x + other->width) < cur->x &&
+                (cur->x   + cur->width)   < other->x)
+                continue;
+        } else
             continue;
 
-        if (((direction == D_LEFT) || (direction == D_RIGHT)) &&
-            (current->rect.y != output->rect.y))
+        /* No candidate yet? Start with this one. */
+        if (!best) {
+            best = output;
             continue;
+        }
 
-        switch (direction) {
-            case D_UP:
-                if (output->rect.y < current->rect.y &&
-                    (!candidate || output->rect.y < candidate->rect.y))
-                    candidate = output;
-                break;
-            case D_DOWN:
-                if (output->rect.y > current->rect.y &&
-                    (!candidate || output->rect.y > candidate->rect.y))
-                    candidate = output;
-                break;
-            case D_LEFT:
-                if (output->rect.x < current->rect.x &&
-                    (!candidate || output->rect.x > candidate->rect.x))
-                    candidate = output;
-                break;
-            case D_RIGHT:
-                if (output->rect.x > current->rect.x &&
-                    (!candidate || output->rect.x < candidate->rect.x))
-                    candidate = output;
-                break;
+        if (close_far == CLOSEST_OUTPUT) {
+            /* Is this output better (closer to the current output) than our
+             * current best bet? */
+            if ((direction == D_RIGHT && other->x < best->rect.x) ||
+                (direction == D_LEFT  && other->x > best->rect.x) ||
+                (direction == D_DOWN  && other->y < best->rect.y) ||
+                (direction == D_UP    && other->y > best->rect.y)) {
+                best = output;
+                continue;
+            }
+        } else {
+            /* Is this output better (farther to the current output) than our
+             * current best bet? */
+            if ((direction == D_RIGHT && other->x > best->rect.x) ||
+                (direction == D_LEFT  && other->x < best->rect.x) ||
+                (direction == D_DOWN  && other->y > best->rect.y) ||
+                (direction == D_UP    && other->y < best->rect.y)) {
+                best = output;
+                continue;
+            }
         }
     }
 
-    return candidate;
+    DLOG("current = %s, best = %s\n", current->name, (best ? best->name : "NULL"));
+    return best;
 }
 
 /*
index 4f34946cc58c13e654c8ad031e68d859f65dde6d..3d598d50ab8014410ad9a5c38eb956564f98cd6a 100644 (file)
@@ -486,7 +486,7 @@ static bool _tree_next(Con *con, char way, orientation_t orientation, bool wrap)
         else
             return false;
 
-        next_output = get_output_next(direction, current_output);
+        next_output = get_output_next(direction, current_output, CLOSEST_OUTPUT);
         if (!next_output)
             return false;
         DLOG("Next output is %s\n", next_output->name);
diff --git a/testcases/t/506-focus-right.t b/testcases/t/506-focus-right.t
new file mode 100644 (file)
index 0000000..1b8be06
--- /dev/null
@@ -0,0 +1,174 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • http://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • http://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • http://build.i3wm.org/docs/ipc.html
+#   (or docs/ipc)
+#
+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
+#   (unless you are already familiar with Perl)
+#
+# Verifies that focus output right works with monitor setups that don’t line up
+# on their x/y coordinates.
+#
+# ticket #771, bug still present in commit dd743f3b55b2f86d9f1f69ef7952ae8ece4de504
+#
+use i3test i3_autostart => 0;
+
+sub test_focus_left_right {
+    my ($config) = @_;
+
+    my $pid = launch_with_config($config);
+
+    my $i3 = i3(get_socket_path(0));
+
+    $x->root->warp_pointer(0, 0);
+    sync_with_i3;
+
+    ############################################################################
+    # Ensure that moving left and right works.
+    ############################################################################
+
+    # First ensure both workspaces have something to focus
+    cmd "workspace 1";
+    my $left_win = open_window;
+
+    cmd "workspace 2";
+    my $right_win = open_window;
+
+    is($x->input_focus, $right_win->id, 'right window focused');
+
+    cmd "focus output left";
+    is($x->input_focus, $left_win->id, 'left window focused');
+
+    cmd "focus output right";
+    is($x->input_focus, $right_win->id, 'right window focused');
+
+    cmd "focus output right";
+    is($x->input_focus, $left_win->id, 'left window focused (wrapping)');
+
+    cmd "focus output left";
+    is($x->input_focus, $right_win->id, 'right window focused (wrapping)');
+
+    ############################################################################
+    # Ensure that moving down from S0 doesn’t crash i3.
+    ############################################################################
+
+    my $second = fresh_workspace(output => 1);
+
+    cmd "focus output down";
+
+    does_i3_live;
+
+    exit_gracefully($pid);
+}
+
+# Screen setup looks like this:
+# +----+
+# |    |--------+
+# | S1 |   S2   |
+# |    |--------+
+# +----+
+#
+my $config = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+fake-outputs 1080x1920+0+0,1920x1080+1080+500
+EOT
+
+test_focus_left_right($config);
+
+# Screen setup looks like this:
+# +----+--------+
+# |    |   S2   |
+# | S1 |--------+
+# |    |
+# +----+
+#
+$config = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+fake-outputs 1080x1920+0+0,1920x200+1080+0
+EOT
+
+test_focus_left_right($config);
+
+# Screen setup looks like this:
+# +----+
+# |    |
+# | S1 |--------+
+# |    |   S2   |
+# +----+--------+
+#
+$config = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+fake-outputs 1080x1920+0+0,1920x200+1080+1720
+EOT
+
+test_focus_left_right($config);
+
+# Screen setup looks like this:
+# +----+        +----+
+# |    |        |    |
+# | S1 |--------+ S3 |
+# |    |   S2   |    |
+# +----+--------+----+
+#
+$config = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+fake-outputs 1080x1920+0+0,1920x200+1080+1720,1080x1920+1280+0
+EOT
+
+my $pid = launch_with_config($config);
+
+my $i3 = i3(get_socket_path(0));
+
+$x->root->warp_pointer(0, 0);
+sync_with_i3;
+
+############################################################################
+# Ensure that focusing right/left works in the expected order.
+############################################################################
+
+sub get_focused_output {
+    my $tree = i3(get_socket_path())->get_tree->recv;
+    my ($focused_id) = @{$tree->{focus}};
+    my ($output) = grep { $_->{id} == $focused_id } @{$tree->{nodes}};
+    return $output->{name};
+}
+
+is(get_focused_output(), 'fake-0', 'focus on fake-0');
+
+cmd 'focus output right';
+is(get_focused_output(), 'fake-1', 'focus on fake-1');
+
+cmd 'focus output right';
+is(get_focused_output(), 'fake-2', 'focus on fake-2');
+
+cmd 'focus output left';
+is(get_focused_output(), 'fake-1', 'focus on fake-1');
+
+cmd 'focus output left';
+is(get_focused_output(), 'fake-0', 'focus on fake-0');
+
+cmd 'focus output left';
+is(get_focused_output(), 'fake-2', 'focus on fake-2 (wrapping)');
+
+cmd 'focus output right';
+is(get_focused_output(), 'fake-0', 'focus on fake-0 (wrapping)');
+
+exit_gracefully($pid);
+
+done_testing;