LibOverride: Fix (unreported) crashes in some cases, preserve active object on Clear, general cleanup.

Inconsistencies in update/tagging code between different code doing the
same 'Clear. liboverride operation lead to crashes in some cases.

Unify deg tagging and WM notifiers accross the three editor-level
codepaths performing the common Make/Reset/Clear operations.

Preserve if possible the active object accross Clear operation.

Several cleanup/rename/re-arangement of code to make it more consistent.

4 files changed, 155 insertions, 91 deletions

diff --git a/source/blender/editors/interface/interface_ops.c b/source/blender/editors/interface/interface_ops.c
index e170cb46925..3a5ba9acaac 100644
--- a/source/blender/editors/interface/interface_ops.c
+++ b/source/blender/editors/interface/interface_ops.c
@@ -820,19 +820,26 @@ static int override_idtemplate_make_exec(bContext *C, wmOperator *UNUSED(op))
     return OPERATOR_CANCELLED;
   }
 
-  if (ID_IS_OVERRIDE_LIBRARY_REAL(owner_id)) {
-    PointerRNA idptr;
-    /* `idptr` is re-assigned to owner property to ensure proper updates etc. Here we also use it
-     * to ensure remapping of the owner property from the linked data to the newly created
-     * liboverride (note that in theory this remapping has already been done by code above), but
-     * only in case owner ID was already an existing liboverride.
-     *
-     * Otherwise, owner ID will also have been overridden, and remapped already to use it's
-     * override of the data too. */
+  PointerRNA idptr;
+  /* `idptr` is re-assigned to owner property to ensure proper updates etc. Here we also use it
+   * to ensure remapping of the owner property from the linked data to the newly created
+   * liboverride (note that in theory this remapping has already been done by code above), but
+   * only in case owner ID was already local ID (override or pure local data).
+   *
+   * Otherwise, owner ID will also have been overridden, and remapped already to use it's
+   * override of the data too. */
+  if (!ID_IS_LINKED(owner_id)) {
     RNA_id_pointer_create(id_override, &idptr);
     RNA_property_pointer_set(&owner_ptr, prop, idptr, NULL);
-    RNA_property_update(C, &owner_ptr, prop);
   }
+  RNA_property_update(C, &owner_ptr, prop);
+
+  /* 'Security' extra tagging, since this process may also affect the owner ID and not only the
+   * used ID, relying on the property update code only is not always enough. */
+  DEG_id_tag_update(&CTX_data_scene(C)->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE);
+  WM_event_add_notifier(C, NC_WINDOW, NULL);
+  WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL);
+  WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL);
 
   return OPERATOR_FINISHED;
 }
@@ -881,6 +888,9 @@ static int override_idtemplate_reset_exec(bContext *C, wmOperator *UNUSED(op))
   RNA_property_pointer_set(&owner_ptr, prop, idptr, NULL);
   RNA_property_update(C, &owner_ptr, prop);
 
+  /* No need for 'security' extra tagging here, since this process will never affect the owner ID.
+   */
+
   return OPERATOR_FINISHED;
 }
 
@@ -919,24 +929,45 @@ static int override_idtemplate_clear_exec(bContext *C, wmOperator *UNUSED(op))
   }
 
   Main *bmain = CTX_data_main(C);
+  ViewLayer *view_layer = CTX_data_view_layer(C);
+  Scene *scene = CTX_data_scene(C);
   ID *id_new = id;
+
   if (BKE_lib_override_library_is_hierarchy_leaf(bmain, id)) {
     id_new = id->override_library->reference;
+    bool do_remap_active = false;
+    if (OBACT(view_layer) == (Object *)id) {
+      BLI_assert(GS(id->name) == ID_OB);
+      BLI_assert(GS(id_new->name) == ID_OB);
+      do_remap_active = true;
+    }
     BKE_libblock_remap(bmain, id, id_new, ID_REMAP_SKIP_INDIRECT_USAGE);
+    if (do_remap_active) {
+      Object *ref_object = (Object *)id_new;
+      Base *basact = BKE_view_layer_base_find(view_layer, ref_object);
+      if (basact != NULL) {
+        view_layer->basact = basact;
+      }
+      DEG_id_tag_update(&scene->id, ID_RECALC_SELECT);
+    }
     BKE_id_delete(bmain, id);
   }
   else {
     BKE_lib_override_library_id_reset(bmain, id, true);
   }
 
-  PointerRNA idptr;
-  /* `idptr` is re-assigned to owner property to ensure proper updates etc. Here we also use it to
-   * ensure remapping of the owner property from the linked data to the newly created liboverride
-   * (note that in theory this remapping has already been done by code above). */
-  RNA_id_pointer_create(id_new, &idptr);
-  RNA_property_pointer_set(&owner_ptr, prop, idptr, NULL);
+  /* Here the affected ID may remain the same, or be replaced by its linked reference. In either
+   * case, the owner ID remains unchanged, and remapping is already handled by internal code, so
+   * calling `RNA_property_update` on it is enough to ensure proper notifiers are sent. */
   RNA_property_update(C, &owner_ptr, prop);
 
