Fix T75542: toggling modifier visibility not working correct with undo speedup

The problem was that in direct_link_id_restore_recalc, recalc_undo_accumulated
should contain the changes from the target state to the current state. However
it had already been cleared at that point, to start accumulating changes up to
the next undo push.

Delaying the clear of this flag seems like the obvious solution, but it's hard
to find the right place for that (if there is one). Instead this splits up the
flag into two separate variables.

Reviewed By: mont29

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

source/blender/blenloader/intern/readfile.c

@@ -2845,11 +2845,11 @@ static int direct_link_id_restore_recalc(const FileData *fd,
      * 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;
+      recalc |= id_current->recalc_up_to_undo_push;
     }
     else {
       /* Redo: tags from current to the target state. */
-      recalc |= id_target->recalc_undo_accumulated;
+      recalc |= id_target->recalc_up_to_undo_push;
     }
   }
 
@@ -2880,11 +2880,11 @@ static void direct_link_id_common(FileData *fd, ID *id, ID *id_old, const int ta
    * the version the file has been saved with. */
   if (fd->memfile == NULL) {
     id->recalc = 0;
-    id->recalc_undo_accumulated = 0;
+    id->recalc_after_undo_push = 0;
   }
   else if ((fd->skip_flags & BLO_READ_SKIP_UNDO_OLD_MAIN) == 0) {
     id->recalc = direct_link_id_restore_recalc(fd, id, id_old, false);
-    id->recalc_undo_accumulated = 0;
+    id->recalc_after_undo_push = 0;
   }
 
   /* Link direct data of overrides. */
@@ -9555,7 +9555,7 @@ static void read_libblock_undo_restore_identical(
 
   /* 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;
+  id_old->recalc_after_undo_push = 0;
 
   /* As usual, proxies require some special love...
    * In `blo_clear_proxy_pointers_from_lib()` we clear all `proxy_from` pointers to local IDs, for

source/blender/blenloader/intern/writefile.c

@@ -1133,11 +1133,6 @@ static void write_nodetree_nolib(WriteData *wd, bNodeTree *ntree)
   for (sock = ntree->outputs.first; sock; sock = sock->next) {
     write_node_socket_interface(wd, sock);
   }
-
-  /* Clear the accumulated recalc flags in case of undo step saving. */
-  if (wd->use_memfile) {
-    ntree->id.recalc_undo_accumulated = 0;
-  }
 }
 
 /**
@@ -2454,11 +2449,6 @@ static void write_collection_nolib(WriteData *wd, Collection *collection)
   LISTBASE_FOREACH (CollectionChild *, child, &collection->children) {
     writestruct(wd, DATA, CollectionChild, 1, child);
   }
-
-  /* Clear the accumulated recalc flags in case of undo step saving. */
-  if (wd->use_memfile) {
-    collection->id.recalc_undo_accumulated = 0;
-  }
 }
 
 static void write_collection(WriteData *wd, Collection *collection, const void *id_address)
@@ -4069,6 +4059,28 @@ static bool write_file_handle(Main *mainvar,
           BKE_lib_override_library_operations_store_start(bmain, override_storage, id);
         }
 
+        if (wd->use_memfile) {
+          /* Record the changes that happened up to this undo push in
+           * recalc_up_to_undo_push, and clear recalc_after_undo_push again
+           * to start accumulating for the next undo push. */
+          id->recalc_up_to_undo_push = id->recalc_after_undo_push;
+          id->recalc_after_undo_push = 0;
+
+          bNodeTree *nodetree = ntreeFromID(id);
+          if (nodetree != NULL) {
+            nodetree->id.recalc_up_to_undo_push = nodetree->id.recalc_after_undo_push;
+            nodetree->id.recalc_after_undo_push = 0;
+          }
+          if (GS(id->name) == ID_SCE) {
+            Scene *scene = (Scene *)id;
+            if (scene->master_collection != NULL) {
+              scene->master_collection->id.recalc_up_to_undo_push =
+                  scene->master_collection->id.recalc_after_undo_push;
+              scene->master_collection->id.recalc_after_undo_push = 0;
+            }
+          }
+        }
+
         memcpy(id_buffer, id, idtype_struct_size);
 
         ((ID *)id_buffer)->tag = 0;
@@ -4206,9 +4218,6 @@ static bool write_file_handle(Main *mainvar,
           /* Very important to do it after every ID write now, otherwise we cannot know whether a
            * specific ID changed or not. */
           mywrite_flush(wd);
-
-          /* Clear the accumulated recalc flags in case of undo step saving. */
-          id->recalc_undo_accumulated = 0;
         }
       }
 

source/blender/depsgraph/intern/depsgraph_tag.cc

@@ -625,7 +625,7 @@ void id_tag_update(Main *bmain, ID *id, int flag, eUpdateSource update_source)
 
   /* Accumulate all tags for an ID between two undo steps, so they can be
    * replayed for undo. */
-  id->recalc_undo_accumulated |= deg_recalc_flags_effective(NULL, flag);
+  id->recalc_after_undo_push |= deg_recalc_flags_effective(NULL, flag);
 }
 
 void graph_id_tag_update(

source/blender/editors/undo/memfile_undo.c

@@ -234,15 +234,15 @@ static void memfile_undosys_step_decode(struct bContext *C,
       /* 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;
+      id->recalc_after_undo_push = 0;
       bNodeTree *nodetree = ntreeFromID(id);
       if (nodetree != NULL) {
-        nodetree->id.recalc_undo_accumulated = 0;
+        nodetree->id.recalc_after_undo_push = 0;
       }
       if (GS(id->name) == ID_SCE) {
         Scene *scene = (Scene *)id;
         if (scene->master_collection != NULL) {
-          scene->master_collection->id.recalc_undo_accumulated = 0;
+          scene->master_collection->id.recalc_after_undo_push = 0;
         }
       }
     }

source/blender/makesdna/DNA_ID.h

@@ -246,11 +246,16 @@ typedef struct ID {
   int icon_id;
   int recalc;
   /**
-   * Used by undo code. Value of recalc is stored there when reading an ID from memfile, and not
-   * touched by anything, which means it can be used as 'reference' recalc value for the next undo
-   * step, when going backward (i.e. actual undo, redo can just use recalc value directly).
+   * Used by undo code. recalc_after_undo_push contains the changes between the
+   * last undo push and the current state. This is accumulated as IDs are tagged
+   * for update in the depsgraph, and only cleared on undo push.
+   *
+   * recalc_up_to_undo_push is saved to undo memory, and is the value of
+   * recalc_after_undo_push at the time of the undo push. This means it can be
+   * used to find the changes between undo states.
    */
-  int recalc_undo_accumulated;
+  int recalc_up_to_undo_push;
+  int recalc_after_undo_push;
 
   /**
    * A session-wide unique identifier for a given ID, that remain the same across potential
@@ -258,8 +263,6 @@ typedef struct ID {
    */
   unsigned int session_uuid;
 
-  char _pad[4];
-
   IDProperty *properties;
 
   /** Reference linked ID which this one overrides. */