GPUViewport: Fix possible hash colision with enabled engines

Also fix engine data validation that was not previously not working.
This commit is contained in:
Clément Foucault 2019-05-07 18:42:28 +02:00
parent ac2e23c739
commit 1ae2385106
3 changed files with 54 additions and 46 deletions

View File

@ -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 |

View File

@ -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__

View File

@ -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);
}