+  /* 'Security' extra tagging, since this process may also affect the owner ID and not only the
+   * used ID, relying on the property update code only is not always enough. */
+  DEG_id_tag_update(&scene->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE);
+  WM_event_add_notifier(C, NC_WINDOW, NULL);
+  WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL);
+  WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL);
+
   return OPERATOR_FINISHED;
 }
 
diff --git a/source/blender/editors/interface/interface_templates.c b/source/blender/editors/interface/interface_templates.c
index 95952b6e8c8..7d27af7220e 100644
--- a/source/blender/editors/interface/interface_templates.c
+++ b/source/blender/editors/interface/interface_templates.c
@@ -885,14 +885,14 @@ static void template_id_liboverride_hierarchy_make(bContext *C,
       C, bmain, owner_id, id, r_undo_push_label);
 
   if (id_override != NULL) {
-    /* Given `idptr` is re-assigned to owner property by caller to ensure proper updates etc. Here
-     * we also use it to ensure remapping of the owner property from the linked data to the newly
-     * created liboverride (note that in theory this remapping has already been done by code
-     * above), but only in case owner ID was already an existing liboverride.
+    /* `idptr` is re-assigned to owner property to ensure proper updates etc. Here we also use it
+     * to ensure remapping of the owner property from the linked data to the newly created
+     * liboverride (note that in theory this remapping has already been done by code above), but
+     * only in case owner ID was already local ID (override or pure local data).
      *
-     * Otherwise, owner ID will also have been overridden, and remapped already to use
-     * it's override of the data too. */
-    if (ID_IS_OVERRIDE_LIBRARY_REAL(owner_id)) {
+     * Otherwise, owner ID will also have been overridden, and remapped already to use it's
+     * override of the data too. */
+    if (!ID_IS_LINKED(owner_id)) {
       RNA_id_pointer_create(id_override, idptr);
     }
   }
diff --git a/source/blender/editors/object/object_relations.c b/source/blender/editors/object/object_relations.c
index 972912e8863..71dfaefe6a3 100644
--- a/source/blender/editors/object/object_relations.c
+++ b/source/blender/editors/object/object_relations.c
@@ -2420,6 +2420,8 @@ static int make_override_library_exec(bContext *C, wmOperator *op)
 
   DEG_id_tag_update(&CTX_data_scene(C)->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE);
   WM_event_add_notifier(C, NC_WINDOW, NULL);
+  WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL);
+  WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL);
 
   return success ? OPERATOR_FINISHED : OPERATOR_CANCELLED;
 }
@@ -2546,7 +2548,8 @@ static int reset_override_library_exec(bContext *C, wmOperator *UNUSED(op))
   }
   FOREACH_SELECTED_OBJECT_END;
 
-  WM_event_add_notifier(C, NC_WM | ND_DATACHANGED, NULL);
+  WM_event_add_notifier(C, NC_WINDOW, NULL);
+  WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL);
   WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL);
 
   return OPERATOR_FINISHED;
@@ -2570,26 +2573,49 @@ void OBJECT_OT_reset_override_library(wmOperatorType *ot)
 static int clear_override_library_exec(bContext *C, wmOperator *UNUSED(op))
 {
   Main *bmain = CTX_data_main(C);
+  ViewLayer *view_layer = CTX_data_view_layer(C);
+  Scene *scene = CTX_data_scene(C);
+  LinkNode *todo_objects = NULL, *todo_object_iter;
 
   /* Make already existing selected liboverrides editable. */
-  FOREACH_SELECTED_OBJECT_BEGIN (CTX_data_view_layer(C), CTX_wm_view3d(C), ob_iter) {
+  FOREACH_SELECTED_OBJECT_BEGIN (view_layer, CTX_wm_view3d(C), ob_iter) {
     if (ID_IS_LINKED(ob_iter)) {
       continue;
     }
+    BLI_linklist_prepend_alloca(&todo_objects, ob_iter);
+  }
+  FOREACH_SELECTED_OBJECT_END;
+
+  for (todo_object_iter = todo_objects; todo_object_iter != NULL;
+       todo_object_iter = todo_object_iter->next) {
+    Object *ob_iter = todo_object_iter->link;
     if (BKE_lib_override_library_is_hierarchy_leaf(bmain, &ob_iter->id)) {
+      bool do_remap_active = false;
+      if (OBACT(view_layer) == ob_iter) {
+        do_remap_active = true;
+      }
       BKE_libblock_remap(bmain,
                          &ob_iter->id,
                          ob_iter->id.override_library->reference,
                          ID_REMAP_SKIP_INDIRECT_USAGE);
+      if (do_remap_active) {
+        Object *ref_object = ob_iter->id.override_library->reference;
+        Base *basact = BKE_view_layer_base_find(view_layer, ref_object);
+        if (basact != NULL) {
+          view_layer->basact = basact;
+        }
+        DEG_id_tag_update(&scene->id, ID_RECALC_SELECT);
+      }
       BKE_id_delete(bmain, &ob_iter->id);
     }
     else {
       BKE_lib_override_library_id_reset(bmain, &ob_iter->id, true);
     }
   }
-  FOREACH_SELECTED_OBJECT_END;
 
-  WM_event_add_notifier(C, NC_WM | ND_DATACHANGED, NULL);
+  DEG_id_tag_update(&scene->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE);
+  WM_event_add_notifier(C, NC_WINDOW, NULL);
+  WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL);
   WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL);
 
   return OPERATOR_FINISHED;
