[Bf-blender-cvs] [748e95ad50f] blender2.8: Fix crash when deleting collections

[Julian Eisel]

Commit: 748e95ad50f260dd67aa83927dd90b389a1f3c59
Author: Julian Eisel
Date:   Wed Jan 10 22:45:44 2018 +0100
Branches: blender2.8
https://developer.blender.org/rB748e95ad50f260dd67aa83927dd90b389a1f3c59

Fix crash when deleting collections

With factory settings, steps to reproduce were:
* Select "Collection 1" (in "RenderLayer")
* Delete
It might crash at this point, although maybe this crash is ASAN only.

However, this was also doing some weird things that I've corrected now. It
called outliner_build_tree in an operator callback. This should only be
called in the main redraw function or so, not in regular handlers.
Instead, we manually cleanup the tree to keep it valid.

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

M	source/blender/editors/space_outliner/outliner_collections.c
M	source/blender/editors/space_outliner/outliner_intern.h
M	source/blender/editors/space_outliner/outliner_tree.c

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

diff --git a/source/blender/editors/space_outliner/outliner_collections.c b/source/blender/editors/space_outliner/outliner_collections.c
index 239e6a9b0fc..2f4d8cc60cb 100644
--- a/source/blender/editors/space_outliner/outliner_collections.c
+++ b/source/blender/editors/space_outliner/outliner_collections.c
@@ -628,6 +628,26 @@ static TreeTraversalAction collection_find_data_to_delete(TreeElement *te, void
 	}
 	else {
 		BLI_gset_add(data->collections_to_delete, scene_collection);
+		return TRAVERSE_SKIP_CHILDS; /* Childs will be gone anyway, no need to recurse deeper. */
+	}
+
+	return TRAVERSE_CONTINUE;
+}
+
+static TreeTraversalAction collection_delete_elements_from_collection(TreeElement *te, void *customdata)
+{
+	struct CollectionDeleteData *data = customdata;
+	SceneCollection *scene_collection = outliner_scene_collection_from_tree_element(te);
+
+	if (!scene_collection) {
+		return TRAVERSE_SKIP_CHILDS;
+	}
+
+	const bool will_be_deleted = BLI_gset_haskey(data->collections_to_delete, scene_collection);
+	if (will_be_deleted) {
+		outliner_free_tree_element(te, te->parent ? &te->parent->subtree : &data->soops->tree);
+		/* Childs are freed now, so don't recurse into them. */
+		return TRAVERSE_SKIP_CHILDS;
 	}
 
 	return TRAVERSE_CONTINUE;
@@ -635,9 +655,7 @@ static TreeTraversalAction collection_find_data_to_delete(TreeElement *te, void
 
 static int collection_delete_exec(bContext *C, wmOperator *UNUSED(op))
 {
-	Main *bmain = CTX_data_main(C);
 	Scene *scene = CTX_data_scene(C);
-	ViewLayer *view_layer = CTX_data_view_layer(C);
 	SpaceOops *soops = CTX_wm_space_outliner(C);
 	struct CollectionDeleteData data = {.scene = scene, .soops = soops};
 
@@ -648,21 +666,20 @@ static int collection_delete_exec(bContext *C, wmOperator *UNUSED(op))
 	/* We first walk over and find the SceneCollections we actually want to delete (ignoring duplicates). */
 	outliner_tree_traverse(soops, &soops->tree, 0, TSE_SELECTED, collection_find_data_to_delete, &data);
 
+	/* Now, delete all tree elements representing a collection that will be deleted. We'll look for a
+	 * new element to select in a few lines, so we can't wait until the tree is recreated on redraw. */
+	outliner_tree_traverse(soops, &soops->tree, 0, 0, collection_delete_elements_from_collection, &data);
+
 	/* Effectively delete the collections. */
 	GSetIterator collections_to_delete_iter;
 	GSET_ITER(collections_to_delete_iter, data.collections_to_delete) {
-
 		SceneCollection *sc = BLI_gsetIterator_getKey(&collections_to_delete_iter);
 		BKE_collection_remove(&data.scene->id, sc);
 	}
 
 	BLI_gset_free(data.collections_to_delete, NULL);
 
-	/* Rebuild the outliner tree before we select the tree element */
-	outliner_build_tree(bmain, scene, view_layer, soops);
-
 	TreeElement *select_te = outliner_tree_element_from_layer_collection(C);
-
 	if (select_te) {
 		outliner_item_select(soops, select_te, false, false);
 	}
@@ -672,6 +689,7 @@ static int collection_delete_exec(bContext *C, wmOperator *UNUSED(op))
 	/* TODO(sergey): Use proper flag for tagging here. */
 	DEG_id_tag_update(&scene->id, 0);
 
+	soops->storeflag |= SO_TREESTORE_REDRAW;
 	WM_main_add_notifier(NC_SCENE | ND_LAYER, NULL);
 
 	return OPERATOR_FINISHED;
diff --git a/source/blender/editors/space_outliner/outliner_intern.h b/source/blender/editors/space_outliner/outliner_intern.h
index f2457b30a15..66f6c7026e6 100644
--- a/source/blender/editors/space_outliner/outliner_intern.h
+++ b/source/blender/editors/space_outliner/outliner_intern.h
@@ -178,8 +178,9 @@ typedef enum {
 
 /* outliner_tree.c ----------------------------------------------- */
 
-void outliner_free_tree(ListBase *lb);
+void outliner_free_tree(ListBase *tree);
 void outliner_cleanup_tree(struct SpaceOops *soops);
+void outliner_free_tree_element(TreeElement *element, ListBase *parent_subtree);
 void outliner_remove_treestore_element(struct SpaceOops *soops, TreeStoreElem *tselem);
 
 void outliner_build_tree(struct Main *mainvar, struct Scene *scene, struct ViewLayer *view_layer, struct SpaceOops *soops);
diff --git a/source/blender/editors/space_outliner/outliner_tree.c b/source/blender/editors/space_outliner/outliner_tree.c
index 3a97964e538..4097751fd0c 100644
--- a/source/blender/editors/space_outliner/outliner_tree.c
+++ b/source/blender/editors/space_outliner/outliner_tree.c
@@ -189,16 +189,11 @@ static void check_persistent(SpaceOops *soops, TreeElement *te, ID *id, short ty
 /* ********************************************************* */
 /* Tree Management */
 
-void outliner_free_tree(ListBase *lb)
+void outliner_free_tree(ListBase *tree)
 {
-	while (lb->first) {
-		TreeElement *te = lb->first;
-		
-		outliner_free_tree(&te->subtree);
-		BLI_remlink(lb, te);
-		
-		if (te->flag & TE_FREE_NAME) MEM_freeN((void *)te->name);
-		MEM_freeN(te);
+	for (TreeElement *element = tree->first, *element_next; element; element = element_next) {
+		element_next = element->next;
+		outliner_free_tree_element(element, tree);
 	}
 }
 
@@ -208,6 +203,25 @@ void outliner_cleanup_tree(SpaceOops *soops)
 	outliner_storage_cleanup(soops);
 }
 
+/**
+ * Free \a element and its sub-tree and remove its link in \a parent_subtree.
+ *
+ * \note Does not remove the TreeStoreElem of \a element!
+ * \param parent_subtree Subtree of the parent element, so the list containing \a element.
+ */
+void outliner_free_tree_element(TreeElement *element, ListBase *parent_subtree)
+{
+	BLI_assert(BLI_findindex(parent_subtree, element) > -1);
+	BLI_remlink(parent_subtree, element);
+
+	outliner_free_tree(&element->subtree);
+
+	if (element->flag & TE_FREE_NAME) {
+		MEM_freeN((void *)element->name);
+	}
+	MEM_freeN(element);
+}
+
 
 /* ********************************************************* */
 
@@ -1780,11 +1794,7 @@ static int outliner_filter_tree(SpaceOops *soops, ListBase *lb)
 			tselem->flag &= ~TSE_SEARCHMATCH;
 			
 			if ((!TSELEM_OPEN(tselem, soops)) || outliner_filter_tree(soops, &te->subtree) == 0) {
-				outliner_free_tree(&te->subtree);
-				BLI_remlink(lb, te);
-				
-				if (te->flag & TE_FREE_NAME) MEM_freeN((void *)te->name);
-				MEM_freeN(te);
+				outliner_free_tree_element(te, lb);
 			}
 		}
 		else {