[Bf-blender-cvs] [1ae2385106c] master: GPUViewport: Fix possible hash colision with enabled engines

Clément Foucault noreply at git.blender.org
Wed May 8 17:59:05 CEST 2019


Commit: 1ae2385106c4bd7bde342a4d3aad74721f781473
Author: Clément Foucault
Date:   Tue May 7 18:42:28 2019 +0200
Branches: master
https://developer.blender.org/rB1ae2385106c4bd7bde342a4d3aad74721f781473

GPUViewport: Fix possible hash colision with enabled engines

Also fix engine data validation that was not previously not working.

===================================================================

M	source/blender/draw/intern/draw_manager.c
M	source/blender/gpu/GPU_viewport.h
M	source/blender/gpu/intern/gpu_viewport.c

===================================================================

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



More information about the Bf-blender-cvs mailing list