From: Michael Stapelberg Date: Wed, 8 Aug 2012 14:22:03 +0000 (+0200) Subject: scratchpad: fix moving scratchpad window X-Git-Tag: 4.3~117 X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=6ba09444308ac4c8c55beaef416cbaafe7470fb0;p=i3%2Fi3 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 4fb7523a..4d553327 100644 --- 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 2a767eb2..3a8d2344 100644 --- 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 6bd08b1e..e75e7fbf 100644 --- 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 acc3c557..da50cee7 100644 --- 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 d6e7ecd9..cee61b9d 100644 --- 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 index 00000000..3d0bcfa6 --- /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 = <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;