[Bf-blender-cvs] [6c8b356203e] undo-experiments: undoexp: Fix major flaw in general logic of unchanged data detection.

Bastien Montagne noreply at git.blender.org
Fri Jan 10 11:16:39 CET 2020


Commit: 6c8b356203ed3cb330c6f6fa8354eafe46509687
Author: Bastien Montagne
Date:   Fri Jan 10 11:10:38 2020 +0100
Branches: undo-experiments
https://developer.blender.org/rB6c8b356203ed3cb330c6f6fa8354eafe46509687

undoexp: Fix major flaw in general logic of unchanged data detection.

Am not sure why it seemed to work that well so far... But using marker
saying that data did not change between n-2 and n-1 step when undoing
from n to n-1 is not really smart.

Fixed that by adding a 'is_identical_future' flag to memfile chunks,
which is set when storing the next undo step. And then using undo
direction to decide which flag to use to detect unchanged data.

This seesm to work to some extend, although for some reason very most
recent undo step seems to be evaluated when doing the first undo, not
sure why... Need to test that with actual working undo code in
undo-experiments-idnames branch anyway.

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

M	source/blender/blenkernel/BKE_blender_undo.h
M	source/blender/blenkernel/intern/blender_undo.c
M	source/blender/blenkernel/intern/blendfile.c
M	source/blender/blenloader/BLO_readfile.h
M	source/blender/blenloader/BLO_undofile.h
M	source/blender/blenloader/intern/readblenentry.c
M	source/blender/blenloader/intern/readfile.c
M	source/blender/blenloader/intern/readfile.h
M	source/blender/blenloader/intern/undofile.c
M	source/blender/editors/undo/memfile_undo.c

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

diff --git a/source/blender/blenkernel/BKE_blender_undo.h b/source/blender/blenkernel/BKE_blender_undo.h
index 7392d3947a2..ae408f2f69a 100644
--- a/source/blender/blenkernel/BKE_blender_undo.h
+++ b/source/blender/blenkernel/BKE_blender_undo.h
@@ -32,7 +32,9 @@ struct bContext;
 
 struct MemFileUndoData *BKE_memfile_undo_encode(struct Main *bmain,
                                                 struct MemFileUndoData *mfu_prev);
-bool BKE_memfile_undo_decode(struct MemFileUndoData *mfu, struct bContext *C);
+bool BKE_memfile_undo_decode(struct MemFileUndoData *mfu,
+                             const int undo_direction,
+                             struct bContext *C);
 void BKE_memfile_undo_free(struct MemFileUndoData *mfu);
 
 #ifdef __cplusplus
diff --git a/source/blender/blenkernel/intern/blender_undo.c b/source/blender/blenkernel/intern/blender_undo.c
index cd950e05415..d4bdcc0ce32 100644
--- a/source/blender/blenkernel/intern/blender_undo.c
+++ b/source/blender/blenkernel/intern/blender_undo.c
@@ -61,7 +61,7 @@
 
 #define UNDO_DISK 0
 
-bool BKE_memfile_undo_decode(MemFileUndoData *mfu, bContext *C)
+bool BKE_memfile_undo_decode(MemFileUndoData *mfu, const int undo_direction, bContext *C)
 {
   Main *bmain = CTX_data_main(C);
   char mainstr[sizeof(bmain->name)];
@@ -76,8 +76,9 @@ bool BKE_memfile_undo_decode(MemFileUndoData *mfu, bContext *C)
     success = BKE_blendfile_read(C, mfu->filename, &(const struct BlendFileReadParams){0}, NULL);
   }
   else {
-    success = BKE_blendfile_read_from_memfile(
-        C, &mfu->memfile, &(const struct BlendFileReadParams){0}, NULL);
+    struct BlendFileReadParams params = {0};
+    params.undo_direction = undo_direction > 0 ? 1 : -1;
+    success = BKE_blendfile_read_from_memfile(C, &mfu->memfile, &params, NULL);
   }
 
   /* Restore, bmain has been re-allocated. */
