Assets: Sanitize threaded preview creation with undo

Basically, this fixes disappearing previews when editing asset metadata
or performing undo/redo actions.

The preview generation in a background job will eventually modify ID
data, but the undo push was done prior to that. So obviously, an undo
then would mean the preview is lost.

This patch makes it so undo/redo will regenerate the preview, if the preview
rendering was invoked but not finished in the undone/redone state.

The preview flag PRV_UNFINISHED wasn't entirely what we needed. So I had to
change it to a slightly different flag, with different semantics.

10 files changed, 115 insertions, 20 deletions

diff --git a/source/blender/blenkernel/intern/icons.cc b/source/blender/blenkernel/intern/icons.cc
index 9cf289935ab..208e911298a 100644
--- a/source/blender/blenkernel/intern/icons.cc
+++ b/source/blender/blenkernel/intern/icons.cc
@@ -250,7 +250,7 @@ static PreviewImage *previewimg_create_ex(size_t deferred_data_size)
   }
 
   for (int i = 0; i < NUM_ICON_SIZES; i++) {
-    prv_img->flag[i] |= (PRV_CHANGED | PRV_UNFINISHED);
+    prv_img->flag[i] |= PRV_CHANGED;
     prv_img->changed_timestamp[i] = 0;
   }
   return prv_img;
@@ -307,7 +307,7 @@ void BKE_previewimg_clear_single(struct PreviewImage *prv, enum eIconSizes size)
     GPU_texture_free(prv->gputexture[size]);
   }
   prv->h[size] = prv->w[size] = 0;
-  prv->flag[size] |= (PRV_CHANGED | PRV_UNFINISHED);
+  prv->flag[size] |= PRV_CHANGED;
   prv->flag[size] &= ~PRV_USER_EDITED;
   prv->changed_timestamp[size] = 0;
 }
@@ -563,7 +563,7 @@ void BKE_previewimg_ensure(PreviewImage *prv, const int size)
           prv->w[ICON_SIZE_PREVIEW] = thumb->x;
           prv->h[ICON_SIZE_PREVIEW] = thumb->y;
           prv->rect[ICON_SIZE_PREVIEW] = (uint *)MEM_dupallocN(thumb->rect);
-          prv->flag[ICON_SIZE_PREVIEW] &= ~(PRV_CHANGED | PRV_USER_EDITED | PRV_UNFINISHED);
+          prv->flag[ICON_SIZE_PREVIEW] &= ~(PRV_CHANGED | PRV_USER_EDITED | PRV_RENDERING);
         }
         if (do_icon) {
           if (thumb->x > thumb->y) {
@@ -582,7 +582,7 @@ void BKE_previewimg_ensure(PreviewImage *prv, const int size)
           prv->w[ICON_SIZE_ICON] = icon_w;
           prv->h[ICON_SIZE_ICON] = icon_h;
           prv->rect[ICON_SIZE_ICON] = (uint *)MEM_dupallocN(thumb->rect);
-          prv->flag[ICON_SIZE_ICON] &= ~(PRV_CHANGED | PRV_USER_EDITED | PRV_UNFINISHED);
+          prv->flag[ICON_SIZE_ICON] &= ~(PRV_CHANGED | PRV_USER_EDITED | PRV_RENDERING);
         }
         IMB_freeImBuf(thumb);
       }
@@ -614,12 +614,12 @@ ImBuf *BKE_previewimg_to_imbuf(PreviewImage *prv, const int size)
 void BKE_previewimg_finish(PreviewImage *prv, const int size)
 {
   /* Previews may be calculated on a thread. */
-  atomic_fetch_and_and_int16(&prv->flag[size], ~PRV_UNFINISHED);
+  atomic_fetch_and_and_int16(&prv->flag[size], ~PRV_RENDERING);
 }
 
 bool BKE_previewimg_is_finished(const PreviewImage *prv, const int size)
 {
-  return (prv->flag[size] & PRV_UNFINISHED) == 0;
+  return (prv->flag[size] & PRV_RENDERING) == 0;
 }
 
 void BKE_previewimg_blend_write(BlendWriter *writer, const PreviewImage *prv)
@@ -653,16 +653,11 @@ void BKE_previewimg_blend_read(BlendDataReader *reader, PreviewImage *prv)
       BLO_read_data_address(reader, &prv->rect[i]);
     }
     prv->gputexture[i] = nullptr;
