Fix T55183, fix T55174: crashes with workspace / view layer relation.

Don't store pointers to ViewLayer in the workspace, only names. Add specific
relation type since the generic mechanism makes the code hard to follow.
Integrate with pointer restore for undo and library remapping code to avoid
data going out of sync.

Also add relation automatically if there doesn't exists one yet in
BKE_workspace_view_layer_get, because in general it's really hard to ensure
it will exist when making arbitrary scene changes.

Differential Revision: https://developer.blender.org/D3432

source/blender/blenkernel/BKE_layer.h

@@ -80,7 +80,7 @@ void BKE_view_layer_copy_data(
         struct ViewLayer *view_layer_dst, const struct ViewLayer *view_layer_src,
         const int flag);
 
-void BKE_view_layer_rename(struct Scene *scene, struct ViewLayer *view_layer, const char *name);
+void BKE_view_layer_rename(struct Main *bmain, struct Scene *scene, struct ViewLayer *view_layer, const char *name);
 
 struct LayerCollection *BKE_layer_collection_get_active(struct ViewLayer *view_layer);
 bool BKE_layer_collection_activate(struct ViewLayer *view_layer, struct LayerCollection *lc);

source/blender/blenkernel/BKE_workspace.h

@@ -53,12 +53,20 @@ void BKE_workspace_layout_remove(
 
 void BKE_workspace_relations_free(
         ListBase *relation_list);
+void BKE_workspace_scene_relations_free_invalid(
+        struct WorkSpace *workspace);
 
 
 /* -------------------------------------------------------------------- */
 /* General Utils */
 
-void BKE_workspace_view_layer_remove_references(
+void BKE_workspace_view_layer_rename(
+        const struct Main *bmain,
+        const struct Scene *scene,
+        const char *old_name,
+        const char *new_name) ATTR_NONNULL();
+
+void BKE_workspace_view_layer_remove(
         const struct Main *bmain,
         const struct ViewLayer *view_layer) ATTR_NONNULL();
 
@@ -92,6 +100,9 @@ struct Base *BKE_workspace_active_base_get(const struct WorkSpace *workspace, co
 struct ViewLayer *BKE_workspace_view_layer_get(
         const struct WorkSpace *workspace,
         const struct Scene *scene) GETTER_ATTRS;
+struct ViewLayer *BKE_workspace_view_layer_exists(
+        const struct WorkSpace *workspace,
+        const struct Scene *scene) GETTER_ATTRS;
 void BKE_workspace_view_layer_set(
         struct WorkSpace *workspace,
         struct ViewLayer *layer,

source/blender/blenkernel/intern/layer.c

@@ -397,7 +397,7 @@ void BKE_view_layer_copy_data(
 	// TODO: not always safe to free BKE_layer_collection_sync(scene_dst, view_layer_dst);
 }
 
-void BKE_view_layer_rename(Scene *scene, ViewLayer *view_layer, const char *newname)
+void BKE_view_layer_rename(Main *bmain, Scene *scene, ViewLayer *view_layer, const char *newname)
 {
 	char oldname[sizeof(view_layer->name)];
 
@@ -418,8 +418,9 @@ void BKE_view_layer_rename(Scene *scene, ViewLayer *view_layer, const char *newn
 		}
 	}
 
-	/* fix all the animation data which may link to this */
+	/* fix all the animation data and workspace which may link to this */
 	BKE_animdata_fix_paths_rename_all(NULL, "view_layers", oldname, view_layer->name);
+	BKE_workspace_view_layer_rename(bmain, scene, oldname, view_layer->name);
 
 	/* Dependency graph uses view layer name based lookups. */
 	DEG_id_tag_update(&scene->id, 0);

source/blender/blenkernel/intern/library_query.c

@@ -932,6 +932,9 @@ void BKE_library_foreach_ID_link(Main *bmain, ID *id, LibraryIDLinkCallback call
 					BKE_workspace_layout_screen_set(layout, screen);
 				}
 
+				for (WorkSpaceSceneRelation *relation = workspace->scene_relations.first; relation; relation = relation->next) {
+					CALLBACK_INVOKE(relation->scene, IDWALK_CB_NOP);
+				}
 				break;
 			}
 			case ID_GD:
@@ -1080,6 +1083,7 @@ bool BKE_library_id_can_use_idtype(ID *id_owner, const short id_type_used)
 		case ID_LP:
 			return ELEM(id_type_used, ID_IM);
 		case ID_WS:
+			return ELEM(id_type_used, ID_SCR, ID_SCE);
 		case ID_IM:
 		case ID_VF:
 		case ID_TXT:

source/blender/blenkernel/intern/workspace.c

@@ -26,6 +26,7 @@
 #define DNA_PRIVATE_WORKSPACE_ALLOW
 
 #include <stdlib.h>
+#include <string.h>
 
 #include "BLI_utildefines.h"
 #include "BLI_string.h"
@@ -117,18 +118,6 @@ static void *workspace_relation_get_data_matching_parent(
 	}
 }
 
-static void workspace_relation_remove_from_value(
-        ListBase *relation_list, const void *value)
-{
-	for (WorkSpaceDataRelation *relation = relation_list->first, *relation_next; relation; relation = relation_next) {
-		relation_next = relation->next;
-
-		if (relation->value == value) {
-			workspace_relation_remove(relation_list, relation);
-		}
-	}
-}
-
 /**
  * Checks if \a screen is already used within any workspace. A screen should never be assigned to multiple
  * WorkSpaceLayouts, but that should be ensured outside of the BKE_workspace module and without such checks.
@@ -168,7 +157,7 @@ WorkSpace *BKE_workspace_add(Main *bmain, const char *name)
 void BKE_workspace_free(WorkSpace *workspace)
 {
 	BKE_workspace_relations_free(&workspace->hook_layout_relations);
-	BKE_workspace_relations_free(&workspace->scene_viewlayer_relations);
+	BLI_freelistN(&workspace->scene_relations);
 
 	BLI_freelistN(&workspace->owner_ids);
 	BLI_freelistN(&workspace->layouts);
@@ -269,16 +258,45 @@ void BKE_workspace_relations_free(
 	}
 }
 
+void BKE_workspace_scene_relations_free_invalid(
+        WorkSpace *workspace)
+{
+	for (WorkSpaceSceneRelation *relation = workspace->scene_relations.first, *relation_next; relation; relation = relation_next) {
+		relation_next = relation->next;
+
+		if (relation->scene == NULL) {
+			BLI_freelinkN(&workspace->scene_relations, relation);
+		}
+		else if (!BLI_findstring(&relation->scene->view_layers, relation->view_layer, offsetof(ViewLayer, name))) {
+			BLI_freelinkN(&workspace->scene_relations, relation);
+		}
+	}
+}
 
 /* -------------------------------------------------------------------- */
 /* General Utils */
 
-void BKE_workspace_view_layer_remove_references(
+void BKE_workspace_view_layer_rename(
         const Main *bmain,
-        const ViewLayer *view_layer)
+        const Scene *scene,
+        const char *old_name,
+        const char *new_name)
 {
 	for (WorkSpace *workspace = bmain->workspaces.first; workspace; workspace = workspace->id.next) {
-		workspace_relation_remove_from_value(&workspace->scene_viewlayer_relations, view_layer);
+		for (WorkSpaceSceneRelation *relation = workspace->scene_relations.first; relation; relation = relation->next) {
+			if (relation->scene == scene && STREQ(relation->view_layer, old_name)) {
+				STRNCPY(relation->view_layer, new_name);
+			}
+		}
+	}
+}
+
+void BKE_workspace_view_layer_remove(
+        const Main *bmain,
+        const ViewLayer *UNUSED(view_layer))
+{
+	for (WorkSpace *workspace = bmain->workspaces.first; workspace; workspace = workspace->id.next) {
+		BKE_workspace_scene_relations_free_invalid(workspace);
 	}
 }
 
@@ -408,13 +426,38 @@ Base *BKE_workspace_active_base_get(const WorkSpace *workspace, const Scene *sce
 	return view_layer->basact;
 }
 
+ViewLayer *BKE_workspace_view_layer_exists(const WorkSpace *workspace, const Scene *scene)
+{
+	WorkSpaceSceneRelation *relation = BLI_findptr(&workspace->scene_relations, scene, offsetof(WorkSpaceSceneRelation, scene));
+	return (relation) ? BLI_findstring(&scene->view_layers, relation->view_layer, offsetof(ViewLayer, name)) : NULL;
+}
+
 ViewLayer *BKE_workspace_view_layer_get(const WorkSpace *workspace, const Scene *scene)
 {
-	return workspace_relation_get_data_matching_parent(&workspace->scene_viewlayer_relations, scene);
+	ViewLayer *layer = BKE_workspace_view_layer_exists(workspace, scene);
+
+	if (layer == NULL) {
+		BKE_workspace_view_layer_set((WorkSpace*)workspace, scene->view_layers.first, (Scene *)scene);
+		layer = scene->view_layers.first;
+	}
+
+	return layer;
 }
+
 void BKE_workspace_view_layer_set(WorkSpace *workspace, ViewLayer *layer, Scene *scene)
 {
-	workspace_relation_ensure_updated(&workspace->scene_viewlayer_relations, scene, layer);
+	WorkSpaceSceneRelation *relation = BLI_findptr(&workspace->scene_relations, scene, offsetof(WorkSpaceSceneRelation, scene));
+	if (relation == NULL) {
+		relation = MEM_callocN(sizeof(*relation), __func__);
+	}
+	else {
+		BLI_remlink(&workspace->scene_relations, relation);
+	}
+
+	/* (Re)insert at the head of the list, for faster lookups. */
+	relation->scene = scene;
+	STRNCPY(relation->view_layer, layer->name);
+	BLI_addhead(&workspace->scene_relations, relation);
 }
 
 ListBase *BKE_workspace_layouts_get(WorkSpace *workspace)
@@ -422,7 +465,6 @@ ListBase *BKE_workspace_layouts_get(WorkSpace *workspace)
 	return &workspace->layouts;
 }
 
-
 const char *BKE_workspace_layout_name_get(const WorkSpaceLayout *layout)
 {
 	return layout->name;

source/blender/blenloader/intern/readfile.c

@@ -2865,6 +2865,19 @@ static void direct_link_cachefile(FileData *fd, CacheFile *cache_file)
 
 /* ************ READ WORKSPACES *************** */
 
+static void lib_link_workspace_scene_data(FileData *fd, WorkSpace *workspace)
+{
+	for (WorkSpaceSceneRelation *relation = workspace->scene_relations.first;
+		 relation != NULL;
+		 relation = relation->next)
+	{
+		relation->scene = newlibadr(fd, workspace->id.lib, relation->scene);
+	}
+
+	/* Free any relations that got lost due to missing datablocks. */
+	BKE_workspace_scene_relations_free_invalid(workspace);
+}
+
 static void lib_link_workspaces(FileData *fd, Main *bmain)
 {
 	for (WorkSpace *workspace = bmain->workspaces.first; workspace; workspace = workspace->id.next) {
@@ -2877,31 +2890,7 @@ static void lib_link_workspaces(FileData *fd, Main *bmain)
 		IDP_LibLinkProperty(id->properties, fd);
 		id_us_ensure_real(id);
 
-		for (WorkSpaceDataRelation *relation = workspace->scene_viewlayer_relations.first,
-		     *relation_next = NULL;
-		     relation != NULL;
-		     relation = relation_next)
-		{
-			relation_next = relation->next;
-
-			relation->parent = newlibadr(fd, id->lib, relation->parent);
-			/* relation->value is set in direct_link_workspace_link_scene_data,
-			 * except when loading linked data. */
-			Scene *scene = relation->parent;
-
-			if (scene) {
-				if (scene->id.lib != NULL) {
-					relation->value = BLI_findstring(&scene->view_layers, relation->value_name, offsetof(ViewLayer, name));
-				}
-				if (relation->value == NULL) {
-					relation->value = scene->view_layers.first;
-				}
-			}
-			else {
-				/* Remove empty relation if scene got lost. */
-				BLI_freelinkN(&workspace->scene_viewlayer_relations, relation);
-			}
-		}
+		lib_link_workspace_scene_data(fd, workspace);
 
 		for (WorkSpaceLayout *layout = layouts->first, *layout_next; layout; layout = layout_next) {
 			bScreen *screen = newlibadr(fd, id->lib, BKE_workspace_layout_screen_get(layout));
@@ -2928,7 +2917,7 @@ static void direct_link_workspace(FileData *fd, WorkSpace *workspace, const Main
 {
 	link_list(fd, BKE_workspace_layouts_get(workspace));
 	link_list(fd, &workspace->hook_layout_relations);
-	link_list(fd, &workspace->scene_viewlayer_relations);
+	link_list(fd, &workspace->scene_relations);
 	link_list(fd, &workspace->owner_ids);
 	link_list(fd, &workspace->tools);
 
@@ -2940,12 +2929,6 @@ static void direct_link_workspace(FileData *fd, WorkSpace *workspace, const Main
 		relation->value = newdataadr(fd, relation->value);
 	}
 
-	if (ID_IS_LINKED(&workspace->id)) {
-		/* Appending workspace so render layer is likely from a different scene. Unset
-		 * now, when activating workspace later we set a valid one from current scene. */
-		BKE_workspace_relations_free(&workspace->scene_viewlayer_relations);
-	}
-
 	/* 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. */
 	for (wmWindowManager *wm = main->wm.first; wm; wm = wm->id.next) {
@@ -6071,31 +6054,7 @@ static void direct_link_sequence_modifiers(FileData *fd, ListBase *lb)
 	}
 }
 
-/**
- * Workspaces store a render layer pointer which can only be read after scene is read.
- */
-static void direct_link_workspace_link_scene_data(
-        FileData *fd, Scene *scene, const ListBase *workspaces)
-{
-	for (WorkSpace *workspace = workspaces->first; workspace; workspace = workspace->id.next) {
-		for (WorkSpaceDataRelation *relation = workspace->scene_viewlayer_relations.first;
-		     relation != NULL;
-		     relation = relation->next)
-		{
-			ViewLayer *view_layer = newdataadr(fd, relation->value);
-			if (view_layer != NULL) {
-				BLI_assert(BLI_findindex(&scene->view_layers, view_layer) != -1);
-				/* relation->parent is set in lib_link_workspaces */
-			}
-			if (UNLIKELY(view_layer == NULL)) {
-				view_layer = scene->view_layers.first;
-			}
-			relation->value = view_layer;
-		}
-	}
-}
-
-static void direct_link_scene(FileData *fd, Scene *sce, Main *bmain)
+static void direct_link_scene(FileData *fd, Scene *sce)
 {
 	Editing *ed;
 	Sequence *seq;
@@ -6361,8 +6320,6 @@ static void direct_link_scene(FileData *fd, Scene *sce, Main *bmain)
 
 	sce->layer_properties = newdataadr(fd, sce->layer_properties);
 	IDP_DirectLinkGroup_OrFree(&sce->layer_properties, (fd->flags & FD_FLAGS_SWITCH_ENDIAN), fd);
-
-	direct_link_workspace_link_scene_data(fd, sce, &bmain->workspaces);
 }
 
 /* ****************** READ GREASE PENCIL ***************** */
@@ -7174,7 +7131,20 @@ static void lib_link_clipboard_restore(struct IDNameLib_Map *id_map)
 	BKE_sequencer_base_recursive_apply(&seqbase_clipboard, lib_link_seq_clipboard_cb, id_map);
 }
 
-static void lib_link_workspace_scene_data_restore(wmWindow *win, Scene *scene)
+static void lib_link_workspace_scene_data_restore(struct IDNameLib_Map *id_map, WorkSpace *workspace)
+{
+	for (WorkSpaceSceneRelation *relation = workspace->scene_relations.first;
+		 relation != NULL;
+		 relation = relation->next)
+	{
+		relation->scene = restore_pointer_by_name(id_map, &relation->scene->id, USER_IGNORE);
+	}
+
+	/* Free any relations that got lost due to missing datablocks or view layers. */
+	BKE_workspace_scene_relations_free_invalid(workspace);
+}
+
+static void lib_link_window_scene_data_restore(wmWindow *win, Scene *scene)
 {
 	bScreen *screen = BKE_workspace_active_screen_get(win->workspace_hook);
 
@@ -7435,12 +7405,14 @@ void blo_lib_link_restore(Main *newmain, wmWindowManager *curwm, Scene *curscene
 	struct IDNameLib_Map *id_map = BKE_main_idmap_create(newmain);
 
 	for (WorkSpace *workspace = newmain->workspaces.first; workspace; workspace = workspace->id.next) {
+		lib_link_workspace_scene_data_restore(id_map, workspace);
+		BKE_workspace_view_layer_set(workspace, cur_view_layer, curscene);
+
 		ListBase *layouts = BKE_workspace_layouts_get(workspace);
 
 		for (WorkSpaceLayout *layout = layouts->first; layout; layout = layout->next) {
 			lib_link_workspace_layout_restore(id_map, newmain, layout);
 		}
-		BKE_workspace_view_layer_set(workspace, cur_view_layer, curscene);
 	}
 
 	for (wmWindow *win = curwm->windows.first; win; win = win->next) {
@@ -7459,7 +7431,7 @@ void blo_lib_link_restore(Main *newmain, wmWindowManager *curwm, Scene *curscene
 		/* keep cursor location through undo */
 		copy_v3_v3(win->scene->cursor.location, oldscene->cursor.location);
 		copy_qt_qt(win->scene->cursor.rotation, oldscene->cursor.rotation);
-		lib_link_workspace_scene_data_restore(win, win->scene);
+		lib_link_window_scene_data_restore(win, win->scene);
 
 		BLI_assert(win->screen == NULL);
 	}
@@ -8371,7 +8343,7 @@ static BHead *read_libblock(FileData *fd, Main *main, BHead *bhead, const short
 			wrong_id = direct_link_screen(fd, (bScreen *)id);
 			break;
 		case ID_SCE:
-			direct_link_scene(fd, (Scene *)id, main);
+			direct_link_scene(fd, (Scene *)id);
 			break;
 		case ID_OB:
 			direct_link_object(fd, (Object *)id);

source/blender/blenloader/intern/writefile.c

@@ -3599,18 +3599,10 @@ static void write_workspace(WriteData *wd, WorkSpace *workspace)
 {
 	ListBase *layouts = BKE_workspace_layouts_get(workspace);
 
-	/* Update the names for file (only need to set on write). */
-	for (WorkSpaceDataRelation *relation = workspace->scene_viewlayer_relations.first;
-	     relation;
-	     relation = relation->next)
-	{
-		STRNCPY(relation->value_name, ((ViewLayer *)relation->value)->name);
-	}
-
 	writestruct(wd, ID_WS, WorkSpace, 1, workspace);
 	writelist(wd, DATA, WorkSpaceLayout, layouts);
 	writelist(wd, DATA, WorkSpaceDataRelation, &workspace->hook_layout_relations);
-	writelist(wd, DATA, WorkSpaceDataRelation, &workspace->scene_viewlayer_relations);
+	writelist(wd, DATA, WorkSpaceDataRelation, &workspace->scene_relations);
 	writelist(wd, DATA, wmOwnerID, &workspace->owner_ids);
 	writelist(wd, DATA, bToolRef, &workspace->tools);
 	for (bToolRef *tref = workspace->tools.first; tref; tref = tref->next) {

source/blender/editors/scene/scene_edit.c

@@ -116,7 +116,7 @@ bool ED_scene_delete(bContext *C, Main *bmain, wmWindow *win, Scene *scene)
 
 static ViewLayer *scene_change_get_new_view_layer(const WorkSpace *workspace, const Scene *scene_new)
 {
-	ViewLayer *layer_new = BKE_workspace_view_layer_get(workspace, scene_new);
+	ViewLayer *layer_new = BKE_workspace_view_layer_exists(workspace, scene_new);
 	return layer_new ? layer_new : BKE_view_layer_default_view(scene_new);
 }
 
@@ -202,10 +202,11 @@ bool ED_scene_view_layer_delete(
 	BLI_assert(BLI_listbase_is_empty(&scene->view_layers) == false);
 
 	ED_workspace_view_layer_unset(bmain, scene, layer, scene->view_layers.first);
-	BKE_workspace_view_layer_remove_references(bmain, layer);
 
 	BKE_view_layer_free(layer);
 
+	BKE_workspace_view_layer_remove(bmain, layer);
+
 	DEG_id_tag_update(&scene->id, 0);
 	DEG_relations_tag_update(bmain);
 	WM_main_add_notifier(NC_SCENE | ND_LAYER | NA_REMOVED, scene);

source/blender/editors/screen/workspace_edit.c

@@ -110,7 +110,7 @@ static void workspace_change_update_view_layer(
         WorkSpace *workspace_new, const WorkSpace *workspace_old,
         Scene *scene)
 {
-	if (!BKE_workspace_view_layer_get(workspace_new, scene)) {
+	if (!BKE_workspace_view_layer_exists(workspace_new, scene)) {
 		BKE_workspace_view_layer_set(workspace_new, BKE_workspace_view_layer_get(workspace_old, scene), scene);
 	}
 }
@@ -199,7 +199,7 @@ bool ED_workspace_change(
 		screen_change_update(C, win, screen_new);
 		workspace_change_update(workspace_new, workspace_old, C, wm);
 
-		BLI_assert(BKE_workspace_view_layer_get(workspace_new, CTX_data_scene(C)) != NULL);
+		BLI_assert(BKE_workspace_view_layer_exists(workspace_new, CTX_data_scene(C)) != NULL);
 		BLI_assert(CTX_wm_workspace(C) == workspace_new);
 
 		WM_toolsystem_unlink_all(C, workspace_old);

source/blender/editors/space_outliner/outliner_draw.c

@@ -402,7 +402,7 @@ static void namebutton_cb(bContext *C, void *tsep, char *oldname)
 					BLI_strncpy(view_layer->name, oldname, sizeof(view_layer->name));
 
 					/* Rename, preserving animation and compositing data. */
-					BKE_view_layer_rename(scene, view_layer, newname);
+					BKE_view_layer_rename(bmain, scene, view_layer, newname);
 					WM_event_add_notifier(C, NC_ID | NA_RENAME, NULL);
 					break;
 				}

source/blender/makesdna/DNA_workspace_types.h

@@ -126,7 +126,7 @@ typedef struct WorkSpace {
 	/* Store for each hook (so for each window) which layout has
 	 * been activated the last time this workspace was visible. */
 	ListBase hook_layout_relations DNA_PRIVATE_WORKSPACE_READ_WRITE; /* WorkSpaceDataRelation */
-	ListBase scene_viewlayer_relations DNA_PRIVATE_WORKSPACE_READ_WRITE; /* WorkSpaceDataRelation */
+	ListBase scene_relations; /* WorkSpaceSceneRelation */
 
 	/* Feature tagging (use for addons) */
 	ListBase owner_ids DNA_PRIVATE_WORKSPACE_READ_WRITE; /* wmOwnerID */
@@ -180,13 +180,17 @@ typedef struct WorkSpaceDataRelation {
 	void *parent;
 	/* The value for this parent-data/workspace relation */
 	void *value;
-
-	/** Use when we reference non-ID data, this allows use to look it up when linking in a workspace. */
-	char value_name[64];  /* MAX_NAME. */
 } WorkSpaceDataRelation;
 
 #endif /* DNA_PRIVATE_WORKSPACE_READ_WRITE */
 
+typedef struct WorkSpaceSceneRelation {
+	struct WorkSpaceSceneRelation *next, *prev;
+
+	struct Scene *scene;
+	char view_layer[64]; /* MAX_NAME */
+} WorkSpaceSceneRelation;
+
 /**
  * Little wrapper to store data that is going to be per window, but comming from the workspace.
  * It allows us to keep workspace and window data completely separate.

source/blender/makesrna/intern/rna_scene.c

@@ -1485,7 +1485,7 @@ void rna_ViewLayer_name_set(PointerRNA *ptr, const char *value)
 {
 	Scene *scene = (Scene *)ptr->id.data;
 	ViewLayer *view_layer = (ViewLayer *)ptr->data;
-	BKE_view_layer_rename(scene, view_layer, value);
+	BKE_view_layer_rename(G.main, scene, view_layer, value);
 }
 
 static void rna_SceneRenderView_name_set(PointerRNA *ptr, const char *value)