Fix: Properly free Vulkan interop handle for Cycles

Unlike OpenGL and Metal, this handle is not shared, but rather Cycles
has to take ownership of it. This required a fair amount of refactoring
to ensure the handle is closed, ownership is properly transferred, and
the handle is recreated once when the pixel buffer is modified.

intern/cycles/app/opengl/display_driver.cpp

@@ -77,7 +77,8 @@ bool OpenGLDisplayDriver::update_begin(const Params &params,
 
     /* Texture did change, and no pixel storage was provided. Tag for an explicit zeroing out to
      * avoid undefined content. */
-    texture_.need_clear = true;
+    texture_.need_zero = true;
+    graphics_interop_buffer_.clear();
   }
 
   /* Update PBO dimensions if needed.
@@ -128,13 +129,13 @@ half4 *OpenGLDisplayDriver::map_texture_buffer()
     LOG(ERROR) << "Error mapping OpenGLDisplayDriver pixel buffer object.";
   }
 
-  if (texture_.need_clear) {
+  if (texture_.need_zero) {
     const int64_t texture_width = texture_.width;
     const int64_t texture_height = texture_.height;
     memset(reinterpret_cast<void *>(mapped_rgba_pixels),
            0,
            texture_width * texture_height * sizeof(half4));
-    texture_.need_clear = false;
+    texture_.need_zero = false;
   }
 
   return mapped_rgba_pixels;
@@ -158,20 +159,19 @@ GraphicsInteropDevice OpenGLDisplayDriver::graphics_interop_get_device()
   return interop_device;
 }
 
-GraphicsInteropBuffer OpenGLDisplayDriver::graphics_interop_get_buffer()
+void OpenGLDisplayDriver::graphics_interop_update_buffer()
 {
-  GraphicsInteropBuffer interop_buffer;
+  if (graphics_interop_buffer_.is_empty()) {
+    graphics_interop_buffer_.assign(GraphicsInteropDevice::OPENGL,
+                                    texture_.gl_pbo_id,
+                                    texture_.buffer_width * texture_.buffer_height *
+                                        sizeof(half4));
+  }
 
-  interop_buffer.width = texture_.buffer_width;
-  interop_buffer.height = texture_.buffer_height;
-  interop_buffer.type = GraphicsInteropDevice::OPENGL;
-  interop_buffer.handle = texture_.gl_pbo_id;
-  interop_buffer.size = texture_.buffer_width * texture_.buffer_height * sizeof(half4);
-
-  interop_buffer.need_clear = texture_.need_clear;
-  texture_.need_clear = false;
-
-  return interop_buffer;
+  if (texture_.need_zero) {
+    graphics_interop_buffer_.zero();
+    texture_.need_zero = false;
+  }
 }
 
 void OpenGLDisplayDriver::graphics_interop_activate()
@@ -190,13 +190,13 @@ void OpenGLDisplayDriver::graphics_interop_deactivate()
 
 void OpenGLDisplayDriver::clear()
 {
-  texture_.need_clear = true;
+  texture_.need_zero = true;
 }
 
 void OpenGLDisplayDriver::draw(const Params &params)
 {
   /* See do_update_begin() for why no locking is required here. */
