[Bf-blender-cvs] [188ccfb0dd6] master: Fix T74662: Prefetching causes random crashes

Richard Antalik noreply at git.blender.org
Wed Mar 25 00:25:52 CET 2020


Commit: 188ccfb0dd6739fb6af8741aeecbc1da82c068ab
Author: Richard Antalik
Date:   Wed Mar 25 00:23:06 2020 +0100
Branches: master
https://developer.blender.org/rB188ccfb0dd6739fb6af8741aeecbc1da82c068ab

Fix T74662: Prefetching causes random crashes

Caused by 18b693bdbd6b, due to lack of thread safety.
Beteween calling BKE_sequencer_cache_get_num_items and BKE_sequencer_cache_iterate
New items could be inserted in the cache.

BKE_sequencer_cache_iterate() now use 2 callbcack functions for initial setup
during which buffers with correct length can be initialized and finally iterating.

Additionally drawing of unselected items was fixed again introduced in 18b693bdbd6b.

T74662 is reporting quite different symptoms, than I get on my machine, so I am not
entirely sure this is complete fix.

Reviewed By: brecht

Differential Revision: https://developer.blender.org/D7220

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

M	source/blender/blenkernel/BKE_sequencer.h
M	source/blender/blenkernel/intern/seqcache.c
M	source/blender/editors/space_sequencer/sequencer_draw.c

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

diff --git a/source/blender/blenkernel/BKE_sequencer.h b/source/blender/blenkernel/BKE_sequencer.h
index df0f0e730a5..556cd7105a9 100644
--- a/source/blender/blenkernel/BKE_sequencer.h
+++ b/source/blender/blenkernel/BKE_sequencer.h
@@ -337,11 +337,14 @@ void BKE_sequencer_cache_cleanup_sequence(struct Scene *scene,
                                           struct Sequence *seq,
                                           struct Sequence *seq_changed,
                                           int invalidate_types);
-void BKE_sequencer_cache_iterate(
-    struct Scene *scene,
-    void *userdata,
-    bool callback(void *userdata, struct Sequence *seq, int cfra, int cache_type, float cost));
-size_t BKE_sequencer_cache_get_num_items(struct Scene *scene);
+void BKE_sequencer_cache_iterate(struct Scene *scene,
+                                 void *userdata,
+                                 bool callback_init(void *userdata, size_t item_count),
+                                 bool callback_iter(void *userdata,
+                                                    struct Sequence *seq,
+                                                    int cfra,
+                                                    int cache_type,
+                                                    float cost));
 bool BKE_sequencer_cache_is_full(struct Scene *scene);
 
 /* **********************************************************************
diff --git a/source/blender/blenkernel/intern/seqcache.c b/source/blender/blenkernel/intern/seqcache.c
index d48930e5665..a2c0434a474 100644
--- a/source/blender/blenkernel/intern/seqcache.c
+++ b/source/blender/blenkernel/intern/seqcache.c
@@ -1401,24 +1401,11 @@ void BKE_sequencer_cache_put(const SeqRenderData *context,
   }
 }
 
-size_t BKE_sequencer_cache_get_num_items(struct Scene *scene)
-{
-  SeqCache *cache = seq_cache_get_from_scene(scene);
-  if (!cache) {
-    return 0;
-  }
-
-  seq_cache_lock(scene);
-  size_t num_items = BLI_ghash_len(cache->hash);
-  seq_cache_unlock(scene);
-
-  return num_items;
-}
-
 void BKE_sequencer_cache_iterate(
     struct Scene *scene,
     void *userdata,
-    bool callback(void *userdata, struct Sequence *seq, int nfra, int cache_type, float cost))
+    bool callback_init(void *userdata, size_t item_count),
+    bool callback_iter(void *userdata, struct Sequence *seq, int nfra, int cache_type, float cost))
 {
   SeqCache *cache = seq_cache_get_from_scene(scene);
   if (!cache) {
@@ -1426,15 +1413,16 @@ void BKE_sequencer_cache_iterate(
   }
 
   seq_cache_lock(scene);
+  bool interrupt = callback_init(userdata, BLI_ghash_len(cache->hash));
+
   GHashIterator gh_iter;
   BLI_ghashIterator_init(&gh_iter, cache->hash);
-  bool interrupt = false;
 
   while (!BLI_ghashIterator_done(&gh_iter) && !interrupt) {
     SeqCacheKey *key = BLI_ghashIterator_getKey(&gh_iter);
     BLI_ghashIterator_step(&gh_iter);
 
-    interrupt = callback(userdata, key->seq, key->nfra, key->type, key->cost);
+    interrupt = callback_iter(userdata, key->seq, key->nfra, key->type, key->cost);
   }
 
   cache->last_key = NULL;
diff --git a/source/blender/editors/space_sequencer/sequencer_draw.c b/source/blender/editors/space_sequencer/sequencer_draw.c
index 058d41ab294..06158e6f1f4 100644
--- a/source/blender/editors/space_sequencer/sequencer_draw.c
+++ b/source/blender/editors/space_sequencer/sequencer_draw.c
@@ -1968,6 +1968,7 @@ typedef struct CacheDrawData {
   struct View2D *v2d;
   float stripe_offs;
   float stripe_ht;
+  int cache_flag;
   GPUVertBuf *raw_vbo;
   GPUVertBuf *preprocessed_vbo;
   GPUVertBuf *composite_vbo;
@@ -1979,7 +1980,25 @@ typedef struct CacheDrawData {
 } CacheDrawData;
 
 /* Called as a callback */
