Fix T66154: viewlayer hide/exclude settings getting lost for linked collections

The problem was that the object and collection pointers in Base and
LayerCollection would get lost of file read. Normally such ID pointers would
be resolved by pointing to an ID_ID placeholder which has the datablock name,
and then replacing it will the real datablock. However ID_ID is only written
for directly linked datablocks.

This adds the concept of an indirectly linked datablock with a weak reference
to it. For this we write an ID_ID_WEAK_REF code, which is a reference that
will only be resolved if the datablock was read for another reason.

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

7 files changed, 101 insertions, 14 deletions

diff --git a/source/blender/blenkernel/BKE_library.h b/source/blender/blenkernel/BKE_library.h
index 5a4a3f29a93..71799bf74f6 100644
--- a/source/blender/blenkernel/BKE_library.h
+++ b/source/blender/blenkernel/BKE_library.h
@@ -175,6 +175,7 @@ void BKE_libblock_management_usercounts_clear(struct Main *bmain, void *idv);
 
 void BKE_id_lib_local_paths(struct Main *bmain, struct Library *lib, struct ID *id);
 void id_lib_extern(struct ID *id);
+void id_lib_indirect_weak_link(struct ID *id);
 void BKE_library_filepath_set(struct Main *bmain, struct Library *lib, const char *filepath);
 void id_us_ensure_real(struct ID *id);
 void id_us_clear_real(struct ID *id);
diff --git a/source/blender/blenkernel/intern/layer.c b/source/blender/blenkernel/intern/layer.c
index f3c0d5da6ee..4198e6ded9c 100644
--- a/source/blender/blenkernel/intern/layer.c
+++ b/source/blender/blenkernel/intern/layer.c
@@ -32,6 +32,7 @@
 #include "BKE_freestyle.h"
 #include "BKE_idprop.h"
 #include "BKE_layer.h"
+#include "BKE_library.h"
 #include "BKE_main.h"
 #include "BKE_node.h"
 #include "BKE_object.h"
@@ -698,6 +699,10 @@ static short layer_collection_sync(ViewLayer *view_layer,
       lc->flag = parent_exclude;
     }
 
+    /* Tag linked collection as a weak reference so we keep the layer
+     * collection pointer on file load and remember exclude state. */
+    id_lib_indirect_weak_link(&collection->id);
+
     /* Collection restrict is inherited. */
     short child_restrict = parent_restrict;
     short child_layer_restrict = parent_layer_restrict;
@@ -735,6 +740,10 @@ static short layer_collection_sync(ViewLayer *view_layer,
         continue;
       }
 
+      /* Tag linked object as a weak reference so we keep the object
+       * base pointer on file load and remember hidden state. */
+      id_lib_indirect_weak_link(&cob->ob->id);
+
       void **base_p;
       Base *base;
       if (BLI_ghash_ensure_p(view_layer->object_bases_hash, cob->ob, &base_p)) {
diff --git a/source/blender/blenkernel/intern/library.c b/source/blender/blenkernel/intern/library.c
index 367c2fbbdda..468d0e97ece 100644
--- a/source/blender/blenkernel/intern/library.c
+++ b/source/blender/blenkernel/intern/library.c
@@ -165,12 +165,23 @@ void id_lib_extern(ID *id)
     BLI_assert(BKE_idcode_is_linkable(GS(id->name)));
     if (id->tag & LIB_TAG_INDIRECT) {
       id->tag &= ~LIB_TAG_INDIRECT;
+      id->flag &= ~LIB_INDIRECT_WEAK_LINK;
       id->tag |= LIB_TAG_EXTERN;
       id->lib->parent = NULL;
     }
   }
 }
 
