Fix T78688: Crash changing workspace with specific fullscreen setup

When switching workspaces we need to have an unused screen layout that
we can activate. The other window now showed the only available screen
layout in fullscreen though.
Usually when there's no unused screen layout we duplicate an existing
one, but that code didn't respect the fullscreen case properly.

This also tries to clean up the logic a bit, but things are still rather
complicated to follow.

Changes in this code are always risky. Of course things worked fine in
my tests, but I wouldn't be surprised if something breaks.

source/blender/editors/include/ED_screen.h

@@ -287,6 +287,12 @@ bool ED_workspace_delete(struct WorkSpace *workspace,
                          struct bContext *C,
                          struct wmWindowManager *wm) ATTR_NONNULL();
 void ED_workspace_scene_data_sync(struct WorkSpaceInstanceHook *hook, Scene *scene) ATTR_NONNULL();
+struct WorkSpaceLayout *ED_workspace_screen_change_ensure_unused_layout(
+    struct Main *bmain,
+    struct WorkSpace *workspace,
+    struct WorkSpaceLayout *layout_new,
+    const struct WorkSpaceLayout *layout_fallback_base,
+    struct wmWindow *win) ATTR_NONNULL();
 struct WorkSpaceLayout *ED_workspace_layout_add(struct Main *bmain,
                                                 struct WorkSpace *workspace,
                                                 struct wmWindow *win,

source/blender/editors/screen/screen_edit.c

@@ -985,39 +985,14 @@ void ED_screen_global_areas_refresh(wmWindow *win)
 /* -------------------------------------------------------------------- */
 /* Screen changing */
 
-static bScreen *screen_fullscreen_find_associated_normal_screen(const Main *bmain, bScreen *screen)
-{
-  for (bScreen *screen_iter = bmain->screens.first; screen_iter;
-       screen_iter = screen_iter->id.next) {
-    if ((screen_iter != screen) && ELEM(screen_iter->state, SCREENMAXIMIZED, SCREENFULL)) {
-      ScrArea *area = screen_iter->areabase.first;
-      if (area && area->full == screen) {
-        return screen_iter;
-      }
-    }
-  }
-
-  return screen;
-}
-
 /**
  * \return the screen to activate.
  * \warning The returned screen may not always equal \a screen_new!
  */
-bScreen *screen_change_prepare(
+void screen_change_prepare(
     bScreen *screen_old, bScreen *screen_new, Main *bmain, bContext *C, wmWindow *win)
 {
-  /* validate screen, it's called with notifier reference */
-  if (BLI_findindex(&bmain->screens, screen_new) == -1) {
-    return NULL;
-  }
-
-  screen_new = screen_fullscreen_find_associated_normal_screen(bmain, screen_new);
-
-  /* check for valid winid */
-  if (!(screen_new->winid == 0 || screen_new->winid == win->winid)) {
-    return NULL;
-  }
+  BLI_assert(BLI_findindex(&bmain->screens, screen_new) != -1);
 
   if (screen_old != screen_new) {
     wmTimer *wt = screen_old->animtimer;
@@ -1038,11 +1013,7 @@ bScreen *screen_change_prepare(
     if (wt) {
       screen_new->animtimer = wt;
     }
-
-    return screen_new;
   }
-
-  return NULL;
 }
 
 void screen_change_update(bContext *C, wmWindow *win, bScreen *screen)
@@ -1075,12 +1046,20 @@ bool ED_screen_change(bContext *C, bScreen *screen)
 {
   Main *bmain = CTX_data_main(C);
   wmWindow *win = CTX_wm_window(C);
+  WorkSpace *workspace = BKE_workspace_active_get(win->workspace_hook);
+  WorkSpaceLayout *layout = BKE_workspace_layout_find(workspace, screen);
   bScreen *screen_old = CTX_wm_screen(C);
-  bScreen *screen_new = screen_change_prepare(screen_old, screen, bmain, C, win);
 
-  if (screen_new) {
-    WorkSpace *workspace = BKE_workspace_active_get(win->workspace_hook);
-    WM_window_set_active_screen(win, workspace, screen);
+  /* Get the actual layout/screen to be activated (guaranteed to be unused, even if that means
+   * having to duplicate an existing one). */
+  WorkSpaceLayout *layout_new = ED_workspace_screen_change_ensure_unused_layout(
+      bmain, workspace, layout, layout, win);
+  bScreen *screen_new = BKE_workspace_layout_screen_get(layout_new);
+
+  screen_change_prepare(screen_old, screen_new, bmain, C, win);
+
+  if (screen_old != screen_new) {
+    WM_window_set_active_screen(win, workspace, screen_new);
     screen_change_update(C, win, screen_new);
 
     return true;

source/blender/editors/screen/screen_intern.h

@@ -50,11 +50,11 @@ bScreen *screen_add(struct Main *bmain, const char *name, const rcti *rect);
 void screen_data_copy(bScreen *to, bScreen *from);
 void screen_new_activate_prepare(const wmWindow *win, bScreen *screen_new);
 void screen_change_update(struct bContext *C, wmWindow *win, bScreen *screen);
-bScreen *screen_change_prepare(bScreen *screen_old,
-                               bScreen *screen_new,
-                               struct Main *bmain,
-                               struct bContext *C,
-                               wmWindow *win);
+void screen_change_prepare(bScreen *screen_old,
+                           bScreen *screen_new,
+                           struct Main *bmain,
+                           struct bContext *C,
+                           wmWindow *win);
 ScrArea *area_split(
     const wmWindow *win, bScreen *screen, ScrArea *area, char dir, float fac, int merge);
 int screen_area_join(struct bContext *C, bScreen *screen, ScrArea *sa1, ScrArea *sa2);

source/blender/editors/screen/workspace_edit.c

@@ -88,22 +88,15 @@ static void workspace_change_update(WorkSpace *workspace_new,
 #endif
 }
 
-static bool workspace_change_find_new_layout_cb(const WorkSpaceLayout *layout, void *UNUSED(arg))
-{
-  /* return false to stop the iterator if we've found a layout that can be activated */
-  return workspace_layout_set_poll(layout) ? false : true;
-}
-
 static WorkSpaceLayout *workspace_change_get_new_layout(Main *bmain,
                                                         WorkSpace *workspace_new,
                                                         wmWindow *win)
 {
-  /* ED_workspace_duplicate may have stored a layout to activate
-   * once the workspace gets activated. */
   WorkSpaceLayout *layout_old = WM_window_get_active_layout(win);
   WorkSpaceLayout *layout_new;
-  bScreen *screen_new;
 
+  /* ED_workspace_duplicate may have stored a layout to activate
+   * once the workspace gets activated. */
   if (win->workspace_hook->temp_workspace_store) {
     layout_new = win->workspace_hook->temp_layout_store;
   }
@@ -113,20 +106,9 @@ static WorkSpaceLayout *workspace_change_get_new_layout(Main *bmain,
       layout_new = workspace_new->layouts.first;
     }
   }
-  screen_new = BKE_workspace_layout_screen_get(layout_new);
 
-  if (screen_new->winid) {
-    /* screen is already used, try to find a free one */
-    WorkSpaceLayout *layout_temp = BKE_workspace_layout_iter_circular(
-        workspace_new, layout_new, workspace_change_find_new_layout_cb, NULL, false);
-    if (!layout_temp) {
-      /* fallback solution: duplicate layout from old workspace */
-      layout_temp = ED_workspace_layout_duplicate(bmain, workspace_new, layout_old, win);
-    }
-    layout_new = layout_temp;
-  }
-
-  return layout_new;
+  return ED_workspace_screen_change_ensure_unused_layout(
+      bmain, workspace_new, layout_new, layout_old, win);
 }
 
 /**
@@ -153,10 +135,7 @@ bool ED_workspace_change(WorkSpace *workspace_new, bContext *C, wmWindowManager
     return false;
   }
 
-  screen_new = screen_change_prepare(screen_old, screen_new, bmain, C, win);
-  if (BKE_workspace_layout_screen_get(layout_new) != screen_new) {
-    layout_new = BKE_workspace_layout_find(workspace_new, screen_new);
-  }
+  screen_change_prepare(screen_old, screen_new, bmain, C, win);
 
   if (screen_new == NULL) {
     return false;

source/blender/editors/screen/workspace_layout_edit.c

@@ -160,6 +160,66 @@ bool ED_workspace_layout_delete(WorkSpace *workspace, WorkSpaceLayout *layout_ol
   return false;
 }
 
+static bool workspace_change_find_new_layout_cb(const WorkSpaceLayout *layout, void *UNUSED(arg))
+{
+  /* return false to stop the iterator if we've found a layout that can be activated */
+  return workspace_layout_set_poll(layout) ? false : true;
+}
+
+static bScreen *screen_fullscreen_find_associated_normal_screen(const Main *bmain, bScreen *screen)
+{
+  for (bScreen *screen_iter = bmain->screens.first; screen_iter;
+       screen_iter = screen_iter->id.next) {
+    if ((screen_iter != screen) && ELEM(screen_iter->state, SCREENMAXIMIZED, SCREENFULL)) {
+      ScrArea *area = screen_iter->areabase.first;
+      if (area && area->full == screen) {
+        return screen_iter;
+      }
+    }
+  }
+
+  return screen;
+}
+
+static bool screen_is_used_by_other_window(const wmWindow *win, const bScreen *screen)
+{
+  return BKE_screen_is_used(screen) && (screen->winid != win->winid);
+}
+
+/**
+ * Make sure there is a non-fullscreen layout to switch to that is not used yet by an other window.
+ * Needed for workspace or screen switching to ensure valid screens.
+ *
+ * \param layout_fallback_base: As last resort, this layout is duplicated and returned.
+ */
+WorkSpaceLayout *ED_workspace_screen_change_ensure_unused_layout(
+    Main *bmain,
+    WorkSpace *workspace,
+    WorkSpaceLayout *layout_new,
+    const WorkSpaceLayout *layout_fallback_base,
+    wmWindow *win)
+{
+  WorkSpaceLayout *layout_temp = layout_new;
+  bScreen *screen_temp = BKE_workspace_layout_screen_get(layout_new);
+
+  screen_temp = screen_fullscreen_find_associated_normal_screen(bmain, screen_temp);
+  layout_temp = BKE_workspace_layout_find(workspace, screen_temp);
+
+  if (screen_is_used_by_other_window(win, screen_temp)) {
+    /* Screen is already used, try to find a free one. */
+    layout_temp = BKE_workspace_layout_iter_circular(
+        workspace, layout_new, workspace_change_find_new_layout_cb, NULL, false);
+    screen_temp = layout_temp ? BKE_workspace_layout_screen_get(layout_temp) : NULL;
+
+    if (!layout_temp || screen_is_used_by_other_window(win, screen_temp)) {
+      /* Fallback solution: duplicate layout. */
+      layout_temp = ED_workspace_layout_duplicate(bmain, workspace, layout_fallback_base, win);
+    }
+  }
+
+  return layout_temp;
+}
+
 static bool workspace_layout_cycle_iter_cb(const WorkSpaceLayout *layout, void *UNUSED(arg))
 {
   /* return false to stop iterator when we have found a layout to activate */