Fix T75542: toggling modifier visibility not working correct with undo speedup
authorBrecht Van Lommel <brecht>
Mon, 13 Apr 2020 15:38:34 +0000 (17:38 +0200)
committerBrecht Van Lommel <brecht@blender.org>
Tue, 14 Apr 2020 09:07:56 +0000 (11:07 +0200)
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
source/blender/blenloader/intern/writefile.c
source/blender/depsgraph/intern/depsgraph_tag.cc
source/blender/editors/undo/memfile_undo.c
source/blender/makesdna/DNA_ID.h

index 799330a07bfda49c286b93e5a3e714fdd0e72c2b..168b4e01e8d6477d89d920b07c4996e238c08004 100644 (file)
@@ -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
index fc6ec80e09a56c058f0a4c11f5376775464bdd88..ee5e6f4610a1ff2f66f4f9eeaa8992b34fae554f 100644 (file)
@@ -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;
         }
       }
 
index 627a93b586909428bd9d3a6109f85b6fd528bbc4..1c56808ea82dd3433e06439a4fedbbd1f66e685e 100644 (file)
@@ -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(
index ebd5b2272b1efc55b70e299a90ab927519192aaf..9954cf8515778b18143220f3f058b27442d2c7c7 100644 (file)
@@ -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;
         }
       }
     }
index 1e894d44f874cbcb1e8d197597e96f310d3b368e..dd3964dfc154ead739546c4368c4dde1ef167f49 100644 (file)
@@ -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. */