-  if (texture_.need_clear) {
+  if (texture_.need_zero) {
     /* Texture is requested to be cleared and was not yet cleared.
      * Do early return which should be equivalent of drawing all-zero texture. */
     return;
@@ -346,6 +346,7 @@ bool OpenGLDisplayDriver::gl_texture_resources_ensure()
 
   /* Creation finished with a success. */
   texture_.is_created = true;
+  graphics_interop_buffer_.clear();
 
   return true;
 }

intern/cycles/app/opengl/display_driver.h

@@ -40,7 +40,7 @@ class OpenGLDisplayDriver : public DisplayDriver {
   void unmap_texture_buffer() override;
 
   GraphicsInteropDevice graphics_interop_get_device() override;
-  GraphicsInteropBuffer graphics_interop_get_buffer() override;
+  void graphics_interop_update_buffer() override;
 
   void draw(const Params &params) override;
 
@@ -85,7 +85,7 @@ class OpenGLDisplayDriver : public DisplayDriver {
     bool need_update = false;
 
     /* Content of the texture is to be filled with zeroes. */
-    std::atomic<bool> need_clear = true;
+    std::atomic<bool> need_zero = true;
 
     /* Dimensions of the texture in pixels. */
     int width = 0;

intern/cycles/blender/display_driver.cpp

@@ -313,8 +313,10 @@ class DisplayGPUPixelBuffer {
     return *this;
   }
 
-  bool gpu_resources_ensure(const uint new_width, const uint new_height)
+  bool gpu_resources_ensure(const uint new_width, const uint new_height, bool &buffer_recreated)
   {
+    buffer_recreated = false;
+
     const size_t required_size = sizeof(half4) * new_width * new_height;
 
     /* Try to re-use the existing PBO if it has usable size. */
@@ -322,6 +324,7 @@ class DisplayGPUPixelBuffer {
       if (new_width != width || new_height != height ||
           GPU_pixel_buffer_size(gpu_pixel_buffer) < required_size)
       {
+        buffer_recreated = true;
         gpu_resources_destroy();
       }
     }
@@ -333,6 +336,7 @@ class DisplayGPUPixelBuffer {
     /* Create pixel buffer if not already created. */
     if (!gpu_pixel_buffer) {
       gpu_pixel_buffer = GPU_pixel_buffer_create(required_size);
+      buffer_recreated = true;
     }
 
     if (gpu_pixel_buffer == nullptr) {
@@ -473,7 +477,7 @@ void BlenderDisplayDriver::next_tile_begin()
 
   /* Moving to the next tile without giving render data for the current tile is not an expected
    * situation. */
-  DCHECK(!need_clear_);
+  DCHECK(!need_zero_);
   /* Texture should have been updated from the PBO at this point. */
   DCHECK(!tiles_->current_tile.need_update_texture_pixels);
 
@@ -504,9 +508,9 @@ bool BlenderDisplayDriver::update_begin(const Params &params,
   /* Clear storage of all finished tiles when display clear is requested.
    * Do it when new tile data is provided to handle the display clear flag in a single place.
    * It also makes the logic reliable from the whether drawing did happen or not point of view. */
-  if (need_clear_) {
+  if (need_zero_) {
     tiles_->finished_tiles.gl_resources_destroy_and_clear();
-    need_clear_ = false;
+    need_zero_ = false;
   }
 
   /* Update PBO dimensions if needed.
@@ -520,15 +524,22 @@ bool BlenderDisplayDriver::update_begin(const Params &params,
    * mode faster. */
   const int buffer_width = params.size.x;
   const int buffer_height = params.size.y;
+  bool interop_recreated = false;
 
-  if (!current_tile_buffer_object.gpu_resources_ensure(buffer_width, buffer_height) ||
+  if (!current_tile_buffer_object.gpu_resources_ensure(
+          buffer_width, buffer_height, interop_recreated) ||
       !current_tile.texture.gpu_resources_ensure(texture_width, texture_height))
   {
+    graphics_interop_buffer_.clear();
     tiles_->current_tile.gpu_resources_destroy();
     gpu_context_disable();
     return false;
   }
 
+  if (interop_recreated) {
+    graphics_interop_buffer_.clear();
+  }
+
   /* Store an updated parameters of the current tile.
    * In theory it is only needed once per update of the tile, but doing it on every update is
    * the easiest and is not expensive. */
@@ -648,36 +659,29 @@ GraphicsInteropDevice BlenderDisplayDriver::graphics_interop_get_device()
   return interop_device;
 }
 
-GraphicsInteropBuffer BlenderDisplayDriver::graphics_interop_get_buffer()
+void BlenderDisplayDriver::graphics_interop_update_buffer()
 {
-  GraphicsInteropBuffer interop_buffer;
+  if (graphics_interop_buffer_.is_empty()) {
+    GraphicsInteropDevice::Type type = GraphicsInteropDevice::NONE;
+    switch (GPU_backend_get_type()) {
+      case GPU_BACKEND_OPENGL:
+        type = GraphicsInteropDevice::OPENGL;
+        break;
+      case GPU_BACKEND_VULKAN:
+        type = GraphicsInteropDevice::VULKAN;
+        break;
+      case GPU_BACKEND_METAL:
+        type = GraphicsInteropDevice::METAL;
+        break;
+      case GPU_BACKEND_NONE:
+      case GPU_BACKEND_ANY:
+        break;
+    }
 
-  interop_buffer.width = tiles_->current_tile.buffer_object.width;
-  interop_buffer.height = tiles_->current_tile.buffer_object.height;
-
-  GPUPixelBufferNativeHandle handle = GPU_pixel_buffer_get_native_handle(
-      tiles_->current_tile.buffer_object.gpu_pixel_buffer);
-
-  switch (GPU_backend_get_type()) {
-    case GPU_BACKEND_OPENGL:
-      interop_buffer.type = GraphicsInteropDevice::OPENGL;
-      break;
-    case GPU_BACKEND_VULKAN:
-      interop_buffer.type = GraphicsInteropDevice::VULKAN;
-      break;
-    case GPU_BACKEND_METAL:
-      interop_buffer.type = GraphicsInteropDevice::METAL;
-      break;
-    case GPU_BACKEND_NONE:
-    case GPU_BACKEND_ANY:
-      interop_buffer.type = GraphicsInteropDevice::NONE;
-      break;
+    GPUPixelBufferNativeHandle handle = GPU_pixel_buffer_get_native_handle(
+        tiles_->current_tile.buffer_object.gpu_pixel_buffer);
+    graphics_interop_buffer_.assign(type, handle.handle, handle.size);
   }
-
-  interop_buffer.handle = handle.handle;
-  interop_buffer.size = handle.size;
-
-  return interop_buffer;
 }
 
 void BlenderDisplayDriver::graphics_interop_activate()
@@ -694,9 +698,9 @@ void BlenderDisplayDriver::graphics_interop_deactivate()
  * Drawing.
  */
 
-void BlenderDisplayDriver::clear()
+void BlenderDisplayDriver::zero()
 {
-  need_clear_ = true;
+  need_zero_ = true;
 }
 
 void BlenderDisplayDriver::set_zoom(const float zoom_x, const float zoom_y)
@@ -808,7 +812,7 @@ void BlenderDisplayDriver::draw(const Params &params)
 {
   gpu_context_lock();
 
-  if (need_clear_) {
+  if (need_zero_) {
     /* Texture is requested to be cleared and was not yet cleared.
      *
      * Do early return which should be equivalent of drawing all-zero texture.
@@ -936,6 +940,8 @@ void BlenderDisplayDriver::gpu_resources_destroy()
 
   display_shader_.reset();
 
+  graphics_interop_buffer_.clear();
+
   tiles_->current_tile.gpu_resources_destroy();
   tiles_->finished_tiles.gl_resources_destroy_and_clear();
 

intern/cycles/blender/display_driver.h

@@ -98,7 +98,7 @@ class BlenderDisplayDriver : public DisplayDriver {
   void graphics_interop_activate() override;
   void graphics_interop_deactivate() override;
 
-  void clear() override;
+  void zero() override;
 
   void set_zoom(const float zoom_x, const float zoom_y);
 
@@ -114,7 +114,7 @@ class BlenderDisplayDriver : public DisplayDriver {
   void unmap_texture_buffer() override;
 
   GraphicsInteropDevice graphics_interop_get_device() override;
-  GraphicsInteropBuffer graphics_interop_get_buffer() override;
+  void graphics_interop_update_buffer() override;
 
   void draw(const Params &params) override;
 
@@ -138,7 +138,7 @@ class BlenderDisplayDriver : public DisplayDriver {
   bool background_;
 
   /* Content of the display is to be filled with zeroes. */
-  std::atomic<bool> need_clear_ = true;
+  std::atomic<bool> need_zero_ = true;
 
   unique_ptr<BlenderDisplayShader> display_shader_;
 

intern/cycles/device/cuda/graphics_interop.cpp

@@ -9,6 +9,14 @@
 #  include "device/cuda/device_impl.h"
 #  include "device/cuda/util.h"
 
+#  include "session/display_driver.h"
+
+#  ifdef _WIN32
+#    include "util/windows.h"
+#  else
+#    include <unistd.h>
+#  endif
+
 CCL_NAMESPACE_BEGIN
 
 CUDADeviceGraphicsInterop::CUDADeviceGraphicsInterop(CUDADeviceQueue *queue)
@@ -22,58 +30,65 @@ CUDADeviceGraphicsInterop::~CUDADeviceGraphicsInterop()
   free();
 }
 
-void CUDADeviceGraphicsInterop::set_buffer(const GraphicsInteropBuffer &interop_buffer)
+void CUDADeviceGraphicsInterop::set_buffer(GraphicsInteropBuffer &interop_buffer)
 {
-  const int64_t new_buffer_area = int64_t(interop_buffer.width) * interop_buffer.height;
+  CUDAContextScope scope(device_);
 
-  assert(interop_buffer.size >= interop_buffer.width * interop_buffer.height * sizeof(half4));
-
-  need_clear_ = interop_buffer.need_clear;
-
-  if (!interop_buffer.need_recreate) {
-    if (native_type_ == interop_buffer.type && native_handle_ == interop_buffer.handle &&
-        native_size_ == interop_buffer.size && buffer_area_ == new_buffer_area)
-    {
-      return;
-    }
+  if (interop_buffer.is_empty()) {
+    free();
+    return;
   }
 
-  CUDAContextScope scope(device_);
+  need_zero_ |= interop_buffer.take_zero();
+
+  if (!interop_buffer.has_new_handle()) {
+    return;
+  }
   free();
 
-  native_type_ = interop_buffer.type;
-  native_handle_ = interop_buffer.handle;
-  native_size_ = interop_buffer.size;
-  buffer_area_ = new_buffer_area;
-
-  switch (interop_buffer.type) {
+  switch (interop_buffer.get_type()) {
     case GraphicsInteropDevice::OPENGL: {
-      const CUresult result = cuGraphicsGLRegisterBuffer(
-          &cu_graphics_resource_, interop_buffer.handle, CU_GRAPHICS_MAP_RESOURCE_FLAGS_NONE);
+      const CUresult result = cuGraphicsGLRegisterBuffer(&cu_graphics_resource_,
+                                                         interop_buffer.take_handle(),
+                                                         CU_GRAPHICS_MAP_RESOURCE_FLAGS_NONE);
       if (result != CUDA_SUCCESS) {
         LOG(ERROR) << "Error registering OpenGL buffer: " << cuewErrorString(result);
+        break;
       }
+
+      buffer_size_ = interop_buffer.get_size();
       break;
     }
     case GraphicsInteropDevice::VULKAN: {
       CUDA_EXTERNAL_MEMORY_HANDLE_DESC external_memory_handle_desc = {};
 #  ifdef _WIN32
+      /* cuImportExternalMemory will not take ownership of the handle. */
+      vulkan_windows_handle_ = interop_buffer.take_handle();
       external_memory_handle_desc.type = CU_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32;
       external_memory_handle_desc.handle.win32.handle = reinterpret_cast<void *>(
-          interop_buffer.handle);
+          vulkan_windows_handle_);
 #  else
+      /* cuImportExternalMemory will take ownership of the handle. */
       external_memory_handle_desc.type = CU_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD;
-      external_memory_handle_desc.handle.fd = interop_buffer.handle;
+      external_memory_handle_desc.handle.fd = interop_buffer.take_handle();
 #  endif
-      external_memory_handle_desc.size = interop_buffer.size;
+      external_memory_handle_desc.size = interop_buffer.get_size();
 
       const CUresult result = cuImportExternalMemory(&cu_external_memory_,
                                                      &external_memory_handle_desc);
       if (result != CUDA_SUCCESS) {
+#  ifdef _WIN32
+        CloseHandle(HANDLE(vulkan_windows_handle_));
+        vulkan_windows_handle_ = 0;
+#  else
+        close(external_memory_handle_desc.handle.fd);
+#  endif
         LOG(ERROR) << "Error importing Vulkan memory: " << cuewErrorString(result);
         break;
       }
 
+      buffer_size_ = interop_buffer.get_size();
+
       CUDA_EXTERNAL_MEMORY_BUFFER_DESC external_memory_buffer_desc = {};
       external_memory_buffer_desc.size = external_memory_handle_desc.size;
       external_memory_buffer_desc.offset = 0;
@@ -111,11 +126,10 @@ device_ptr CUDADeviceGraphicsInterop::map()
     cu_buffer = cu_external_memory_ptr_;
   }
 
-  if (cu_buffer && need_clear_) {
-    cuda_device_assert(
-        device_, cuMemsetD8Async(cu_buffer, 0, buffer_area_ * sizeof(half4), queue_->stream()));
+  if (cu_buffer && need_zero_) {
+    cuda_device_assert(device_, cuMemsetD8Async(cu_buffer, 0, buffer_size_, queue_->stream()));
 
-    need_clear_ = false;
+    need_zero_ = false;
   }
 
   return static_cast<device_ptr>(cu_buffer);
@@ -144,6 +158,17 @@ void CUDADeviceGraphicsInterop::free()
   }
 
   cu_external_memory_ptr_ = 0;
+
+#  ifdef _WIN32
+  if (vulkan_windows_handle_) {
+    CloseHandle(HANDLE(vulkan_windows_handle_));
+    vulkan_windows_handle_ = 0;
+  }
+#  endif
+
+  buffer_size_ = 0;
+
+  need_zero_ = false;
 }
 
 CCL_NAMESPACE_END

intern/cycles/device/cuda/graphics_interop.h

@@ -30,7 +30,7 @@ class CUDADeviceGraphicsInterop : public DeviceGraphicsInterop {
   CUDADeviceGraphicsInterop &operator=(const CUDADeviceGraphicsInterop &other) = delete;
   CUDADeviceGraphicsInterop &operator=(CUDADeviceGraphicsInterop &&other) = delete;
 
-  void set_buffer(const GraphicsInteropBuffer &interop_buffer) override;
+  void set_buffer(GraphicsInteropBuffer &interop_buffer) override;
 
   device_ptr map() override;
   void unmap() override;
@@ -39,22 +39,22 @@ class CUDADeviceGraphicsInterop : public DeviceGraphicsInterop {
   CUDADeviceQueue *queue_ = nullptr;
   CUDADevice *device_ = nullptr;
 
-  /* Native handle. */
-  GraphicsInteropDevice::Type native_type_ = GraphicsInteropDevice::NONE;
-  int64_t native_handle_ = 0;
-  size_t native_size_ = 0;
-
-  /* Buffer area in pixels of the corresponding PBO. */
-  int64_t buffer_area_ = 0;
+  /* Size of the buffer in bytes. */
+  size_t buffer_size_ = 0;
 
   /* The destination was requested to be cleared. */
-  bool need_clear_ = false;
+  bool need_zero_ = false;
 
   /* CUDA resources. */
   CUgraphicsResource cu_graphics_resource_ = nullptr;
   CUexternalMemory cu_external_memory_ = nullptr;
   CUdeviceptr cu_external_memory_ptr_ = 0;
 
+  /* Vulkan handle to free. */
+#  ifdef _WIN32
+  int64_t vulkan_windows_handle_ = 0;
+#  endif
+
   void free();
 };
 

intern/cycles/device/graphics_interop.h

@@ -21,7 +21,7 @@ class DeviceGraphicsInterop {
 
   /* Update this device-side graphics interoperability buffer with the given destination
    * resource information. */
-  virtual void set_buffer(const GraphicsInteropBuffer &interop_buffer) = 0;
+  virtual void set_buffer(GraphicsInteropBuffer &interop_buffer) = 0;
 
   virtual device_ptr map() = 0;
   virtual void unmap() = 0;

intern/cycles/device/hip/graphics_interop.cpp

@@ -22,38 +22,35 @@ HIPDeviceGraphicsInterop::~HIPDeviceGraphicsInterop()
   free();
 }
 
-void HIPDeviceGraphicsInterop::set_buffer(const GraphicsInteropBuffer &interop_buffer)
+void HIPDeviceGraphicsInterop::set_buffer(GraphicsInteropBuffer &interop_buffer)
 {
-  const int64_t new_buffer_area = int64_t(interop_buffer.width) * interop_buffer.height;
+  HIPContextScope scope(device_);
 
-  assert(interop_buffer.size >= interop_buffer.width * interop_buffer.height * sizeof(half4));
-
-  need_clear_ = interop_buffer.need_clear;
-
-  if (!interop_buffer.need_recreate) {
-    if (native_type_ == interop_buffer.type && native_handle_ == interop_buffer.handle &&
-        native_size_ == interop_buffer.size && buffer_area_ == new_buffer_area)
-    {
-      return;
-    }
+  if (interop_buffer.is_empty()) {
+    free();
+    return;
+  }
+
+  need_zero_ |= interop_buffer.take_zero();
+
+  if (!interop_buffer.has_new_handle()) {
+    return;
   }
 
-  HIPContextScope scope(device_);
   free();
 
-  native_type_ = interop_buffer.type;
-  native_handle_ = interop_buffer.handle;
-  native_size_ = interop_buffer.size;
-  buffer_area_ = new_buffer_area;
-
-  switch (interop_buffer.type) {
+  switch (interop_buffer.get_type()) {
     case GraphicsInteropDevice::OPENGL: {
       const hipError_t result = hipGraphicsGLRegisterBuffer(
-          &hip_graphics_resource_, interop_buffer.handle, hipGraphicsRegisterFlagsNone);
+          &hip_graphics_resource_, interop_buffer.take_handle(), hipGraphicsRegisterFlagsNone);
 
       if (result != hipSuccess) {
         LOG(ERROR) << "Error registering OpenGL buffer: " << hipewErrorString(result);
+        break;
       }
+
+      buffer_size_ = interop_buffer.get_size();
+
       break;
     }
     case GraphicsInteropDevice::VULKAN:
@@ -82,11 +79,10 @@ device_ptr HIPDeviceGraphicsInterop::map()
     hip_buffer = hip_external_memory_ptr_;
   }
 
-  if (hip_buffer && need_clear_) {
-    hip_device_assert(
-        device_, hipMemsetD8Async(hip_buffer, 0, buffer_area_ * sizeof(half4), queue_->stream()));
+  if (hip_buffer && need_zero_) {
+    hip_device_assert(device_, hipMemsetD8Async(hip_buffer, 0, buffer_size_, queue_->stream()));
 
-    need_clear_ = false;
+    need_zero_ = false;
   }
 
   return static_cast<device_ptr>(hip_buffer);
@@ -110,6 +106,10 @@ void HIPDeviceGraphicsInterop::free()
   }
 
   hip_external_memory_ptr_ = 0;
+
+  buffer_size_ = 0;
+
+  need_zero_ = false;
 }
 
 CCL_NAMESPACE_END

intern/cycles/device/hip/graphics_interop.h

@@ -28,7 +28,7 @@ class HIPDeviceGraphicsInterop : public DeviceGraphicsInterop {
   HIPDeviceGraphicsInterop &operator=(const HIPDeviceGraphicsInterop &other) = delete;
   HIPDeviceGraphicsInterop &operator=(HIPDeviceGraphicsInterop &&other) = delete;
 
-  void set_buffer(const GraphicsInteropBuffer &interop_buffer) override;
+  void set_buffer(GraphicsInteropBuffer &interop_buffer) override;
 
   device_ptr map() override;
   void unmap() override;
@@ -37,16 +37,11 @@ class HIPDeviceGraphicsInterop : public DeviceGraphicsInterop {
   HIPDeviceQueue *queue_ = nullptr;
   HIPDevice *device_ = nullptr;
 
-  /* Native handle. */
-  GraphicsInteropDevice::Type native_type_ = GraphicsInteropDevice::NONE;
-  int64_t native_handle_ = 0;
-  size_t native_size_ = 0;
-
-  /* Buffer area in pixels of the corresponding PBO. */
-  int64_t buffer_area_ = 0;
+  /* Size of the buffer in bytes. */
+  size_t buffer_size_ = 0;
 
   /* The destination was requested to be cleared. */
-  bool need_clear_ = false;
+  bool need_zero_ = false;
 
   hipGraphicsResource hip_graphics_resource_ = nullptr;
   hipDeviceptr_t hip_external_memory_ptr_ = 0;

intern/cycles/device/metal/graphics_interop.h

@@ -26,7 +26,7 @@ class MetalDeviceGraphicsInterop : public DeviceGraphicsInterop {
   MetalDeviceGraphicsInterop &operator=(const MetalDeviceGraphicsInterop &other) = delete;
   MetalDeviceGraphicsInterop &operator=(MetalDeviceGraphicsInterop &&other) = delete;
 
-  void set_buffer(const GraphicsInteropBuffer &interop_buffer) override;
+  void set_buffer(GraphicsInteropBuffer &interop_buffer) override;
 
   device_ptr map() override;
   void unmap() override;
@@ -40,7 +40,7 @@ class MetalDeviceGraphicsInterop : public DeviceGraphicsInterop {
   size_t size_ = 0;
 
   /* The destination was requested to be cleared. */
-  bool need_clear_ = false;
+  bool need_zero_ = false;
 };
 
 CCL_NAMESPACE_END

intern/cycles/device/metal/graphics_interop.mm

@@ -17,21 +17,29 @@ MetalDeviceGraphicsInterop::MetalDeviceGraphicsInterop(MetalDeviceQueue *queue)
 
 MetalDeviceGraphicsInterop::~MetalDeviceGraphicsInterop() = default;
 
-void MetalDeviceGraphicsInterop::set_buffer(const GraphicsInteropBuffer &interop_buffer)
+void MetalDeviceGraphicsInterop::set_buffer(GraphicsInteropBuffer &interop_buffer)
 {
-  /* Trivial implementation due to unified memory. */
-  if (interop_buffer.type == GraphicsInteropDevice::METAL) {
-    need_clear_ |= interop_buffer.need_clear;
-    mem_.mtlBuffer = reinterpret_cast<id<MTLBuffer>>(interop_buffer.handle);
-    size_ = interop_buffer.width * interop_buffer.height * sizeof(half4);
+  if (interop_buffer.is_empty()) {
+    mem_.mtlBuffer = nullptr;
+    size_ = 0;
+    return;
   }
+
+  need_zero_ |= interop_buffer.take_zero();
+
+  if (!interop_buffer.has_new_handle()) {
+    return;
+  }
+
+  mem_.mtlBuffer = reinterpret_cast<id<MTLBuffer>>(interop_buffer.take_handle());
+  size_ = interop_buffer.get_size();
 }
 
 device_ptr MetalDeviceGraphicsInterop::map()
 {
-  if (mem_.mtlBuffer && need_clear_) {
+  if (mem_.mtlBuffer && need_zero_) {
     memset([mem_.mtlBuffer contents], 0, size_);
-    need_clear_ = false;
+    need_zero_ = false;
   }
 
   return device_ptr(&mem_);

intern/cycles/hydra/display_driver.cpp

@@ -72,6 +72,7 @@ void HdCyclesDisplayDriver::gl_context_create()
 
   if (!gl_pbo_id_) {
     glGenBuffers(1, &gl_pbo_id_);
+    graphics_interop_buffer_.clear();
   }
 }
 
@@ -142,6 +143,7 @@ bool HdCyclesDisplayDriver::update_begin(const Params &params,
     glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
 
     pbo_size_ = params.full_size;
+    graphics_interop_buffer_.clear();
   }
 
   need_update_ = true;
@@ -179,9 +181,9 @@ half4 *HdCyclesDisplayDriver::map_texture_buffer()
   auto *const mapped_rgba_pixels = static_cast<half4 *>(
       glMapBuffer(GL_PIXEL_UNPACK_BUFFER, GL_WRITE_ONLY));
 
-  if (need_clear_ && mapped_rgba_pixels) {
+  if (need_zero_ && mapped_rgba_pixels) {
     memset(mapped_rgba_pixels, 0, sizeof(half4) * pbo_size_.x * pbo_size_.y);
-    need_clear_ = false;
+    need_zero_ = false;
   }
 
   return mapped_rgba_pixels;
@@ -201,20 +203,17 @@ GraphicsInteropDevice HdCyclesDisplayDriver::graphics_interop_get_device()
   return interop_device;
 }
 
-GraphicsInteropBuffer HdCyclesDisplayDriver::graphics_interop_get_buffer()
+void HdCyclesDisplayDriver::graphics_interop_update_buffer()
 {
-  GraphicsInteropBuffer interop_buffer;
+  if (graphics_interop_buffer_.is_empty()) {
+    graphics_interop_buffer_.assign(
+        GraphicsInteropDevice::OPENGL, gl_pbo_id_, pbo_size_.x * pbo_size_.y * sizeof(half4));
+  }
 
-  interop_buffer.width = pbo_size_.x;
-  interop_buffer.height = pbo_size_.y;
-  interop_buffer.type = GraphicsInteropDevice::OPENGL;
-  interop_buffer.handle = gl_pbo_id_;
-  interop_buffer.size = pbo_size_.x * pbo_size_.y * sizeof(half4);
-
-  interop_buffer.need_clear = need_clear_;
-  need_clear_ = false;
-
-  return interop_buffer;
+  if (need_zero_) {
+    graphics_interop_buffer_.zero();
+    need_zero_ = false;
+  }
 }
 
 void HdCyclesDisplayDriver::graphics_interop_activate()
@@ -227,9 +226,9 @@ void HdCyclesDisplayDriver::graphics_interop_deactivate()
   gl_context_disable();
 }
 
-void HdCyclesDisplayDriver::clear()
+void HdCyclesDisplayDriver::zero()
 {
-  need_clear_ = true;
+  need_zero_ = true;
 }
 
 void HdCyclesDisplayDriver::draw(const Params &params)

intern/cycles/hydra/display_driver.h

@@ -33,12 +33,12 @@ class HdCyclesDisplayDriver final : public CCL_NS::DisplayDriver {
   void unmap_texture_buffer() override;
 
   GraphicsInteropDevice graphics_interop_get_device() override;
-  GraphicsInteropBuffer graphics_interop_get_buffer() override;
+  void graphics_interop_update_buffer() override;
 
   void graphics_interop_activate() override;
   void graphics_interop_deactivate() override;
 
-  void clear() override;
+  void zero() override;
 
   void draw(const Params &params) override;
 
@@ -60,7 +60,8 @@ class HdCyclesDisplayDriver final : public CCL_NS::DisplayDriver {
   unsigned int gl_pbo_id_ = 0;
   CCL_NS::int2 pbo_size_ = CCL_NS::make_int2(0, 0);
   bool need_update_ = false;
-  std::atomic_bool need_clear_ = false;
+  std::atomic_bool need_zero_ = false;
+  std::atomic_bool need_recreate_interop_ = false;
 
   void *gl_render_sync_ = nullptr;
   void *gl_upload_sync_ = nullptr;

intern/cycles/integrator/path_trace.cpp

@@ -654,10 +654,10 @@ void PathTrace::set_display_driver(unique_ptr<DisplayDriver> driver)
   }
 }
 
-void PathTrace::clear_display()
+void PathTrace::zero_display()
 {
   if (display_) {
-    display_->clear();
+    display_->zero();
   }
 }
 

intern/cycles/integrator/path_trace.h

@@ -107,7 +107,7 @@ class PathTrace {
   void set_display_driver(unique_ptr<DisplayDriver> driver);
 
   /* Clear the display buffer by filling it in with all zeroes. */
-  void clear_display();
+  void zero_display();
 
   /* Perform drawing of the current state of the DisplayDriver. */
   void draw();

intern/cycles/integrator/path_trace_display.cpp

@@ -5,6 +5,7 @@
 #include "integrator/path_trace_display.h"
 
 #include "session/buffers.h"
+#include "session/display_driver.h"
 
 #include "util/log.h"
 
@@ -191,26 +192,30 @@ GraphicsInteropDevice PathTraceDisplay::graphics_interop_get_device()
   return driver_->graphics_interop_get_device();
 }
 
-GraphicsInteropBuffer PathTraceDisplay::graphics_interop_get_buffer()
+GraphicsInteropBuffer &PathTraceDisplay::graphics_interop_get_buffer()
 {
+  GraphicsInteropBuffer &interop_buffer = driver_->graphics_interop_get_buffer();
   DCHECK(!texture_buffer_state_.is_mapped);
   DCHECK(update_state_.is_active);
 
   if (texture_buffer_state_.is_mapped) {
     LOG(ERROR)
         << "Attempt to use graphics interoperability mode while the texture buffer is mapped.";
-    return GraphicsInteropBuffer();
+    interop_buffer.clear();
+    return interop_buffer;
   }
 
   if (!update_state_.is_active) {
     LOG(ERROR) << "Attempt to use graphics interoperability outside of PathTraceDisplay update.";
-    return GraphicsInteropBuffer();
+    interop_buffer.clear();
+    return interop_buffer;
   }
 
   /* Assume that interop will write new values to the texture. */
   mark_texture_updated();
 
-  return driver_->graphics_interop_get_buffer();
+  driver_->graphics_interop_update_buffer();
+  return interop_buffer;
 }
 
 void PathTraceDisplay::graphics_interop_activate()
@@ -227,9 +232,9 @@ void PathTraceDisplay::graphics_interop_deactivate()
  * Drawing.
  */
 
-void PathTraceDisplay::clear()
+void PathTraceDisplay::zero()
 {
-  driver_->clear();
+  driver_->zero();
 }
 
 bool PathTraceDisplay::draw()

intern/cycles/integrator/path_trace_display.h

@@ -116,7 +116,7 @@ class PathTraceDisplay {
   /* Get PathTraceDisplay graphics interoperability information which acts as a destination for the
    * device API. */
   GraphicsInteropDevice graphics_interop_get_device();
-  GraphicsInteropBuffer graphics_interop_get_buffer();
+  GraphicsInteropBuffer &graphics_interop_get_buffer();
 
   /* (De)activate GPU display for graphics interoperability outside of regular display update
    * routines. */
@@ -138,7 +138,7 @@ class PathTraceDisplay {
    *
    * If the GPU display supports graphics interoperability then the zeroing the display is to be
    * delegated to the device via the `GraphicsInterop`. */
-  void clear();
+  void zero();
 
   /* Draw the current state of the texture.
    *

intern/cycles/integrator/path_trace_work_gpu.cpp

@@ -1041,7 +1041,7 @@ bool PathTraceWorkGPU::copy_to_display_interop(PathTraceDisplay *display,
     device_graphics_interop_ = queue_->graphics_interop_create();
   }
 
-  const GraphicsInteropBuffer interop_buffer = display->graphics_interop_get_buffer();
+  GraphicsInteropBuffer &interop_buffer = display->graphics_interop_get_buffer();
   device_graphics_interop_->set_buffer(interop_buffer);
 
   const device_ptr d_rgba_half = device_graphics_interop_->map();

intern/cycles/session/CMakeLists.txt

@@ -11,6 +11,7 @@ set(INC_SYS
 
 set(SRC
   buffers.cpp
+  display_driver.cpp
   denoising.cpp
   merge.cpp
   session.cpp

intern/cycles/session/display_driver.cpp

@@ -0,0 +1,86 @@
+/* SPDX-FileCopyrightText: 2021-2025 Blender Foundation
+ *
+ * SPDX-License-Identifier: Apache-2.0 */
+
+#include "session/display_driver.h"
+
+#ifdef _WIN32
+#  include "util/windows.h"
+#else
+#  include <unistd.h>
+#endif
+
+CCL_NAMESPACE_BEGIN
+
+GraphicsInteropBuffer::~GraphicsInteropBuffer()
+{
+  clear();
+}
+
+void GraphicsInteropBuffer::assign(GraphicsInteropDevice::Type type, int64_t handle, size_t size)
+{
+  clear();
+
+  type_ = type;
+  handle_ = handle;
+  own_handle_ = true;
+  size_ = size;
+}
+
+bool GraphicsInteropBuffer::is_empty() const
+{
+  return handle_ == 0;
+}
+
+void GraphicsInteropBuffer::zero()
+{
+  need_zero_ = true;
+}
+
+void GraphicsInteropBuffer::clear()
+{
+  if (type_ == GraphicsInteropDevice::VULKAN && handle_ && own_handle_) {
+#ifdef _WIN32
+    CloseHandle(HANDLE(handle_));
+#else
+    close(handle_);
+#endif
+  }
+
+  type_ = GraphicsInteropDevice::NONE;
+  handle_ = 0;
+  size_ = 0;
+  need_zero_ = false;
+  own_handle_ = false;
+}
+
+GraphicsInteropDevice::Type GraphicsInteropBuffer::get_type() const
+{
+  return type_;
+}
+
+size_t GraphicsInteropBuffer::get_size() const
+{
+  return size_;
+}
+
+bool GraphicsInteropBuffer::has_new_handle() const
+{
+  return own_handle_;
+}
+
+bool GraphicsInteropBuffer::take_zero()
+{
+  bool need_zero = need_zero_;
+  need_zero_ = false;
+  return need_zero;
+}
+
+int64_t GraphicsInteropBuffer::take_handle()
+{
+  assert(own_handle_);
+  own_handle_ = false;
+  return handle_;
+}
+
+CCL_NAMESPACE_END

intern/cycles/session/display_driver.h

@@ -33,34 +33,55 @@ class GraphicsInteropDevice {
  * with RGBA channels. */
 class GraphicsInteropBuffer {
  public:
-  /* Dimensions of the buffer, in pixels. */
-  int width = 0;
-  int height = 0;
+  GraphicsInteropBuffer() = default;
+  ~GraphicsInteropBuffer();
 
+  GraphicsInteropBuffer(const GraphicsInteropBuffer &other) = delete;
+  GraphicsInteropBuffer &operator=(const GraphicsInteropBuffer &other) = delete;
+  GraphicsInteropBuffer(GraphicsInteropBuffer &&other) = delete;
+  GraphicsInteropBuffer &operator=(GraphicsInteropBuffer &&other) = delete;
+
+  /* Display Driver API. */
+
+  /* Assign handle. For Vulkan, this transfers ownership of the handle. */
+  void assign(GraphicsInteropDevice::Type type, int64_t handle, size_t size);
+  /* Is a handle assigned? */
+  bool is_empty() const;
+  /* Zero memory. */
+  void zero();
+  /* Clear handle. */
+  void clear();
+
+  /* Device graphics interop API. */
+
+  /* Get type of handle. */
+  GraphicsInteropDevice::Type get_type() const;
+  /* Get size of buffer. */
+  size_t get_size() const;
+
+  /* Is there a new handle to take ownership of? */
+  bool has_new_handle() const;
+  /* Take ownership of the handle. */
+  int64_t take_handle();
+
+  /* Take ownership of zeroing the buffer. */
+  bool take_zero();
+
+ protected:
   /* The handle is expected to be:
    * - OpenGL: pixel buffer object ID.
    * - Vulkan on Windows: opaque handle for VkBuffer.
    * - Vulkan on Unix: opaque file descriptor for VkBuffer.
    * - Metal: MTLBuffer with unified memory. */
-  GraphicsInteropDevice::Type type = GraphicsInteropDevice::NONE;
-  int64_t handle = 0;
+  GraphicsInteropDevice::Type type_ = GraphicsInteropDevice::NONE;
+  int64_t handle_ = 0;
+  bool own_handle_ = false;
 
-  /* Actual size of the memory, which must be `>= width * height sizeof(half4)`. */
-  size_t size = 0;
+  /* Actual size of the memory, which must be `>= width * height * sizeof(half4)`. */
+  size_t size_ = 0;
 
   /* Clear the entire buffer before doing partial write to it. */
-  bool need_clear = false;
-
-  /* Enforce re-creation of the graphics interop object.
-   *
-   * When this field is true then the graphics interop will be re-created no matter what the
-   * rest of the configuration is.
-   * When this field is false the graphics interop will be re-created if the PBO or buffer size
-   * did change.
-   *
-   * This allows to ensure graphics interop is re-created when there is a possibility that an
-   * underlying PBO was re-allocated but did not change its ID. */
-  bool need_recreate = false;
+  bool need_zero_ = false;
 };
 
 /* Display driver for efficient interactive display of renders.
@@ -130,14 +151,19 @@ class DisplayDriver {
   virtual half4 *map_texture_buffer() = 0;
   virtual void unmap_texture_buffer() = 0;
 
+  GraphicsInteropBuffer graphics_interop_buffer_;
+
+  /* Graphics interop to avoid CPU - GPU transfer. See GraphicsInteropBuffer for details. */
   virtual GraphicsInteropDevice graphics_interop_get_device()
   {
     return GraphicsInteropDevice();
   }
 
-  virtual GraphicsInteropBuffer graphics_interop_get_buffer()
+  virtual void graphics_interop_update_buffer() {}
+
+  GraphicsInteropBuffer &graphics_interop_get_buffer()
   {
-    return GraphicsInteropBuffer();
+    return graphics_interop_buffer_;
   }
 
   /* (De)activate graphics context required for editing or deleting the graphics interop
@@ -149,7 +175,7 @@ class DisplayDriver {
   virtual void graphics_interop_deactivate(){};
 
   /* Clear the display buffer by filling it with zeros. */
-  virtual void clear() = 0;
+  virtual void zero() = 0;
 
   /* Draw the render using the native graphics API.
    *

intern/cycles/session/session.cpp

@@ -159,7 +159,7 @@ bool Session::ready_to_reset()
 
 void Session::run_main_render_loop()
 {
-  path_trace_->clear_display();
+  path_trace_->zero_display();
 
   while (true) {
     RenderWork render_work = run_update_for_next_iteration();

source/blender/gpu/GPU_texture.hh

@@ -1357,6 +1357,13 @@ size_t GPU_pixel_buffer_size(GPUPixelBuffer *pixel_buf);
 
 /**
  * Return the native handle of the \a pix_buf to use for graphic interoperability registration.
+ *
+ * - OpenGL: pixel buffer object ID.
+ * - Vulkan on Windows: opaque handle for VkBuffer.
+ * - Vulkan on Unix: opaque file descriptor for VkBuffer.
+ * - Metal: MTLBuffer with unified memory.
+ *
+ * For Vulkan, the caller is responsible for closing the handle.
  */
 struct GPUPixelBufferNativeHandle {
   int64_t handle = 0;

source/blender/gpu/vulkan/vk_pixel_buffer.cc

@@ -40,7 +40,6 @@ void VKPixelBuffer::create(bool memory_export)
 
   buffer_initialized_ = true;
   buffer_memory_export_ = memory_export;
-  native_handle_ = GPUPixelBufferNativeHandle{};
 }
 
 void *VKPixelBuffer::map()
@@ -57,16 +56,13 @@ void VKPixelBuffer::unmap()
 
 GPUPixelBufferNativeHandle VKPixelBuffer::get_native_handle()
 {
-  /* Initialize once. */
-  if (buffer_initialized_) {
-    return native_handle_;
-  }
-
   VKDevice &device = VKBackend::get().device;
 
+  GPUPixelBufferNativeHandle native_handle;
+
   /* Functionality supported? */
   if (!device.extensions_get().external_memory) {
-    return native_handle_;
+    return native_handle;
   }
 
   /* Create buffer. */
@@ -77,7 +73,7 @@ GPUPixelBufferNativeHandle VKPixelBuffer::get_native_handle()
   VkDeviceMemory memory = buffer_.export_memory_get(memory_size);
   if (memory == nullptr) {
     CLOG_ERROR(&LOG, "Failed to get device memory for Vulkan pixel buffer");
-    return native_handle_;
+    return native_handle;
   }
 
 #ifdef _WIN32
@@ -91,11 +87,11 @@ GPUPixelBufferNativeHandle VKPixelBuffer::get_native_handle()
   HANDLE handle = 0;
   if (device.functions.vkGetMemoryWin32Handle(device.vk_handle(), &info, &handle) != VK_SUCCESS) {
     CLOG_ERROR(&LOG, "Failed to get Windows handle for Vulkan pixel buffer");
-    return native_handle_;
+    return native_handle;
   }
 
-  native_handle_.handle = int64_t(handle);
-  native_handle_.size = memory_size;
+  native_handle.handle = int64_t(handle);
+  native_handle.size = memory_size;
 #else
   /* Opaque file descriptor. */
   VkMemoryGetFdInfoKHR info = {};
@@ -107,14 +103,14 @@ GPUPixelBufferNativeHandle VKPixelBuffer::get_native_handle()
   int fd = -1;
   if (device.functions.vkGetMemoryFd(device.vk_handle(), &info, &fd) != VK_SUCCESS) {
     CLOG_ERROR(&LOG, "Failed to get file descriptor for Vulkan pixel buffer");
-    return native_handle_;
+    return native_handle;
   }
 
-  native_handle_.handle = int64_t(fd);
-  native_handle_.size = memory_size;
+  native_handle.handle = int64_t(fd);
+  native_handle.size = memory_size;
 #endif
 
-  return native_handle_;
+  return native_handle;
 }
 
 size_t VKPixelBuffer::get_size()

source/blender/gpu/vulkan/vk_pixel_buffer.hh

@@ -18,7 +18,6 @@ class VKPixelBuffer : public PixelBuffer {
   VKBuffer buffer_;
   bool buffer_initialized_ = false;
   bool buffer_memory_export_ = false;
-  GPUPixelBufferNativeHandle native_handle_;
 
  public:
   VKPixelBuffer(size_t size);