[Bf-blender-cvs] [668b61996f5] asset-browser: Apply D9974: Sanitize threaded preview creation with undo
Julian Eisel
noreply at git.blender.org
Wed May 19 19:31:21 CEST 2021
Commit: 668b61996f5e4a8bea03161a3fb9372d896382f2
Author: Julian Eisel
Date: Wed May 19 19:29:58 2021 +0200
Branches: asset-browser
https://developer.blender.org/rB668b61996f5e4a8bea03161a3fb9372d896382f2
Apply D9974: Sanitize threaded preview creation with undo
With undo/redo after creating assets, previews of the "Current File"
asset library would often go missing, e.g. see T83884.
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.
===================================================================
M source/blender/blenkernel/intern/icons.cc
M source/blender/editors/interface/interface_icons.c
M source/blender/editors/undo/memfile_undo.c
M source/blender/editors/util/ed_util_ops.cc
M source/blender/makesdna/DNA_ID.h
===================================================================
diff --git a/source/blender/blenkernel/intern/icons.cc b/source/blender/blenkernel/intern/icons.cc
index 7e9f81f9c83..3380ba94e80 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)
@@ -659,16 +659,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;
@@ -698,7 +693,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/interface/interface_icons.c b/source/blender/editors/interface/interface_icons.c
index ad9a58165ea..68abd476c6b 100644
--- a/source/blender/editors/interface/interface_icons.c
+++ b/source/blender/editors/interface/interface_icons.c
@@ -1268,7 +1268,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");
}
@@ -1417,6 +1417,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/undo/memfile_undo.c b/source/blender/editors/undo/memfile_undo.c
index 9189adaf4d1..0ce47702eec 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,9 +49,12 @@
#include "WM_types.h"
#include "ED_object.h"
+#include "ED_render.h"
#include "ED_undo.h"
#include "ED_util.h"
+#include "UI_interface_icons.h"
+
#include "../blenloader/BLO_undofile.h"
#include "undo_intern.h"
@@ -142,6 +146,33 @@ 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 triggers the
+ * generation again.
+ */
+static void memfile_undosys_unfinished_id_previews_restart(bContext *C, 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)) {
+ BKE_previewimg_clear_single(preview, i);
+ UI_icon_render_id(C, NULL, id, i, true);
+ }
+ }
+}
+
static void memfile_undosys_step_decode(struct bContext *C,
struct Main *bmain,
UndoStep *us_p,
@@ -188,6 +219,8 @@ static void memfile_undosys_step_decode(struct bContext *C,
}
ED_editors_exit(bmain, false);
+ /* Ensure there's no preview job running. Below the jobs will be restarted if needed. */
+ 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);
@@ -221,6 +254,10 @@ static void memfile_undosys_step_decode(struct bContext *C,
BKE_library_foreach_ID_link(
bmain, id, memfile_undosys_step_id_reused_cb, NULL, IDWALK_READONLY);
}
+ else {
+ /* Restart preview generation if the undo state was generating previews. */
+ memfile_undosys_unfinished_id_previews_restart(C, id);
+ }
/* Tag depsgraph to update data-block for changes that happened between the
* current and the target state, see direct_link_id_restore_recalc(). */
@@ -251,6 +288,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(C, 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 e2978e15565..92c2baa980e 100644
--- a/source/blender/editors/util/ed_util_ops.cc
+++ b/source/blender/editors/util/ed_util_ops.cc
@@ -115,7 +115,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 8bf9afafa1b..cf45fb08da8 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -376,7 +376,7 @@ typedef struct Library {
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 */
More information about the Bf-blender-cvs
mailing list