+void id_lib_indirect_weak_link(ID *id)
+{
+  if (id && ID_IS_LINKED(id)) {
+    BLI_assert(BKE_idcode_is_linkable(GS(id->name)));
+    if (id->tag & LIB_TAG_INDIRECT) {
+      id->flag |= LIB_INDIRECT_WEAK_LINK;
+    }
+  }
+}
+
 /**
  * Ensure we have a real user
  *
@@ -1768,6 +1779,7 @@ void id_clear_lib_data_ex(Main *bmain, ID *id, const bool id_in_mainlist)
 
   id->lib = NULL;
   id->tag &= ~(LIB_TAG_INDIRECT | LIB_TAG_EXTERN);
+  id->flag &= ~LIB_INDIRECT_WEAK_LINK;
   if (id_in_mainlist) {
     if (BKE_id_new_name_validate(which_libbase(bmain, GS(id->name)), id, NULL)) {
       bmain->is_memfile_undo_written = false;
@@ -1981,6 +1993,7 @@ void BKE_library_make_local(Main *bmain,
 
       if (id->lib == NULL) {
         id->tag &= ~(LIB_TAG_EXTERN | LIB_TAG_INDIRECT | LIB_TAG_NEW);
+        id->flag &= ~LIB_INDIRECT_WEAK_LINK;
       }
       /* The check on the fourth line (LIB_TAG_PRE_EXISTING) is done so it's possible to tag data
        * you don't want to be made local, used for appending data,
diff --git a/source/blender/blenkernel/intern/library_remap.c b/source/blender/blenkernel/intern/library_remap.c
index 7554dad5fb0..45dbb4b6ec1 100644
--- a/source/blender/blenkernel/intern/library_remap.c
+++ b/source/blender/blenkernel/intern/library_remap.c
@@ -497,6 +497,7 @@ static void libblock_remap_data(
   if (new_id && (new_id->tag & LIB_TAG_INDIRECT) &&
       (r_id_remap_data->status & ID_REMAP_IS_LINKED_DIRECT)) {
     new_id->tag &= ~LIB_TAG_INDIRECT;
+    new_id->flag &= ~LIB_INDIRECT_WEAK_LINK;
     new_id->tag |= LIB_TAG_EXTERN;
   }
 
diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index 2593e4d05e1..87db0ce1ad2 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -2657,6 +2657,7 @@ static void direct_link_id(FileData *fd, ID *id)
    * function are still in a clear tag state.
    * (glowering at certain nodetree fake data-lock here...). */
   id->tag = 0;
+  id->flag &= ~LIB_INDIRECT_WEAK_LINK;
 
   /* Link direct data of overrides. */
   if (id->override_library) {
@@ -9073,7 +9074,12 @@ static BHead *read_data_into_oldnewmap(FileData *fd, BHead *bhead, const char *a
   return bhead;
 }
 
-static BHead *read_libblock(FileData *fd, Main *main, BHead *bhead, const int tag, ID **r_id)
+static BHead *read_libblock(FileData *fd,
+                            Main *main,
+                            BHead *bhead,
+                            const int tag,
+                            const bool placeholder_set_indirect_extern,
+                            ID **r_id)
 {
   /* this routine reads a libblock and its direct data. Use link functions to connect it all
    */
@@ -9193,7 +9199,16 @@ static BHead *read_libblock(FileData *fd, Main *main, BHead *bhead, const int ta
   /* this case cannot be direct_linked: it's just the ID part */
   if (bhead->code == ID_LINK_PLACEHOLDER) {
     /* That way, we know which data-lock needs do_versions (required currently for linking). */
-    id->tag = tag | LIB_TAG_NEED_LINK | LIB_TAG_NEW;
+    id->tag = tag | LIB_TAG_ID_LINK_PLACEHOLDER | LIB_TAG_NEED_LINK | LIB_TAG_NEW;
+
+    if (placeholder_set_indirect_extern) {
+      if (id->flag & LIB_INDIRECT_WEAK_LINK) {
+        id->tag |= LIB_TAG_INDIRECT;
+      }
+      else {
+        id->tag |= LIB_TAG_EXTERN;
+      }
+    }
 
     return blo_bhead_next(fd, bhead);
   }
@@ -9756,8 +9771,7 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
            * to the file format definition. So we can use the entry at the
            * end of mainlist, added in direct_link_library. */
           Main *libmain = mainlist.last;
-          bhead = read_libblock(
-              fd, libmain, bhead, LIB_TAG_ID_LINK_PLACEHOLDER | LIB_TAG_EXTERN, NULL);
+          bhead = read_libblock(fd, libmain, bhead, 0, true, NULL);
         }
         break;
       /* in 2.50+ files, the file identifier for screens is patched, forward compatibility */
@@ -9770,7 +9784,7 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
           bhead = blo_bhead_next(fd, bhead);
         }
         else {
-          bhead = read_libblock(fd, bfd->main, bhead, LIB_TAG_LOCAL, NULL);
+          bhead = read_libblock(fd, bfd->main, bhead, LIB_TAG_LOCAL, false, NULL);
         }
     }
   }
