Images: changes to avoid losing new images that have not been saved
authorBrecht Van Lommel <brechtvanlommel@gmail.com>
Sun, 4 Mar 2018 21:45:22 +0000 (22:45 +0100)
committerBrecht Van Lommel <brechtvanlommel@gmail.com>
Fri, 17 May 2019 18:03:26 +0000 (20:03 +0200)
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".

release/scripts/startup/bl_ui/space_image.py
source/blender/blenkernel/BKE_image.h
source/blender/blenkernel/intern/bpath.c
source/blender/blenkernel/intern/image.c
source/blender/editors/space_image/image_buttons.c
source/blender/editors/space_image/image_ops.c

index 5ea3545..6a65841 100644 (file)
@@ -237,13 +237,14 @@ class IMAGE_MT_image(Menu):
 
             layout.menu("IMAGE_MT_image_invert")
 
-            if not show_render:
-                layout.separator()
-                if ima.packed_file:
-                    layout.operator("image.pack", text="Repack")
+        if ima and not show_render:
+            if ima.packed_file:
+                if len(ima.filepath):
+                    layout.separator()
                     layout.operator("image.unpack", text="Unpack")
-                else:
-                    layout.operator("image.pack", text="Pack")
+            else:
+                layout.separator()
+                layout.operator("image.pack", text="Pack")
 
 
 class IMAGE_MT_image_invert(Menu):
index 9e5a761..b9f2123 100644 (file)
@@ -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,
index 6f0c189..fa7af53 100644 (file)
@@ -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) &&
index 18c42da..d59ead2 100644 (file)
@@ -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)
 {
index 5989639..68af854 100644 (file)
@@ -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);
index 03e1d80..07c2d21 100644 (file)
@@ -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 {