diff --git a/include/randr.h b/include/randr.h index 019e1f74..b5c02144 100644 --- a/include/randr.h +++ b/include/randr.h @@ -18,6 +18,11 @@ 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 diff --git a/src/commands.c b/src/commands.c index 51ac28b1..4a8258e2 100644 --- a/src/commands.c +++ b/src/commands.c @@ -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); diff --git a/src/randr.c b/src/randr.c index 8b6ba1d9..97915704 100644 --- a/src/randr.c +++ b/src/randr.c @@ -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; } /* diff --git a/src/tree.c b/src/tree.c index 4f34946c..3d598d50 100644 --- a/src/tree.c +++ b/src/tree.c @@ -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 index 00000000..1b8be06d --- /dev/null +++ b/testcases/t/506-focus-right.t @@ -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 = <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;