-    /* For now consider previews read from file as finished to not confuse File Browser preview
-     * loading. That could be smarter and check if there's a preview job running instead.
-     * If the preview is tagged as changed, it needs to be updated anyway, so don't remove the tag.
-     */
-    if ((prv->flag[i] & PRV_CHANGED) == 0) {
-      BKE_previewimg_finish(prv, i);
-    }
-    else {
-      /* Only for old files that didn't write the flag. */
-      prv->flag[i] |= PRV_UNFINISHED;
+
+    /* PRV_RENDERING is a runtime only flag currently, but don't mess with it on undo! It gets
+     * special handling in #memfile_undosys_restart_unfinished_id_previews() then. */
+    if (!BLO_read_data_is_undo(reader)) {
+      prv->flag[i] &= ~PRV_RENDERING;
     }
   }
   prv->icon_id = 0;
@@ -692,7 +687,7 @@ void BKE_icon_changed(const int icon_id)
     /* If we have previews, they all are now invalid changed. */
     if (p_prv && *p_prv) {
       for (int i = 0; i < NUM_ICON_SIZES; i++) {
-        (*p_prv)->flag[i] |= (PRV_CHANGED | PRV_UNFINISHED);
+        (*p_prv)->flag[i] |= PRV_CHANGED;
         (*p_prv)->changed_timestamp[i]++;
       }
     }
diff --git a/source/blender/editors/include/ED_render.h b/source/blender/editors/include/ED_render.h
index 2d042b8b0e3..0e03000efba 100644
--- a/source/blender/editors/include/ED_render.h
+++ b/source/blender/editors/include/ED_render.h
@@ -41,6 +41,7 @@ struct bContext;
 struct bScreen;
 struct wmWindow;
 struct wmWindowManager;
+enum eIconSizes;
 
 /* render_ops.c */
 
@@ -105,6 +106,11 @@ void ED_preview_icon_job(const struct bContext *C,
                          int sizex,
                          int sizey,
                          const bool delay);
+
+void ED_preview_restart_queue_free(void);
+void ED_preview_restart_queue_add(struct ID *id, enum eIconSizes size);
+void ED_preview_restart_queue_work(const struct bContext *C);
+
 void ED_preview_kill_jobs(struct wmWindowManager *wm, struct Main *bmain);
 
 void ED_preview_draw(const struct bContext *C, void *idp, void *parentp, void *slot, rcti *rect);
diff --git a/source/blender/editors/interface/interface_icons.c b/source/blender/editors/interface/interface_icons.c
index 6f119d55d3c..311965ac502 100644
--- a/source/blender/editors/interface/interface_icons.c
+++ b/source/blender/editors/interface/interface_icons.c
@@ -1270,7 +1270,7 @@ static void icon_create_rect(struct PreviewImage *prv_img, enum eIconSizes size)
   else if (!prv_img->rect[size]) {
     prv_img->w[size] = render_size;
     prv_img->h[size] = render_size;
-    prv_img->flag[size] |= (PRV_CHANGED | PRV_UNFINISHED);
+    prv_img->flag[size] |= PRV_CHANGED;
     prv_img->changed_timestamp[size] = 0;
     prv_img->rect[size] = MEM_callocN(render_size * render_size * sizeof(uint), "prv_rect");
   }
@@ -1419,6 +1419,7 @@ static void icon_set_image(const bContext *C,
 
   const bool delay = prv_img->rect[size] != NULL;
   icon_create_rect(prv_img, size);
+  prv_img->flag[size] |= PRV_RENDERING;
 
   if (use_job && (!id || BKE_previewimg_id_supports_jobs(id))) {
     /* Job (background) version */
diff --git a/source/blender/editors/render/render_preview.c b/source/blender/editors/render/render_preview.c
index 0cc944436b2..25353dc30fe 100644
--- a/source/blender/editors/render/render_preview.c
+++ b/source/blender/editors/render/render_preview.c
@@ -103,6 +103,8 @@
 #include "ED_view3d.h"
 #include "ED_view3d_offscreen.h"
 
+#include "UI_interface_icons.h"
+
 #ifndef NDEBUG
 /* Used for database init assert(). */
 #  include "BLI_threads.h"
@@ -1948,4 +1950,45 @@ void ED_preview_kill_jobs(wmWindowManager *wm, Main *UNUSED(bmain))
   }
 }
 
