Fix #141253: Bring back the global Draw lock

Alternative solution to #141392 / #141564.

As a recap, the DST global lock (which prevented running drawing code
from multiple threads concurrently) was removed for 4.5 (#134690).
One unforeseen issue is that Images (and their GPUTextures) are shared
across dependency graphs (and therefore multiple threads), meaning we
are running into data race issues with them.

@fclem did #141392 and I continued it #141564. However, this is only a
partial solution, parts of the GPUTexture API and the whole BKE_image
API are still unsafe.
Trying to solve all the possible underlying issues seems unrealistic for
4.5 given the time frame and that the extension of the code affected by
this issue is quite large.

So this PR just brings the 4.4 locking behavior instead, which, while
risky on its own, seems much safer to me than the alternative.

This effectively undoes the improvements from #134690 by disabling
concurrent rendering, but instead of reverting all the code, it just
ensures we hold the lock in the same places we did in 4.4.
This means there's some redundant code that is not technically needed
anymore, like the `submission_mutex`, but it's probably best to make as
few modifications as possible, given how close we are to release and
that this is only intended as a temporary measure.

Pull Request: https://projects.blender.org/blender/blender/pulls/141618

source/blender/draw/DRW_engine.hh

@@ -128,13 +128,20 @@ void DRW_render_gpencil(RenderEngine *engine, Depsgraph *depsgraph);
 void DRW_render_context_enable(Render *render);
 void DRW_render_context_disable(Render *render);
 
+void DRW_mutexes_init();
+void DRW_mutexes_exit();
+
+/* Mutex to lock the drw manager and avoid concurrent context usage.
+ * Equivalent to the old DST lock.
+ * Brought back to 4.5 due to unforeseen issues causing data races and race conditions with Images
+ * and GPUTextures. (See #141253) */
+void DRW_lock_start();
+void DRW_lock_end();
+
 /* Critical section for GPUShader usage. Can be removed when we have threadsafe GPUShader class. */
 void DRW_submission_start();
 void DRW_submission_end();
 
-void DRW_submission_mutex_init();
-void DRW_submission_mutex_exit();
-
 void DRW_gpu_context_create();
 void DRW_gpu_context_destroy();
 /**

source/blender/draw/engines/eevee/eevee_material.cc

@@ -207,13 +207,17 @@ void MaterialModule::end_sync()
     for (auto i : range) {
       GPUMaterialTexture *tex = texture_loading_queue_[i];
       ImageUser *iuser = tex->iuser_available ? &tex->iuser : nullptr;
-      BKE_image_tag_time(tex->ima);
       BKE_image_get_tile(tex->ima, 0);
       ImBuf *imbuf = BKE_image_acquire_ibuf(tex->ima, iuser, nullptr);
       BKE_image_release_ibuf(tex->ima, imbuf, nullptr);
     }
   });
 
+  /* Tag time is not thread-safe. */
+  for (GPUMaterialTexture *tex : texture_loading_queue_) {
+    BKE_image_tag_time(tex->ima);
+  }
+
   /* Upload to the GPU (create GPUTexture). This part still requires a valid GPU context and
    * is not easily parallelized. */
   for (GPUMaterialTexture *tex : texture_loading_queue_) {

source/blender/draw/intern/draw_gpu_context.cc

@@ -28,18 +28,33 @@
  * between render engine instances, we cannot allow pass submissions in a concurrent manner.
  * \{ */
 
+static TicketMutex *draw_mutex = nullptr;
 static TicketMutex *submission_mutex = nullptr;
 
-void DRW_submission_mutex_init()
+void DRW_mutexes_init()
 {
+  draw_mutex = BLI_ticket_mutex_alloc();
   submission_mutex = BLI_ticket_mutex_alloc();
 }
 
-void DRW_submission_mutex_exit()
+void DRW_mutexes_exit()
 {
+  BLI_ticket_mutex_free(draw_mutex);
   BLI_ticket_mutex_free(submission_mutex);
 }
 
+void DRW_lock_start()
+{
+  bool locked = BLI_ticket_mutex_lock_check_recursive(draw_mutex);
+  BLI_assert(locked);
+  UNUSED_VARS_NDEBUG(locked);
+}
+
+void DRW_lock_end()
+{
+  BLI_ticket_mutex_unlock(draw_mutex);
+}
+
 void DRW_submission_start()
 {
   bool locked = BLI_ticket_mutex_lock_check_recursive(submission_mutex);
@@ -95,6 +110,7 @@ class ContextShared {
 
   void enable()
   {
+    DRW_lock_start();
     /* IMPORTANT: We don't support immediate mode in render mode!
      * This shall remain in effect until immediate mode supports
      * multiple threads. */
@@ -124,6 +140,7 @@ class ContextShared {
     GPU_render_end();
 
     BLI_ticket_mutex_unlock(mutex_);
+    DRW_lock_end();
   }
 };
 
@@ -145,7 +162,7 @@ void DRW_gpu_context_create()
 {
   BLI_assert(viewport_context == nullptr); /* Ensure it's called once */
 
-  DRW_submission_mutex_init();
+  DRW_mutexes_init();
 
   viewport_context = MEM_new<ContextShared>(__func__);
   preview_context = MEM_new<ContextShared>(__func__);
@@ -164,7 +181,7 @@ void DRW_gpu_context_destroy()
   if (viewport_context == nullptr) {
     return;
   }
-  DRW_submission_mutex_exit();
+  DRW_mutexes_exit();
 
   MEM_SAFE_DELETE(viewport_context);
   MEM_SAFE_DELETE(preview_context);
@@ -237,12 +254,14 @@ void DRW_system_gpu_render_context_enable(void *re_system_gpu_context)
   /* If thread is main you should use DRW_gpu_context_enable(). */
   BLI_assert(!BLI_thread_is_main());
 
+  DRW_lock_start();
   WM_system_gpu_context_activate(re_system_gpu_context);
 }
 
 void DRW_system_gpu_render_context_disable(void *re_system_gpu_context)
 {
   WM_system_gpu_context_release(re_system_gpu_context);
+  DRW_lock_end();
 }
 
 void DRW_blender_gpu_render_context_enable(void *re_gpu_context)
@@ -346,6 +365,7 @@ void DRW_xr_drawing_begin()
 {
   /* XXX: See comment on #DRW_system_gpu_context_get(). */
 
+  DRW_lock_start();
   BLI_ticket_mutex_lock(viewport_context->mutex_);
 }
 
@@ -354,6 +374,7 @@ void DRW_xr_drawing_end()
   /* XXX: See comment on #DRW_system_gpu_context_get(). */
 
   BLI_ticket_mutex_unlock(viewport_context->mutex_);
+  DRW_lock_end();
 }
 
 #endif

source/blender/draw/tests/draw_testing.cc

@@ -14,12 +14,12 @@ namespace blender::draw {
 void DrawOpenGLTest::SetUp()
 {
   GPUOpenGLTest::SetUp();
-  DRW_submission_mutex_init();
+  DRW_mutexes_init();
 }
 
 void DrawOpenGLTest::TearDown()
 {
-  DRW_submission_mutex_exit();
+  DRW_mutexes_exit();
   GPUOpenGLTest::TearDown();
 }
 #endif
@@ -28,12 +28,12 @@ void DrawOpenGLTest::TearDown()
 void DrawMetalTest::SetUp()
 {
   GPUMetalTest::SetUp();
-  DRW_submission_mutex_init();
+  DRW_mutexes_init();
 }
 
 void DrawMetalTest::TearDown()
 {
-  DRW_submission_mutex_exit();
+  DRW_mutexes_exit();
   GPUMetalTest::TearDown();
 }
 #endif
@@ -42,12 +42,12 @@ void DrawMetalTest::TearDown()
 void DrawVulkanTest::SetUp()
 {
   GPUVulkanTest::SetUp();
-  DRW_submission_mutex_init();
+  DRW_mutexes_init();
 }
 
 void DrawVulkanTest::TearDown()
 {
-  DRW_submission_mutex_exit();
+  DRW_mutexes_exit();
   GPUVulkanTest::TearDown();
 }
 #endif