[Bf-blender-cvs] [cc7f8319d8a] undo-experiments-idnames: undoexp: much better handling of idname-based re-using old IDs.

Bastien Montagne noreply at git.blender.org
Mon Jan 6 10:43:34 CET 2020


Commit: cc7f8319d8aedee782ca9f51f9cf20bc52c02478
Author: Bastien Montagne
Date:   Mon Jan 6 10:38:32 2020 +0100
Branches: undo-experiments-idnames
https://developer.blender.org/rBcc7f8319d8aedee782ca9f51f9cf20bc52c02478

undoexp: much better handling of idname-based re-using old IDs.

This commit adds obvious usage of a proper mapping to find local IDs
in old Main based on their names.

It also adds handling of potential pointer collisions between old Main
realm and read .blend memfile realm. That last part is theoritical
currently though, testing it actually works won't be easy (pointer
collisions will happen, but they are not very likely to happen...).

We are still missing some way to prevent all that new code to be ran
over an undo step that contains ID renaming (ID creation/deletion should
be safe, but idname-based search cannot work whith renaming).

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

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

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

diff --git a/source/blender/blenloader/intern/readblenentry.c b/source/blender/blenloader/intern/readblenentry.c
index a4b96c9e59c..0c010a59477 100644
--- a/source/blender/blenloader/intern/readblenentry.c
+++ b/source/blender/blenloader/intern/readblenentry.c
@@ -384,6 +384,14 @@ BlendFileData *BLO_read_from_memfile(Main *oldmain,
     /* add the library pointers in oldmap lookup */
     blo_add_library_pointer_map(&old_mainlist, fd);
 
+    /* Build idmap of old main (we only care about local data here, so we can do that after
+     * split_main() call. */
+    blo_make_idmap_from_main(fd, old_mainlist.first);
+
+    /* Create sibling mapping of libmap (i.e. old ID pointer values to new valid IDs), but for the
+     * addresses from old main. */
+    blo_make_undo_reused_libmap(fd);
+
     /* makes lookup of existing images in old main */
     blo_make_image_pointer_map(fd, oldmain);
 
diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index f5a72f0924b..e85e4d32677 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -1588,6 +1588,12 @@ void blo_filedata_free(FileData *fd)
     if (fd->libmap && !(fd->flags & FD_FLAGS_NOT_MY_LIBMAP)) {
       oldnewmap_free(fd->libmap);
     }
+    if (fd->libmap_undo_reused != NULL) {
+      oldnewmap_free(fd->libmap_undo_reused);
+    }
+    if (fd->old_idmap != NULL) {
+      BKE_main_idmap_destroy(fd->old_idmap);
+    }
     if (fd->bheadmap) {
       MEM_freeN(fd->bheadmap);
     }
@@ -2232,6 +2238,23 @@ void blo_add_library_pointer_map(ListBase *old_mainlist, FileData *fd)
   fd->old_mainlist = old_mainlist;
 }
 
+/* Build idmap of old main (we only care about local data here, so we can do that after
+ * split_main() call. */
+void blo_make_idmap_from_main(FileData *fd, Main *bmain)
+{
+  if (fd->old_idmap != NULL) {
+    BKE_main_idmap_destroy(fd->old_idmap);
+  }
+  fd->old_idmap = BKE_main_idmap_create(bmain, false, NULL);
+}
+
+/* Create sibling mapping of libmap (i.e. old ID pointer values to new valid IDs), but for the
+ * addresses from old main. */
+void blo_make_undo_reused_libmap(FileData *fd)
+{
+  fd->libmap_undo_reused = oldnewmap_new();
+}
+
 /** \} */
 
 /* -------------------------------------------------------------------- */
