[Bf-blender-cvs] [d704c293c2e] lanpr-under-gp: Fix (unrepported) utterly broken logic in readfile for Workspaces.

Bastien Montagne noreply at git.blender.org
Fri Oct 2 07:40:24 CEST 2020


Commit: d704c293c2edcea5249b5dee709d2bc8865c753b
Author: Bastien Montagne
Date:   Tue Sep 29 12:14:04 2020 +0200
Branches: lanpr-under-gp
https://developer.blender.org/rBd704c293c2edcea5249b5dee709d2bc8865c753b

Fix (unrepported) utterly broken logic in readfile for Workspaces.

Remove the attempt to update the active layout pointers of each window
from whithin `direct_link_workspace`.

This piece of code was a nonsense for at least to critical reasons:
* Do not, never, ever, access data from another datablock within the
  direct_link_... functions. Just don't. Don't try to be smart.
* Since it was trying (and failing) to update the active layout of every
  window for every workspace, it was effectively setting those
  `act_layout` pointers to NULL (remapping can only ever happen once,
  trying to remap and already remapped new pointer is bound to fail in
  any case).

Luckily (and funnily), this piece of code was actually harmless, since
setting the active layout would be overridden/redone later, in
`lib_link_windowmanager`, when updating their `workspace_hook` in
`lib_link_workspace_instance_hook`.

Note that the similar horror with `WorkSpaceDataRelation->parent` (which
points at a window) is kept for now, because that one is needed. Hope to
refactor it soon though.

===================================================================

M	source/blender/blenloader/intern/readfile.c

===================================================================

diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index 491214570eb..a0bb457a4c4 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -2573,7 +2573,7 @@ static void lib_link_workspaces(BlendLibReader *reader, WorkSpace *workspace)
   }
 }
 
-static void direct_link_workspace(BlendDataReader *reader, WorkSpace *workspace, const Main *main)
+static void direct_link_workspace(BlendDataReader *reader, WorkSpace *workspace)
 {
   BLO_read_list(reader, &workspace->layouts);
   BLO_read_list(reader, &workspace->hook_layout_relations);
@@ -2582,18 +2582,14 @@ static void direct_link_workspace(BlendDataReader *reader, WorkSpace *workspace,
 
   LISTBASE_FOREACH (WorkSpaceDataRelation *, relation, &workspace->hook_layout_relations) {
     /* data from window - need to access through global oldnew-map */
+    /* XXX This is absolutely not acceptable. There is no acceptable reasons to mess with other
+     * ID's data in read code, and certainly never, ever in `direct_link_` functions.
+     * Kept for now because it seems to work, but it should be refactored. Probably store and use
+     * window's `winid`, just like it was already done for screens? */
     relation->parent = newglobadr(reader->fd, relation->parent);
     BLO_read_data_address(reader, &relation->value);
   }
 
-  /* Same issue/fix as in direct_link_workspace_link_scene_data: Can't read workspace data
-   * when reading windows, so have to update windows after/when reading workspaces. */
-  LISTBASE_FOREACH (wmWindowManager *, wm, &main->wm) {
-    LISTBASE_FOREACH (wmWindow *, win, &wm->windows) {
-      BLO_read_data_address(reader, &win->workspace_hook->act_layout);
-    }
-  }
-
   LISTBASE_FOREACH (bToolRef *, tref, &workspace->tools) {
     tref->runtime = NULL;
     BLO_read_data_address(reader, &tref->properties);
@@ -6371,7 +6367,7 @@ static bool direct_link_id(FileData *fd, Main *main, const int tag, ID *id, ID *
       direct_link_particlesettings(&reader, (ParticleSettings *)id);
       break;
     case ID_WS:
-      direct_link_workspace(&reader, (WorkSpace *)id, main);
+      direct_link_workspace(&reader, (WorkSpace *)id);
       break;
     case ID_ME:
     case ID_LT:



More information about the Bf-blender-cvs mailing list