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
This commit is contained in:
Michael Stapelberg 2012-09-22 16:49:37 +02:00
parent 89ca48be20
commit 19883108a9
5 changed files with 242 additions and 76 deletions

View File

@ -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

View 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);

View 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 doesnt overlap the other ones 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 doesnt overlap the other ones 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;
}
/*

View 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);

View File

@ -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 dont 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 doesnt 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;