LibOverride: refactor of relationships handling in library overrides.

First step towards a better handling of relationships between IDs in
override context, especially when a resync is needed.

First, introduce a new flag to override operations,
`IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE`, for ID pointers.
It keeps track of whether an RNA ID pointer has been kept to its
'natural overriden ID' (in override hierarchy context), or has actually
been re-assigned to some other data-block.

Second, refactor how we deal with relationships between IDs in override
hierarchy code, especially in resync case. This will fixe several cases
listed in T83811, especially the case where an ID pointer to an existing
override needs to be updated to a new one due to a matching change in
linked data.

4 files changed, 202 insertions, 124 deletions

diff --git a/source/blender/blenkernel/BKE_lib_override.h b/source/blender/blenkernel/BKE_lib_override.h
index c9a9c26e222..5fd451dc986 100644
--- a/source/blender/blenkernel/BKE_lib_override.h
+++ b/source/blender/blenkernel/BKE_lib_override.h
@@ -65,15 +65,6 @@ struct ID *BKE_lib_override_library_create_from_id(struct Main *bmain,
                                                    struct ID *reference_id,
                                                    const bool do_tagged_remap);
 bool BKE_lib_override_library_create_from_tag(struct Main *bmain);
-void BKE_lib_override_library_dependencies_tag(struct Main *bmain,
-                                               struct ID *id_root,
-                                               const uint tag,
-                                               const bool do_create_main_relashionships);
-void BKE_lib_override_library_override_group_tag(struct Main *bmain,
-                                                 struct ID *id_root,
-                                                 const uint tag,
-                                                 const uint missing_tag,
-                                                 const bool do_create_main_relashionships);
 bool BKE_lib_override_library_create(struct Main *bmain,
                                      struct Scene *scene,
                                      struct ViewLayer *view_layer,
diff --git a/source/blender/blenkernel/intern/lib_override.c b/source/blender/blenkernel/intern/lib_override.c
index 388f50e12cf..b0438500413 100644
--- a/source/blender/blenkernel/intern/lib_override.c
+++ b/source/blender/blenkernel/intern/lib_override.c
@@ -368,29 +368,50 @@ bool BKE_lib_override_library_create_from_tag(Main *bmain)
   return success;
 }
 
