Properly close disabled outputs restored during a restart. (#2337)

If an output is disabled during a restart, for example because a binding
such as

    bindsym $mod+Shift+r exec "xrandr --auto", restart

is used, it can happen that we first write the layout to disk and only
then receive the RandR change events. This leads to a situation where
the restored tree will contain these outputs, but the restarted i3
process will not receive the RandR events, thus the internal output in i3
is marked disabled.

This patch finds these cases after a restart and force-disables the
affected outputs.

fixes #2326

diff --git a/include/randr.h b/include/randr.h
index 8d4b4defa42e46919eb24435dba89e83bebd6849..30dd955dabeafafee2453851786d95b9fa9d2b5e 100644 (file)
--- a/include/randr.h
+++ b/include/randr.h
@@ -60,6 +60,12 @@ void init_ws_for_output(Output *output, Con *content);
  */
 void randr_query_outputs(void);
 
+/**
+ * Disables the output and moves its content.
+ *
+ */
+void randr_disable_output(Output *output);
+
 /**
  * Returns the first output which is active.
  *

diff --git a/src/main.c b/src/main.c
index b2ce17d88761dc3be52d1a64e581d32b0c40452d..8bbd23baf6a86ff5fa35853e684012f14104f00c 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -623,7 +623,7 @@ int main(int argc, char *argv[]) {
     grab_all_keys(conn);
 
     bool needs_tree_init = true;
-    if (layout_path) {
+    if (layout_path != NULL) {
         LOG("Trying to restore the layout from \"%s\".\n", layout_path);
         needs_tree_init = !tree_restore(layout_path, greply);
         if (delete_layout_path) {
@@ -633,7 +633,6 @@ int main(int argc, char *argv[]) {
              * sockets) left. */
             rmdir(dir);
         }
-        free(layout_path);
     }
     if (needs_tree_init)
         tree_init(greply);
@@ -658,6 +657,33 @@ int main(int argc, char *argv[]) {
         randr_init(&randr_base);
     }
 
+    /* We need to force disabling outputs which have been loaded from the
+     * layout file but are no longer active. This can happen if the output has
+     * been disabled in the short time between writing the restart layout file
+     * and restarting i3. See #2326. */
+    if (layout_path != NULL && randr_base > -1) {
+        Con *con;
+        TAILQ_FOREACH(con, &(croot->nodes_head), nodes) {
+            Output *output;
+            TAILQ_FOREACH(output, &outputs, outputs) {
+                if (output->active || strcmp(con->name, output->name) != 0)
+                    continue;
+
+                /* This will correctly correlate the output with its content
+                 * container. We need to make the connection to properly
+                 * disable the output. */
+                if (output->con == NULL) {
+                    output_init_con(output);
+                    output->changed = false;
+                }
+
+                output->to_be_disabled = true;
+                randr_disable_output(output);
+            }
+        }
+    }
+    FREE(layout_path);
+
     scratchpad_fix_resolution();
 
     xcb_query_pointer_reply_t *pointerreply;

