Undo: change depsgraph recalc flags handling to improve performance

These changes only have an effect when the experimental Undo Speedup preference
is enabled.

* For DEG_id_tag_update, accumulate recalc flags immediately before the undo
  push happens instead of afterwards. Otherwise the undo state does not
  contain enough flags, and the current state may contain too many flags.

  This also means we call DEG_id_tag_update after undo with the accumulated
  flags to ensure they are flushed to other datablocks.

* For undo, accumulate recalc flags in id->recalc and clear accumulated flags
  immediately. Not clearing would cause circular behavior where accumulated
  flags may never end up being cleared.

  This matches what happens after an undo push where these are also cleared,
  indicating that the undo state and current in-memory state match exactly.

* Don't change id->recalc of identical datablocks, it should not be needed.
  There is one exception for armatures where pointers across datablocks
  exist which otherwise would cause problems. There may be a better solution
  to this but it seems to work in agent 327 production files.

* This contains a change in undofile.c to avoid detecting all datablocks as
  changed for the first of the two undo steps, where we restore to the state
  of the last undo push before going to the one before.

  Without this the whole system is much less efficient. However this is unsafe
  in the sense that if an app handler or operators edits a datablock after an
  undo push, that change will not be undone.

  It can be argued that this is acceptable behavior, since a following undo push
  will include that change and this may already have unexpected side effects.

Ref T60695

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

source/blender/blenkernel/intern/scene.c