-static bool draw_cache_view_cb(
+static bool draw_cache_view_init_cb(void *userdata, size_t item_count)
+{
+  if (item_count == 0) {
+    return true;
+  }
+
+  CacheDrawData *drawdata = userdata;
+  /* We can not get item count per cache type, so using total item count is safe. */
+  size_t max_vert_count = item_count * 6;
+  GPU_vertbuf_data_alloc(drawdata->raw_vbo, max_vert_count);
+  GPU_vertbuf_data_alloc(drawdata->preprocessed_vbo, max_vert_count);
+  GPU_vertbuf_data_alloc(drawdata->composite_vbo, max_vert_count);
+  GPU_vertbuf_data_alloc(drawdata->final_out_vbo, max_vert_count);
+
+  return false;
+}
+
+/* Called as a callback */
+static bool draw_cache_view_iter_cb(
     void *userdata, struct Sequence *seq, int nfra, int cache_type, float UNUSED(cost))
 {
   CacheDrawData *drawdata = userdata;
@@ -1987,44 +2006,43 @@ static bool draw_cache_view_cb(
   float stripe_bot, stripe_top, stripe_offs, stripe_ht;
   GPUVertBuf *vbo;
   size_t *vert_count;
-  switch (cache_type) {
-    case SEQ_CACHE_STORE_FINAL_OUT:
-      stripe_ht = UI_view2d_region_to_view_y(v2d, 4.0f * UI_DPI_FAC * U.pixelsize) - v2d->cur.ymin;
-      stripe_bot = UI_view2d_region_to_view_y(v2d, V2D_SCROLL_HANDLE_HEIGHT);
-      stripe_top = stripe_bot + stripe_ht;
-      vbo = drawdata->final_out_vbo;
-      vert_count = &drawdata->final_out_vert_count;
-      break;
-
-    case SEQ_CACHE_STORE_RAW:
-      stripe_offs = drawdata->stripe_offs;
-      stripe_ht = drawdata->stripe_ht;
-      stripe_bot = seq->machine + SEQ_STRIP_OFSBOTTOM + stripe_offs;
-      stripe_top = stripe_bot + stripe_ht;
-      vbo = drawdata->raw_vbo;
-      vert_count = &drawdata->raw_vert_count;
-      break;
 
-    case SEQ_CACHE_STORE_PREPROCESSED:
-      stripe_offs = drawdata->stripe_offs;
-      stripe_ht = drawdata->stripe_ht;
-      stripe_bot = seq->machine + SEQ_STRIP_OFSBOTTOM + (stripe_offs + stripe_ht) + stripe_offs;
-      stripe_top = stripe_bot + stripe_ht;
-      vbo = drawdata->preprocessed_vbo;
-      vert_count = &drawdata->preprocessed_vert_count;
-      break;
-
-    case SEQ_CACHE_STORE_COMPOSITE:
-      stripe_offs = drawdata->stripe_offs;
-      stripe_ht = drawdata->stripe_ht;
-      stripe_top = seq->machine + SEQ_STRIP_OFSTOP - stripe_offs;
-      stripe_bot = stripe_top - stripe_ht;
-      vbo = drawdata->composite_vbo;
-      vert_count = &drawdata->composite_vert_count;
-      break;
-
-    default:
-      return true;
+  if ((cache_type & SEQ_CACHE_STORE_FINAL_OUT) &&
+      (drawdata->cache_flag & SEQ_CACHE_VIEW_FINAL_OUT)) {
+    stripe_ht = UI_view2d_region_to_view_y(v2d, 4.0f * UI_DPI_FAC * U.pixelsize) - v2d->cur.ymin;
+    stripe_bot = UI_view2d_region_to_view_y(v2d, V2D_SCROLL_HANDLE_HEIGHT);
+    stripe_top = stripe_bot + stripe_ht;
+    vbo = drawdata->final_out_vbo;
+    vert_count = &drawdata->final_out_vert_count;
+  }
+  else if ((cache_type & SEQ_CACHE_STORE_RAW) && (drawdata->cache_flag & SEQ_CACHE_VIEW_RAW)) {
+    stripe_offs = drawdata->stripe_offs;
+    stripe_ht = drawdata->stripe_ht;
+    stripe_bot = seq->machine + SEQ_STRIP_OFSBOTTOM + stripe_offs;
+    stripe_top = stripe_bot + stripe_ht;
+    vbo = drawdata->raw_vbo;
+    vert_count = &drawdata->raw_vert_count;
+  }
+  else if ((cache_type & SEQ_CACHE_STORE_PREPROCESSED) &&
+           (drawdata->cache_flag & SEQ_CACHE_VIEW_PREPROCESSED)) {
+    stripe_offs = drawdata->stripe_offs;
+    stripe_ht = drawdata->stripe_ht;
+    stripe_bot = seq->machine + SEQ_STRIP_OFSBOTTOM + (stripe_offs + stripe_ht) + stripe_offs;
+    stripe_top = stripe_bot + stripe_ht;
+    vbo = drawdata->preprocessed_vbo;
+    vert_count = &drawdata->preprocessed_vert_count;
+  }
+  else if ((cache_type & SEQ_CACHE_STORE_COMPOSITE) &&
+           (drawdata->cache_flag & SEQ_CACHE_VIEW_COMPOSITE)) {
+    stripe_offs = drawdata->stripe_offs;
+    stripe_ht = drawdata->stripe_ht;
+    stripe_top = seq->machine + SEQ_STRIP_OFSTOP - stripe_offs;
+    stripe_bot = stripe_top - stripe_ht;
+    vbo = drawdata->composite_vbo;
+    vert_count = &drawdata->composite_vert_count;
+  }
+  else {
+    return false;
   }
 
   int cfra = seq->start + nfra;
@@ -2048,10 +2066,10 @@ static void draw_cache_view_batch(
     GPUVertBuf *vbo, size_t vert_count, float col_r, float col_g, float col_b, float col_a)
 {
   GPUBatch *batch = GPU_batch_create_ex(GPU_PRIM_TRIS, vbo, NULL, GPU_BATCH_OWNS_VBO);
-  GPU_vertbuf_data_len_set(vbo, vert_count);
-  GPU_batch_program_set_builtin(batch, GPU_SHADER_2D_UNIFORM_COLOR);
-  GPU_batch_uniform_4f(batch, "color", col_r, col_g, col_b, col_a);
   if (vert_count > 0) {
+    GPU_vertbuf_data_len_set(vbo, vert_count);
+    GPU_batch_program_set_builtin(batch, GPU_SHADER_2D_UNIFORM_COLOR);
+    GPU_batch_uniform_4f(batch, "color", col_r, col_g, col_b, col_a);
     GPU_batch_draw(batch);
   }
   GPU_batch_discard(batch);
@@ -2127,42 +2145,32 @@ static void draw_cache_view(const bContext *C)
 
   immUnbindProgram();
 
-  size_t cache_num_items = BKE_sequencer_cache_get_num_items(scene);
-
-  if (cache_num_items > 0) {
-    GPUVertFormat format = {0};
-    GPU_vertformat_attr_add(&format, "pos", GPU_COMP_F32, 2, GPU_FETCH_FLOAT);
-
-    CacheDrawData userdata;
-    userdata.v2d = v2d;
-    userdata.stripe_offs = stripe_offs;
-    userdata.stripe_ht = stripe_ht;
-    userdata.raw_vert_count = 0;
-    userdata.preprocessed_vert_count = 0;
-    userdata.composite_vert_count = 0;
-    userdata.final_out_vert_count = 0;
-    userdata.raw_vbo = GPU_vertbuf_create_with_format(&format);
-    userdata.preprocessed_vbo = GPU_vertbuf_create_with_format(&format);
-    userdata.composite_vbo = GPU_vertbuf_create_with_format(&format);
-    userdata.final_out_vbo = GPU_vertbuf_create_with_format(&format);
-
-    /* We can not get item count per cache type, so using total item count is safe. */
-    size_t max_vert_count = cache_num_items * 6;
-    GPU_vertbuf_data_alloc(userdata.raw_vbo, max_vert_count);
-    GPU_vertbuf_data_alloc(userdata.preprocessed_vbo, max_vert_count);
-    GPU_vertbuf_data_alloc(userdata.composite_vbo, max_vert_count);
-    GPU_vertbuf_data_alloc(userdata.final_out_vbo, max_vert_count);
-
-    BKE_sequencer_cache_iterate(scene, &userdata, draw_cache_view_cb);
-
-    draw_cache_view_batch(userdata.raw_vbo, userdata.raw_vert_count, 1.0f, 0.1f, 0.02f, 0.4f);
-    draw_cache_view_batch(
-        userdata.preprocessed_vbo, userdata.preprocessed_vert_count, 0.1f, 0.1f, 0.75f, 0.4f);
-    draw_cache_view_batch(
-        userdata.composite_vbo, userdata.composit

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list