[Bf-blender-cvs] [151cc02b6f8] master: Image: support storing full image buffers for each undo step

Campbell Barton noreply at git.blender.org
Tue Oct 1 16:49:49 CEST 2019


Commit: 151cc02b6f8234c94a65618ae0437403e1aba689
Author: Campbell Barton
Date:   Wed Oct 2 00:07:06 2019 +1000
Branches: master
https://developer.blender.org/rB151cc02b6f8234c94a65618ae0437403e1aba689

Image: support storing full image buffers for each undo step

Update image undo to store buffers for each step:

- Undo buffers share tiles to avoid using too much memory.
- Undo support for different sized buffers
  allowing operations such as crop or resize.
- Paint tiles have been split into separate API/storage.
- Painting speed wont be impacted significantly
  since storing the extra tiles is done after the stroke & only
  for the first undo step.

Resolves T61263, see D5939 for details.

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

M	source/blender/editors/include/ED_paint.h
M	source/blender/editors/sculpt_paint/CMakeLists.txt
M	source/blender/editors/sculpt_paint/paint_image_undo.c
M	source/blender/editors/space_image/image_ops.c
M	source/blender/imbuf/IMB_imbuf.h
M	source/blender/imbuf/intern/rectop.c

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

diff --git a/source/blender/editors/include/ED_paint.h b/source/blender/editors/include/ED_paint.h
index 88cc8a85897..81252ad2516 100644
--- a/source/blender/editors/include/ED_paint.h
+++ b/source/blender/editors/include/ED_paint.h
@@ -42,6 +42,10 @@ void ED_imapaint_bucket_fill(struct bContext *C, float color[3], struct wmOperat
 
 /* paint_image_undo.c */
 void ED_image_undo_push_begin(const char *name, int paint_mode);
+void ED_image_undo_push_begin_with_image(const char *name,
+                                         struct Image *image,
+                                         struct ImBuf *ibuf);
+
 void ED_image_undo_push_end(void);
 void ED_image_undo_restore(struct UndoStep *us);
 
diff --git a/source/blender/editors/sculpt_paint/CMakeLists.txt b/source/blender/editors/sculpt_paint/CMakeLists.txt
index 752a5c36010..c8057686c5e 100644
--- a/source/blender/editors/sculpt_paint/CMakeLists.txt
+++ b/source/blender/editors/sculpt_paint/CMakeLists.txt
@@ -31,6 +31,7 @@ set(INC
   ../../render/extern/include
   ../../windowmanager
   ../../../../intern/atomic
+  ../../../../intern/clog
   ../../../../intern/glew-mx
   ../../../../intern/guardedalloc
 )
diff --git a/source/blender/editors/sculpt_paint/paint_image_undo.c b/source/blender/editors/sculpt_paint/paint_image_undo.c
index 6b095188ba4..4cc12296e37 100644
--- a/source/blender/editors/sculpt_paint/paint_image_undo.c
+++ b/source/blender/editors/sculpt_paint/paint_image_undo.c
@@ -15,8 +15,23 @@
 
 /** \file
  * \ingroup edsculpt
+ *
+ * Overview
+ * ========
+ *
+ * - Each undo step is a #ImageUndoStep
+ * - Each #ImageUndoStep stores a list of #UndoImageHandle
+ *   - Each #UndoImageHandle stores a list of #UndoImageBuf
+ *     (this is the undo systems equivalent of an #ImBuf).
+ *     - Each #UndoImageBuf stores an array of #UndoImageTile
+ *       The tiles are shared between #UndoImageBuf's to avoid duplication.
+ *
+ * When the undo system manages an image, there will always be a full copy (as a #UndoImageBuf)
+ * each new undo step only stores modified tiles.
  */
 
+#include "CLG_log.h"
+
 #include "MEM_guardedalloc.h"
 
 #include "BLI_math.h"
@@ -51,335 +66,575 @@
 
 #include "paint_intern.h"
 
+static CLG_LogRef LOG = {"ed.image.undo"};
+
 /* -------------------------------------------------------------------- */
-/** \name Undo Conversion
+/** \name Thread Locking
  * \{ */
 
-typedef struct UndoImageTile {
-  struct UndoImageTile *next, *prev;
-
-  char ibufname[IMB_FILENAME_SIZE];
-
-  union {
-    float *fp;
-    unsigned int *uint;
-    void *pt;
-  } rect;
-
-  unsigned short *mask;
-
-  int x, y;
-
-  /* TODO(campbell): avoid storing the ID per tile,
-   * adds unnecessary overhead restoring undo steps when most tiles share the same image. */
-  UndoRefID_Image image_ref;
-
-  short source;
-  bool use_float;
-  char gen_type;
-  bool valid;
-
-  size_t undo_size;
-} UndoImageTile;
-
 /* this is a static resource for non-globality,
  * Maybe it should be exposed as part of the
  * paint operation, but for now just give a public interface */
-static SpinLock undolock;
+static SpinLock paint_tiles_lock;
 
 void image_undo_init_locks(void)
 {
-  BLI_spin_init(&undolock);
+  BLI_spin_init(&paint_tiles_lock);
 }
 
 void image_undo_end_locks(void)
 {
-  BLI_spin_end(&undolock);
+  BLI_spin_end(&paint_tiles_lock);
 }
 
-/* UNDO */
-typedef enum {
-  COPY = 0,
-  RESTORE = 1,
-  RESTORE_COPY = 2,
-} CopyMode;
+/** \} */
+
+/* -------------------------------------------------------------------- */
+/** \name Paint Tiles
+ *
+ * Created on demand while painting,
+ * use to access the previous state for some paint operations.
+ *
+ * These buffers are also used for undo when available.
+ *
+ * \{ */
 
-static void undo_copy_tile(UndoImageTile *tile, ImBuf *tmpibuf, ImBuf *ibuf, CopyMode mode)
+static ImBuf *imbuf_alloc_temp_tile(void)
 {
-  if (mode == COPY) {
-    /* copy or swap contents of tile->rect and region in ibuf->rect */
-    IMB_rectcpy(tmpibuf,
-                ibuf,
-                0,
-                0,
-                tile->x * IMAPAINT_TILE_SIZE,
-                tile->y * IMAPAINT_TILE_SIZE,
-                IMAPAINT_TILE_SIZE,
-                IMAPAINT_TILE_SIZE);
+  return IMB_allocImBuf(IMAPAINT_TILE_SIZE, IMAPAINT_TILE_SIZE, 32, IB_rectfloat | IB_rect);
+}
 
-    if (ibuf->rect_float) {
-      SWAP(float *, tmpibuf->rect_float, tile->rect.fp);
-    }
-    else {
-      SWAP(unsigned int *, tmpibuf->rect, tile->rect.uint);
-    }
+typedef struct PaintTile {
+  struct PaintTile *next, *prev;
+  Image *image;
+  ImBuf *ibuf;
+  union {
+    float *fp;
+    uint *uint;
+    void *pt;
+  } rect;
+  ushort *mask;
+  bool valid;
+  bool use_float;
+  int x, y;
+} PaintTile;
+
+static void ptile_free(PaintTile *ptile)
+{
+  if (ptile->rect.pt) {
+    MEM_freeN(ptile->rect.pt);
   }
-  else {
-    if (mode == RESTORE_COPY) {
-      IMB_rectcpy(tmpibuf,
-                  ibuf,
-                  0,
-                  0,
-                  tile->x * IMAPAINT_TILE_SIZE,
-                  tile->y * IMAPAINT_TILE_SIZE,
-                  IMAPAINT_TILE_SIZE,
-                  IMAPAINT_TILE_SIZE);
-    }
-    /* swap to the tmpbuf for easy copying */
-    if (ibuf->rect_float) {
-      SWAP(float *, tmpibuf->rect_float, tile->rect.fp);
-    }
-    else {
-      SWAP(unsigned int *, tmpibuf->rect, tile->rect.uint);
-    }
+  MEM_freeN(ptile);
+}
 
-    IMB_rectcpy(ibuf,
-                tmpibuf,
-                tile->x * IMAPAINT_TILE_SIZE,
-                tile->y * IMAPAINT_TILE_SIZE,
-                0,
-                0,
-                IMAPAINT_TILE_SIZE,
-                IMAPAINT_TILE_SIZE);
+static void ptile_free_list(ListBase *paint_tiles)
+{
+  for (PaintTile *ptile = paint_tiles->first, *ptile_next; ptile; ptile = ptile_next) {
+    ptile_next = ptile->next;
+    ptile_free(ptile);
+  }
+  BLI_listbase_clear(paint_tiles);
+}
 
-    if (mode == RESTORE) {
-      if (ibuf->rect_float) {
-        SWAP(float *, tmpibuf->rect_float, tile->rect.fp);
-      }
-      else {
-        SWAP(unsigned int *, tmpibuf->rect, tile->rect.uint);
-      }
-    }
+static void ptile_invalidate_list(ListBase *paint_tiles)
+{
+  for (PaintTile *ptile = paint_tiles->first; ptile; ptile = ptile->next) {
+    ptile->valid = false;
   }
 }
 
-void *image_undo_find_tile(ListBase *undo_tiles,
-                           Image *ima,
+void *image_undo_find_tile(ListBase *paint_tiles,
+                           Image *image,
                            ImBuf *ibuf,
                            int x_tile,
                            int y_tile,
-                           unsigned short **mask,
+                           ushort **r_mask,
                            bool validate)
 {
-  UndoImageTile *tile;
-  const bool use_float = (ibuf->rect_float != NULL);
-
-  for (tile = undo_tiles->first; tile; tile = tile->next) {
-    if (tile->x == x_tile && tile->y == y_tile && ima->gen_type == tile->gen_type &&
-        ima->source == tile->source) {
-      if (tile->use_float == use_float) {
-        if (STREQ(tile->ibufname, ibuf->name)) {
-          if (mask) {
-            /* allocate mask if requested */
-            if (!tile->mask) {
-              tile->mask = MEM_callocN(sizeof(unsigned short) * IMAPAINT_TILE_SIZE *
-                                           IMAPAINT_TILE_SIZE,
-                                       "UndoImageTile.mask");
-            }
-
-            *mask = tile->mask;
-          }
-          if (validate) {
-            tile->valid = true;
+  for (PaintTile *ptile = paint_tiles->first; ptile; ptile = ptile->next) {
+    if (ptile->x == x_tile && ptile->y == y_tile) {
+      if (ptile->image == image && ptile->ibuf == ibuf) {
+        if (r_mask) {
+          /* allocate mask if requested. */
+          if (!ptile->mask) {
+            ptile->mask = MEM_callocN(sizeof(ushort) * SQUARE(IMAPAINT_TILE_SIZE),
+                                      "UndoImageTile.mask");
           }
-          return tile->rect.pt;
+          *r_mask = ptile->mask;
         }
+        if (validate) {
+          ptile->valid = true;
+        }
+        return ptile->rect.pt;
       }
     }
   }
-
   return NULL;
 }
 
-void *image_undo_push_tile(ListBase *undo_tiles,
-                           Image *ima,
+void image_undo_remove_masks(void)
+{
+  ListBase *paint_tiles = ED_image_undo_get_tiles();
+  for (PaintTile *ptile = paint_tiles->first; ptile; ptile = ptile->next) {
+    MEM_SAFE_FREE(ptile->mask);
+  }
+}
+
+void *image_undo_push_tile(ListBase *paint_tiles,
+                           Image *image,
                            ImBuf *ibuf,
                            ImBuf **tmpibuf,
                            int x_tile,
                            int y_tile,
-                           unsigned short **mask,
+                           ushort **r_mask,
                            bool **valid,
                            bool proj,
                            bool find_prev)
 {
-  UndoImageTile *tile;
-  int allocsize;
-  const bool use_float = (ibuf->rect_float != NULL);
-  void *data;
+  const bool has_float = (ibuf->rect_float != NULL);
 
   /* check if tile is already pushed */
 
   /* in projective painting we keep accounting of tiles, so if we need one pushed, just push! */
   if (find_prev) {
-    data = image_undo_find_tile(undo_tiles, ima, ibuf, x_tile, y_tile, mask, true);
+    void *data = image_undo_find_tile(paint_tiles, image, ibuf, x_tile, y_tile, r_mask, true);
     if (data) {
       return data;
     }
   }
 
   if (*tmpibuf == NULL) {
-    *tmpibuf = IMB_allocImBuf(IMAPAINT_TILE_SIZE, IMAPAINT_TILE_SIZE, 32, IB_rectfloat | IB_rect);
+    *tmpibuf = imbuf_alloc_temp_tile();
   }
 
-  tile = MEM_callocN(sizeof(UndoImageTile), "UndoImageTile");
-  tile->x = x_tile;
-  tile->y = y_tile;
+  PaintTile *ptile = MEM_callocN(sizeof(PaintTile), "PaintTile");
+
+  ptile->image = image;
+  ptile->ibuf = ibuf;
+
+  ptile->x = x_tile;
+  ptile->y = y_tile;
 
   /* add mask explicitly here */
-  if (mask) {
-    *mask = tile->mask = MEM_callocN(
-        sizeof(unsigned short) * IMAPAINT_TILE_SIZE * IMAPAINT_TILE_SIZE, "UndoImageTile.mask");
+  if (r_mask) {
+    *r_ma

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list