diff --git a/source/blender/editors/space_outliner/outliner_tools.cc b/source/blender/editors/space_outliner/outliner_tools.cc
index e5c417daa0d..d6305c836ff 100644
--- a/source/blender/editors/space_outliner/outliner_tools.cc
+++ b/source/blender/editors/space_outliner/outliner_tools.cc
@@ -1274,22 +1274,68 @@ static void id_override_library_reset_fn(bContext *C,
   OutlinerLibOverrideData *data = reinterpret_cast<OutlinerLibOverrideData *>(user_data);
   const bool do_hierarchy = data->do_hierarchy;
 
-  if (ID_IS_OVERRIDE_LIBRARY_REAL(id_root)) {
-    Main *bmain = CTX_data_main(C);
+  if (!ID_IS_OVERRIDE_LIBRARY_REAL(id_root)) {
+    CLOG_WARN(&LOG, "Could not reset library override of data block '%s'", id_root->name);
+    return;
+  }
 
-    if (do_hierarchy) {
-      BKE_lib_override_library_id_hierarchy_reset(bmain, id_root, false);
+  Main *bmain = CTX_data_main(C);
+
+  if (do_hierarchy) {
+    BKE_lib_override_library_id_hierarchy_reset(bmain, id_root, false);
+  }
+  else {
+    BKE_lib_override_library_id_reset(bmain, id_root, false);
+  }
+}
+
+static void id_override_library_clear_single_fn(bContext *C,
+                                                ReportList *reports,
+                                                Scene *scene,
+                                                TreeElement *UNUSED(te),
+                                                TreeStoreElem *UNUSED(tsep),
+                                                TreeStoreElem *tselem,
+                                                void *UNUSED(user_data))
+{
+  BLI_assert(TSE_IS_REAL_ID(tselem));
+  Main *bmain = CTX_data_main(C);
+  ViewLayer *view_layer = CTX_data_view_layer(C);
+  ID *id = tselem->id;
+
+  if (!ID_IS_OVERRIDE_LIBRARY_REAL(id)) {
+    BKE_reportf(reports,
+                RPT_WARNING,
+                "Cannot clear embedded library override id '%s', only overrides of real "
+                "data-blocks can be directly deleted",
+                id->name);
+    return;
+  }
+
+  /* If given ID is not using any other override (it's a 'leaf' in the override hierarchy),
+   * delete it and remap its usages to its linked reference. Otherwise, keep it as a reset system
+   * override. */
+  if (BKE_lib_override_library_is_hierarchy_leaf(bmain, id)) {
+    bool do_remap_active = false;
+    if (OBACT(view_layer) == reinterpret_cast<Object *>(id)) {
+      BLI_assert(GS(id->name) == ID_OB);
+      do_remap_active = true;
     }
-    else {
-      BKE_lib_override_library_id_reset(bmain, id_root, false);
+    BKE_libblock_remap(bmain, id, id->override_library->reference, ID_REMAP_SKIP_INDIRECT_USAGE);
+    if (do_remap_active) {
+      Object *ref_object = reinterpret_cast<Object *>(id->override_library->reference);
+      Base *basact = BKE_view_layer_base_find(view_layer, ref_object);
+      if (basact != nullptr) {
+        view_layer->basact = basact;
+      }
+      DEG_id_tag_update(&scene->id, ID_RECALC_SELECT);
     }
-
-    WM_event_add_notifier(C, NC_WM | ND_DATACHANGED, nullptr);
-    WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, nullptr);
+    BKE_id_delete(bmain, id);
   }
   else {
-    CLOG_WARN(&LOG, "Could not reset library override of data block '%s'", id_root->name);
+    BKE_lib_override_library_id_reset(bmain, id, true);
   }
+
+  DEG_id_tag_update(&scene->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE);
 }
 
 static void id_override_library_resync_fn(bContext *UNUSED(C),
@@ -1340,13 +1386,13 @@ static void id_override_library_resync_hierarchy_process(bContext *C,
   WM_event_add_notifier(C, NC_WINDOW, nullptr);
 }
 
-static void id_override_library_clear_hierarchy_fn(bContext *UNUSED(C),
-                                                   ReportList *UNUSED(reports),
-                                                   Scene *UNUSED(scene),
-                                                   TreeElement *UNUSED(te),
-                                                   TreeStoreElem *UNUSED(tsep),
-                                                   TreeStoreElem *tselem,
-                                                   void *user_data)
+static void id_override_library_delete_hierarchy_fn(bContext *UNUSED(C),
+                                                    ReportList *UNUSED(reports),
+                                                    Scene *UNUSED(scene),
+                                                    TreeElement *UNUSED(te),
+                                                    TreeStoreElem *UNUSED(tsep),
+                                                    TreeStoreElem *tselem,
+                                                    void *user_data)
 {
   OutlinerLibOverrideData *data = reinterpret_cast<OutlinerLibOverrideData *>(user_data);
 
@@ -1366,52 +1412,15 @@ static void id_override_library_clear_hierarchy_fn(bContext *UNUSED(C),
 }
 
 /* Clear (delete) a hierarchy of library overrides. */
-static void id_override_library_clear_hierarchy_process(bContext *C,
-                                                        ReportList *UNUSED(reports),
-                                                        OutlinerLibOverrideData &data)
+static void id_override_library_delete_hierarchy_process(bContext *C,
+                                                         ReportList *UNUSED(reports),
+                                                         OutlinerLibOverrideData &data)
 {
   Main *bmain = CTX_data_main(C);
 
   for (auto &&id_hierarchy_root : data.id_hierarchy_roots.keys()) {
     BKE_lib_override_library_delete(bmain, id_hierarchy_root);
   }
-
-  WM_event_add_notifier(C, NC_WINDOW, nullptr);
-}
-
-static void id_override_library_clear_single_fn(bContext *C,
-                                                ReportList *reports,
-                                                Scene *UNUSED(scene),
-                                                TreeElement *UNUSED(te),
-                                                TreeStoreElem *UNUSED(tsep),
-                                                TreeStoreElem *tselem,
-                                                void *UNUSED(user_data))
-{
-  BLI_assert(TSE_IS_REAL_ID(tselem));
-  Main *bmain = CTX_data_main(C);
-  ID *id = tselem->id;
-
-  if (!ID_IS_OVERRIDE_LIBRARY_REAL(id)) {
-    BKE_reportf(reports,
-                RPT_WARNING,
-                "Cannot clear embedded library override id '%s', only overrides of real "
-                "data-blocks can be directly deleted",
-                id->name);
-    return;
-  }
-
-  /* If given ID is not using any other override (it's a 'leaf' in the override hierarchy),
-   * delete it and remap its usages to its linked reference. Otherwise, keep it as a reset system
-   * override. */
-  if (BKE_lib_override_library_is_hierarchy_leaf(bmain, id)) {
-    BKE_libblock_remap(bmain, id, id->override_library->reference, ID_REMAP_SKIP_INDIRECT_USAGE);
-    BKE_id_delete(bmain, id);
-  }
-  else {
-    BKE_lib_override_library_id_reset(bmain, id, true);
-  }
-
-  WM_event_add_notifier(C, NC_WINDOW, nullptr);
 }
 
 static void id_fake_user_set_fn(bContext *UNUSED(C),
@@ -1785,11 +1794,11 @@ static int outliner_liboverride_operation_exec(bContext *C, wmOperator *op)
                                                   op->reports,
                                                   scene,
                                                   space_outliner,
-                                                  id_override_library_clear_hierarchy_fn,
+                                                  id_override_library_delete_hierarchy_fn,
                                                   OUTLINER_LIB_SELECTIONSET_SELECTED,
                                                   nullptr);
 
-      id_override_library_clear_hierarchy_process(C, op->reports, override_data);
+      id_override_library_delete_hierarchy_process(C, op->reports, override_data);
 
       ED_undo_push(C, "Delete Overridden Data Hierarchy");
       break;
@@ -1799,11 +1808,9 @@ static int outliner_liboverride_operation_exec(bContext *C, wmOperator *op)
       break;
   }
 
-  /* wrong notifier still... */
-  WM_event_add_notifier(C, NC_ID | NA_EDITED, nullptr);
-
-  /* XXX: this is just so that outliner is always up to date. */
-  WM_event_add_notifier(C, NC_SPACE | ND_SPACE_OUTLINER, nullptr);
+  WM_event_add_notifier(C, NC_WINDOW, nullptr);
+  WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, nullptr);
+  WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, nullptr);
 
   return OPERATOR_FINISHED;
 }