Fix #128608: Vulkan: Sync issues when sharing context between threads

Resources are shared, when running multiple contexts on the same thread.
Cycles uses the same context on multiple threads and expected same resources.

This change will introduce a single render graph per context and an updated
resource management. Render graphs are not shared anymore; Resource pools
are still shared, but garbage collection depends on the thread and if
background rendering is used.

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

source/blender/gpu/vulkan/vk_backend.cc

@@ -375,7 +375,7 @@ Context *VKBackend::context_alloc(void *ghost_window, void *ghost_context)
     device.init(ghost_context);
   }
 
-  VKContext *context = new VKContext(ghost_window, ghost_context, device.current_thread_data());
+  VKContext *context = new VKContext(ghost_window, ghost_context, device.resources);
   device.context_register(*context);
   GHOST_SetVulkanSwapBuffersCallbacks((GHOST_ContextHandle)ghost_context,
                                       VKContext::swap_buffers_pre_callback,
@@ -455,20 +455,25 @@ void VKBackend::render_end()
   VKThreadData &thread_data = device.current_thread_data();
   thread_data.rendering_depth -= 1;
   BLI_assert_msg(thread_data.rendering_depth >= 0, "Unbalanced `GPU_render_begin/end`");
-
-  if (G.background || !BLI_thread_is_main()) {
-    /* When **not** running on the main thread (or doing background rendering) we assume that there
-     * is no swap chain in play. Rendering happens on a single thread and when finished all the
-     * resources have been used and are in a state that they can be discarded. It can still be that
-     * a non-main thread discards a resource that is in use by another thread. We move discarded
-     * resources to a device global discard pool (`device.orphaned_data`). The next time the main
-     * thread goes to the next swap chain image the device global discard pool will be added to the
-     * discard pool of the new swap chain image.*/
+  if (G.background) {
+    /* Garbage collection when performing background rendering. In this case the rendering is
+     * already 'thread-safe'. We move the resources to the device discard list and we destroy it
+     * the next frame. */
     if (thread_data.rendering_depth == 0) {
       VKResourcePool &resource_pool = thread_data.resource_pool_get();
-      resource_pool.discard_pool.destroy_discarded_resources(device);
+      device.orphaned_data.destroy_discarded_resources(device);
+      device.orphaned_data.move_data(resource_pool.discard_pool);
+      resource_pool.reset();
+    }
+  }
+
+  else if (!BLI_thread_is_main()) {
+    /* Foreground rendering using a worker/render thread. In this case we move the resources to the
+     * device discard list and it will be cleared by the main thread. */
+    if (thread_data.rendering_depth == 0) {
+      VKResourcePool &resource_pool = thread_data.resource_pool_get();
+      device.orphaned_data.move_data(resource_pool.discard_pool);
       resource_pool.reset();
-      resource_pool.discard_pool.move_data(device.orphaned_data);
     }
   }
 }

source/blender/gpu/vulkan/vk_context.cc

@@ -21,14 +21,15 @@
 
 namespace blender::gpu {
 
-VKContext::VKContext(void *ghost_window, void *ghost_context, VKThreadData &thread_data)
-    : thread_data_(thread_data), render_graph(thread_data_.render_graph)
+VKContext::VKContext(void *ghost_window,
+                     void *ghost_context,
+                     render_graph::VKResourceStateTracker &resources)
+    : render_graph(std::make_unique<render_graph::VKCommandBufferWrapper>(), resources)
 {
   ghost_window_ = ghost_window;
   ghost_context_ = ghost_context;
 
   state_manager = new VKStateManager();
-  imm = &thread_data.resource_pool_get().immediate;
 
   back_left = new VKFrameBuffer("back_left");
   front_left = new VKFrameBuffer("front_left");
@@ -57,9 +58,9 @@ void VKContext::sync_backbuffer()
   if (ghost_window_) {
     GHOST_VulkanSwapChainData swap_chain_data = {};
     GHOST_GetVulkanSwapChainFormat((GHOST_WindowHandle)ghost_window_, &swap_chain_data);
-    if (assign_if_different(thread_data_.resource_pool_index, swap_chain_data.swap_chain_index)) {
-      thread_data_.resource_pool_index = swap_chain_data.swap_chain_index;
-      VKResourcePool &resource_pool = thread_data_.resource_pool_get();
+    VKThreadData &thread_data = thread_data_.value().get();
+    if (assign_if_different(thread_data.resource_pool_index, swap_chain_data.swap_chain_index)) {
+      VKResourcePool &resource_pool = thread_data.resource_pool_get();
       imm = &resource_pool.immediate;
       resource_pool.discard_pool.destroy_discarded_resources(device);
       resource_pool.reset();
@@ -108,6 +109,12 @@ void VKContext::activate()
   /* Make sure no other context is already bound to this thread. */
   BLI_assert(is_active_ == false);
 
+  VKDevice &device = VKBackend::get().device;
+  VKThreadData &thread_data = device.current_thread_data();
+  thread_data_ = std::reference_wrapper<VKThreadData>(thread_data);
+
+  imm = &thread_data.resource_pool_get().immediate;
+
   is_active_ = true;
 
   sync_backbuffer();
@@ -117,21 +124,16 @@ void VKContext::activate()
 
 void VKContext::deactivate()
 {
-  rendering_end();
+  flush_render_graph();
   immDeactivate();
+  imm = nullptr;
+  thread_data_.reset();
   is_active_ = false;
 }
 
 void VKContext::begin_frame() {}
 
-void VKContext::end_frame()
-{
-  /* Enable this to track how resources are managed per thread and resource pool. */
-#if 0
-  VKDevice &device = VKBackend::get().device;
-  device.debug_print();
-#endif
-}
+void VKContext::end_frame() {}
 
 void VKContext::flush() {}
 
@@ -161,12 +163,12 @@ void VKContext::memory_statistics_get(int *r_total_mem_kb, int *r_free_mem_kb)
 
 VKDescriptorPools &VKContext::descriptor_pools_get()
 {
-  return thread_data_.resource_pool_get().descriptor_pools;
+  return thread_data_.value().get().resource_pool_get().descriptor_pools;
 }
 
 VKDescriptorSetTracker &VKContext::descriptor_set_get()
 {
-  return thread_data_.resource_pool_get().descriptor_set;
+  return thread_data_.value().get().resource_pool_get().descriptor_set;
 }
 
 VKStateManager &VKContext::state_manager_get() const

source/blender/gpu/vulkan/vk_context.hh

@@ -36,12 +36,14 @@ class VKContext : public Context, NonCopyable {
   /* Reusable data. Stored inside context to limit reallocations. */
   render_graph::VKResourceAccessInfo access_info_ = {};
 
-  VKThreadData &thread_data_;
+  std::optional<std::reference_wrapper<VKThreadData>> thread_data_;
 
  public:
-  render_graph::VKRenderGraph &render_graph;
+  render_graph::VKRenderGraph render_graph;
 
-  VKContext(void *ghost_window, void *ghost_context, VKThreadData &thread_data);
+  VKContext(void *ghost_window,
+            void *ghost_context,
+            render_graph::VKResourceStateTracker &resources);
   virtual ~VKContext();
 
   void activate() override;

source/blender/gpu/vulkan/vk_device.cc

@@ -333,11 +333,7 @@ std::string VKDevice::driver_version() const
 /** \name VKThreadData
  * \{ */
 
-VKThreadData::VKThreadData(VKDevice &device,
-                           pthread_t thread_id,
-                           std::unique_ptr<render_graph::VKCommandBufferInterface> command_buffer,
-                           render_graph::VKResourceStateTracker &resources)
-    : thread_id(thread_id), render_graph(std::move(command_buffer), resources)
+VKThreadData::VKThreadData(VKDevice &device, pthread_t thread_id) : thread_id(thread_id)
 {
   for (VKResourcePool &resource_pool : resource_pools) {
     resource_pool.init(device);
@@ -368,11 +364,7 @@ VKThreadData &VKDevice::current_thread_data()
     }
   }
 
-  VKThreadData *thread_data = new VKThreadData(
-      *this,
-      current_thread_id,
-      std::make_unique<render_graph::VKCommandBufferWrapper>(),
-      resources);
+  VKThreadData *thread_data = new VKThreadData(*this, current_thread_id);
   thread_data_.append(thread_data);
   return *thread_data;
 }
@@ -381,9 +373,11 @@ VKDiscardPool &VKDevice::discard_pool_for_current_thread()
 {
   std::scoped_lock mutex(resources.mutex);
   pthread_t current_thread_id = pthread_self();
-  for (VKThreadData *thread_data : thread_data_) {
-    if (pthread_equal(thread_data->thread_id, current_thread_id)) {
-      return thread_data->resource_pool_get().discard_pool;
+  if (BLI_thread_is_main()) {
+    for (VKThreadData *thread_data : thread_data_) {
+      if (pthread_equal(thread_data->thread_id, current_thread_id)) {
+        return thread_data->resource_pool_get().discard_pool;
+      }
     }
   }
 
@@ -393,19 +387,11 @@ VKDiscardPool &VKDevice::discard_pool_for_current_thread()
 void VKDevice::context_register(VKContext &context)
 {
   contexts_.append(std::reference_wrapper(context));
-  current_thread_data().num_contexts += 1;
 }
 
 void VKDevice::context_unregister(VKContext &context)
 {
   contexts_.remove(contexts_.first_index_of(std::reference_wrapper(context)));
-
-  auto &thread_data = current_thread_data();
-  thread_data.num_contexts -= 1;
-  BLI_assert(thread_data.num_contexts >= 0);
-  if (thread_data.num_contexts == 0) {
-    discard_pool_for_current_thread().destroy_discarded_resources(*this);
-  }
 }
 Span<std::reference_wrapper<VKContext>> VKDevice::contexts_get() const
 {
@@ -488,7 +474,6 @@ void VKDevice::debug_print()
     const bool is_main = pthread_equal(thread_data->thread_id, pthread_self());
     os << "ThreadData" << (is_main ? " (main-thread)" : "") << ")\n";
     os << " Rendering_depth: " << thread_data->rendering_depth << "\n";
-    os << " Number of contexts: " << thread_data->num_contexts << "\n";
     for (int resource_pool_index : IndexRange(thread_data->resource_pools.size())) {
       const VKResourcePool &resource_pool = thread_data->resource_pools[resource_pool_index];
       const bool is_active = thread_data->resource_pool_index == resource_pool_index;

source/blender/gpu/vulkan/vk_device.hh

@@ -71,7 +71,6 @@ class VKThreadData : public NonCopyable, NonMovable {
  public:
   /** Thread ID this instance belongs to. */
   pthread_t thread_id;
-  render_graph::VKRenderGraph render_graph;
   /**
    * Index of the active resource pool. Is in sync with the active swap chain image or cycled when
    * rendering.
@@ -91,16 +90,7 @@ class VKThreadData : public NonCopyable, NonMovable {
    */
   int32_t rendering_depth = 0;
 
-  /**
-   * Number of contexts registered in the current thread.
-   * Discarded resources are destroyed when all contexts are unregistered.
-   */
-  int32_t num_contexts = 0;
-
-  VKThreadData(VKDevice &device,
-               pthread_t thread_id,
-               std::unique_ptr<render_graph::VKCommandBufferInterface> command_buffer,
-               render_graph::VKResourceStateTracker &resources);
+  VKThreadData(VKDevice &device, pthread_t thread_id);
   void deinit(VKDevice &device);
 
   /**