-static bool lib_override_hierarchy_recursive_tag(Main *bmain,
-                                                 ID *id,
-                                                 const uint tag,
-                                                 const uint missing_tag,
-                                                 Library *override_group_lib_reference)
+typedef struct LibOverrideGroupTagData {
+  ID *id_root;
+  uint tag;
+  uint missing_tag;
+} LibOverrideGroupTagData;
+
+static int lib_override_linked_group_tag_cb(LibraryIDLinkCallbackData *cb_data)
 {
-  void **entry_vp = BLI_ghash_lookup_p(bmain->relations->relations_from_pointers, id);
-  if (entry_vp == NULL) {
-    /* This ID is not used by nor using any other ID. */
-    return (id->tag & tag) != 0;
+  if (cb_data->cb_flag & (IDWALK_CB_EMBEDDED | IDWALK_CB_LOOPBACK)) {
+    return IDWALK_RET_STOP_RECURSION;
   }
 
-  MainIDRelationsEntry *entry = *entry_vp;
-  if (entry->tags & MAINIDRELATIONS_ENTRY_TAGS_PROCESSED) {
-    /* This ID has already been processed. */
-    return (id->tag & tag) != 0;
+  LibOverrideGroupTagData *data = cb_data->user_data;
+  const uint tag = data->tag;
+  const uint missing_tag = data->missing_tag;
+
+  ID *id_root = data->id_root;
+  Library *library_root = id_root->lib;
+  ID *id = *cb_data->id_pointer;
+  ID *id_owner = cb_data->id_owner;
+
+  BLI_assert(id_owner == cb_data->id_self);
+
+  if (ELEM(id, NULL, id_owner)) {
+    return IDWALK_RET_NOP;
+  }
+
+  BLI_assert(id_owner->lib == library_root);
+
+  if (*(uint *)&id->tag & (tag | missing_tag)) {
+    /* Already processed and tagged, nothing else to do here. */
+    return IDWALK_RET_STOP_RECURSION;
+  }
+
+  if (id->lib != library_root) {
+    /* We do not override data-blocks from other libraries, nor do we process them. */
+    return IDWALK_RET_STOP_RECURSION;
   }
 
-  /* Note: in case some reference ID is missing from linked data (and therefore its override uses
-   * a placeholder as reference), use `missing_tag` instead of `tag` for this override. */
-  if (override_group_lib_reference != NULL && ID_IS_OVERRIDE_LIBRARY_REAL(id) &&
-      id->override_library->reference->lib == override_group_lib_reference) {
-    if (id->override_library->reference->tag & LIB_TAG_MISSING) {
+  /* We tag all collections and objects for override. And we also tag all other data-blocks which
+   * would use one of those.
+   * Note: missing IDs (aka placeholders) are never overridden. */
+  if (ELEM(GS(id->name), ID_OB, ID_GR)) {
+    if ((id->tag & LIB_TAG_MISSING)) {
       id->tag |= missing_tag;
     }
     else {
@@ -398,6 +419,26 @@ static bool lib_override_hierarchy_recursive_tag(Main *bmain,
     }
   }
 
+  return IDWALK_RET_NOP;
+}
+
+/* Tag all IDs in dependency relationships whithin an override hierarchy/group.
+ *
+ * Note: this is typically called to complete `lib_override_linked_group_tag()`.
+ * Note: BMain's relations mapping won't be valid anymore after that call.
+ */
+static bool lib_override_hierarchy_dependencies_recursive_tag(Main *bmain,
+                                                              ID *id,
+                                                              const uint tag,
+                                                              const uint missing_tag)
+{
+  MainIDRelationsEntry *entry = BLI_ghash_lookup(bmain->relations->relations_from_pointers, id);
+  BLI_assert(entry != NULL);
+
+  if (entry->tags & MAINIDRELATIONS_ENTRY_TAGS_PROCESSED) {
+    /* This ID has already been processed. */
+    return (*(uint *)&id->tag & tag) != 0;
+  }
   /* This way we won't process again that ID, should we encounter it again through another
    * relationship hierarchy. */
   entry->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
@@ -410,81 +451,69 @@ static bool lib_override_hierarchy_recursive_tag(Main *bmain,
       continue;
     }
     /* We only consider IDs from the same library. */
-    if (*to_id_entry->id_pointer.to != NULL && (*to_id_entry->id_pointer.to)->lib == id->lib) {
-      const bool needs_tag = lib_override_hierarchy_recursive_tag(
-          bmain, *to_id_entry->id_pointer.to, tag, missing_tag, override_group_lib_reference);
-      if (needs_tag && override_group_lib_reference == NULL) {
+    ID *to_id = *to_id_entry->id_pointer.to;
+    if (to_id != NULL && to_id->lib == id->lib) {
+      if (lib_override_hierarchy_dependencies_recursive_tag(bmain, to_id, tag, missing_tag)) {
         id->tag |= tag;
       }
     }
   }
 
-  return (id->tag & tag) != 0;
+  return (*(uint *)&id->tag & tag) != 0;
 }
 
-/**
- * Tag all IDs in given \a bmain that are being used by given \a id_root ID or its dependencies,
- * recursively.
- * It detects and tag only chains of dependencies marked at both ends by given tag.
+/* This will tag at least all 'boundary' linked IDs for a potential override group.
  *
- * This will include all local IDs, and all IDs from the same library as the \a id_root.
+ * Note that you will then need to call `lib_override_hierarchy_dependencies_recursive_tag` to
+ * complete tagging of all dependencies whitin theoverride group.
  *
- * \param id_root: The root of the hierarchy of dependencies to be tagged.
- * \param do_create_main_relashionships: Whether main relations needs to be created or already
- * exist (in any case, they will be freed by this function).
+ * We currently only consider Collections and Objects (that are not used as bone shapes) as valid
+ * boundary IDs to define an override group.
  */
-void BKE_lib_override_library_dependencies_tag(Main *bmain,
-                                               ID *id_root,
-                                               const uint tag,
-                                               const bool do_create_main_relashionships)
+static void lib_override_linked_group_tag(Main *bmain,
+                                          ID *id,
+                                          const uint tag,
+                                          const uint missing_tag)
 {
-  if (do_create_main_relashionships) {
-    BKE_main_relations_create(bmain, 0);
-  }
-
-  /* We tag all intermediary data-blocks in-between two overridden ones (e.g. if a shape-key
-   * has a driver using an armature object's bone, we need to override the shape-key/obdata,
-   * the objects using them, etc.) */
-  lib_override_hierarchy_recursive_tag(bmain, id_root, tag, 0, NULL);
+  BKE_main_relations_create(bmain, 0);
 
-  BKE_main_relations_free(bmain);
-}
+  if (ELEM(GS(id->name), ID_OB, ID_GR)) {
+    LibOverrideGroupTagData data = {.id_root = id, .tag = tag, .missing_tag = missing_tag};
+    /* Tag all collections and objects. */
+    BKE_library_foreach_ID_link(
+        bmain, id, lib_override_linked_group_tag_cb, &data, IDWALK_READONLY | IDWALK_RECURSE);
 
-/**
- * Tag all IDs in given \a bmain that are part of the same \a id_root liboverride ID group.
- * That is, all other liboverride IDs (in)directly used by \a is_root one, and sharing the same
- * library for their reference IDs.
- *
- * \param id_root: The root of the hierarchy of liboverride dependencies to be tagged.
- * \param do_create_main_relashionships: Whether main relations needs to be created or already
- * exist (in any case, they will be freed by this function).
- */
-void BKE_lib_override_library_override_group_tag(Main *bmain,
-                                                 ID *id_root,
-                                                 const uint tag,
-                                                 const uint missing_tag,
-                                                 const bool do_create_main_relashionships)
-{
-  if (do_create_main_relashionships) {
-    BKE_main_relations_create(bmain, 0);
+    /* Then, we remove (untag) bone shape objects, you shall never want to directly/explicitely
+     * override those. */
+    LISTBASE_FOREACH (Object *, ob, &bmain->objects) {
+      if (ob->type == OB_ARMATURE && ob->pose != NULL && (ob->id.tag & tag)) {
+        for (bPoseChannel *pchan = ob->pose->chanbase.first; pchan != NULL; pchan = pchan->next) {
+          if (pchan->custom != NULL) {
+            pchan->custom->id.tag &= ~(tag | missing_tag);
+          }
+        }
+      }
+    }
   }
 
-  /* We tag all liboverride data-blocks from the same library as reference one,
-   * being used by the root ID. */
-  lib_override_hierarchy_recursive_tag(
-      bmain, id_root, tag, missing_tag, id_root->override_library->reference->lib);
+  lib_override_hierarchy_dependencies_recursive_tag(bmain, id, tag, missing_tag);
 
   BKE_main_relations_free(bmain);
 }
 
-static int lib_override_library_make_tag_ids_cb(LibraryIDLinkCallbackData *cb_data)
+static int lib_override_local_group_tag_cb(LibraryIDLinkCallbackData *cb_data)
 {
-  if (cb_data->cb_flag & (IDWALK_CB_EMBEDDED | IDWALK_CB_LOOPBACK)) {
+  if (cb_data->cb_flag &
+      (IDWALK_CB_EMBEDDED | IDWALK_CB_LOOPBACK | IDWALK_CB_OVERRIDE_LIBRARY_REFERENCE)) {
     return IDWALK_RET_STOP_RECURSION;
   }
 
-  ID *id_root = cb_data->user_data;
-  Library *library_root = id_root->lib;
+  LibOverrideGroupTagData *data = cb_data->user_data;
+  const uint tag = data->tag;
+  const uint missing_tag = data->missing_tag;
+
+  ID *id_root = data->id_root;
+  Library *library_reference_root = id_root->override_library->reference->lib;
   ID *id = *cb_data->id_pointer;
   ID *id_owner = cb_data->id_owner;
 
@@ -494,62 +523,67 @@ static int lib_override_library_make_tag_ids_cb(LibraryIDLinkCallbackData *cb_da
     return IDWALK_RET_NOP;
   }
 
-  BLI_assert(id->lib != NULL);
-  BLI_assert(id_owner->lib == library_root);
-
-  if (id->tag & LIB_TAG_DOIT) {
+  if (*(uint *)&id->tag & (tag | missing_tag)) {
     /* Already processed and tagged, nothing else to do here. */
     return IDWALK_RET_STOP_RECURSION;
   }
 
-  if (id->lib != library_root) {
+  if (!ID_IS_OVERRIDE_LIBRARY(id) || ID_IS_LINKED(id)) {
+    /* Fully local, or linked ID, those are never part of a local override group. */
+    return IDWALK_RET_STOP_RECURSION;
+  }
+
+  /* NOTE: Since we rejected embedded data too at the beginning of this function, id should only be
+   * a real override now.
+   *
+   * However, our usual trouble maker, Key, is not considered as an embedded ID currently, yet it
+   * is never a real override either. Enjoy. */
+  if (!ID_IS_OVERRIDE_LIBRARY_REAL(id)) {
+    return IDWALK_RET_NOP;
+  }
+
+  if (id->override_library->reference->lib != library_reference_root) {
     /* We do not override data-blocks from other libraries, nor do we process them. */
     return IDWALK_RET_STOP_RECURSION;
   }
 
-  /* We tag all collections and objects for override. And we also tag all other data-blocks which
-   * would use one of those.
-   * Note: missing IDs (aka placeholders) are never overridden. */
-  if (ELEM(GS(id->name), ID_OB, ID_GR) && !(id->tag & LIB_TAG_MISSING)) {
-    id->tag |= LIB_TAG_DOIT;
+  if (id->override_library->reference->tag & LIB_TAG_MISSING) {
+    id->tag |= missing_tag;
+  }
+  else {
+    id->tag |= tag;
   }
 
   return IDWALK_RET_NOP;
 }
 
+/* This will tag at least all 'boundary' linked IDs for a potential override group.
+ *
+ * Note that you will then need to call `lib_override_hierarchy_dependencies_recursive_tag` to
+ * complete tagging of all dependencies whitin theoverride group.
+ *
+ * We currently only consider Collections and Objects (that are not used as bone shapes) as valid
+ * boundary IDs to define an override group.
+ */
+static void lib_override_local_group_tag(Main *bmain,
+                                         ID *id,
+                                         const uint tag,
+                                         const uint missing_tag)
+{
+  LibOverrideGroupTagData data = {.id_root = id, .tag = tag, .missing_tag = missing_tag};
+  /* Tag all local overrides in id_root's group. */
+  BKE_library_foreach_ID_link(
+      bmain, id, lib_override_local_group_tag_cb, &data, IDWALK_READONLY | IDWALK_RECURSE);
+}
+
 static bool lib_override_library_create_do(Main *bmain, ID *id_root)
 {
   id_root->tag |= LIB_TAG_DOIT;
 
   BKE_main_relations_create(bmain, 0);
 
-  if (ELEM(GS(id_root->name), ID_OB, ID_GR)) {
-    /* Tag all collections and objects. */
-    BKE_library_foreach_ID_link(bmain,
-                                id_root,
-                                lib_override_library_make_tag_ids_cb,
-                                id_root,
-                                IDWALK_READONLY | IDWALK_RECURSE);
-
-    /* Then, we remove (untag) bone shape objects, you shall never want to override those
-     * (hopefully). */
-    LISTBASE_FOREACH (Object *, ob, &bmain->objects) {
-      if (ob->type == OB_ARMATURE && ob->pose != NULL && (ob->id.tag & LIB_TAG_DOIT)) {
-        for (bPoseChannel *pchan = ob->pose->chanbase.first; pchan != NULL; pchan = pchan->next) {
-          if (pchan->custom != NULL) {
-            pchan->custom->id.tag &= ~LIB_TAG_DOIT;
-          }
-        }
-      }
-    }
-  }
-
-  /* Now tag all non-object/collection IDs 'in-between' two tagged ones, as those are part of an
-   * override chain and therefore also need to be overridden.
-   * One very common cases are e.g. drivers on geometry or materials of an overridden object, that
-   * are using another overridden object as parameter. */
-  /* Note that this call will also free the main relations data we created above. */
-  BKE_lib_override_library_dependencies_tag(bmain, id_root, LIB_TAG_DOIT, false);
+  lib_override_linked_group_tag(bmain, id_root, LIB_TAG_DOIT, LIB_TAG_MISSING);
+  lib_override_hierarchy_dependencies_recursive_tag(bmain, id_root, LIB_TAG_DOIT, LIB_TAG_MISSING);
 
   return BKE_lib_override_library_create_from_tag(bmain);
 }
@@ -742,14 +776,14 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
 {
   BLI_assert(ID_IS_OVERRIDE_LIBRARY_REAL(id_root));
 
-  /* Tag all collections and objects, as well as other IDs using them. */
   id_root->tag |= LIB_TAG_DOIT;
   ID *id_root_reference = id_root->override_library->reference;
 
-  /* Make a mapping 'linked reference IDs' -> 'Local override IDs' of existing overrides, and tag
-   * linked reference ones to be overridden again. */
-  BKE_lib_override_library_override_group_tag(bmain, id_root, LIB_TAG_DOIT, LIB_TAG_MISSING, true);
+  lib_override_local_group_tag(bmain, id_root, LIB_TAG_DOIT, LIB_TAG_MISSING);
 
+  lib_override_linked_group_tag(bmain, id_root_reference, LIB_TAG_DOIT, LIB_TAG_MISSING);
+
+  /* Make a mapping 'linked reference IDs' -> 'Local override IDs' of existing overrides. */
   GHash *linkedref_to_old_override = BLI_ghash_new(
       BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp, __func__);
   ID *id;
@@ -760,7 +794,7 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
        * same linked ID in a same hierarchy. */
       if (!BLI_ghash_haskey(linkedref_to_old_override, id->override_library->reference)) {
         BLI_ghash_insert(linkedref_to_old_override, id->override_library->reference, id);
-        id->override_library->reference->tag |= LIB_TAG_DOIT;
+        BLI_assert(id->override_library->reference->tag & LIB_TAG_DOIT);
       }
     }
   }
@@ -770,7 +804,7 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
   /* Note that this call also remaps all pointers of tagged IDs from old override IDs to new
    * override IDs (including within the old overrides themselves, since those are tagged too
    * above). */
-  const bool success = lib_override_library_create_do(bmain, id_root_reference);
+  const bool success = BKE_lib_override_library_create_from_tag(bmain);
 
   if (!success) {
     return success;
@@ -833,6 +867,23 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
         RNA_id_pointer_create(id_override_old, &rnaptr_src);
         RNA_id_pointer_create(id_override_new, &rnaptr_dst);
 
+        /* We remove any operation tagged with `IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE`,
+         * that way the potentially new pointer will be properly kept, when old one is still valid
+         * too (typical case: assigning new ID to some usage, while old one remains used elsewhere
+         * in the override hierarchy). */
+        LISTBASE_FOREACH_MUTABLE (
+            IDOverrideLibraryProperty *, op, &id_override_new->override_library->properties) {
+          LISTBASE_FOREACH_MUTABLE (IDOverrideLibraryPropertyOperation *, opop, &op->operations) {
+            if (opop->flag & IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE) {
+              lib_override_library_property_operation_clear(opop);
+              BLI_freelinkN(&op->operations, opop);
+            }
+          }
+          if (BLI_listbase_is_empty(&op->operations)) {
+            BKE_lib_override_library_property_delete(id_override_new->override_library, op);
+          }
+        }
+
         RNA_struct_override_apply(
             bmain, &rnaptr_dst, &rnaptr_src, NULL, id_override_new->override_library);
       }
@@ -905,7 +956,7 @@ void BKE_lib_override_library_delete(Main *bmain, ID *id_root)
   id_root->tag |= LIB_TAG_DOIT;
 
   /* Tag all library overrides in the chains of dependencies from the given root one. */
-  BKE_lib_override_library_override_group_tag(bmain, id_root, LIB_TAG_DOIT, LIB_TAG_DOIT, true);
+  lib_override_local_group_tag(bmain, id_root, LIB_TAG_DOIT, LIB_TAG_DOIT);
 
   ID *id;
   FOREACH_MAIN_ID_BEGIN (bmain, id) {
diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h
index 89c7ba0cca7..2c7e82e6587 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -193,6 +193,10 @@ enum {
   IDOVERRIDE_LIBRARY_FLAG_MANDATORY = 1 << 0,
   /** User cannot change that override operation. */
   IDOVERRIDE_LIBRARY_FLAG_LOCKED = 1 << 1,
+
+  /** For overrides of ID pointers: this override still matches (follows) the hierarchy of the
+   *  reference linked data. */
+  IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE = 1 << 8,
 };
 
 /** A single overridden property, contain all operations on this one. */
diff --git a/source/blender/makesrna/intern/rna_rna.c b/source/blender/makesrna/intern/rna_rna.c
index 826e1202efc..cad0d77607b 100644
--- a/source/blender/makesrna/intern/rna_rna.c
+++ b/source/blender/makesrna/intern/rna_rna.c
@@ -1314,9 +1314,9 @@ static int rna_property_override_diff_propptr(Main *bmain,
             BLI_assert(op->rna_prop_type == property_type);
           }
 
+          IDOverrideLibraryPropertyOperation *opop = NULL;
           if (created || rna_itemname_a != NULL || rna_itemname_b != NULL ||
               rna_itemindex_a != -1 || rna_itemindex_b != -1) {
-            IDOverrideLibraryPropertyOperation *opop;
             opop = BKE_lib_override_library_property_operation_get(op,
                                                                    IDOVERRIDE_LIBRARY_OP_REPLACE,
                                                                    rna_itemname_b,
@@ -1337,6 +1337,38 @@ static int rna_property_override_diff_propptr(Main *bmain,
           else {
             BKE_lib_override_library_operations_tag(op, IDOVERRIDE_LIBRARY_TAG_UNUSED, false);
           }
+
+          if (is_id && no_ownership) {
+            if (opop == NULL) {
+              opop = BKE_lib_override_library_property_operation_find(op,
+                                                                      rna_itemname_b,
+                                                                      rna_itemname_a,
+                                                                      rna_itemindex_b,
+                                                                      rna_itemindex_a,
+                                                                      true,
+                                                                      NULL);
+              BLI_assert(opop != NULL);
+            }
+
+            BLI_assert(propptr_a->data == propptr_a->owner_id);
+            BLI_assert(propptr_b->data == propptr_b->owner_id);
+            ID *id_a = propptr_a->data;
+            ID *id_b = propptr_b->data;
+            if (ELEM(NULL, id_a, id_b)) {
+              /* In case one of the pointer is NULL and not the other, we consider that the
+               * override is not matching its reference anymore. */
+              opop->flag &= ~IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE;
+            }
+            else if (id_a->override_library != NULL && id_a->override_library->reference == id_b) {
+              opop->flag |= IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE;
+            }
+            else if (id_b->override_library != NULL && id_b->override_library->reference == id_a) {
+              opop->flag |= IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE;
+            }
+            else {
+              opop->flag &= ~IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE;
+            }
+          }
         }
       }