+typedef struct PreviewRestartQueueEntry {
+  struct PreviewRestartQueueEntry *next, *prev;
+
+  enum eIconSizes size;
+  ID *id;
+} PreviewRestartQueueEntry;
+
+static ListBase /* #PreviewRestartQueueEntry */ G_restart_previews_queue;
+
+void ED_preview_restart_queue_free(void)
+{
+  BLI_freelistN(&G_restart_previews_queue);
+}
+
+void ED_preview_restart_queue_add(ID *id, enum eIconSizes size)
+{
+  PreviewRestartQueueEntry *queue_entry = MEM_mallocN(sizeof(*queue_entry), __func__);
+  queue_entry->size = size;
+  queue_entry->id = id;
+  BLI_addtail(&G_restart_previews_queue, queue_entry);
+}
+
+void ED_preview_restart_queue_work(const bContext *C)
+{
+  LISTBASE_FOREACH_MUTABLE (PreviewRestartQueueEntry *, queue_entry, &G_restart_previews_queue) {
+    PreviewImage *preview = BKE_previewimg_id_get(queue_entry->id);
+    if (!preview) {
+      continue;
+    }
+    if (preview->flag[queue_entry->size] & PRV_USER_EDITED) {
+      /* Don't touch custom previews. */
+      continue;
+    }
+
+    BKE_previewimg_clear_single(preview, queue_entry->size);
+    UI_icon_render_id(C, NULL, queue_entry->id, queue_entry->size, true);
+
+    BLI_freelinkN(&G_restart_previews_queue, queue_entry);
+  }
+}
+
 /** \} */
diff --git a/source/blender/editors/undo/memfile_undo.c b/source/blender/editors/undo/memfile_undo.c
index 1bdc2b2251e..ee7221bd2a8 100644
--- a/source/blender/editors/undo/memfile_undo.c
+++ b/source/blender/editors/undo/memfile_undo.c
@@ -35,6 +35,7 @@
 
 #include "BKE_blender_undo.h"
 #include "BKE_context.h"
+#include "BKE_icons.h"
 #include "BKE_lib_id.h"
 #include "BKE_lib_query.h"
 #include "BKE_main.h"
@@ -48,6 +49,7 @@
 #include "WM_types.h"
 
 #include "ED_object.h"
+#include "ED_render.h"
 #include "ED_undo.h"
 #include "ED_util.h"
 
@@ -142,6 +144,32 @@ static int memfile_undosys_step_id_reused_cb(LibraryIDLinkCallbackData *cb_data)
   return IDWALK_RET_NOP;
 }
 
+/**
+ * ID previews may be generated in a parallel job. So whatever operation generates the preview
+ * likely does the undo push before the preview is actually done and stored in the ID. Hence they
+ * get some extra treatement here:
+ * When undoing back to the moment the preview generation was triggered, this function schedules
+ * the preview for regeneration.
+ */
+static void memfile_undosys_unfinished_id_previews_restart(ID *id)
+{
+  PreviewImage *preview = BKE_previewimg_id_get(id);
+  if (!preview) {
+    return;
+  }
+
+  for (int i = 0; i < NUM_ICON_SIZES; i++) {
+    if (preview->flag[i] & PRV_USER_EDITED) {
+      /* Don't modify custom previews. */
+      continue;
+    }
+
+    if (!BKE_previewimg_is_finished(preview, i)) {
+      ED_preview_restart_queue_add(id, i);
+    }
+  }
+}
+
 static void memfile_undosys_step_decode(struct bContext *C,
                                         struct Main *bmain,
                                         UndoStep *us_p,
@@ -188,6 +216,9 @@ static void memfile_undosys_step_decode(struct bContext *C,
   }
 
   ED_editors_exit(bmain, false);
+  /* Ensure there's no preview job running. Unfinished previews will be scheduled for regeneration
+   * via #memfile_undosys_unfinished_id_previews_restart(). */
+  ED_preview_kill_jobs(CTX_wm_manager(C), bmain);
 
   MemFileUndoStep *us = (MemFileUndoStep *)us_p;
   BKE_memfile_undo_decode(us->data, undo_direction, use_old_bmain_data, C);
@@ -239,6 +270,9 @@ static void memfile_undosys_step_decode(struct bContext *C,
               bmain, &scene->master_collection->id, scene->master_collection->id.recalc);
         }
       }
