Merge pull request #1609 from Deiz/consistent-center

Handle floating centering in one function and test for consistency

include/floating.h

@@ -64,6 +64,12 @@ void floating_raise_con(Con *con);
  */
 bool floating_maybe_reassign_ws(Con *con);
 
+/**
+ * Centers a floating con above the specified rect.
+ *
+ */
+void floating_center(Con *con, Rect rect);
+
 #if 0
 /**
  * Removes the floating client from its workspace and attaches it to the new

src/commands.c

@@ -1761,25 +1761,18 @@ void cmd_move_window_to_center(I3_CMD, char *method) {
     }
 
     if (strcmp(method, "absolute") == 0) {
-        Rect *rect = &focused->parent->rect;
-
         DLOG("moving to absolute center\n");
-        rect->x = croot->rect.width / 2 - rect->width / 2;
+        floating_center(focused->parent, croot->rect);
-        rect->y = croot->rect.height / 2 - rect->height / 2;
 
         floating_maybe_reassign_ws(focused->parent);
         cmd_output->needs_tree_render = true;
     }
 
     if (strcmp(method, "position") == 0) {
-        Rect *wsrect = &con_get_workspace(focused)->rect;
-        Rect newrect = focused->parent->rect;
-
         DLOG("moving to center\n");
-        newrect.x = wsrect->x + wsrect->width / 2 - newrect.width / 2;
+        floating_center(focused->parent, con_get_workspace(focused)->rect);
-        newrect.y = wsrect->y + wsrect->height / 2 - newrect.height / 2;
 
-        floating_reposition(focused->parent, newrect);
+        cmd_output->needs_tree_render = true;
     }
 
     // XXX: default reply for now, make this a better reply

src/floating.c

@@ -246,12 +246,10 @@ void floating_enable(Con *con, bool automatic) {
         if (con->window && con->window->leader != XCB_NONE &&
             (leader = con_by_window_id(con->window->leader)) != NULL) {
             DLOG("Centering above leader\n");
-            nc->rect.x = leader->rect.x + (leader->rect.width / 2) - (nc->rect.width / 2);
+            floating_center(nc, leader->rect);
-            nc->rect.y = leader->rect.y + (leader->rect.height / 2) - (nc->rect.height / 2);
         } else {
             /* center the window on workspace as fallback */
-            nc->rect.x = ws->rect.x + (ws->rect.width / 2) - (nc->rect.width / 2);
+            floating_center(nc, ws->rect);
-            nc->rect.y = ws->rect.y + (ws->rect.height / 2) - (nc->rect.height / 2);
         }
     }
 
@@ -310,8 +308,7 @@ void floating_enable(Con *con, bool automatic) {
     }
 
     ELOG("No output found at destination coordinates, centering floating window on current ws\n");
-    nc->rect.x = ws->rect.x + (ws->rect.width / 2) - (nc->rect.width / 2);
+    floating_center(nc, ws->rect);
-    nc->rect.y = ws->rect.y + (ws->rect.height / 2) - (nc->rect.height / 2);
 
     ipc_send_window_event("floating", con);
 }
@@ -420,6 +417,15 @@ bool floating_maybe_reassign_ws(Con *con) {
     return true;
 }
 
+/*
+ * Centers a floating con above the specified rect.
+ *
+ */
+void floating_center(Con *con, Rect rect) {
+    con->rect.x = rect.x + (rect.width / 2) - (con->rect.width / 2);
+    con->rect.y = rect.y + (rect.height / 2) - (con->rect.height / 2);
+}
+
 DRAGGING_CB(drag_window_callback) {
     const struct xcb_button_press_event_t *event = extra;
 

src/scratchpad.c

@@ -198,10 +198,7 @@ void scratchpad_show(Con *con) {
         con->rect.width = output->rect.width * 0.5;
         con->rect.height = output->rect.height * 0.75;
         floating_check_size(con);
-        con->rect.x = output->rect.x +
+        floating_center(con, con_get_workspace(con)->rect);
-                      ((output->rect.width / 2.0) - (con->rect.width / 2.0));
-        con->rect.y = output->rect.y +
-                      ((output->rect.height / 2.0) - (con->rect.height / 2.0));
     }
 
     /* Activate active workspace if window is from another workspace to ensure

testcases/t/241-consistent-center.t

@@ -0,0 +1,106 @@
+#!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 most of i3's centering methods produce consistent results.
+# Decorations are disabled to avoid floating_enable's logic which shifts
+# windows upwards dependent on their decoration height.
+#
+use i3test i3_autostart => 0;
+
+my $config = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+new_window none
+new_float none
+EOT
+
+my $pid = launch_with_config($config);
+
+#####################################################################
+# Open a floating window, verifying that its initial position is
+# centered, and also verify that both centering methods leave it in
+# its original spot.
+#####################################################################
+
+my $first = open_floating_window;
+
+my $initial = $first->rect;
+is(int($initial->{x} + $initial->{width} / 2), int($x->root->rect->width / 2),
+   'x coordinates match');
+is(int($initial->{y} + $initial->{height} / 2), int($x->root->rect->height / 2),
+   'y coordinates match');
+
+cmd 'move position center';
+
+my $new = $first->rect;
+is($initial->{x}, $new->{x}, 'x coordinates match');
+is($initial->{y}, $new->{y}, 'y coordinates match');
+
+cmd 'move absolute position center';
+
+$new = $first->rect;
+is($initial->{x}, $new->{x}, 'x coordinates match');
+is($initial->{y}, $new->{y}, 'y coordinates match');
+
+#####################################################################
+# Create a second window and move it into and out of the scratchpad.
+# Because it hasn't been moved or resized, it should be floated in
+# the center of the screen when pulled out of the scratchpad.
+#####################################################################
+
+my $second = open_window;
+
+cmd 'move scratchpad, scratchpad show';
+
+$new = $second->rect;
+my $mid_init = $initial->{x} + int($initial->{width} / 2);
+my $mid_new = $new->{x} + int($new->{width} / 2);
+is($mid_init, $mid_new, 'x midpoint is ws center');
+
+$mid_init = $initial->{y} + int($initial->{height} / 2);
+$mid_new = $new->{y} + int($new->{height} / 2);
+is($mid_init, $mid_new, 'y midpoint is ws center');
+
+#####################################################################
+# Verify that manually floating a tiled window results in proper
+# centering.
+#####################################################################
+
+my $third = open_window;
+
+cmd 'floating enable';
+
+$new = $third->rect;
+is($initial->{x}, $new->{x}, 'x coordinates match');
+is($initial->{y}, $new->{y}, 'y coordinates match');
+
+#####################################################################
+# Create a child window of the previous window, which should result
+# in the new window being centered over the last one.
+#####################################################################
+
+my $fourth = open_window( dont_map => 1, client_leader => $third );
+$fourth->map;
+sync_with_i3;
+
+my $child = $fourth->rect;
+is($new->{x}, $child->{x}, 'x coordinates match');
+is($new->{y}, $child->{y}, 'y coordinates match');
+
+exit_gracefully($pid);
+
+done_testing;