[Bf-blender-cvs] [2a061a5610d] undo-experiments-swap-reread-datablocks: undoexp: initial implementation of ID swapping.

Bastien Montagne noreply at git.blender.org
Tue Jan 28 21:26:10 CET 2020


Commit: 2a061a5610d45a93f31bfde41bc0fe8cb7c11680
Author: Bastien Montagne
Date:   Tue Jan 28 21:08:34 2020 +0100
Branches: undo-experiments-swap-reread-datablocks
https://developer.blender.org/rB2a061a5610d45a93f31bfde41bc0fe8cb7c11680

undoexp: initial implementation of ID swapping.

Goal is to reuse same address also for changed IDs during an undo step.
This will allow for a given ID to almost always keep its same address
along a whole editing session, reducing a lot useless updates required
by current memfile undo.

Note that this commit has a lot of things to be investigated & fixed
still, at the very least:
 * Refcounting is not really taken care of yet. This is fine for now
   since we remap everything still, but at some point plan is to not remap
   (liblink) reused unchanged IDs at all...
 * We keep the double libmap for now, getting rid of it requires further
   investigations (especially in some corner cases).
 * 'UI' IDs (WM, screen and WS) are likely troublemakers given their
   current weird specific handling. This might actually be seriously
   simplified with this new approach?
 * Since even changed IDs keep the same address, we are going to have to
   improve a lot the `id->recalc` handling - unless we accept a
   brute-force complete depsgraph update of the changed IDs.

Last point is especially interesting, as with current code, updates do
happen as expected most of the time (proper updates are missing
sometimes). This means that even undo moving of a very highly
modifier-subdivided object can be done "instantaneously" as that object
updates does not requires a geometry update.

Handling it properly will likely require to store the 'future' update
flag (as we do currently with the future unchanged status). Not sure
we'll catch all cases though, if that goes too far we may just force
full update as a first step. :/

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

M	source/blender/blenloader/intern/readfile.c

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

diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index a468c9e83fc..a6d5d55b2ed 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -9444,12 +9444,14 @@ static BHead *read_libblock(FileData *fd,
   /* read libblock */
   fd->are_memchunks_identical = true;
   id = read_struct(fd, bhead, "lib block");
+  const short idcode = id != NULL ? GS(id->name) : 0;
 
   BHead *id_bhead = bhead;
+  /* Used when undoing from memfile, we swap changed IDs into their old addresses when found. */
+  ID *id_old = NULL;
+  bool do_id_swap = false;
 
   if (id) {
-    const short idcode = GS(id->name);
-
     if (id_bhead->code != ID_LINK_PLACEHOLDER) {
       /* need a name for the mallocN, just for debugging and sane prints on leaks */
       allocname = dataname(idcode);
@@ -9472,7 +9474,7 @@ static BHead *read_libblock(FileData *fd,
 
         /* Find the 'current' existing ID we want to reuse instead of the one we would read from
          * the undo memfile. */
-        ID *id_old = BKE_main_idmap_lookup(fd->old_idmap, idcode, id->name + 2, NULL);
+        id_old = BKE_main_idmap_lookup(fd->old_idmap, idcode, id->name + 2, NULL);
         if (id_old != NULL) {
           MEM_freeN(id);
           id = id_old;
@@ -9512,20 +9514,30 @@ static BHead *read_libblock(FileData *fd,
     /* do after read_struct, for dna reconstruct */
     lb = which_libbase(main, idcode);
     if (lb) {
-      /* for ID_LINK_PLACEHOLDER check */
-      oldnewmap_insert(fd->libmap, id_bhead->old, id, id_bhead->code);
-
       /* Some re-used old IDs might also use newly read ones, so we have to check for old memory
        * addresses for those as well. */
       if (fd->memfile != NULL && (fd->skip_flags & BLO_READ_SKIP_UNDO_OLD_MAIN) == 0 &&
           id->lib == NULL) {
         BLI_assert(fd->old_idmap != NULL);
-        ID *id_old = BKE_main_idmap_lookup(fd->old_idmap, idcode, id->name + 2, NULL);
+        if (id_old == NULL) {
+          id_old = BKE_main_idmap_lookup(fd->old_idmap, idcode, id->name + 2, NULL);
+        }
         if (id_old != NULL) {
-          oldnewmap_insert(fd->libmap_undo_reused, id_old, id, id_bhead->code);
+          BLI_assert(MEM_allocN_len(id) == MEM_allocN_len(id_old));
+          /* UI IDs are always re-used from old bmain at higher-level calling code, so never swap
+           * those. Besides maybe custom properties, no other ID should have pointers to those
+           * anyway...
+           * And linked IDs are handled separately as well. */
+          do_id_swap = !ELEM(idcode, ID_WM, ID_SCR, ID_WS) &&
+                       !(id_bhead->code == ID_LINK_PLACEHOLDER);
+          oldnewmap_insert(
+              fd->libmap_undo_reused, id_old, do_id_swap ? id_old : id, id_bhead->code);
         }
       }
 
+      /* for ID_LINK_PLACEHOLDER check */
+      oldnewmap_insert(fd->libmap, id_bhead->old, do_id_swap ? id_old : id, id_bhead->code);
+
       BLI_addtail(lb, id);
     }
     else {
@@ -9537,7 +9549,7 @@ static BHead *read_libblock(FileData *fd,
   }
 
   if (r_id) {
-    *r_id = id;
+    *r_id = do_id_swap ? id_old : id;
   }
   if (!id) {
     return blo_bhead_next(fd, id_bhead);
@@ -9585,7 +9597,7 @@ static BHead *read_libblock(FileData *fd,
   /* Note: doing this after direct_link_id(), which resets that field. */
   id->tag = tag | LIB_TAG_NEED_LINK | LIB_TAG_NEW;
 
-  switch (GS(id->name)) {
+  switch (idcode) {
     case ID_WM:
       direct_link_windowmanager(fd, (wmWindowManager *)id);
       break;
@@ -9702,6 +9714,29 @@ static BHead *read_libblock(FileData *fd,
   if (wrong_id) {
     BKE_id_free(main, id);
   }
+  else if (do_id_swap) {
+    /* During memfile undo, if an ID changed and we cannot directly re-use existing one from old
+     * bmain, we do a full read of the new id from the memfile, and then fully swap its content
+     * with the old id. This allows us to keep the same pointer even for modified data, which helps
+     * reducing further detected changes by the depsgraph (since unchanged IDs remain fully
+     * unchanged, even if they are using/pointing to a changed one). */
+
+    BLI_assert((fd->skip_flags & BLO_READ_SKIP_UNDO_OLD_MAIN) == 0);
+
+    Main *old_bmain = fd->old_mainlist->first;
+    BLI_assert(old_bmain == BKE_main_idmap_main_get(fd->old_idmap));
+    BLI_assert(id_old != NULL);
+
+    ListBase *old_lb = which_libbase(old_bmain, idcode);
+    ListBase *new_lb = which_libbase(main, idcode);
+    BLI_remlink(old_lb, id_old);
+    BLI_remlink(new_lb, id);
+
+    BKE_id_full_swap(NULL, id, id_old);
+
+    BLI_addtail(new_lb, id_old);
+    BLI_addtail(old_lb, id);
+  }
 
   return (bhead);
 }



More information about the Bf-blender-cvs mailing list