Images: changes to avoid losing new images that have not been saved

The basic idea is that such new images will be packed into the .blend file
when saving all modified images from the quit dialog. To make this workflow
nicer a number of changes were made to how this type of packed image is
handled.

* "Save Modified Images" now packs generated images into the .blend file.
* "Save As" for packed images now automatically unpacks the image, so that
  it's easy to save automatically packed images. "Save Copy" keeps it packed.
* "Save" for packed images now re-saves the image into the .blend file, so
  that it's effectively the equivalent of "Save" for non-packed images.
* Empty image filepaths are no longer remapped when saving the .blend file.
  Previously it would become e.g. "//../../" which makes no sense for generated
  images with no filepath yet.
* Hide unpack button and filepath for such packed images with no filepath.
  Unpacking does not work in a predictable way without a filepath, better
  to just "Save As".

5 files changed, 99 insertions, 51 deletions

diff --git a/source/blender/blenkernel/BKE_image.h b/source/blender/blenkernel/BKE_image.h
index 9e5a761d02d..b9f2123b2bd 100644
--- a/source/blender/blenkernel/BKE_image.h
+++ b/source/blender/blenkernel/BKE_image.h
@@ -325,6 +325,7 @@ void BKE_image_mark_dirty(struct Image *image, struct ImBuf *ibuf);
 int BKE_image_sequence_guess_offset(struct Image *image);
 bool BKE_image_has_anim(struct Image *image);
 bool BKE_image_has_packedfile(struct Image *image);