@@ -10018,7 +10032,7 @@ static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
     if (id == NULL) {
       /* ID has not been read yet, add placeholder to the main of the
        * library it belongs to, so that it will be read later. */
-      read_libblock(fd, libmain, bhead, LIB_TAG_ID_LINK_PLACEHOLDER | LIB_TAG_INDIRECT, NULL);
+      read_libblock(fd, libmain, bhead, LIB_TAG_INDIRECT, false, NULL);
       // commented because this can print way too much
       // if (G.debug & G_DEBUG) printf("expand_doit: other lib %s\n", lib->name);
 
@@ -10026,6 +10040,12 @@ static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
       libmain->curlib->parent = mainvar->curlib;
     }
     else {
+      /* Convert any previously read weak link to regular link
+       * to signal that we want to read this datablock. */
+      if (id->tag & LIB_TAG_ID_LINK_PLACEHOLDER) {
+        id->flag &= ~LIB_INDIRECT_WEAK_LINK;
+      }
+
       /* "id" is either a placeholder or real ID that is already in the
        * main of the library (A) it belongs to. However it might have been
        * put there by another library (C) which only updated its own
@@ -10067,9 +10087,15 @@ static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
 
     ID *id = is_yet_read(fd, mainvar, bhead);
     if (id == NULL) {
-      read_libblock(fd, mainvar, bhead, LIB_TAG_NEED_EXPAND | LIB_TAG_INDIRECT, NULL);
+      read_libblock(fd, mainvar, bhead, LIB_TAG_NEED_EXPAND | LIB_TAG_INDIRECT, false, NULL);
     }
     else {
+      /* Convert any previously read weak link to regular link
+       * to signal that we want to read this datablock. */
+      if (id->tag & LIB_TAG_ID_LINK_PLACEHOLDER) {
+        id->flag &= ~LIB_INDIRECT_WEAK_LINK;
+      }
+
       /* this is actually only needed on UI call? when ID was already read before,
        * and another append happens which invokes same ID...
        * in that case the lookup table needs this entry */
@@ -11203,6 +11229,7 @@ static void add_loose_objects_to_scene(Main *mainvar,
         BKE_scene_object_base_flag_sync_from_base(base);
 
         ob->id.tag &= ~LIB_TAG_INDIRECT;
+        ob->id.flag &= ~LIB_INDIRECT_WEAK_LINK;
         ob->id.tag |= LIB_TAG_EXTERN;
       }
     }
@@ -11295,6 +11322,7 @@ static void add_collections_to_scene(Main *mainvar,
 
         /* Those are kept for safety and consistency, but should not be needed anymore? */
         collection->id.tag &= ~LIB_TAG_INDIRECT;
+        collection->id.flag &= ~LIB_INDIRECT_WEAK_LINK;
         collection->id.tag |= LIB_TAG_EXTERN;
       }
     }
