[Bf-blender-cvs] [b18d0244fc0] master: LayerCollection: Refactor of resync-with-Collection-hierarchy process.

Bastien Montagne noreply at git.blender.org
Wed Jul 28 15:51:13 CEST 2021


Commit: b18d0244fc075ad39693c9cfc980ab44a00bfd65
Author: Bastien Montagne
Date:   Wed Jul 28 15:28:42 2021 +0200
Branches: master
https://developer.blender.org/rBb18d0244fc075ad39693c9cfc980ab44a00bfd65

LayerCollection: Refactor of resync-with-Collection-hierarchy process.

The goal of this refactor is to improve resync of LayerCollections
hierarchy to match again Collection one.

Current code would destroy and re-create valid layers whenever a parent
collection would be removed, which leads to losing way too often
layer-related settings when editing collection hierarchies.

While this could be partially addressed from operators side, there was
no way to fix those issues from lower level, more generic ID management
code like ID remapping or library override resync processes.

The new code builds a shallow wrapper around existing (aka old) layers
hierarchy, does a set of checks to define the status of all existing
layers, and try to find the closest matching unused layer in cases where
layers and collections hierarchies do not match anymore.

The intent is to both re-use as much as possible existing layers, and
to pick the 'best' possible layer to re-use, following those heuristics:
 * Prefer layers children of current one first (in old hierarchy), and only
   use those from other higher-level hierarchies if no (grand-)child is found.
 * Prefer to use closest layers available in the old hierarchy.

NOTE: The new code is about 12%-15% slower than the previous one, which is
expected given the increased complexity. Note that this would not be an
issue in practice if this code was not called way too often (needs to
be converted to lazy update instead, which is a long known TODO).

NOTE: The LayerCollectionResync code uses its own built-in version of
FIFO queue, as performances in this code is currently a critical point
(it can get called tens of thousands of times during a single (heavy)
ID management operation currently, in a production file e.g.).

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

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

M	source/blender/blenkernel/intern/layer.c

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

diff --git a/source/blender/blenkernel/intern/layer.c b/source/blender/blenkernel/intern/layer.c
index 23ea306c1a8..262950bfab5 100644
--- a/source/blender/blenkernel/intern/layer.c
+++ b/source/blender/blenkernel/intern/layer.c
@@ -23,7 +23,10 @@
 
 #include <string.h>
 
+#include "CLG_log.h"
+
 #include "BLI_listbase.h"
+#include "BLI_mempool.h"
 #include "BLI_string.h"
 #include "BLI_string_utf8.h"
 #include "BLI_string_utils.h"
@@ -63,6 +66,8 @@
 
 #include "BLO_read_write.h"
 
+static CLG_LogRef LOG = {"bke.layercollection"};
+
 /* Set of flags which are dependent on a collection settings. */
 static const short g_base_collection_flags = (BASE_VISIBLE_DEPSGRAPH | BASE_VISIBLE_VIEWLAYER |
                                               BASE_SELECTABLE | BASE_ENABLED_VIEWPORT |
@@ -771,6 +776,194 @@ void BKE_layer_collection_resync_allow(void)
   no_resync = false;
 }
 