+bool BKE_image_has_filepath(struct Image *ima);
 bool BKE_image_is_animated(struct Image *image);
 void BKE_image_file_format_set(struct Image *image,
                                int ftype,
diff --git a/source/blender/blenkernel/intern/bpath.c b/source/blender/blenkernel/intern/bpath.c
index 6f0c1891b05..fa7af53df2d 100644
--- a/source/blender/blenkernel/intern/bpath.c
+++ b/source/blender/blenkernel/intern/bpath.c
@@ -450,7 +450,10 @@ void BKE_bpath_traverse_id(
       Image *ima;
       ima = (Image *)id;
       if (BKE_image_has_packedfile(ima) == false || (flag & BKE_BPATH_TRAVERSE_SKIP_PACKED) == 0) {
-        if (ELEM(ima->source, IMA_SRC_FILE, IMA_SRC_MOVIE, IMA_SRC_SEQUENCE)) {
+        /* Skip empty file paths, these are typically from generated images and
+         * don't make sense to add directories to until the image has been saved
+         * once to give it a meaningful value. */
+        if (ELEM(ima->source, IMA_SRC_FILE, IMA_SRC_MOVIE, IMA_SRC_SEQUENCE) && ima->name[0]) {
           if (rewrite_path_fixed(ima->name, visit_cb, absbase, bpath_user_data)) {
             if (flag & BKE_BPATH_TRAVERSE_RELOAD_EDITED) {
               if (!BKE_image_has_packedfile(ima) &&
diff --git a/source/blender/blenkernel/intern/image.c b/source/blender/blenkernel/intern/image.c
index 18c42da0bd4..d59ead25396 100644
--- a/source/blender/blenkernel/intern/image.c
+++ b/source/blender/blenkernel/intern/image.c
@@ -470,7 +470,7 @@ bool BKE_image_scale(Image *image, int width, int height)
 
   if (ibuf) {
     IMB_scaleImBuf(ibuf, width, height);
-    ibuf->userflags |= IB_BITMAPDIRTY;
+    BKE_image_mark_dirty(image, ibuf);
   }
 
   BKE_image_release_ibuf(image, ibuf, lock);
@@ -646,7 +646,6 @@ static ImBuf *add_ibuf_size(unsigned int width,
   }
 
   STRNCPY(ibuf->name, name);
-  ibuf->userflags |= IB_BITMAPDIRTY;
 
   switch (gen_type) {
     case IMA_GENTYPE_GRID:
@@ -5081,6 +5080,13 @@ bool BKE_image_has_packedfile(Image *ima)
   return (BLI_listbase_is_empty(&ima->packedfiles) == false);
 }
 
+bool BKE_image_has_filepath(Image *ima)
+{
+  /* This could be improved to detect cases like //../../, currently path
+   * remapping empty file paths empty. */
+  return ima->name[0] != '\0';
+}
+
 /* Checks the image buffer changes with time (not keyframed values). */
 bool BKE_image_is_animated(Image *image)
 {
diff --git a/source/blender/editors/space_image/image_buttons.c b/source/blender/editors/space_image/image_buttons.c
index 59896391cca..68af854e367 100644
--- a/source/blender/editors/space_image/image_buttons.c
+++ b/source/blender/editors/space_image/image_buttons.c
@@ -928,11 +928,16 @@ void uiTemplateImage(uiLayout *layout,
       layout = uiLayoutColumn(layout, false);
       uiLayoutSetEnabled(layout, !is_dirty);
 
+      /* Image source */
       uiItemR(layout, &imaptr, "source", 0, NULL, ICON_NONE);
 
-      if (ima->source != IMA_SRC_GENERATED) {
+      /* Filepath */
+      const bool is_packed = BKE_image_has_packedfile(ima);
+      const bool no_filepath = is_packed && !BKE_image_has_filepath(ima);
+
+      if ((ima->source != IMA_SRC_GENERATED) && !no_filepath) {
         row = uiLayoutRow(layout, true);
-        if (BKE_image_has_packedfile(ima)) {
+        if (is_packed) {
           uiItemO(row, "", ICON_PACKAGE, "image.unpack");
         }
         else {
@@ -940,12 +945,12 @@ void uiTemplateImage(uiLayout *layout,
         }
 
         row = uiLayoutRow(row, true);
-        uiLayoutSetEnabled(row, BKE_image_has_packedfile(ima) == false);
+        uiLayoutSetEnabled(row, is_packed == false);
         uiItemR(row, &imaptr, "filepath", 0, "", ICON_NONE);
         uiItemO(row, "", ICON_FILE_REFRESH, "image.reload");
       }
 
-      /* multilayer? */
+      /* Image layers and Info */
       if (ima->type == IMA_TYPE_MULTILAYER && ima->rr) {
         const float dpi_fac = UI_DPI_FAC;
         uiblock_layer_pass_buttons(layout, ima, ima->rr, iuser, 230 * dpi_fac, NULL);
diff --git a/source/blender/editors/space_image/image_ops.c b/source/blender/editors/space_image/image_ops.c
index 03e1d80e267..07c2d216a93 100644
--- a/source/blender/editors/space_image/image_ops.c
+++ b/source/blender/editors/space_image/image_ops.c
@@ -244,42 +244,6 @@ static bool imbuf_format_writeable(const ImBuf *ibuf)
   return (BKE_image_imtype_to_ftype(im_format.imtype, &options_dummy) == ibuf->ftype);
 }
 
-static bool space_image_file_exists_poll(bContext *C)
-{
-  if (image_buffer_exists_from_context(C) == false) {
-    return false;
-  }
-
-  Main *bmain = CTX_data_main(C);
-  Image *ima = image_from_context(C);
-  ImageUser *iuser = image_user_from_context(C);
-  void *lock;
-  ImBuf *ibuf = BKE_image_acquire_ibuf(ima, iuser, &lock);
-  bool ret = false;
-
-  if (ibuf) {
-    char name[FILE_MAX];
-    BLI_strncpy(name, ibuf->name, FILE_MAX);
-    BLI_path_abs(name, BKE_main_blendfile_path(bmain));
-
-    if (BLI_exists(name) == false) {
-      CTX_wm_operator_poll_msg_set(C, "image file not found");
-    }
-    else if (!BLI_file_is_writable(name)) {
-      CTX_wm_operator_poll_msg_set(C, "image path can't be written to");
-    }
-    else if (!imbuf_format_writeable(ibuf)) {
-      CTX_wm_operator_poll_msg_set(C, "image format is read-only");
-    }
-    else {
-      ret = true;
-    }
-  }
-
-  BKE_image_release_ibuf(ima, ibuf, lock);
-  return ret;
-}
-
 bool space_image_main_region_poll(bContext *C)
 {
   SpaceImage *sima = CTX_wm_space_image(C);
@@ -1882,7 +1846,12 @@ static int image_save_as_exec(bContext *C, wmOperator *op)
 
   save_image_op(C, op, &opts);
 
+  if (opts.save_copy == false) {
+    BKE_image_free_packedfiles(image);
+  }
+
   image_save_as_free(op);
+
   return OPERATOR_FINISHED;
 }
 
@@ -2035,6 +2004,61 @@ void IMAGE_OT_save_as(wmOperatorType *ot)
 
 /******************** save image operator ********************/
 
+static bool image_file_path_saveable(bContext *C, Image *ima, ImageUser *iuser)
+{
+  /* Can always repack images. */
+  if (BKE_image_has_packedfile(ima)) {
+    return true;
+  }
+
+  /* Test for valid filepath. */
+  void *lock;
+  ImBuf *ibuf = BKE_image_acquire_ibuf(ima, iuser, &lock);
+  bool ret = false;
+
+  if (ibuf) {
+    Main *bmain = CTX_data_main(C);
+    char name[FILE_MAX];
+    BLI_strncpy(name, ibuf->name, FILE_MAX);
+    BLI_path_abs(name, BKE_main_blendfile_path(bmain));
+
+    if (BLI_exists(name) == false) {
+      CTX_wm_operator_poll_msg_set(C, "image file not found");
+    }
+    else if (!BLI_file_is_writable(name)) {
+      CTX_wm_operator_poll_msg_set(C, "image path can't be written to");
+    }
+    else if (!imbuf_format_writeable(ibuf)) {
+      CTX_wm_operator_poll_msg_set(C, "image format is read-only");
+    }
+    else {
+      ret = true;
+    }
+  }
+
+  BKE_image_release_ibuf(ima, ibuf, lock);
+  return ret;
+}
+
+static bool image_save_poll(bContext *C)
+{
+  /* Can't save if there are no pixels. */
+  if (image_buffer_exists_from_context(C) == false) {
+    return false;
+  }
+
+  Image *ima = image_from_context(C);
+  ImageUser *iuser = image_user_from_context(C);
+
+  /* Images without a filepath will go to save as. */
+  if (!BKE_image_has_filepath(ima)) {
+    return true;
+  }
+
+  /* Check if there is a valid file path and image format we can write. */
+  return image_file_path_saveable(C, ima, iuser);
+}
+
 static int image_save_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
@@ -2043,6 +2067,12 @@ static int image_save_exec(bContext *C, wmOperator *op)
   Scene *scene = CTX_data_scene(C);
   ImageSaveOptions opts;
 
+  if (BKE_image_has_packedfile(image)) {
+    /* Save packed files to memory. */
+    BKE_image_memorypack(image);
+    return OPERATOR_FINISHED;
+  }
+
   BKE_image_save_options_init(&opts, bmain, scene);
   if (image_save_options_init(bmain, &opts, image, iuser, false, false) == 0) {
     return OPERATOR_CANCELLED;
@@ -2066,13 +2096,15 @@ static int image_save_exec(bContext *C, wmOperator *op)
 
 static int image_save_invoke(bContext *C, wmOperator *op, const wmEvent *UNUSED(event))
 {
-  if (space_image_file_exists_poll(C)) {
-    return image_save_exec(C, op);
-  }
-  else {
+  Image *ima = image_from_context(C);
+
+  if (!BKE_image_has_packedfile(ima) && !BKE_image_has_filepath(ima)) {
     WM_operator_name_call(C, "IMAGE_OT_save_as", WM_OP_INVOKE_DEFAULT, NULL);
     return OPERATOR_CANCELLED;
   }
+  else {
+    return image_save_exec(C, op);
+  }
 }
 
 void IMAGE_OT_save(wmOperatorType *ot)
@@ -2085,7 +2117,7 @@ void IMAGE_OT_save(wmOperatorType *ot)
   /* api callbacks */
   ot->exec = image_save_exec;
   ot->invoke = image_save_invoke;
-  ot->poll = image_save_as_poll;
+  ot->poll = image_save_poll;
 
   /* flags */
   ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
@@ -2192,7 +2224,8 @@ static bool image_should_be_saved_when_modified(Image *ima)
 
 static bool image_should_be_saved(Image *ima)
 {
-  if (BKE_image_is_dirty(ima) && (ima->source == IMA_SRC_FILE)) {
+  if (BKE_image_is_dirty(ima) &&
+      (ima->source == IMA_SRC_FILE || ima->source == IMA_SRC_GENERATED)) {
     return image_should_be_saved_when_modified(ima);
   }
   else {
@@ -2219,7 +2252,7 @@ int ED_image_save_all_modified_info(const bContext *C, ReportList *reports)
 
   for (Image *ima = bmain->images.first; ima; ima = ima->id.next) {
     if (image_should_be_saved(ima)) {
-      if (BKE_image_has_packedfile(ima)) {
+      if (BKE_image_has_packedfile(ima) || (ima->source == IMA_SRC_GENERATED)) {
         if (ima->id.lib == NULL) {
           num_saveable_images++;
         }
@@ -2268,7 +2301,7 @@ bool ED_image_save_all_modified(const bContext *C, ReportList *reports)
 
   for (Image *ima = bmain->images.first; ima; ima = ima->id.next) {
     if (image_should_be_saved(ima)) {
-      if (BKE_image_has_packedfile(ima)) {
+      if (BKE_image_has_packedfile(ima) || (ima->source == IMA_SRC_GENERATED)) {
         BKE_image_memorypack(ima);
       }
       else {