[Bf-blender-cvs] [554ed613ae5] blender-v2.83-release: Fix T77774: New undo code broken by 'make local' behavior.

[Bastien Montagne]

Commit: 554ed613ae5f26b205f65ed950ae50baf9f009b5
Author: Bastien Montagne
Date:   Mon Jun 15 15:10:17 2020 +0200
Branches: blender-v2.83-release
https://developer.blender.org/rB554ed613ae5f26b205f65ed950ae50baf9f009b5

Fix T77774: New undo code broken by 'make local' behavior.

This is actually a nice issue due to too much optimization...
* Making an ID local just reuse the linked one whenever possible, instead of
  actually making a copy of it.
* Therefore, the collection containing that ID is seen as unchanged, since
  the pointer itself remained the same.
* But on undo step, there is no way to reuse that local object, which then
  gets deleted, and linked one gets re-created - at a different address.
* Collection, however, since unchanged, is not updated at all and thus keeps
  reference to the to-be-deleted local object, instead of the linked one.
* Issue gets even worse with viewlayers, this leads to the crash.

To address this, this patch adds a 'virtual' update flags that does nothing
in update case, but will ensure that the affected IDs using the one made local
are properly detected as changed across the relevant undo step.

Note that the recalc flags were chosen mostly for a logical reason, and also
because they are already properly dealt with and cleared by undo code,
so this looks like the optimal solution.

Reviewed By: brecht

Maniphest Tasks: T77774

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

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

M	source/blender/blenkernel/intern/lib_id.c
M	source/blender/makesdna/DNA_ID.h

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

diff --git a/source/blender/blenkernel/intern/lib_id.c b/source/blender/blenkernel/intern/lib_id.c
index b4513fc2805..7ae6a6aab8c 100644
--- a/source/blender/blenkernel/intern/lib_id.c
+++ b/source/blender/blenkernel/intern/lib_id.c
@@ -177,6 +177,16 @@ static void lib_id_library_local_paths(Main *bmain, Library *lib, ID *id)
                         (void *)bpath_user_data);
 }
 
+static int lib_id_clear_library_data_users_update_cb(LibraryIDLinkCallbackData *cb_data)
+{
+  ID *id = cb_data->user_data;
+  if (*cb_data->id_pointer == id) {
+    DEG_id_tag_update_ex(cb_data->bmain, cb_data->id_owner, ID_RECALC_TAG_FOR_UNDO);
+    return IDWALK_RET_STOP_ITER;
+  }
+  return IDWALK_RET_NOP;
+}
+
 /**
  * Pull an ID out of a library (make it local). Only call this for IDs that
  * don't have other library users.
@@ -199,6 +209,21 @@ static void lib_id_clear_library_data_ex(Main *bmain, ID *id)
     }
   }
 
+  /* Conceptually, an ID made local is not the same as the linked one anymore. Reflect that by
+   * regenerating its session UUID. */
+  BKE_lib_libblock_session_uuid_renew(id);
+
+  /* We need to tag this IDs and all of its users, conceptually new local ID and original linked
+   * ones are two completely different data-blocks that were virtually remaped, even though in
+   * reality they remain the same data. For undo this info is critical now. */
+  DEG_id_tag_update_ex(bmain, id, ID_RECALC_COPY_ON_WRITE);
+  ID *id_iter;
+  FOREACH_MAIN_ID_BEGIN (bmain, id_iter) {
+    BKE_library_foreach_ID_link(
+        bmain, id_iter, lib_id_clear_library_data_users_update_cb, id, IDWALK_READONLY);
+  }
+  FOREACH_MAIN_ID_END;
+
   /* Internal shape key blocks inside data-blocks also stores id->lib,
    * make sure this stays in sync (note that we do not need any explicit handling for real EMBEDDED
    * IDs here, this is down automatically in `lib_id_expand_local_cb()`. */
@@ -1101,8 +1126,10 @@ void BKE_lib_libblock_session_uuid_ensure(ID *id)
 /**
  * Re-generate a new session-wise uuid for the given \a id.
  *
- * \warning This has a very specific use-case (to handle UI-related data-blocks that are kept
- * across new file reading, when we do keep existing UI). No other usage is expected currently.
+ * \warning This has a few very specific use-cases, no other usage is expected currently:
+ *   - To handle UI-related data-blocks that are kept across new file reading, when we do keep
+ * existing UI.
+ *   - For IDs that are made local without needing any copying.
  */
 void BKE_lib_libblock_session_uuid_renew(ID *id)
 {
diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h
index 7ce84e0092c..0cf45fa4bcc 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -680,6 +680,15 @@ typedef enum IDRecalcFlag {
    * input file or for color space changes. */
   ID_RECALC_SOURCE = (1 << 23),
 
+  /* Virtual recalc tag/marker required for undo in some cases, where actual data does not change
+   * and hence do not require an update, but conceptually we are dealing with something new.
+   *
+   * Current known case: linked IDs made local without requiring any copy. While their users do not
+   * require any update, they have actually been 'virtually' remapped from the linked ID to the
+   * local one.
+   */
+  ID_RECALC_TAG_FOR_UNDO = (1 << 24),
+
   /***************************************************************************
    * Pseudonyms, to have more semantic meaning in the actual code without
    * using too much low-level and implementation specific tags. */