[Bf-blender-cvs] [85f060dc94f] cycles-x: Fix race condition between draw and render threads
Sergey Sharybin
noreply at git.blender.org
Fri Sep 17 10:25:02 CEST 2021
Commit: 85f060dc94f141c195ec01fe68725807f4eb7908
Author: Sergey Sharybin
Date: Thu Sep 16 18:01:55 2021 +0200
Branches: cycles-x
https://developer.blender.org/rB85f060dc94f141c195ec01fe68725807f4eb7908
Fix race condition between draw and render threads
Caused crashes like the one William was fixing in the D12498, or the
crash when rendering multiple view layers.
Need to guarantee that the render engine is not freed during drawing,
and that the external engine is not in the middle of re-allocation due
to `update()` which might cause session reset (happens in Cycles).
Had to introduce a separate lock. The reason for this comes from the
fact that we need to acquire the engine early on in the draw manager,
and release it only when drawing is done. However, some other engines
(like overlay) might be requesting ImBuf for the image space, so that
they know dimensions. Such acquisition is guarded by the resultmutex.
This means reusing `resultmutex` for the `RenderEngine::draw()` would
cause a recursive lock.
Not entirely happy with implicit release in the external engine code,
but not sure there is an existing way of making it explicit without
introducing new draw engine callback.
Differential Revision: https://developer.blender.org/D12521
===================================================================
M source/blender/draw/engines/external/external_engine.c
M source/blender/draw/engines/external/external_engine.h
M source/blender/draw/intern/draw_manager.c
M source/blender/render/RE_engine.h
M source/blender/render/intern/engine.c
M source/blender/render/intern/pipeline.c
M source/blender/render/intern/render_types.h
===================================================================
diff --git a/source/blender/draw/engines/external/external_engine.c b/source/blender/draw/engines/external/external_engine.c
index 70201136f48..de21a69e550 100644
--- a/source/blender/draw/engines/external/external_engine.c
+++ b/source/blender/draw/engines/external/external_engine.c
@@ -296,24 +296,6 @@ static void external_draw_scene_do_v3d(void *vedata)
}
}
-/* Get render engine of the current scene.
- *
- * Note that existence of the engine does not correlate with the rendering scene of the scene:
- * the engine could be non-NULL if the scene used persistent data. Or, if the scene is just
- * beginning to render the engine might not be existing yet. */
-static RenderEngine *external_engine_get(void)
-{
- const DRWContextState *draw_ctx = DRW_context_state_get();
- Scene *scene = draw_ctx->scene;
-
- Render *re = RE_GetSceneRender(scene);
- if (re == NULL) {
- return NULL;
- }
-
- return RE_engine_get(re);
-}
-
/* Configure current matrix stack so that the external engine can use the same drawing code for
* both viewport and image editor drawing.
*
@@ -362,10 +344,14 @@ static void external_image_space_matrix_set(const RenderEngine *engine)
static void external_draw_scene_do_image(void *UNUSED(vedata))
{
- RenderEngine *engine = external_engine_get();
- if (engine == NULL) {
- return;
- }
+ const DRWContextState *draw_ctx = DRW_context_state_get();
+ Scene *scene = draw_ctx->scene;
+ Render *re = RE_GetSceneRender(scene);
+ RenderEngine *engine = RE_engine_get(re);
+
+ /* Is tested before enabling the drawing engine. */
+ BLI_assert(re != NULL);
+ BLI_assert(engine != NULL);
const DefaultFramebufferList *dfbl = DRW_viewport_framebuffer_list_get();
@@ -387,7 +373,6 @@ static void external_draw_scene_do_image(void *UNUSED(vedata))
BLI_assert(engine_type != NULL);
BLI_assert(engine_type->draw != NULL);
- const DRWContextState *draw_ctx = DRW_context_state_get();
engine_type->draw(engine, draw_ctx->evil_C, draw_ctx->depsgraph);
GPU_debug_group_end();
@@ -397,6 +382,8 @@ static void external_draw_scene_do_image(void *UNUSED(vedata))
DRW_state_reset();
GPU_bgl_end();
+
+ RE_engine_draw_release(re);
}
static void external_draw_scene_do(void *vedata)
@@ -496,10 +483,11 @@ RenderEngineType DRW_engine_viewport_external_type = {
{NULL, NULL, NULL},
};
-bool DRW_engine_external_use_for_image_editor(void)
+bool DRW_engine_external_acquire_for_image_editor(void)
{
const DRWContextState *draw_ctx = DRW_context_state_get();
const SpaceLink *space_data = draw_ctx->space_data;
+ Scene *scene = draw_ctx->scene;
if (space_data == NULL) {
return false;
@@ -520,23 +508,10 @@ bool DRW_engine_external_use_for_image_editor(void)
return false;
}
- RenderEngine *engine = external_engine_get();
- if (engine == NULL) {
- return false;
- }
-
- if (!RE_engine_is_rendering(engine->re)) {
- return false;
- }
-
- const RenderEngineType *engine_type = engine->type;
- BLI_assert(engine_type != NULL);
-
- if (engine_type->draw == NULL) {
- return false;
- }
+ /* Render is allocated on main thread, so it is safe to access it from here. */
+ Render *re = RE_GetSceneRender(scene);
- return true;
+ return RE_engine_draw_acquire(re);
}
#undef EXTERNAL_ENGINE
diff --git a/source/blender/draw/engines/external/external_engine.h b/source/blender/draw/engines/external/external_engine.h
index d3f260b3d50..14ec4e2d3c5 100644
--- a/source/blender/draw/engines/external/external_engine.h
+++ b/source/blender/draw/engines/external/external_engine.h
@@ -25,5 +25,9 @@
extern DrawEngineType draw_engine_external_type;
extern RenderEngineType DRW_engine_viewport_external_type;
-/* Check whether an external engine is to be used to draw content of an image editor. */
-bool DRW_engine_external_use_for_image_editor(void);
+/* Check whether an external engine is to be used to draw content of an image editor.
+ * If the drawing is possible, the render engine is "acquired" so that it is not freed by the
+ * render engine for until drawing is finished.
+ *
+ * NOTE: Released by the draw engine when it is done drawing. */
+bool DRW_engine_external_acquire_for_image_editor(void);
diff --git a/source/blender/draw/intern/draw_manager.c b/source/blender/draw/intern/draw_manager.c
index 46a763e051e..e65fdce5f2e 100644
--- a/source/blender/draw/intern/draw_manager.c
+++ b/source/blender/draw/intern/draw_manager.c
@@ -1199,7 +1199,7 @@ static void drw_engines_enable_basic(void)
static void drw_engine_enable_image_editor(void)
{
- if (DRW_engine_external_use_for_image_editor()) {
+ if (DRW_engine_external_acquire_for_image_editor()) {
use_drw_engine(&draw_engine_external_type);
}
else {
diff --git a/source/blender/render/RE_engine.h b/source/blender/render/RE_engine.h
index 27ae19a9c96..b545f689bb2 100644
--- a/source/blender/render/RE_engine.h
+++ b/source/blender/render/RE_engine.h
@@ -76,6 +76,7 @@ extern "C" {
#define RE_ENGINE_DO_UPDATE 8
#define RE_ENGINE_RENDERING 16
#define RE_ENGINE_HIGHLIGHT_TILES 32
+#define RE_ENGINE_CAN_DRAW 64
extern ListBase R_engines;
@@ -245,7 +246,15 @@ bool RE_engine_use_persistent_data(struct RenderEngine *engine);
struct RenderEngine *RE_engine_get(const struct Render *re);
-bool RE_engine_is_rendering(const struct Render *re);
+/* Acquire render engine for drawing via its `draw()` callback.
+ *
+ * If drawing is not possible false is returned. If drawing is possible then the engine is
+ * "acquired" so that it can not be freed by the render pipeline.
+ *
+ * Drawing is possible if the engine has the `draw()` callback and it is in its `render()`
+ * callback. */
+bool RE_engine_draw_acquire(struct Render *re);
+void RE_engine_draw_release(struct Render *re);
/* NOTE: Only used for Cycles's BLenderGPUDisplay integration with the draw manager. A subject
* for re-consideration. Do not use this functionality. */
diff --git a/source/blender/render/intern/engine.c b/source/blender/render/intern/engine.c
index e84a625acd5..2eec4496768 100644
--- a/source/blender/render/intern/engine.c
+++ b/source/blender/render/intern/engine.c
@@ -892,8 +892,16 @@ static void engine_render_view_layer(Render *re,
DRW_render_context_enable(engine->re);
}
+ BLI_mutex_lock(&engine->re->engine_draw_mutex);
+ re->engine->flag |= RE_ENGINE_CAN_DRAW;
+ BLI_mutex_unlock(&engine->re->engine_draw_mutex);
+
engine->type->render(engine, engine->depsgraph);
+ BLI_mutex_lock(&engine->re->engine_draw_mutex);
+ re->engine->flag &= ~RE_ENGINE_CAN_DRAW;
+ BLI_mutex_unlock(&engine->re->engine_draw_mutex);
+
if (use_gpu_context) {
DRW_render_context_disable(engine->re);
}
@@ -1100,12 +1108,23 @@ struct RenderEngine *RE_engine_get(const Render *re)
return re->engine;
}
-bool RE_engine_is_rendering(const Render *re)
+bool RE_engine_draw_acquire(Render *re)
{
- if (re->engine == NULL) {
+ BLI_mutex_lock(&re->engine_draw_mutex);
+
+ RenderEngine *engine = re->engine;
+
+ if (engine == NULL || engine->type->draw == NULL || (engine->flag & RE_ENGINE_CAN_DRAW) == 0) {
+ BLI_mutex_unlock(&re->engine_draw_mutex);
return false;
}
- return re->engine->flag & RE_ENGINE_RENDERING;
+
+ return true;
+}
+
+void RE_engine_draw_release(Render *re)
+{
+ BLI_mutex_unlock(&re->engine_draw_mutex);
}
/* -------------------------------------------------------------------- */
diff --git a/source/blender/render/intern/pipeline.c b/source/blender/render/intern/pipeline.c
index 36b1fce651c..72ff920561d 100644
--- a/source/blender/render/intern/pipeline.c
+++ b/source/blender/render/intern/pipeline.c
@@ -567,6 +567,7 @@ Render *RE_NewRender(const char *name)
BLI_addtail(&RenderGlobal.renderlist, re);
BLI_strncpy(re->name, name, RE_MAXNAME);
BLI_rw_mutex_init(&re->resultmutex);
+ BLI_mutex_init(&re->engine_draw_mutex);
BLI_mutex_init(&re->highlighted_tiles_mutex);
}
@@ -631,6 +632,7 @@ void RE_FreeRender(Render *re)
}
BLI_rw_mutex_end(&re->resultmutex);
+ BLI_mutex_end(&re->engine_draw_mutex);
BLI_mutex_end(&re->highlighted_tiles_mutex);
BLI_freelistN(&re->view_layers);
diff --git a/source/blender/render/intern/render_types.h b/source/blender/render/intern/render_types.h
index 3243e440659..ca4f72350e1 100644
--- a/source/blender/render/intern/render_types.h
+++ b/source/blender/render/intern/render_types.h
@@ -71,6 +71,9 @@ struct Render {
* to not conflict with writes, so no lock used for that */
ThreadRWMutex resultmutex;
+ /* Guard for drawing render result using engine's `draw()` callback. */
+ ThreadMutex engine_draw_mutex;
+
/** Window size, display rect, viewplane.
* \note Buffer width and height with percentage applied
* without border & crop. convert to long before multiplying together to avoid overflow. */
More information about the Bf-blender-cvs
mailing list