@@ -2167,8 +2167,6 @@ GHash *BKE_scene_undo_depsgraphs_extract(Main *bmain)
 void BKE_scene_undo_depsgraphs_restore(Main *bmain, GHash *depsgraph_extract)
 {
   for (Scene *scene = bmain->scenes.first; scene != NULL; scene = scene->id.next) {
-    BLI_assert(scene->depsgraph_hash == NULL);
-
     for (ViewLayer *view_layer = scene->view_layers.first; view_layer != NULL;
          view_layer = view_layer->next) {
       char key_full[MAX_ID_NAME + FILE_MAX + MAX_NAME] = {0};

source/blender/blenloader/intern/readfile.c

@@ -2800,6 +2800,62 @@ static void direct_link_id_private_id(FileData *fd, ID *id, ID *id_old)
   }
 }
 
+static int direct_link_id_restore_recalc_exceptions(const ID *id_current)
+{
+  /* Exception for armature objects, where the pose has direct points to the
+   * armature databolock. */
+  if (GS(id_current->name) == ID_OB && ((Object *)id_current)->pose) {
+    return ID_RECALC_GEOMETRY;
+  }
+
+  return 0;
+}
+
+static int direct_link_id_restore_recalc(const FileData *fd,
+                                         const ID *id_target,
+                                         const ID *id_current,
+                                         const bool is_identical)
+{
+  /* These are the evaluations that had not been performed yet at the time the
+   * target undo state was written. These need to be done again, since they may
+   * flush back changes to the original datablock. */
+  int recalc = id_target->recalc;
+
+  if (id_current == NULL) {
+    /* ID does not currently exist in the database, so also will not exist in
+     * the dependency graphs. That means it will be newly created and as a
+     * result also fully re-evaluated regardless of the recalc flag set here. */
+    recalc |= ID_RECALC_ALL;
+  }
+  else {
+    /* If the contents datablock changed, the depsgraph needs to copy the
+     * datablock again to ensure it matches the original datablock. */
+    if (!is_identical) {
+      recalc |= ID_RECALC_COPY_ON_WRITE;
+    }
+
+    /* Special exceptions. */
+    recalc |= direct_link_id_restore_recalc_exceptions(id_current);
+
+    /* Evaluations for the current state that have not been performed yet
+     * by the time we are perfoming this undo step. */
+    recalc |= id_current->recalc;
+
+    /* Tags that were set between the target state and the current state,
+     * that we need to perform again. */
+    if (fd->undo_direction < 0) {
+      /* Undo: tags from target to the current state. */
+      recalc |= id_current->recalc_undo_accumulated;
+    }
+    else {
+      /* Redo: tags from current to the target state. */
+      recalc |= id_target->recalc_undo_accumulated;
+    }
+  }
+
+  return recalc;
+}
+
 static void direct_link_id_common(FileData *fd, ID *id, ID *id_old, const int tag)
 {
   /*link direct data of ID properties*/
@@ -2827,29 +2883,8 @@ static void direct_link_id_common(FileData *fd, ID *id, ID *id_old, const int ta
     id->recalc_undo_accumulated = 0;
   }
   else if ((fd->skip_flags & BLO_READ_SKIP_UNDO_OLD_MAIN) == 0) {
-    if (fd->undo_direction < 0) {
-      /* We are coming from the future (i.e. do an actual undo, and not a redo), and we found an
-       * old (aka existing) ID: we use its 'accumulated recalc flags since last memfile undo step
-       * saving' as recalc flags of our newly read ID. */
-      if (id_old != NULL) {
-        id->recalc = id_old->recalc_undo_accumulated;
-      }
-    }
-    else {
-      /* We are coming from the past (i.e. do a redo), we use saved 'accumulated
-       * recalc flags since last memfile undo step saving' as recalc flags of our newly read ID. */
-      id->recalc = id->recalc_undo_accumulated;
-    }
-    /* In any case, we need to flush the depsgraph's CoWs, as even if the ID address itself did not
-     * change, internal data most likely have. */
-    id->recalc |= ID_RECALC_COPY_ON_WRITE;
-
-    /* We need to 'accumulate' the accumulated recalc flags of all undo steps until we actually
-     * perform a depsgraph update, otherwise we'd only ever use the flags from one of the steps,
-     * and never get proper flags matching all others. */
-    if (id_old != NULL) {
-      id->recalc_undo_accumulated |= id_old->recalc_undo_accumulated;
-    }
+    id->recalc = direct_link_id_restore_recalc(fd, id, id_old, false);
+    id->recalc_undo_accumulated = 0;
   }
 
   /* Link direct data of overrides. */
@@ -9498,7 +9533,7 @@ static bool read_libblock_undo_restore_linked(FileData *fd, Main *main, const ID
 
 /* For undo, restore unchanged datablock from old main. */
 static void read_libblock_undo_restore_identical(
-    FileData *fd, Main *main, const ID *id, ID *id_old, const int tag)
+    FileData *fd, Main *main, const ID *UNUSED(id), ID *id_old, const int tag)
 {
   BLI_assert((fd->skip_flags & BLO_READ_SKIP_UNDO_OLD_MAIN) == 0);
   BLI_assert(id_old != NULL);
@@ -9511,10 +9546,6 @@ static void read_libblock_undo_restore_identical(
   id_old->newid = NULL;
   id_old->orig_id = NULL;
 
-  /* About recalc: since that ID did not change at all, we know that its recalc fields also
-   * remained unchanged, so no need to handle neither recalc nor recalc_undo_future here.
-   */
-
   const short idcode = GS(id_old->name);
   Main *old_bmain = fd->old_mainlist->first;
   ListBase *old_lb = which_libbase(old_bmain, idcode);
@@ -9522,31 +9553,9 @@ static void read_libblock_undo_restore_identical(
   BLI_remlink(old_lb, id_old);
   BLI_addtail(new_lb, id_old);
 
-  /* Even though we re-use the old ID as-is, it does not mean that we are 100% safe from
-   * needing some depsgraph updates for it (it could depend on another ID which address
-   * did not change, but which actual content might have been re-read from the memfile).
-   * IMPORTANT: Do not fully overwrite recalc flag here, depsgraph may not have been ran
-   * yet for previous undo step(s), we do not want to erase flags set by those.
-   */
-  if (fd->undo_direction < 0) {
-    /* We are coming from the future (i.e. do an actual undo, and not a redo), we use our
-     * old reused ID's 'accumulated recalc flags since last memfile undo step saving' as
-     * recalc flags. */
-    id_old->recalc |= id_old->recalc_undo_accumulated;
-  }
-  else {
-    /* We are coming from the past (i.e. do a redo), we use the saved 'accumulated recalc
-     * flags since last memfile undo step saving' from the newly read ID as recalc flags.
-     */
-    id_old->recalc |= id->recalc_undo_accumulated;
-  }
-  /* There is no need to flush the depsgraph's CoWs here, since that ID's data itself did
-   * not change. */
-
-  /* We need to 'accumulate' the accumulated recalc flags of all undo steps until we
-   * actually perform a depsgraph update, otherwise we'd only ever use the flags from one
-   * of the steps, and never get proper flags matching all others. */
-  id_old->recalc_undo_accumulated |= id->recalc_undo_accumulated;
+  /* Recalc flags, mostly these just remain as they are. */
+  id_old->recalc |= direct_link_id_restore_recalc_exceptions(id_old);
+  id_old->recalc_undo_accumulated = 0;
 }
 
 /* For undo, store changed datablock at old address. */
@@ -10217,6 +10226,10 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
   BlendFileData *bfd;
   ListBase mainlist = {NULL, NULL};
 
+  if (fd->memfile != NULL) {
+    DEBUG_PRINTF("\nUNDO: read step\n");
+  }
+
   bfd = MEM_callocN(sizeof(BlendFileData), "blendfiledata");
 
   bfd->main = BKE_main_new();

source/blender/blenloader/intern/undofile.c

@@ -106,7 +106,10 @@ void memfile_chunk_add(MemFile *memfile, const char *buf, uint size, MemFileChun
   curchunk->size = size;
   curchunk->buf = NULL;
   curchunk->is_identical = false;
-  curchunk->is_identical_future = false;
+  /* This is unsafe in the sense that an app handler or other code that does not
+   * perform an undo push may make changes after the last undo push that
+   * will then not be undo. Though it's not entirely clear that is wrong behavior. */
+  curchunk->is_identical_future = true;
   BLI_addtail(&memfile->chunks, curchunk);
 
   /* we compare compchunk with buf */

source/blender/depsgraph/intern/depsgraph_tag.cc

@@ -622,6 +622,10 @@ void id_tag_update(Main *bmain, ID *id, int flag, eUpdateSource update_source)
   for (DEG::Depsgraph *depsgraph : DEG::get_all_registered_graphs(bmain)) {
     graph_id_tag_update(bmain, depsgraph, id, flag, update_source);
   }
+
+  /* Accumulate all tags for an ID between two undo steps, so they can be
+   * replayed for undo. */
+  id->recalc_undo_accumulated |= flag;
 }
 
 void graph_id_tag_update(
@@ -820,13 +824,10 @@ void DEG_ids_check_recalc(
 
 static void deg_graph_clear_id_recalc_flags(ID *id)
 {
-  /* Keep incremental track of used recalc flags, to get proper update when undoing. */
-  id->recalc_undo_accumulated |= id->recalc;
   id->recalc &= ~ID_RECALC_ALL;
   bNodeTree *ntree = ntreeFromID(id);
   /* Clear embedded node trees too. */
   if (ntree) {
-    ntree->id.recalc_undo_accumulated |= ntree->id.recalc;
     ntree->id.recalc &= ~ID_RECALC_ALL;
   }
   /* XXX And what about scene's master collection here? */

source/blender/editors/undo/memfile_undo.c

@@ -25,14 +25,17 @@
 
 #include "BLI_ghash.h"
 
+#include "DNA_node_types.h"
 #include "DNA_object_enums.h"
 #include "DNA_object_types.h"
+#include "DNA_scene_types.h"
 
 #include "BKE_blender_undo.h"
 #include "BKE_context.h"
 #include "BKE_lib_id.h"
 #include "BKE_lib_query.h"
 #include "BKE_main.h"
+#include "BKE_node.h"
 #include "BKE_scene.h"
 #include "BKE_undo_system.h"
 
@@ -106,7 +109,6 @@ static int memfile_undosys_step_id_reused_cb(LibraryIDLinkCallbackData *cb_data)
   ID *id_self = cb_data->id_self;
   ID **id_pointer = cb_data->id_pointer;
   BLI_assert((id_self->tag & LIB_TAG_UNDO_OLD_ID_REUSED) != 0);
-  Main *bmain = cb_data->user_data;
 
   ID *id = *id_pointer;
   if (id != NULL && id->lib == NULL && (id->tag & LIB_TAG_UNDO_OLD_ID_REUSED) == 0) {
@@ -129,9 +131,6 @@ static int memfile_undosys_step_id_reused_cb(LibraryIDLinkCallbackData *cb_data)
       }
     }
 
-    /* In case an old, re-used ID is using a newly read data-block (i.e. one of its ID pointers got
-     * updated), we have to tell the depsgraph about it. */
-    DEG_id_tag_update_ex(bmain, id_self, ID_RECALC_COPY_ON_WRITE);
     return do_stop_iter ? IDWALK_RET_STOP_ITER : IDWALK_RET_NOP;
   }
 
@@ -219,10 +218,35 @@ static void memfile_undosys_step_decode(struct bContext *C,
         BKE_library_foreach_ID_link(
             bmain, id, memfile_undosys_step_id_reused_cb, bmain, IDWALK_READONLY);
       }
+
+      /* Tag depsgraph to update datablock for changes that happend between the
+       * current and the target state, see direct_link_id_restore_recalc(). */
+      if (id->recalc) {
+        DEG_id_tag_update_ex(bmain, id, id->recalc);
+      }
     }
     FOREACH_MAIN_ID_END;
 
-    BKE_main_id_tag_all(bmain, LIB_TAG_UNDO_OLD_ID_REUSED, false);
+    FOREACH_MAIN_ID_BEGIN (bmain, id) {
+      /* Clear temporary tag. */
+      id->tag &= ~LIB_TAG_UNDO_OLD_ID_REUSED;
+
+      /* We only start accumulating from this point, any tags set up to here
+       * are already part of the current undo state. This is done in a second
+       * loop because DEG_id_tag_update may set tags on other datablocks. */
+      id->recalc_undo_accumulated = 0;
+      bNodeTree *nodetree = ntreeFromID(id);
+      if (nodetree != NULL) {
+        nodetree->id.recalc_undo_accumulated = 0;
+      }
+      if (GS(id->name) == ID_SCE) {
+        Scene *scene = (Scene *)id;
+        if (scene->master_collection != NULL) {
+          scene->master_collection->id.recalc_undo_accumulated = 0;
+        }
+      }
+    }
+    FOREACH_MAIN_ID_END;
   }
 
   WM_event_add_notifier(C, NC_SCENE | ND_LAYER_CONTENT, CTX_data_scene(C));