+typedef struct LayerCollectionResync {
+  struct LayerCollectionResync *prev, *next;
+
+  /* Temp data used to generate a queue during valid layer search. See
+   * #layer_collection_resync_find. */
+  struct LayerCollectionResync *queue_next;
+
+  /* LayerCollection and Collection wrapped by this data. */
+  LayerCollection *layer;
+  Collection *collection;
+
+  /* Hierarchical relationships in the old, existing ViewLayer state (except for newly created
+   * layers). */
+  struct LayerCollectionResync *parent_layer_resync;
+  ListBase children_layer_resync;
+
+  /* This layer still points to a valid collection. */
+  bool is_usable;
+  /* This layer is still valid as a parent, i.e. at least one of its original layer children is
+   * usable and matches one of its current children collections. */
+  bool is_valid_as_parent;
+  /* This layer is still valid as a child, i.e. its original layer parent is usable and matches one
+   * of its current parents collections. */
+  bool is_valid_as_child;
+  /* This layer is still fully valid in the new collection hierarchy, i.e. itself and all of its
+   * parents fully match the current collection hierarchy.
+   * OR
+   * This layer has already been re-used to match the new collections hierarchy. */
+  bool is_used;
+} LayerCollectionResync;
+
+static LayerCollectionResync *layer_collection_resync_create_recurse(
+    LayerCollectionResync *parent_layer_resync, LayerCollection *layer, BLI_mempool *mempool)
+{
+  LayerCollectionResync *layer_resync = BLI_mempool_calloc(mempool);
+
+  layer_resync->layer = layer;
+  layer_resync->collection = layer->collection;
+  layer_resync->parent_layer_resync = parent_layer_resync;
+  if (parent_layer_resync != NULL) {
+    BLI_addtail(&parent_layer_resync->children_layer_resync, layer_resync);
+  }
+
+  layer_resync->is_usable = (layer->collection != NULL);
+  layer_resync->is_valid_as_child =
+      layer_resync->is_usable && (parent_layer_resync == NULL ||
+                                  (parent_layer_resync->is_usable &&
+                                   BLI_findptr(&parent_layer_resync->layer->collection->children,
+                                               layer->collection,
+                                               offsetof(CollectionChild, collection)) != NULL));
+  if (layer_resync->is_valid_as_child) {
+    layer_resync->is_used = parent_layer_resync != NULL ? parent_layer_resync->is_used : true;
+  }
+  else {
+    layer_resync->is_used = false;
+  }
+
+  if (BLI_listbase_is_empty(&layer->layer_collections)) {
+    layer_resync->is_valid_as_parent = layer_resync->is_usable;
+  }
+  else {
+    LISTBASE_FOREACH (LayerCollection *, child_layer, &layer->layer_collections) {
+      LayerCollectionResync *child_layer_resync = layer_collection_resync_create_recurse(
+          layer_resync, child_layer, mempool);
+      if (layer_resync->is_usable && child_layer_resync->is_valid_as_child) {
+        layer_resync->is_valid_as_parent = true;
+      }
+    }
+  }
+
+  CLOG_INFO(&LOG,
+            4,
+            "Old LayerCollection for %s is...\n\tusable: %d\n\tvalid parent: %d\n\tvalid child: "
+            "%d\n\tused: %d\n",
+            layer_resync->collection ? layer_resync->collection->id.name : "<NONE>",
+            layer_resync->is_usable,
+            layer_resync->is_valid_as_parent,
+            layer_resync->is_valid_as_child,
+            layer_resync->is_used);
+
+  return layer_resync;
+}
+
+static LayerCollectionResync *layer_collection_resync_find(LayerCollectionResync *layer_resync,
+                                                           Collection *child_collection)
+{
+  /* Given the given parent, valid layer collection, find in the old hierarchy the best possible
+   * unused layer matching the given child collection.
+   *
+   * This uses the following heuristics:
+   *  - Prefer a layer descendant of the given parent one if possible.
+   *  - Prefer a layer as closely related as possible from the given parent.
+   *  - Do not used layers that are not head (highest possible ancestor) of a local valid hierarchy
+   *    branch, since we can assume we could then re-use its ancestor instead.
+   *
+   * A queue is used to ensure this order of preferences.
+   */
+
+  BLI_assert(layer_resync->collection != child_collection);
+  BLI_assert(child_collection != NULL);
+
+  LayerCollectionResync *current_layer_resync = NULL;
+  LayerCollectionResync *root_layer_resync = layer_resync;
+
+  LayerCollectionResync *queue_head = layer_resync, *queue_tail = layer_resync;
+  layer_resync->queue_next = NULL;
+
+  while (queue_head != NULL) {
+    current_layer_resync = queue_head;
+    queue_head = current_layer_resync->queue_next;
+
+    if (current_layer_resync->collection == child_collection &&
+        (current_layer_resync->parent_layer_resync == layer_resync ||
+         (!current_layer_resync->is_used && !current_layer_resync->is_valid_as_child))) {
+      /* This layer is a valid candidate, because its collection matches the seeked one, AND:
+       *  - It is a direct child of the initial given parent ('unchanged hierarchy' case), OR
+       *  - It is not currently used, and not part of a valid hierarchy (sub-)chain.
+       */
+      break;
+    }
+
+    /* Else, add all its direct children for further searching. */
+    LISTBASE_FOREACH (LayerCollectionResync *,
+                      child_layer_resync,
+                      &current_layer_resync->children_layer_resync) {
+      /* Add to tail of the queue. */
+      queue_tail->queue_next = child_layer_resync;
+      child_layer_resync->queue_next = NULL;
+      queue_tail = child_layer_resync;
+      if (queue_head == NULL) {
+        queue_head = queue_tail;
+      }
+    }
+
+    /* If all descendants from current layer have been processed, go one step higher and
+     * process all of its other siblings. */
+    if (queue_head == NULL && root_layer_resync->parent_layer_resync != NULL) {
+      LISTBASE_FOREACH (LayerCollectionResync *,
+                        sibling_layer_resync,
+                        &root_layer_resync->parent_layer_resync->children_layer_resync) {
+        if (sibling_layer_resync == root_layer_resync) {
+          continue;
+        }
+        /* Add to tail of the queue. */
+        queue_tail->queue_next = sibling_layer_resync;
+        sibling_layer_resync->queue_next = NULL;
+        queue_tail = sibling_layer_resync;
+        if (queue_head == NULL) {
+          queue_head = queue_tail;
+        }
+      }
+      root_layer_resync = root_layer_resync->parent_layer_resync;
+    }
+
+    current_layer_resync = NULL;
+  }
+
+  return current_layer_resync;
+}
+
+static void layer_collection_resync_unused_layers_free(ViewLayer *view_layer,
+                                                       LayerCollectionResync *layer_resync)
+{
+  LISTBASE_FOREACH (
+      LayerCollectionResync *, child_layer_resync, &layer_resync->children_layer_resync) {
+    layer_collection_resync_unused_layers_free(view_layer, child_layer_resync);
+  }
+
+  if (!layer_resync->is_used) {
+    CLOG_INFO(&LOG,
+              4,
+              "Freeing unused LayerCollection for %s",
+              layer_resync->collection != NULL ? layer_resync->collection->id.name :
+                                                 "<Deleted Collection>");
+
+    if (layer_resync->layer == view_layer->active_collection) {
+      view_layer->active_collection = NULL;
+    }
+
+    /* We do not want to go recursive here, this is handled through the LayerCollectionResync data
+     * wrapper. */
+    MEM_freeN(layer_resync->layer);
+    layer_resync->layer = NULL;
+    layer_resync->collection = NULL;
+    layer_resync->is_usable = false;
+  }
+}
+
 static void layer_collection_objects_sync(ViewLayer *view_layer,
                                           LayerCollection *layer,
                                           ListBase *r_lb_new_object_bases,
@@ -839,55 +1032,91 @@ static void layer_collection_objects_sync(ViewLayer *view_layer,
 }
 
 static void layer_collection_sync(ViewLayer *view_layer,
-                                  const ListBase *lb_children_collections,
-                                  ListBase *r_lb_children_layers,
+                                  LayerCollectionResync *layer_resync,
+                                  BLI_mempool *layer_resync_mempool,
                                   ListBase *r_lb_new_object_bases,
                                   const short parent_layer_flag,
                                   const short parent_collection_restrict,
                                   const short parent_layer_restrict,
                                   const ushort parent_local_collections_bits)
 {
-  /* TODO: support recovery after removal of intermediate collections, reordering, ..
-   * For local edits we can make editing operating do the appropriate thing, but for
-   * linking we can only sync after the fact. */
-
-  /* Remove layer collections that no longer have a corresponding scene collection. */
-  LISTBASE_FOREACH_MUTABLE (LayerCollection *, child_layer, r_lb_children_layers) {
-    /* Note that ID remap can set child_layer->collection to NULL when deleting collections. */
-    Collection *child_collection = (child_layer->collection != NULL) ?
-                         

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list