diff options
author | Clément Foucault <foucault.clem@gmail.com> | 2019-05-07 19:42:28 +0300 |
---|---|---|
committer | Clément Foucault <foucault.clem@gmail.com> | 2019-05-08 18:49:28 +0300 |
commit | 1ae2385106c4bd7bde342a4d3aad74721f781473 (patch) | |
tree | f388757924d366b18d4cfb0d65a34a5a603ce745 /source | |
parent | ac2e23c739ab118ad1a6c516c454bc78b5cf6807 (diff) |
GPUViewport: Fix possible hash colision with enabled engines
Also fix engine data validation that was not previously not working.
Diffstat (limited to 'source')
-rw-r--r-- | source/blender/draw/intern/draw_manager.c | 28 | ||||
-rw-r--r-- | source/blender/gpu/GPU_viewport.h | 2 | ||||
-rw-r--r-- | source/blender/gpu/intern/gpu_viewport.c | 70 |
3 files changed, 54 insertions, 46 deletions
diff --git a/source/blender/draw/intern/draw_manager.c b/source/blender/draw/intern/draw_manager.c index 05fb893bec5..6afb1fdf854 100644 --- a/source/blender/draw/intern/draw_manager.c +++ b/source/blender/draw/intern/draw_manager.c @@ -22,6 +22,7 @@ #include <stdio.h> +#include "BLI_alloca.h" #include "BLI_listbase.h" #include "BLI_memblock.h" #include "BLI_rect.h" @@ -30,6 +31,7 @@ #include "BLF_api.h" +#include "BKE_anim.h" #include "BKE_colortools.h" #include "BKE_curve.h" #include "BKE_global.h" @@ -1108,7 +1110,7 @@ static void drw_engines_cache_populate(Object *ob) /* TODO: in the future it would be nice to generate once for all viewports. * But we need threaded DRW manager first. */ - drw_batch_cache_generate_requested(ob); + drw_batch_cache_generate_requested(DST.dupli_source ? DST.dupli_source->ob : ob); /* ... and clearing it here too because theses draw data are * from a mempool and must not be free individually by depsgraph. */ @@ -1402,17 +1404,19 @@ static void drw_engines_disable(void) BLI_freelistN(&DST.enabled_engines); } -static uint DRW_engines_get_hash(void) +static void drw_engines_data_validate(void) { - uint hash = 0; - /* The cache depends on enabled engines */ - /* FIXME : if collision occurs ... segfault */ + int enabled_engines = BLI_listbase_count(&DST.enabled_engines); + void **engine_handle_array = BLI_array_alloca(engine_handle_array, enabled_engines + 1); + int i = 0; + for (LinkData *link = DST.enabled_engines.first; link; link = link->next) { DrawEngineType *engine = link->data; - hash += BLI_ghashutil_strhash_p(engine->idname); + engine_handle_array[i++] = engine; } + engine_handle_array[i] = NULL; - return hash; + GPU_viewport_engines_data_validate(DST.viewport, engine_handle_array); } /* -------------------------------------------------------------------- */ @@ -1525,8 +1529,6 @@ void DRW_draw_render_loop_ex(struct Depsgraph *depsgraph, DST.viewport = viewport; /* Setup viewport */ - GPU_viewport_engines_data_validate(DST.viewport, DRW_engines_get_hash()); - DST.draw_ctx = (DRWContextState){ .ar = ar, .rv3d = rv3d, @@ -1546,6 +1548,8 @@ void DRW_draw_render_loop_ex(struct Depsgraph *depsgraph, /* Get list of enabled engines */ drw_engines_enable(view_layer, engine_type); + drw_engines_data_validate(); + /* Update ubos */ DRW_globals_update(); @@ -2029,7 +2033,7 @@ void DRW_render_object_iter( DST.ob_state = NULL; callback(vedata, ob, engine, depsgraph); - drw_batch_cache_generate_requested(ob); + drw_batch_cache_generate_requested(DST.dupli_source ? DST.dupli_source->ob : ob); } } DEG_OBJECT_ITER_END; @@ -2262,14 +2266,10 @@ void DRW_draw_select_loop(struct Depsgraph *depsgraph, drw_engines_world_update(scene); if (use_obedit) { -# if 0 - drw_engines_cache_populate(obact); -# else FOREACH_OBJECT_IN_MODE_BEGIN (view_layer, v3d, obact->type, obact->mode, ob_iter) { drw_engines_cache_populate(ob_iter); } FOREACH_OBJECT_IN_MODE_END; -# endif } else { const int iter_flag = DEG_ITER_OBJECT_FLAG_LINKED_DIRECTLY | diff --git a/source/blender/gpu/GPU_viewport.h b/source/blender/gpu/GPU_viewport.h index 70626ccb39b..e61cfe363df 100644 --- a/source/blender/gpu/GPU_viewport.h +++ b/source/blender/gpu/GPU_viewport.h @@ -118,7 +118,7 @@ GPUTexture *GPU_viewport_color_texture(GPUViewport *viewport); GPUTexture *GPU_viewport_texture_pool_query( GPUViewport *viewport, void *engine, int width, int height, int format); -bool GPU_viewport_engines_data_validate(GPUViewport *viewport, unsigned int hash); +bool GPU_viewport_engines_data_validate(GPUViewport *viewport, void **engine_handle_array); void GPU_viewport_cache_release(GPUViewport *viewport); #endif // __GPU_VIEWPORT_H__ diff --git a/source/blender/gpu/intern/gpu_viewport.c b/source/blender/gpu/intern/gpu_viewport.c index 88373dc9fe1..db083244b01 100644 --- a/source/blender/gpu/intern/gpu_viewport.c +++ b/source/blender/gpu/intern/gpu_viewport.c @@ -48,6 +48,8 @@ static const int default_fbl_len = (sizeof(DefaultFramebufferList)) / sizeof(void *); static const int default_txl_len = (sizeof(DefaultTextureList)) / sizeof(void *); +#define MAX_ENABLE_ENGINE 8 + /* Maximum number of simultaneous engine enabled at the same time. * Setting it lower than the real number will do lead to * higher VRAM usage due to sub-efficient buffer reuse. */ @@ -64,8 +66,11 @@ struct GPUViewport { int samples; int flag; - ListBase data; /* ViewportEngineData wrapped in LinkData */ - uint data_hash; /* If hash mismatch we free all ViewportEngineData in this viewport */ + /* If engine_handles mismatch we free all ViewportEngineData in this viewport */ + struct { + void *handle; + ViewportEngineData *data; + } engine_data[MAX_ENABLE_ENGINE]; DefaultFramebufferList *fbl; DefaultTextureList *txl; @@ -172,7 +177,6 @@ void GPU_viewport_clear_from_offscreen(GPUViewport *viewport) void *GPU_viewport_engine_data_create(GPUViewport *viewport, void *engine_type) { - LinkData *ld = MEM_callocN(sizeof(LinkData), "LinkData"); ViewportEngineData *data = MEM_callocN(sizeof(ViewportEngineData), "ViewportEngineData"); int fbl_len, txl_len, psl_len, stl_len; @@ -185,20 +189,25 @@ void *GPU_viewport_engine_data_create(GPUViewport *viewport, void *engine_type) data->psl = MEM_callocN((sizeof(void *) * psl_len) + sizeof(PassList), "PassList"); data->stl = MEM_callocN((sizeof(void *) * stl_len) + sizeof(StorageList), "StorageList"); - ld->data = data; - BLI_addtail(&viewport->data, ld); + for (int i = 0; i < MAX_ENABLE_ENGINE; i++) { + if (viewport->engine_data[i].handle == NULL) { + viewport->engine_data[i].handle = engine_type; + viewport->engine_data[i].data = data; + return data; + } + } - return data; + BLI_assert(!"Too many draw engines enabled at the same time"); + return NULL; } static void gpu_viewport_engines_data_free(GPUViewport *viewport) { int fbl_len, txl_len, psl_len, stl_len; - LinkData *next; - for (LinkData *link = viewport->data.first; link; link = next) { - next = link->next; - ViewportEngineData *data = link->data; + for (int i = 0; i < MAX_ENABLE_ENGINE && viewport->engine_data[i].handle; i++) { + ViewportEngineData *data = viewport->engine_data[i].data; + DRW_engine_viewport_data_size_get(data->engine_type, &fbl_len, &txl_len, &psl_len, &stl_len); gpu_viewport_buffers_free(data->fbl, fbl_len, data->txl, txl_len); @@ -219,19 +228,20 @@ static void gpu_viewport_engines_data_free(GPUViewport *viewport) MEM_freeN(data); - BLI_remlink(&viewport->data, link); - MEM_freeN(link); + /* Mark as unused*/ + viewport->engine_data[i].handle = NULL; } gpu_viewport_texture_pool_free(viewport); } -void *GPU_viewport_engine_data_get(GPUViewport *viewport, void *engine_type) +void *GPU_viewport_engine_data_get(GPUViewport *viewport, void *engine_handle) { - for (LinkData *link = viewport->data.first; link; link = link->next) { - ViewportEngineData *vdata = link->data; - if (vdata->engine_type == engine_type) { - return vdata; + BLI_assert(engine_handle != NULL); + + for (int i = 0; i < MAX_ENABLE_ENGINE; i++) { + if (viewport->engine_data[i].handle == engine_handle) { + return viewport->engine_data[i].data; } } return NULL; @@ -352,24 +362,22 @@ static void gpu_viewport_texture_pool_free(GPUViewport *viewport) BLI_freelistN(&viewport->tex_pool); } -bool GPU_viewport_engines_data_validate(GPUViewport *viewport, uint hash) +/* Takes an NULL terminated array of engine_handle. Returns true is data is still valid. */ +bool GPU_viewport_engines_data_validate(GPUViewport *viewport, void **engine_handle_array) { - bool dirty = false; - - if (viewport->data_hash != hash) { - gpu_viewport_engines_data_free(viewport); - dirty = true; + for (int i = 0; i < MAX_ENABLE_ENGINE && engine_handle_array[i]; i++) { + if (viewport->engine_data[i].handle != engine_handle_array[i]) { + gpu_viewport_engines_data_free(viewport); + return false; + } } - - viewport->data_hash = hash; - - return dirty; + return true; } void GPU_viewport_cache_release(GPUViewport *viewport) { - for (LinkData *link = viewport->data.first; link; link = link->next) { - ViewportEngineData *data = link->data; + for (int i = 0; i < MAX_ENABLE_ENGINE && viewport->engine_data[i].handle; i++) { + ViewportEngineData *data = viewport->engine_data[i].data; int psl_len; DRW_engine_viewport_data_size_get(data->engine_type, NULL, NULL, &psl_len, NULL); gpu_viewport_passes_free(data->psl, psl_len); @@ -468,8 +476,8 @@ void GPU_viewport_bind(GPUViewport *viewport, const rcti *rect) (TextureList *)viewport->txl, default_txl_len); - for (LinkData *link = viewport->data.first; link; link = link->next) { - ViewportEngineData *data = link->data; + for (int i = 0; i < MAX_ENABLE_ENGINE && viewport->engine_data[i].handle; i++) { + ViewportEngineData *data = viewport->engine_data[i].data; DRW_engine_viewport_data_size_get(data->engine_type, &fbl_len, &txl_len, NULL, NULL); gpu_viewport_buffers_free(data->fbl, fbl_len, data->txl, txl_len); } |