From 5f0933f07a548719a850d9cac01aae6709b9dc0b Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Thu, 27 Oct 2016 09:51:10 +0200 Subject: Fix T49829: Removal of custom icon previews during add-on unregister crashes Blender. Issue was happening when removal of custom icons was done while they were still being rendered by preview job. Now add a 'deffered deletion' system, to prevent main thread to delete preview image until loading thread is done with them. Note that ideally, calling `ED_preview_kill_jobs()` on custom icon removal would have been simpler, but we don't have easy access to context here... --- source/blender/blenkernel/BKE_icons.h | 1 + source/blender/blenkernel/intern/icons.c | 20 ++++++++++---- source/blender/blenloader/intern/readfile.c | 1 + source/blender/editors/interface/interface_icons.c | 2 +- source/blender/editors/render/render_preview.c | 31 ++++++++++++++++++---- source/blender/makesdna/DNA_ID.h | 13 ++++++--- 6 files changed, 54 insertions(+), 14 deletions(-) (limited to 'source/blender') diff --git a/source/blender/blenkernel/BKE_icons.h b/source/blender/blenkernel/BKE_icons.h index efef8d4be78..6944c5ccd28 100644 --- a/source/blender/blenkernel/BKE_icons.h +++ b/source/blender/blenkernel/BKE_icons.h @@ -114,6 +114,7 @@ struct PreviewImage *BKE_previewimg_cached_thumbnail_read( const char *name, const char *path, const int source, bool force_update); void BKE_previewimg_cached_release(const char *name); +void BKE_previewimg_cached_release_pointer(struct PreviewImage *prv); #define ICON_RENDER_DEFAULT_HEIGHT 32 diff --git a/source/blender/blenkernel/intern/icons.c b/source/blender/blenkernel/intern/icons.c index 2d5b15c8f9d..7669c4ba112 100644 --- a/source/blender/blenkernel/intern/icons.c +++ b/source/blender/blenkernel/intern/icons.c @@ -143,7 +143,7 @@ static PreviewImage *previewimg_create_ex(size_t deferred_data_size) memset(prv_img, 0, sizeof(*prv_img)); /* leave deferred data dirty */ if (deferred_data_size) { - prv_img->use_deferred = true; + prv_img->tag |= PRV_TAG_DEFFERED; } for (i = 0; i < NUM_ICON_SIZES; ++i) { @@ -355,11 +355,14 @@ PreviewImage *BKE_previewimg_cached_thumbnail_read( return prv; } -void BKE_previewimg_cached_release(const char *name) +void BKE_previewimg_cached_release_pointer(PreviewImage *prv) { - PreviewImage *prv = BLI_ghash_popkey(gCachedPreviews, name, MEM_freeN); - if (prv) { + if (prv->tag & PRV_TAG_DEFFERED_RENDERING) { + /* We cannot delete the preview while it is being loaded in another thread... */ + prv->tag |= PRV_TAG_DEFFERED_DELETE; + return; + } if (prv->icon_id) { BKE_icon_delete(prv->icon_id); } @@ -367,11 +370,18 @@ void BKE_previewimg_cached_release(const char *name) } } +void BKE_previewimg_cached_release(const char *name) +{ + PreviewImage *prv = BLI_ghash_popkey(gCachedPreviews, name, MEM_freeN); + + BKE_previewimg_cached_release_pointer(prv); +} + /** Handle deferred (lazy) loading/generation of preview image, if needed. * For now, only used with file thumbnails. */ void BKE_previewimg_ensure(PreviewImage *prv, const int size) { - if (prv->use_deferred) { + if ((prv->tag & PRV_TAG_DEFFERED) != 0) { const bool do_icon = ((size == ICON_SIZE_ICON) && !prv->rect[ICON_SIZE_ICON]); const bool do_preview = ((size == ICON_SIZE_PREVIEW) && !prv->rect[ICON_SIZE_PREVIEW]); diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c index c1da78d3877..41b275751d1 100644 --- a/source/blender/blenloader/intern/readfile.c +++ b/source/blender/blenloader/intern/readfile.c @@ -2147,6 +2147,7 @@ static PreviewImage *direct_link_preview_image(FileData *fd, PreviewImage *old_p prv->gputexture[i] = NULL; } prv->icon_id = 0; + prv->tag = 0; } return prv; diff --git a/source/blender/editors/interface/interface_icons.c b/source/blender/editors/interface/interface_icons.c index 22a450d2523..ff9d2840e9c 100644 --- a/source/blender/editors/interface/interface_icons.c +++ b/source/blender/editors/interface/interface_icons.c @@ -1088,7 +1088,7 @@ void ui_icon_ensure_deferred(const bContext *C, const int icon_id, const bool bi if (prv) { const int size = big ? ICON_SIZE_PREVIEW : ICON_SIZE_ICON; - if (id || prv->use_deferred) { + if (id || (prv->tag & PRV_TAG_DEFFERED) != 0) { ui_id_preview_image_render_size(C, NULL, id, prv, size, true); } } diff --git a/source/blender/editors/render/render_preview.c b/source/blender/editors/render/render_preview.c index ddbf59b2cf7..87c08dc6583 100644 --- a/source/blender/editors/render/render_preview.c +++ b/source/blender/editors/render/render_preview.c @@ -1080,13 +1080,19 @@ static void icon_preview_add_size(IconPreview *ip, unsigned int *rect, int sizex static void icon_preview_startjob_all_sizes(void *customdata, short *stop, short *do_update, float *progress) { IconPreview *ip = (IconPreview *)customdata; - IconPreviewSize *cur_size = ip->sizes.first; + IconPreviewSize *cur_size; const bool use_new_shading = BKE_scene_use_new_shading_nodes(ip->scene); - while (cur_size) { + for (cur_size = ip->sizes.first; cur_size; cur_size = cur_size->next) { PreviewImage *prv = ip->owner; + + if (prv->tag & PRV_TAG_DEFFERED_DELETE) { + /* Non-thread-protected reading is not an issue here. */ + continue; + } + ShaderPreview *sp = MEM_callocN(sizeof(ShaderPreview), "Icon ShaderPreview"); - const bool is_render = !prv->use_deferred; + const bool is_render = !(prv->tag & PRV_TAG_DEFFERED); /* construct shader preview from image size and previewcustomdata */ sp->scene = ip->scene; @@ -1117,8 +1123,6 @@ static void icon_preview_startjob_all_sizes(void *customdata, short *stop, short common_preview_startjob(sp, stop, do_update, progress); shader_preview_free(sp); - - cur_size = cur_size->next; } } @@ -1147,6 +1151,15 @@ static void icon_preview_endjob(void *customdata) } #endif } + + if (ip->owner) { + PreviewImage *prv_img = ip->owner; + prv_img->tag &= ~PRV_TAG_DEFFERED_RENDERING; + if (prv_img->tag & PRV_TAG_DEFFERED_DELETE) { + BLI_assert(prv_img->tag & PRV_TAG_DEFFERED); + BKE_previewimg_cached_release_pointer(prv_img); + } + } } static void icon_preview_free(void *customdata) @@ -1205,6 +1218,14 @@ void ED_preview_icon_job(const bContext *C, void *owner, ID *id, unsigned int *r icon_preview_add_size(ip, rect, sizex, sizey); + /* Special threading hack: warn main code that this preview is being rendered and cannot be freed... */ + { + PreviewImage *prv_img = owner; + if (prv_img->tag & PRV_TAG_DEFFERED) { + prv_img->tag |= PRV_TAG_DEFFERED_RENDERING; + } + } + /* setup job */ WM_jobs_customdata_set(wm_job, ip, icon_preview_free); WM_jobs_timer(wm_job, 0.1, NC_WINDOW, NC_WINDOW); diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h index 5c1bfc229da..feeb2d5e4d7 100644 --- a/source/blender/makesdna/DNA_ID.h +++ b/source/blender/makesdna/DNA_ID.h @@ -172,6 +172,13 @@ enum ePreviewImage_Flag { PRV_USER_EDITED = (1 << 1), /* if user-edited, do not auto-update this anymore! */ }; +/* for PreviewImage->tag */ +enum { + PRV_TAG_DEFFERED = (1 << 0), /* Actual loading of preview is deffered. */ + PRV_TAG_DEFFERED_RENDERING = (1 << 1), /* Deffered preview is being loaded. */ + PRV_TAG_DEFFERED_DELETE = (1 << 2), /* Deffered preview should be deleted asap. */ +}; + typedef struct PreviewImage { /* All values of 2 are really NUM_ICON_SIZES */ unsigned int w[2]; @@ -184,12 +191,12 @@ typedef struct PreviewImage { struct GPUTexture *gputexture[2]; int icon_id; /* Used by previews outside of ID context. */ - char pad[3]; - char use_deferred; /* for now a mere bool, if we add more deferred loading methods we can switch to bitflag. */ + short tag; /* Runtime data. */ + char pad[2]; } PreviewImage; #define PRV_DEFERRED_DATA(prv) \ - (CHECK_TYPE_INLINE(prv, PreviewImage *), BLI_assert((prv)->use_deferred), (void *)((prv) + 1)) + (CHECK_TYPE_INLINE(prv, PreviewImage *), BLI_assert((prv)->tag & PRV_TAG_DEFFERED), (void *)((prv) + 1)) /** * Defines for working with IDs. -- cgit v1.2.3