@@ -9447,15 +9470,15 @@ static BHead *read_libblock(FileData *fd,
                    fd->are_memchunks_identical);
 
       if (fd->are_memchunks_identical && !ELEM(idcode, ID_WM, ID_SCR, ID_WS)) {
-        BLI_assert(fd->memfile);
+        BLI_assert(fd->memfile != NULL);
+        BLI_assert(fd->old_idmap != NULL);
+        /* This code should only ever be reached for local data-blocks. */
+        BLI_assert(main->curlib == NULL);
 
         /* Find the 'current' existing ID we want to reuse instead of the one we would read from
          * the undo memfile. */
-        Main *old_bmain = fd->old_mainlist->first;
-        ListBase *old_lb = which_libbase(old_bmain, idcode);
-        BLI_assert(old_lb != NULL);
-        ID *id_old = NULL;
-        if ((id_old = BLI_findstring(old_lb, id->name, offsetof(ID, name))) != NULL) {
+        ID *id_old = BKE_main_idmap_lookup(fd->old_idmap, idcode, id->name + 2, NULL);
+        if (id_old != NULL) {
           MEM_freeN(id);
           id = id_old;
 
@@ -9470,8 +9493,11 @@ static BHead *read_libblock(FileData *fd,
           id->orig_id = NULL;
 
           oldnewmap_insert(fd->libmap, id_bhead->old, id, id_bhead->code);
-          oldnewmap_insert(fd->libmap, id, id, id_bhead->code);
+          oldnewmap_insert(fd->libmap_undo_reused, id, id, id_bhead->code);
 
+          Main *old_bmain = fd->old_mainlist->first;
+          BLI_assert(old_bmain == BKE_main_idmap_main_get(fd->old_idmap));
+          ListBase *old_lb = which_libbase(old_bmain, idcode);
           ListBase *new_lb = which_libbase(main, idcode);
           BLI_remlink(old_lb, id);
           BLI_addtail(new_lb, id);
@@ -9494,12 +9520,12 @@ static BHead *read_libblock(FileData *fd,
       /* for ID_LINK_PLACEHOLDER check */
       oldnewmap_insert(fd->libmap, id_bhead->old, id, id_bhead->code);
 
-      if (fd->old_mainlist != NULL) {
-        Main *old_bmain = fd->old_mainlist->first;
-        ListBase *old_lb = which_libbase(old_bmain, idcode);
-        ID *id_old;
-        if ((id_old = BLI_findstring(old_lb, id->name, offsetof(ID, name))) != NULL) {
-          oldnewmap_insert(fd->libmap, id_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->old_idmap != NULL && id->lib == NULL) {
+        ID *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);
         }
       }
 
@@ -10033,6 +10059,78 @@ static BHead *read_userdef(BlendFileData *bfd, FileData *fd, BHead *bhead)
 /** \name Read File (Internal)
  * \{ */
 
+static int blo_undo_merge_remap_colliding_pointers_cb(void *user_data,
+                                                      ID *UNUSED(id_self),
+                                                      ID **id_pointer,
+                                                      int UNUSED(cb_flag))
+{
+  void **old_new_oldpointers = user_data;
+  if (*id_pointer == old_new_oldpointers[0]) {
+    *id_pointer = old_new_oldpointers[1];
+  }
+
+  return IDWALK_RET_NOP;
+}
+
+static void blo_undo_merge_and_fix_collisions_in_libmaps(FileData *fd)
+{
+  for (int i = fd->libmap->nentries; i-- > 0;) {
+    OldNew *entry = &fd->libmap->entries[i];
+    const void *oldp = entry->oldp;
+
+    OldNew *reused_entry = oldnewmap_lookup_entry(fd->libmap_undo_reused, oldp);
+    /* If both entries are pointing to same new ID of same type (stored in `nr`), there is no
+     * collision. */
+    if (reused_entry != NULL && reused_entry->newp != entry->newp &&
+        reused_entry->nr != entry->nr) {
+      const void *orig_oldp = oldp;
+      /* We have a pointer collision, find a new free oldp value.
+       * Note that we can only check libmap_undo_reused here as well, in the (rather unlikely) case
+       * we'd reach another used oldp value in a libmap item not yet processed, it will simply be
+       * detected as used/colliding too when its turn comes. */
+      for (oldp = (const char *)oldp + 1;
+           oldnewmap_lookup_entry(fd->libmap_undo_reused, oldp) != NULL;
+           oldp = (const char *)oldp + 1)
+        ;
+
+      DEBUG_PRINTF(
+          "%s: found same old pointer value used by both re-used IDs and newly read-from-memfile "
+          "IDs, remapped the laters from %p to %p\n",
+          __func__,
+          orig_oldp,
+          oldp);
+
+      /* Now we need to remap all orig_oldp pointers in local main to the new oldp value. */
+      Main *bmain = fd->mainlist->first;
+      ID *id;
+      const void *old_new_oldpointers[2] = {orig_oldp, oldp};
+      FOREACH_MAIN_ID_BEGIN (bmain, id) {
+        if (id->tag & LIB_TAG_UNDO_OLD_ID_REUSED) {
+          continue;
+        }
+        BKE_library_foreach_ID_link(bmain,
+                                    id,
+                                    blo_undo_merge_remap_colliding_pointers_cb,
+                                    old_new_oldpointers,
+                                    IDWALK_NOP);
+      }
+      FOREACH_MAIN_ID_END;
+    }
+
+    /* Inserting into libmap_undo_reused, which will be our final, merged libmap.
+     * We are doing this in that direction (and not from libmap_undo_reused to libmap) because this
+     * helps us reducing the changes to reused IDs (the fewer of their pointers we have to remap to
+     * a new address, the less likely they are to be detected as 'changed' in later undo/redo
+     * steps). */
+    oldnewmap_insert(fd->libmap_undo_reused, oldp, entry->newp, entry->nr);
+  }
+
+  /* Finally, we put our final, merged and collision-free libmap at its rightful place. */
+  oldnewmap_free(fd->libmap);
+  fd->libmap = fd->libmap_undo_reused;
+  fd->libmap_undo_reused = NULL;
+}
+
 BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
 {
   BHead *bhead = blo_bhead_first(fd);
@@ -10141,6 +10239,18 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
 
     blo_join_main(&mainlist);
 
+    if (fd->memfile != NULL) {
+      /* In case of undo, we have to handle possible 'pointer collisions' between newly read
+       * data-blocks and those re-used from the old bmain. */
+      BLI_assert(fd->libmap_undo_reused != NULL);
+
+      blo_undo_merge_and_fix_collisions_in_libmaps(fd);
+    }
+    else {
+      BLI_assert(fd->libmap_undo_reused == NULL);
+      BLI_assert(fd->old_idmap == NULL);
+    }
+
     lib_link_all(fd, bfd->main);
 
     /* Skip in undo case. */
diff --git a/source/blender/blenloader/intern/readfile.h b/source/blender/blenloader/intern/readfile.h
index 93715d01458..ee69e3cd0f1 100644
--- a/source/blender/blenloader/intern/readfile.h
+++ b/source/blender/blenloader/intern/readfile.h
@@ -30,6 +30,7 @@
 #include "DNA_space_types.h"
 #include "DNA_windowmanager_types.h" /* for ReportType */
 
+struct IDNameLib_Map;
 struct Key;
 struct MemFile;
 struct Object;
@@ -38,6 +39,8 @@ struct PartEff;
 struct ReportList;
 struct View3D;
 
+typedef struct IDNameLib_Map IDNameLib_Map;
+
 enum eFileDataFlag {
   FD_FLAGS_SWITCH_ENDIAN = 1 << 0,
   FD_FLAGS_FILE_POINTSIZE_IS_4 = 1 << 1,
@@ -113,6 +116,7 @@ typedef struct FileData {
   struct OldNewMap *datamap;
   struct OldNewMap *globmap;
   struct OldNewMap *libmap;
+  struct OldNewMap *libmap_undo_reused; /* Used for undo. */
   struct OldNewMap *imamap;
   struct OldNewMap *movieclipmap;
   struct OldNewMap *scenemap;
@@ -128,6 +132,7 @@ typedef struct FileData {
   ListBase *mainlist;
   /** Used for undo. */
   ListBase *old_mainlist;
+  IDNameLib_Map *old_idmap;
 
   struct ReportList *reports;
 } FileData;
@@ -157,6 +162,8 @@ void blo_end_sound_pointer_map(FileData *fd, struct Main *oldmain);
 void blo_make_packed_pointer_map(FileData *fd, struct Main *oldmain);
 void blo_end_packed_pointer_map(FileD

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list