From 19883108a9973bd1dfef15600e8fd7b1f55a7ca9 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Sat, 22 Sep 2012 16:49:37 +0200 Subject: [PATCH] Make get_output_next() work with non-aligned RandR setups (+test) (Thanks Feh, swh, Moritz) 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 | 7 +- src/commands.c | 16 ++-- src/randr.c | 119 +++++++++++------------ src/tree.c | 2 +- testcases/t/506-focus-right.t | 174 ++++++++++++++++++++++++++++++++++ 5 files changed, 242 insertions(+), 76 deletions(-) create mode 100644 testcases/t/506-focus-right.t 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; -- 2.39.2