diff --git a/src/randr.c b/src/randr.c
index 6aed40cff4c9114b232520bf16359877d69fe444..512e0d2833f9c5bb8d6ba326e9469a91b598a3da 100644 (file)
--- a/src/randr.c
+++ b/src/randr.c
@@ -622,7 +622,7 @@ static void handle_output(xcb_connection_t *conn, xcb_randr_output_t id,
  *
  */
 void randr_query_outputs(void) {
-    Output *output, *other, *first;
+    Output *output, *other;
     xcb_randr_get_output_primary_cookie_t pcookie;
     xcb_randr_get_screen_resources_current_cookie_t rcookie;
 
@@ -733,84 +733,7 @@ void randr_query_outputs(void) {
      * because the user disabled them or because they are clones) */
     TAILQ_FOREACH(output, &outputs, outputs) {
         if (output->to_be_disabled) {
-            output->active = false;
-            DLOG("Output %s disabled, re-assigning workspaces/docks\n", output->name);
-
-            first = get_first_output();
-
-            /* TODO: refactor the following code into a nice function. maybe
-             * use an on_destroy callback which is implement differently for
-             * different container types (CT_CONTENT vs. CT_DOCKAREA)? */
-            Con *first_content = output_get_content(first->con);
-
-            if (output->con != NULL) {
-                /* We need to move the workspaces from the disappearing output to the first output */
-                /* 1: Get the con to focus next, if the disappearing ws is focused */
-                Con *next = NULL;
-                if (TAILQ_FIRST(&(croot->focus_head)) == output->con) {
-                    DLOG("This output (%p) was focused! Getting next\n", output->con);
-                    next = focused;
-                    DLOG("next = %p\n", next);
-                }
-
-                /* 2: iterate through workspaces and re-assign them, fixing the coordinates
-                 * of floating containers as we go */
-                Con *current;
-                Con *old_content = output_get_content(output->con);
-                while (!TAILQ_EMPTY(&(old_content->nodes_head))) {
-                    current = TAILQ_FIRST(&(old_content->nodes_head));
-                    if (current != next && TAILQ_EMPTY(&(current->focus_head))) {
-                        /* the workspace is empty and not focused, get rid of it */
-                        DLOG("Getting rid of current = %p / %s (empty, unfocused)\n", current, current->name);
-                        tree_close_internal(current, DONT_KILL_WINDOW, false, false);
-                        continue;
-                    }
-                    DLOG("Detaching current = %p / %s\n", current, current->name);
-                    con_detach(current);
-                    DLOG("Re-attaching current = %p / %s\n", current, current->name);
-                    con_attach(current, first_content, false);
-                    DLOG("Fixing the coordinates of floating containers\n");
-                    Con *floating_con;
-                    TAILQ_FOREACH(floating_con, &(current->floating_head), floating_windows)
-                    floating_fix_coordinates(floating_con, &(output->con->rect), &(first->con->rect));
-                    DLOG("Done, next\n");
-                }
-                DLOG("re-attached all workspaces\n");
-
-                if (next) {
-                    DLOG("now focusing next = %p\n", next);
-                    con_focus(next);
-                    workspace_show(con_get_workspace(next));
-                }
-
-                /* 3: move the dock clients to the first output */
-                Con *child;
-                TAILQ_FOREACH(child, &(output->con->nodes_head), nodes) {
-                    if (child->type != CT_DOCKAREA)
-                        continue;
-                    DLOG("Handling dock con %p\n", child);
-                    Con *dock;
-                    while (!TAILQ_EMPTY(&(child->nodes_head))) {
-                        dock = TAILQ_FIRST(&(child->nodes_head));
-                        Con *nc;
-                        Match *match;
-                        nc = con_for_window(first->con, dock->window, &match);
-                        DLOG("Moving dock client %p to nc %p\n", dock, nc);
-                        con_detach(dock);
-                        DLOG("Re-attaching\n");
-                        con_attach(dock, nc, false);
-                        DLOG("Done\n");
-                    }
-                }
-
-                DLOG("destroying disappearing con %p\n", output->con);
-                tree_close_internal(output->con, DONT_KILL_WINDOW, true, false);
-                DLOG("Done. Should be fine now\n");
-                output->con = NULL;
-            }
-
-            output->to_be_disabled = false;
-            output->changed = false;
+            randr_disable_output(output);
         }
 
         if (output->changed) {
@@ -846,6 +769,94 @@ void randr_query_outputs(void) {
     FREE(primary);
 }
 
+/*
+ * Disables the output and moves its content.
+ *
+ */
+void randr_disable_output(Output *output) {
+    assert(output->to_be_disabled);
+
+    output->active = false;
+    DLOG("Output %s disabled, re-assigning workspaces/docks\n", output->name);
+
+    Output *first = get_first_output();
+
+    /* TODO: refactor the following code into a nice function. maybe
+     * use an on_destroy callback which is implement differently for
+     * different container types (CT_CONTENT vs. CT_DOCKAREA)? */
+    Con *first_content = output_get_content(first->con);
+
+    if (output->con != NULL) {
+        /* We need to move the workspaces from the disappearing output to the first output */
+        /* 1: Get the con to focus next, if the disappearing ws is focused */
+        Con *next = NULL;
+        if (TAILQ_FIRST(&(croot->focus_head)) == output->con) {
+            DLOG("This output (%p) was focused! Getting next\n", output->con);
+            next = focused;
+            DLOG("next = %p\n", next);
+        }
+
+        /* 2: iterate through workspaces and re-assign them, fixing the coordinates
+         * of floating containers as we go */
+        Con *current;
+        Con *old_content = output_get_content(output->con);
+        while (!TAILQ_EMPTY(&(old_content->nodes_head))) {
+            current = TAILQ_FIRST(&(old_content->nodes_head));
+            if (current != next && TAILQ_EMPTY(&(current->focus_head))) {
+                /* the workspace is empty and not focused, get rid of it */
+                DLOG("Getting rid of current = %p / %s (empty, unfocused)\n", current, current->name);
+                tree_close_internal(current, DONT_KILL_WINDOW, false, false);
+                continue;
+            }
+            DLOG("Detaching current = %p / %s\n", current, current->name);
+            con_detach(current);
+            DLOG("Re-attaching current = %p / %s\n", current, current->name);
+            con_attach(current, first_content, false);
+            DLOG("Fixing the coordinates of floating containers\n");
+            Con *floating_con;
+            TAILQ_FOREACH(floating_con, &(current->floating_head), floating_windows) {
+                floating_fix_coordinates(floating_con, &(output->con->rect), &(first->con->rect));
+            }
+            DLOG("Done, next\n");
+        }
+        DLOG("re-attached all workspaces\n");
+
+        if (next) {
+            DLOG("now focusing next = %p\n", next);
+            con_focus(next);
+            workspace_show(con_get_workspace(next));
+        }
+
+        /* 3: move the dock clients to the first output */
+        Con *child;
+        TAILQ_FOREACH(child, &(output->con->nodes_head), nodes) {
+            if (child->type != CT_DOCKAREA)
+                continue;
+            DLOG("Handling dock con %p\n", child);
+            Con *dock;
+            while (!TAILQ_EMPTY(&(child->nodes_head))) {
+                dock = TAILQ_FIRST(&(child->nodes_head));
+                Con *nc;
+                Match *match;
+                nc = con_for_window(first->con, dock->window, &match);
+                DLOG("Moving dock client %p to nc %p\n", dock, nc);
+                con_detach(dock);
+                DLOG("Re-attaching\n");
+                con_attach(dock, nc, false);
+                DLOG("Done\n");
+            }
+        }
+
+        DLOG("destroying disappearing con %p\n", output->con);
+        tree_close_internal(output->con, DONT_KILL_WINDOW, true, false);
+        DLOG("Done. Should be fine now\n");
+        output->con = NULL;
+    }
+
+    output->to_be_disabled = false;
+    output->changed = false;
+}
+
 /*
  * We have just established a connection to the X server and need the initial
  * XRandR information to setup workspaces for each screen.