diff --git a/source/blender/blenkernel/intern/blendfile.c b/source/blender/blenkernel/intern/blendfile.c
index 62173393256..6c48104662c 100644
--- a/source/blender/blenkernel/intern/blendfile.c
+++ b/source/blender/blenkernel/intern/blendfile.c
@@ -473,8 +473,7 @@ bool BKE_blendfile_read_from_memfile(bContext *C,
   Main *bmain = CTX_data_main(C);
   BlendFileData *bfd;
 
-  bfd = BLO_read_from_memfile(
-      bmain, BKE_main_blendfile_path(bmain), memfile, params->skip_flags, reports);
+  bfd = BLO_read_from_memfile(bmain, BKE_main_blendfile_path(bmain), memfile, params, reports);
   if (bfd) {
     /* remove the unused screens and wm */
     while (bfd->main->wm.first) {
diff --git a/source/blender/blenloader/BLO_readfile.h b/source/blender/blenloader/BLO_readfile.h
index adf3bf00d48..5b8cfa94520 100644
--- a/source/blender/blenloader/BLO_readfile.h
+++ b/source/blender/blenloader/BLO_readfile.h
@@ -76,6 +76,9 @@ typedef struct WorkspaceConfigFileData {
 struct BlendFileReadParams {
   uint skip_flags : 2; /* eBLOReadSkip */
   uint is_startup : 1;
+
+  /** Whether we are reading the memfile for an undo (< 0) or a redo (> 0). */
+  int undo_direction : 2;
 };
 
 /* skip reading some data-block types (may want to skip screen data too). */
@@ -96,7 +99,7 @@ BlendFileData *BLO_read_from_memory(const void *mem,
 BlendFileData *BLO_read_from_memfile(struct Main *oldmain,
                                      const char *filename,
                                      struct MemFile *memfile,
-                                     eBLOReadSkip skip_flags,
+                                     const struct BlendFileReadParams *params,
                                      struct ReportList *reports);
 
 void BLO_blendfiledata_free(BlendFileData *bfd);
diff --git a/source/blender/blenloader/BLO_undofile.h b/source/blender/blenloader/BLO_undofile.h
index 0388b3f3520..5f1142cc20e 100644
--- a/source/blender/blenloader/BLO_undofile.h
+++ b/source/blender/blenloader/BLO_undofile.h
@@ -34,6 +34,10 @@ typedef struct {
   unsigned int size;
   /** When true, this chunk doesn't own the memory, it's shared with a previous #MemFileChunk */
   bool is_identical;
+  /** When true, this chunk is also identical to the one in the next step (used by undo code to
+   * detect unchanged IDs).
+   * Defined when writing the next step (i.e. last undo step has those always false). */
+  bool is_identical_future;
 } MemFileChunk;
 
 typedef struct MemFile {
diff --git a/source/blender/blenloader/intern/readblenentry.c b/source/blender/blenloader/intern/readblenentry.c
index a4b96c9e59c..ce03001c5b1 100644
--- a/source/blender/blenloader/intern/readblenentry.c
+++ b/source/blender/blenloader/intern/readblenentry.c
@@ -363,17 +363,17 @@ BlendFileData *BLO_read_from_memory(const void *mem,
 BlendFileData *BLO_read_from_memfile(Main *oldmain,
                                      const char *filename,
                                      MemFile *memfile,
-                                     eBLOReadSkip skip_flags,
+                                     const struct BlendFileReadParams *params,
                                      ReportList *reports)
 {
   BlendFileData *bfd = NULL;
   FileData *fd;
   ListBase old_mainlist;
 
-  fd = blo_filedata_from_memfile(memfile, reports);
+  fd = blo_filedata_from_memfile(memfile, params, reports);
   if (fd) {
     fd->reports = reports;
-    fd->skip_flags = skip_flags;
+    fd->skip_flags = params->skip_flags;
     BLI_strncpy(fd->relabase, filename, sizeof(fd->relabase));
 
     /* clear ob->proxy_from pointers in old main */
diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index 50ff35c862e..ff8e29ff9b5 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -1260,7 +1260,13 @@ static int fd_read_from_memfile(FileData *filedata,
       filedata->file_offset += readsize;
       seek += readsize;
       if (r_is_memchunck_identical != NULL) {
-        *r_is_memchunck_identical = chunk->is_identical;
+        /* `is_identical` of current chunk represent whether it changed compared to previous undo
+         * step. this is fine in redo case (filedata->undo_direction > 0), but not in undo case,
+         * where we need an extra flag defined when saving the next (future) step after the one we
+         * want to restore, as we are supposed to 'come from' that future undo step, and not the
+         * one before current one. */
+        *r_is_memchunck_identical = filedata->undo_direction > 0 ? chunk->is_identical :
+                                                                   chunk->is_identical_future;
       }
     } while (totread < size);
 
@@ -1502,7 +1508,9 @@ FileData *blo_filedata_from_memory(const void *mem, int memsize, ReportList *rep
   }
 }
 
-FileData *blo_filedata_from_memfile(MemFile *memfile, ReportList *reports)
+FileData *blo_filedata_from_memfile(MemFile *memfile,
+                                    const struct BlendFileReadParams *params,
+                                    ReportList *reports)
 {
   if (!memfile) {
     BKE_report(reports, RPT_WARNING, "Unable to open blend <memory>");
@@ -1511,6 +1519,7 @@ FileData *blo_filedata_from_memfile(MemFile *memfile, ReportList *reports)
   else {
     FileData *fd = filedata_new();
     fd->memfile = memfile;
+    fd->undo_direction = params->undo_direction;
 
     fd->read = fd_read_from_memfile;
     fd->flags |= FD_FLAGS_NOT_MY_BUFFER;
@@ -9435,6 +9444,9 @@ static BHead *read_libblock(FileData *fd,
        * and eval, not actual file reading. */
       bhead = read_data_into_oldnewmap(fd, id_bhead, allocname);
 
+      DEBUG_PRINTF(
+          "%s: ID %s is unchanged: %d\n", __func__, id->name, fd->are_memchunks_identical);
+
       if (fd->are_memchunks_identical && !ELEM(idcode, ID_WM, ID_SCR, ID_WS)) {
         BLI_assert(fd->memfile);
 
diff --git a/source/blender/blenloader/intern/readfile.h b/source/blender/blenloader/intern/readfile.h
index 93715d01458..952133be4ef 100644
--- a/source/blender/blenloader/intern/readfile.h
+++ b/source/blender/blenloader/intern/readfile.h
@@ -86,6 +86,9 @@ typedef struct FileData {
    * Updated by `fd_read_from_memfile()`, user is responsible to reset it to true when needed.
    * Used to detect unchanged IDs. */
   bool are_memchunks_identical;
+  /** Whether we are undoing (< 0) or redoing (> 0), used to choose which 'unchanged' flag to use
+   * to detect unchanged data from memfile. */
+  short undo_direction;
 
   /** Variables needed for reading from file. */
   gzFile gzfiledes;
@@ -143,7 +146,9 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath);
 
 FileData *blo_filedata_from_file(const char *filepath, struct ReportList *reports);
 FileData *blo_filedata_from_memory(const void *buffer, int buffersize, struct ReportList *reports);
-FileData *blo_filedata_from_memfile(struct MemFile *memfile, struct ReportList *reports);
+FileData *blo_filedata_from_memfile(struct MemFile *memfile,
+                                    const struct BlendFileReadParams *params,
+                                    struct ReportList *reports);
 
 void blo_clear_proxy_pointers_from_lib(struct Main *oldmain);
 void blo_make_image_pointer_map(FileData *fd, struct Main *oldmain);
diff --git a/source/blender/blenloader/intern/undofile.c b/source/blender/blenloader/intern/undofile.c
index 95a4771b313..03cc60c09f6 100644
--- a/source/blender/blenloader/intern/undofile.c
+++ b/source/blender/blenloader/intern/undofile.c
@@ -98,6 +98,7 @@ 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;
   BLI_addtail(&memfile->chunks, curchunk);
 
   /* we compare compchunk with buf */
@@ -107,6 +108,7 @@ void memfile_chunk_add(MemFile *memfile, const char *buf, uint size, MemFileChun
       if (memcmp(compchunk->buf, buf, size) == 0) {
         curchunk->buf = compchunk->buf;
         curchunk->is_identical = true;
+        compchunk->is_identical_future = true;
       }
     }
     *compchunk_step = compchunk->next;
diff --git a/source/blender/editors/undo/memfile_undo.c b/source/blender/editors/undo/memfile_undo.c
index a5f30409aa6..15a052c7cb8 100

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list