Fix bug/crash in ID bulk deletion code.

This is complex situation. Tagged ID deletion (used to delete several
data-blocks at once) removes IDs to be deleted from Main.

But when we remove deleted IDs' usages of other IDs (using
`BKE_libblock_relink_ex`), some specific post-process is required on
some types, like Collections. Those post-processes would in some cases
rely on data actually being in Main.

That failing condition would lead in existing code on missing processing
the very ID (collection) we were working on, leading to missing removing
some child collection pointers, leading to the crash (later on in
LayerCollection resync process).

For now we go with an optimization & fix that avoids processing all
collections in Main when we actually know which one we are working one
(case of `BKE_libblock_relink_ex`, but not of
`BKE_libblock_remap_locked`).

This is however yet another demonstration of the need to rework that
whole collection/layer resync process, since it is not only extremely
inneficient currently, but it also requires valid Main/ID state way too
deep into the remapping code.

NOTE: This fix may very well not catch/address all possible fail cases
here, dealing with the double parent/child relationships of collections
is challenging...

Issue reported by @eyecandy from the studio, thanks.

1 files changed, 11 insertions, 4 deletions

diff --git a/source/blender/blenkernel/intern/lib_remap.c b/source/blender/blenkernel/intern/lib_remap.c
index 1f597bbb9a6..b32b97dc250 100644
--- a/source/blender/blenkernel/intern/lib_remap.c
+++ b/source/blender/blenkernel/intern/lib_remap.c
@@ -303,6 +303,7 @@ static void libblock_remap_data_postprocess_object_update(Main *bmain,
 /* Can be called with both old_collection and new_collection being NULL,
  * this means we have to check whole Main database then. */
 static void libblock_remap_data_postprocess_collection_update(Main *bmain,
+                                                              Collection *owner_collection,
                                                               Collection *UNUSED(old_collection),
                                                               Collection *new_collection)
 {
@@ -311,7 +312,7 @@ static void libblock_remap_data_postprocess_collection_update(Main *bmain,
      * and BKE_main_collection_sync_remap() does not tolerate any of those, so for now always check
      * whole existing collections for NULL pointers.
      * I'd consider optimizing that whole collection remapping process a TODO for later. */
-    BKE_collections_child_remove_nulls(bmain, NULL /*old_collection*/);
+    BKE_collections_child_remove_nulls(bmain, owner_collection, NULL /*old_collection*/);
   }
   else {
     /* Temp safe fix, but a "tad" brute force... We should probably be able to use parents from
@@ -523,7 +524,7 @@ void BKE_libblock_remap_locked(Main *bmain, void *old_idv, void *new_idv, const
       break;
     case ID_GR:
       libblock_remap_data_postprocess_collection_update(
-          bmain, (Collection *)old_id, (Collection *)new_id);
+          bmain, NULL, (Collection *)old_id, (Collection *)new_id);
       break;
     case ID_ME:
     case ID_CU:
@@ -628,6 +629,12 @@ void BKE_libblock_relink_ex(
   switch (GS(id->name)) {
     case ID_SCE:
     case ID_GR: {
+      /* Note: here we know which collection we have affected, so at lest for NULL children
+       * detection we can only process that one.
+       * This is also a required fix in case `id` would not be in Main anymore, which can happen
+       * e.g. when called from `id_delete`. */
+      Collection *owner_collection = (GS(id->name) == ID_GR) ? (Collection *)id :
+                                                               ((Scene *)id)->master_collection;
       if (old_id) {
         switch (GS(old_id->name)) {
           case ID_OB:
@@ -636,7 +643,7 @@ void BKE_libblock_relink_ex(
             break;
           case ID_GR:
             libblock_remap_data_postprocess_collection_update(
-                bmain, (Collection *)old_id, (Collection *)new_id);
+                bmain, owner_collection, (Collection *)old_id, (Collection *)new_id);
             break;
           default:
             break;
@@ -644,7 +651,7 @@ void BKE_libblock_relink_ex(
       }
       else {
         /* No choice but to check whole objects/collections. */
-        libblock_remap_data_postprocess_collection_update(bmain, NULL, NULL);
+        libblock_remap_data_postprocess_collection_update(bmain, owner_collection, NULL, NULL);
         libblock_remap_data_postprocess_object_update(bmain, NULL, NULL);
       }
       break;