Fix T81545: Moving nested collections changes view layer flags

The code that restored collection flags after they are rebuilt when
moving a collection didn't take into account collection children. The
flag for the active collection was properly restored, but all of its
children would take on the exclude flag of the collection the active
collection was dragged into.

This commit builds a temporary tree structure to store the flags for
the moving collection and its children. Then it reapplies these flags
after `BKE_main_collection_sync`.

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

1 files changed, 110 insertions, 39 deletions

diff --git a/source/blender/blenkernel/intern/collection.c b/source/blender/blenkernel/intern/collection.c
index b4db33b1c48..0ed6f94ce79 100644
--- a/source/blender/blenkernel/intern/collection.c
+++ b/source/blender/blenkernel/intern/collection.c
@@ -21,7 +21,6 @@
 #include <string.h>
 
 #include "BLI_blenlib.h"
-#include "BLI_ghash.h"
 #include "BLI_iterator.h"
 #include "BLI_listbase.h"
 #include "BLI_math_base.h"
@@ -1543,6 +1542,112 @@ bool BKE_collection_objects_select(ViewLayer *view_layer, Collection *collection
 /** \name Collection move (outliner drag & drop)
  * \{ */
 
+/* Local temporary storage for layer collection flags. */
+typedef struct LayerCollectionFlag {
+  struct LayerCollectionFlag *next, *prev;
+  /** The view layer for the collections being moved, NULL for their children. */
+  ViewLayer *view_layer;
+  /** The original #LayerCollection's collection field. */
+  Collection *collection;
+  /** The original #LayerCollection's flag. */
+  int flag;
+  /** Corresponds to #LayerCollection->layer_collections. */
+  ListBase children;
+} LayerCollectionFlag;
+
+static void layer_collection_flags_store_recursive(const LayerCollection *layer_collection,
+                                                   LayerCollectionFlag *flag)
+{
+  flag->collection = layer_collection->collection;
+  flag->flag = layer_collection->flag;
+
+  LISTBASE_FOREACH (const LayerCollection *, child, &layer_collection->layer_collections) {
+    LayerCollectionFlag *child_flag = MEM_callocN(sizeof(LayerCollectionFlag), __func__);
+    BLI_addtail(&flag->children, child_flag);
+    layer_collection_flags_store_recursive(child, child_flag);
+  }
+}
+
+/**
+ * For every view layer, find the \a collection and save flags
+ * for it and its children in a temporary tree structure.
+ */
+static void layer_collection_flags_store(Main *bmain,
+                                         const Collection *collection,
+                                         ListBase *r_layer_level_list)
+{
+  BLI_listbase_clear(r_layer_level_list);
+  LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) {
+    LISTBASE_FOREACH (ViewLayer *, view_layer, &scene->view_layers) {
+      LayerCollection *layer_collection = BKE_layer_collection_first_from_scene_collection(
+          view_layer, collection);
+      /* Skip this view layer if the collection isn't found for some reason. */
+      if (layer_collection == NULL) {
+        continue;
+      }
+
+      /* Store the flags for the collection and all of its children. */
+      LayerCollectionFlag *flag = MEM_callocN(sizeof(LayerCollectionFlag), __func__);
+      flag->view_layer = view_layer;
+
+      /* Recursively save flags from collection children. */
+      layer_collection_flags_store_recursive(layer_collection, flag);
+
+      BLI_addtail(r_layer_level_list, flag);
+    }
+  }
+}
+
+static void layer_collection_flags_restore_recursive(LayerCollection *layer_collection,
+                                                     LayerCollectionFlag *flag)
+{
+  /* There should be a flag struct for every layer collection. */
+  BLI_assert(BLI_listbase_count(&layer_collection->layer_collections) ==
+             BLI_listbase_count(&flag->children));
+  /* The flag and the the layer collection should actually correspond. */
+  BLI_assert(flag->collection == layer_collection->collection);
+
+  LayerCollectionFlag *child_flag = flag->children.first;
+  LISTBASE_FOREACH (LayerCollection *, child, &layer_collection->layer_collections) {
+    layer_collection_flags_restore_recursive(child, child_flag);
+
+    child_flag = child_flag->next;
+  }
+  BLI_freelistN(&flag->children);
+
+  /* We treat exclude as a special case.
+   *
+   * If in a different view layer the parent collection was disabled (e.g., background)
+   * and now we moved a new collection to be part of the background this collection should
+   * probably be disabled.
+   *
+   * Note: If we were to also keep the exclude flag we would need to re-sync the collections.
+   */
+  layer_collection->flag = flag->flag | (layer_collection->flag & LAYER_COLLECTION_EXCLUDE);
+}
+
+/**
+ * Restore a collection's (and its children's) flags for each view layer
+ * from the structure built in #layer_collection_flags_store.
+ */
+static void layer_collection_flags_restore(ListBase *flags, const Collection *collection)
+{
+  LISTBASE_FOREACH (LayerCollectionFlag *, flag, flags) {
+    ViewLayer *view_layer = flag->view_layer;
+    /* The top level of flag structs must have this set. */
+    BLI_assert(view_layer != NULL);
+
+    LayerCollection *layer_collection = BKE_layer_collection_first_from_scene_collection(
+        view_layer, collection);
+    /* The flags should only be added if the collection is in the view layer. */
+    BLI_assert(layer_collection != NULL);
+
+    layer_collection_flags_restore_recursive(layer_collection, flag);
+  }
+
+  BLI_freelistN(flags);
+}
+
 bool BKE_collection_move(Main *bmain,
                          Collection *to_parent,
                          Collection *from_parent,
@@ -1585,48 +1690,14 @@ bool BKE_collection_move(Main *bmain,
 
   /* Make sure we store the flag of the layer collections before we remove and re-create them.
    * Otherwise they will get lost and everything will be copied from the new parent collection. */
-  GHash *view_layer_hash = BLI_ghash_new(BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp, __func__);
-
-  for (Scene *scene = bmain->scenes.first; scene; scene = scene->id.next) {
-    LISTBASE_FOREACH (ViewLayer *, view_layer, &scene->view_layers) {
-
-      LayerCollection *layer_collection = BKE_layer_collection_first_from_scene_collection(
-          view_layer, collection);
-
-      if (layer_collection == NULL) {
-        continue;
-      }
-
-      BLI_ghash_insert(view_layer_hash, view_layer, POINTER_FROM_INT(layer_collection->flag));
-    }
-  }
+  ListBase layer_flags;
+  layer_collection_flags_store(bmain, collection, &layer_flags);
 
   /* Create and remove layer collections. */
   BKE_main_collection_sync(bmain);
 
-  /* Restore back the original layer collection flags. */
-  GHashIterator gh_iter;
-  GHASH_ITER (gh_iter, view_layer_hash) {
-    ViewLayer *view_layer = BLI_ghashIterator_getKey(&gh_iter);
-
-    LayerCollection *layer_collection = BKE_layer_collection_first_from_scene_collection(
-        view_layer, collection);
-
-    if (layer_collection) {
-      /* We treat exclude as a special case.
-       *
-       * If in a different view layer the parent collection was disabled (e.g., background)
-       * and now we moved a new collection to be part of the background this collection should
-       * probably be disabled.
-       *
-       * Note: If we were to also keep the exclude flag we would need to re-sync the collections.
-       */
-      layer_collection->flag = POINTER_AS_INT(BLI_ghashIterator_getValue(&gh_iter)) |
-                               (layer_collection->flag & LAYER_COLLECTION_EXCLUDE);
-    }
-  }
-
-  BLI_ghash_free(view_layer_hash, NULL, NULL);
+  /* Restore the original layer collection flags. */
+  layer_collection_flags_restore(&layer_flags, collection);
 
   /* We need to sync it again to pass the correct flags to the collections objects. */
   BKE_main_collection_sync(bmain);