Merge pull request #3397 from orestisf1993/randr-enable-disable

Fix bugs in enabling & disabling randr outputs

include/randr.h

@@ -48,7 +48,7 @@ void output_init_con(Output *output);
  * • Create the first unused workspace.
  *
  */
-void init_ws_for_output(Output *output, Con *content);
+void init_ws_for_output(Output *output);
 
 /**
  * Initializes the specified output, assigning the specified workspace to it.

src/fake_outputs.c

@@ -74,7 +74,7 @@ void fake_outputs_init(const char *output_spec) {
             else
                 TAILQ_INSERT_TAIL(&outputs, new_output, outputs);
             output_init_con(new_output);
-            init_ws_for_output(new_output, output_get_content(new_output->con));
+            init_ws_for_output(new_output);
             num_screens++;
         }
         new_output->primary = primary;

src/randr.c

@@ -420,7 +420,10 @@ void output_init_con(Output *output) {
  * • Create the first unused workspace.
  *
  */
-void init_ws_for_output(Output *output, Con *content) {
+void init_ws_for_output(Output *output) {
+    Con *content = output_get_content(output->con);
+    Con *previous_focus = con_get_workspace(focused);
+
     /* go through all assignments and move the existing workspaces to this output */
     struct Workspace_Assignment *assignment;
     TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) {
@@ -442,49 +445,18 @@ void init_ws_for_output(Output *output, Con *content) {
             continue;
         }
 
-        /* if so, move it over */
+        DLOG("Moving workspace \"%s\" from output \"%s\" to \"%s\" due to assignment\n",
-        LOG("Moving workspace \"%s\" from output \"%s\" to \"%s\" due to assignment\n",
+             workspace->name, workspace_out->name, output_primary_name(output));
-            workspace->name, workspace_out->name, output_primary_name(output));
+        /* Need to copy output's rect since content is not yet rendered. We
-
+         * can't call render_con here because render_output only proceeds if a
-        /* if the workspace is currently visible on that output, we need to
+         * workspace exists. */
-         * switch to a different workspace - otherwise the output would end up
+        content->rect = output->con->rect;
-         * with no active workspace */
+        workspace_move_to_output(workspace, output);
-        bool visible = workspace_is_visible(workspace);
-        Con *previous = NULL;
-        if (visible && (previous = TAILQ_NEXT(workspace, focused))) {
-            LOG("Switching to previously used workspace \"%s\" on output \"%s\"\n",
-                previous->name, workspace_out->name);
-            workspace_show(previous);
-        }
-
-        /* Render the output on which the workspace was to get correct Rects.
-         * Then, we need to work with the "content" container, since we cannot
-         * be sure that the workspace itself was rendered at all (in case it’s
-         * invisible, it won’t be rendered). */
-        render_con(workspace_out, false);
-        Con *ws_out_content = output_get_content(workspace_out);
-
-        Con *floating_con;
-        TAILQ_FOREACH(floating_con, &(workspace->floating_head), floating_windows)
-        /* NB: We use output->con here because content is not yet rendered,
-             * so it has a rect of {0, 0, 0, 0}. */
-        floating_fix_coordinates(floating_con, &(ws_out_content->rect), &(output->con->rect));
-
-        con_detach(workspace);
-        con_attach(workspace, content, false);
-
-        /* In case the workspace we just moved was visible but there was no
-         * other workspace to switch to, we need to initialize the source
-         * output as well */
-        if (visible && previous == NULL) {
-            LOG("There is no workspace left on \"%s\", re-initializing\n",
-                workspace_out->name);
-            init_ws_for_output(get_output_by_name(workspace_out->name, true),
-                               output_get_content(workspace_out));
-            DLOG("Done re-initializing, continuing with \"%s\"\n", output_primary_name(output));
-        }
     }
 
+    /* Temporarily set the focused container, might not be initialized yet. */
+    focused = content;
+
     /* if a workspace exists, we are done now */
     if (!TAILQ_EMPTY(&(content->nodes_head))) {
         /* ensure that one of the workspaces is actually visible (in fullscreen
@@ -493,10 +465,9 @@ void init_ws_for_output(Output *output, Con *content) {
         GREP_FIRST(visible, content, child->fullscreen_mode == CF_OUTPUT);
         if (!visible) {
             visible = TAILQ_FIRST(&(content->nodes_head));
-            focused = content;
             workspace_show(visible);
         }
-        return;
+        goto restore_focus;
     }
 
     /* otherwise, we create the first assigned ws for this output */
@@ -507,17 +478,18 @@ void init_ws_for_output(Output *output, Con *content) {
 
         LOG("Initializing first assigned workspace \"%s\" for output \"%s\"\n",
             assignment->name, assignment->output);
-        focused = content;
         workspace_show_by_name(assignment->name);
-        return;
+        goto restore_focus;
     }
 
     /* if there is still no workspace, we create the first free workspace */
     DLOG("Now adding a workspace\n");
-    Con *ws = create_workspace_on_output(output, content);
+    workspace_show(create_workspace_on_output(output, content));
 
-    /* TODO: Set focus in main.c */
+restore_focus:
-    con_focus(ws);
+    if (previous_focus) {
+        workspace_show(previous_focus);
+    }
 }
 
 /*
@@ -937,7 +909,7 @@ void randr_query_outputs(void) {
         if (!TAILQ_EMPTY(&(content->nodes_head)))
             continue;
         DLOG("Should add ws for output %s\n", output_primary_name(output));
-        init_ws_for_output(output, content);
+        init_ws_for_output(output);
     }
 
     /* Focus the primary screen, if possible */
@@ -976,13 +948,8 @@ void randr_disable_output(Output *output) {
 
     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 */
+        /* 1: Get the con to focus next */
-        Con *next = NULL;
+        Con *next = focused;
-        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 */
@@ -1005,15 +972,12 @@ void randr_disable_output(Output *output) {
             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) {
+        /* Restore focus after con_detach / con_attach */
-            DLOG("now focusing next = %p\n", next);
+        DLOG("now focusing next = %p\n", next);
-            con_activate(next);
+        con_focus(next);
-            workspace_show(con_get_workspace(next));
+        workspace_show(con_get_workspace(next));
-        }
 
         /* 3: move the dock clients to the first output */
         Con *child;
@@ -1050,7 +1014,7 @@ void randr_disable_output(Output *output) {
 static void fallback_to_root_output(void) {
     root_output->active = true;
     output_init_con(root_output);
-    init_ws_for_output(root_output, output_get_content(root_output->con));
+    init_ws_for_output(root_output);
 }
 
 /*

src/workspace.c

@@ -972,7 +972,11 @@ bool workspace_move_to_output(Con *ws, Output *output) {
     LOG("got output %p with content %p\n", output, content);
 
     Con *previously_visible_ws = TAILQ_FIRST(&(content->focus_head));
-    LOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name);
+    if (previously_visible_ws) {
+        DLOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name);
+    } else {
+        DLOG("No previously visible workspace on output.\n");
+    }
 
     bool workspace_was_visible = workspace_is_visible(ws);
     if (con_num_children(ws->parent) == 1) {
@@ -1030,6 +1034,10 @@ bool workspace_move_to_output(Con *ws, Output *output) {
         workspace_show(ws);
     }
 
+    if (!previously_visible_ws) {
+        return true;
+    }
+
     /* NB: We cannot simply work with previously_visible_ws since it might
      * have been cleaned up by workspace_show() already, depending on the
      * focus order/number of other workspaces on the output.

src/xinerama.c

@@ -71,7 +71,7 @@ static void query_screens(xcb_connection_t *conn) {
             else
                 TAILQ_INSERT_TAIL(&outputs, s, outputs);
             output_init_con(s);
-            init_ws_for_output(s, output_get_content(s->con));
+            init_ws_for_output(s);
             num_screens++;
         }
 
@@ -98,7 +98,7 @@ static void use_root_output(xcb_connection_t *conn) {
     s->active = true;
     TAILQ_INSERT_TAIL(&outputs, s, outputs);
     output_init_con(s);
-    init_ws_for_output(s, output_get_content(s->con));
+    init_ws_for_output(s);
 }
 
 /*