@@ -11319,7 +11347,7 @@ static ID *link_named_part(
     if (id == NULL) {
       /* not read yet */
       const int tag = force_indirect ? LIB_TAG_INDIRECT : LIB_TAG_EXTERN;
-      read_libblock(fd, mainl, bhead, tag | LIB_TAG_NEED_EXPAND, &id);
+      read_libblock(fd, mainl, bhead, tag | LIB_TAG_NEED_EXPAND, false, &id);
 
       if (id) {
         /* sort by name in list */
@@ -11335,6 +11363,7 @@ static ID *link_named_part(
       oldnewmap_insert(fd->libmap, bhead->old, id, bhead->code);
       if (!force_indirect && (id->tag & LIB_TAG_INDIRECT)) {
         id->tag &= ~LIB_TAG_INDIRECT;
+        id->flag &= ~LIB_INDIRECT_WEAK_LINK;
         id->tag |= LIB_TAG_EXTERN;
       }
     }
@@ -11373,7 +11402,7 @@ int BLO_library_link_copypaste(Main *mainl, BlendHandle *bh, const unsigned int
     if (BKE_idcode_is_valid(bhead->code) && BKE_idcode_is_linkable(bhead->code) &&
         (id_types_mask == 0 ||
          (BKE_idcode_to_idfilter((short)bhead->code) & id_types_mask) != 0)) {
-      read_libblock(fd, mainl, bhead, LIB_TAG_NEED_EXPAND | LIB_TAG_INDIRECT, &id);
+      read_libblock(fd, mainl, bhead, LIB_TAG_NEED_EXPAND | LIB_TAG_INDIRECT, false, &id);
       num_directly_linked++;
     }
 
@@ -11659,7 +11688,7 @@ static int has_linked_ids_to_read(Main *mainvar)
 
   while (a--) {
     for (ID *id = lbarray[a]->first; id; id = id->next) {
-      if (id->tag & LIB_TAG_ID_LINK_PLACEHOLDER) {
+      if ((id->tag & LIB_TAG_ID_LINK_PLACEHOLDER) && !(id->flag & LIB_INDIRECT_WEAK_LINK)) {
         return true;
       }
     }
@@ -11690,11 +11719,12 @@ static void read_library_linked_id(
   }
 
   id->tag &= ~LIB_TAG_ID_LINK_PLACEHOLDER;
+  id->flag &= ~LIB_INDIRECT_WEAK_LINK;
 
   if (bhead) {
     id->tag |= LIB_TAG_NEED_EXPAND;
     // printf("read lib block %s\n", id->name);
-    read_libblock(fd, mainvar, bhead, id->tag, r_id);
+    read_libblock(fd, mainvar, bhead, id->tag, false, r_id);
   }
   else {
     blo_reportf_wrap(reports,
@@ -11728,7 +11758,7 @@ static void read_library_linked_ids(FileData *basefd,
 
     while (id) {
       ID *id_next = id->next;
-      if (id->tag & LIB_TAG_ID_LINK_PLACEHOLDER) {
+      if ((id->tag & LIB_TAG_ID_LINK_PLACEHOLDER) && !(id->flag & LIB_INDIRECT_WEAK_LINK)) {
         BLI_remlink(lbarray[a], id);
 
         /* When playing with lib renaming and such, you may end with cases where
@@ -11764,6 +11794,28 @@ static void read_library_linked_ids(FileData *basefd,
   BLI_ghash_free(loaded_ids, NULL, NULL);
 }
 
+static void read_library_clear_weak_links(FileData *basefd, ListBase *mainlist, Main *mainvar)
+{
+  /* Any remaining weak links at this point have been lost, silently drop
+   * those by setting them to NULL pointers. */
+  ListBase *lbarray[MAX_LIBARRAY];
+  int a = set_listbasepointers(mainvar, lbarray);
+
+  while (a--) {
+    ID *id = lbarray[a]->first;
+
+    while (id) {
+      ID *id_next = id->next;
+      if ((id->tag & LIB_TAG_ID_LINK_PLACEHOLDER) && (id->flag & LIB_INDIRECT_WEAK_LINK)) {
+        /* printf("Dropping weak link to %s\n", id->name); */
+        change_link_placeholder_to_real_ID_pointer(mainlist, basefd, id, NULL);
+        BLI_freelinkN(lbarray[a], id);
+      }
+      id = id_next;
+    }
+  }
+}
+
 static FileData *read_library_file_data(FileData *basefd,
                                         ListBase *mainlist,
                                         Main *mainl,
@@ -11890,6 +11942,9 @@ static void read_libraries(FileData *basefd, ListBase *mainlist)
 
   Main *main_newid = BKE_main_new();
   for (Main *mainptr = mainl->next; mainptr; mainptr = mainptr->next) {
+    /* Drop weak links for which no datablock was found. */
+    read_library_clear_weak_links(basefd, mainlist, mainptr);
+
     /* Do versioning for newly added linked data-locks. If no data-locks
      * were read from a library versionfile will still be zero and we can
      * skip it. */
diff --git a/source/blender/blenloader/intern/writefile.c b/source/blender/blenloader/intern/writefile.c
index f20397617cc..aae5072c8de 100644
--- a/source/blender/blenloader/intern/writefile.c
+++ b/source/blender/blenloader/intern/writefile.c
@@ -3636,7 +3636,9 @@ static void write_libraries(WriteData *wd, Main *main)
       found_one = false;
       while (!found_one && tot--) {
         for (id = lbarray[tot]->first; id; id = id->next) {
-          if (id->us > 0 && (id->tag & LIB_TAG_EXTERN)) {
+          if (id->us > 0 &&
+              ((id->tag & LIB_TAG_EXTERN) ||
+               ((id->tag & LIB_TAG_INDIRECT) && (id->flag & LIB_INDIRECT_WEAK_LINK)))) {
             found_one = true;
             break;
           }
@@ -3666,7 +3668,9 @@ static void write_libraries(WriteData *wd, Main *main)
       /* Write link placeholders for all direct linked IDs. */
       while (a--) {
         for (id = lbarray[a]->first; id; id = id->next) {
-          if (id->us > 0 && (id->tag & LIB_TAG_EXTERN)) {
+          if (id->us > 0 &&
+              ((id->tag & LIB_TAG_EXTERN) ||
+               ((id->tag & LIB_TAG_INDIRECT) && (id->flag & LIB_INDIRECT_WEAK_LINK)))) {
             if (!BKE_idcode_is_linkable(GS(id->name))) {
               printf(
                   "ERROR: write file: data-block '%s' from lib '%s' is not linkable "
diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h
index a1bc71ade62..a6212e09567 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -477,6 +477,10 @@ enum {
   /* The datablock structure is a sub-object of a different one.
    * Direct persistent references are not allowed. */
   LIB_PRIVATE_DATA = 1 << 10,
+  /* Datablock is from a library and linked indirectly, with LIB_TAG_INDIRECT
+   * tag set. But the current .blend file also has a weak pointer to it that
+   * we want to restore if possible, and silently drop if it's missing. */
+  LIB_INDIRECT_WEAK_LINK = 1 << 11,
 };
 
 /**