+
+      /* Restart preview generation if the undo state was generating previews. */
+      memfile_undosys_unfinished_id_previews_restart(id);
     }
     FOREACH_MAIN_ID_END;
 
@@ -263,6 +297,14 @@ static void memfile_undosys_step_decode(struct bContext *C,
     }
     FOREACH_MAIN_ID_END;
   }
+  else {
+    ID *id = NULL;
+    FOREACH_MAIN_ID_BEGIN (bmain, id) {
+      /* Restart preview generation if the undo state was generating previews. */
+      memfile_undosys_unfinished_id_previews_restart(id);
+    }
+    FOREACH_MAIN_ID_END;
+  }
 
   WM_event_add_notifier(C, NC_SCENE | ND_LAYER_CONTENT, CTX_data_scene(C));
 }
diff --git a/source/blender/editors/util/ed_util_ops.cc b/source/blender/editors/util/ed_util_ops.cc
index 7d32d252718..df6e093c57a 100644
--- a/source/blender/editors/util/ed_util_ops.cc
+++ b/source/blender/editors/util/ed_util_ops.cc
@@ -110,7 +110,7 @@ static void ED_OT_lib_id_load_custom_preview(wmOperatorType *ot)
   ot->invoke = WM_operator_filesel;
 
   /* flags */
-  ot->flag = OPTYPE_INTERNAL;
+  ot->flag = OPTYPE_UNDO | OPTYPE_INTERNAL;
 
   WM_operator_properties_filesel(ot,
                                  FILE_TYPE_FOLDER | FILE_TYPE_IMAGE,
diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h
index 2c04d0b06ef..6a478f9abb5 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -461,7 +461,7 @@ typedef struct LibraryWeakReference {
 enum ePreviewImage_Flag {
   PRV_CHANGED = (1 << 0),
   PRV_USER_EDITED = (1 << 1), /* if user-edited, do not auto-update this anymore! */
-  PRV_UNFINISHED = (1 << 2),  /* The preview is not done rendering yet. */
+  PRV_RENDERING = (1 << 2),   /* Rendering was invoked. Cleared on file read. */
 };
 
 /* for PreviewImage->tag */
diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index b780fef289a..c5efb239a88 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -64,6 +64,7 @@
 #include "ED_asset.h"
 #include "ED_fileselect.h"
 #include "ED_info.h"
+#include "ED_render.h"
 #include "ED_screen.h"
 #include "ED_undo.h"
 #include "ED_util.h"
@@ -457,6 +458,9 @@ void wm_event_do_notifiers(bContext *C)
         else if (note->data == ND_DATACHANGED) {
           wm_window_title(wm, win);
         }
+        else if (note->data == ND_UNDO) {
+          ED_preview_restart_queue_work(C);
+        }
       }
       if (note->window == win) {
         if (note->category == NC_SCREEN) {
diff --git a/source/blender/windowmanager/intern/wm_files.c b/source/blender/windowmanager/intern/wm_files.c
index a301b17227d..e95ce98352a 100644
--- a/source/blender/windowmanager/intern/wm_files.c
+++ b/source/blender/windowmanager/intern/wm_files.c
@@ -111,6 +111,7 @@
 #include "ED_fileselect.h"
 #include "ED_image.h"
 #include "ED_outliner.h"
+#include "ED_render.h"
 #include "ED_screen.h"
 #include "ED_undo.h"
 #include "ED_util.h"
@@ -620,6 +621,8 @@ static void wm_file_read_pre(bContext *C, bool use_data, bool UNUSED(use_userdef
   /* Always do this as both startup and preferences may have loaded in many font's
    * at a different zoom level to the file being loaded. */
   UI_view2d_zoom_cache_reset();
+
+  ED_preview_restart_queue_free();
 }
 
 /**
diff --git a/source/blender/windowmanager/intern/wm_init_exit.c b/source/blender/windowmanager/intern/wm_init_exit.c
index c382af03c4a..1a45e535a4a 100644
--- a/source/blender/windowmanager/intern/wm_init_exit.c
+++ b/source/blender/windowmanager/intern/wm_init_exit.c
@@ -547,6 +547,7 @@ void WM_exit_ex(bContext *C, const bool do_python)
   RE_engines_exit();
 
   ED_preview_free_dbase(); /* frees a Main dbase, before BKE_blender_free! */
+  ED_preview_restart_queue_free();
   ED_assetlist_storage_exit();
 
   if (wm) {