Make wgpu-hal resources copyable (Metal and GL only)

CHANGELOG.md

@@ -123,6 +123,7 @@ Additionally `Surface::get_default_config` now returns an Option and returns Non
 - The `strict_assert` family of macros was moved to `wgpu-types`. By @i509VCB in [#3051](https://github.com/gfx-rs/wgpu/pull/3051)
 - Add missing `DEPTH_BIAS_CLAMP` and `FULL_DRAW_INDEX_UINT32` downlevel flags. By @teoxoy in [#3316](https://github.com/gfx-rs/wgpu/pull/3316)
 - Make `ObjectId` structure and invariants idiomatic. By @teoxoy in [#3347](https://github.com/gfx-rs/wgpu/pull/3347)
+- When importing external objects, the `DropGuard` is to be provided to wgpu instead of wgpu-hal. By @kvark in [#3199](https://github.com/gfx-rs/wgpu/pull/3199)
 
 #### WebGPU
 

wgpu-core/src/device/mod.rs

@@ -706,6 +706,7 @@ impl<A: HalApi> Device<A> {
             },
             life_guard: LifeGuard::new(desc.label.borrow_or_default()),
             clear_mode,
+            drop_guard: None,
         }
     }
 
@@ -3789,6 +3790,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         hal_texture: A::Texture,
         device_id: id::DeviceId,
         desc: &resource::TextureDescriptor,
+        drop_guard: Option<resource::DropGuard>,
         id_in: Input<G, id::TextureId>,
     ) -> (id::TextureId, Option<resource::CreateTextureError>) {
         profiling::scope!("Device::create_texture");
@@ -3837,6 +3839,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             }
 
             texture.initialization_status = TextureInitTracker::new(desc.mip_level_count, 0);
+            texture.drop_guard = drop_guard;
 
             let ref_count = texture.life_guard.add_ref();
 

wgpu-core/src/present.rs

@@ -197,6 +197,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         clear_views,
                         is_color: true,
                     },
+                    drop_guard: None,
                 };
 
                 let ref_count = texture.life_guard.add_ref();

wgpu-core/src/resource.rs

@@ -13,6 +13,9 @@ use thiserror::Error;
 
 use std::{borrow::Borrow, num::NonZeroU8, ops::Range, ptr::NonNull};
 
+/// Drop guard to signal that wgpu is no longer using an externally created object.
+pub type DropGuard = Box<dyn std::any::Any + Send + Sync>;
+
 /// The status code provided to the buffer mapping callback.
 ///
 /// This is very similar to `BufferAccessResult`, except that this is FFI-friendly.
@@ -345,6 +348,7 @@ pub struct Texture<A: hal::Api> {
     pub(crate) full_range: TextureSelector,
     pub(crate) life_guard: LifeGuard,
     pub(crate) clear_mode: TextureClearMode<A>,
+    pub(crate) drop_guard: Option<DropGuard>,
 }
 
 impl<A: hal::Api> Texture<A> {

wgpu-hal/src/empty.rs

@@ -7,7 +7,7 @@ pub struct Api;
 pub struct Context;
 #[derive(Debug)]
 pub struct Encoder;
-#[derive(Debug)]
+#[derive(Clone, Copy, Debug)]
 pub struct Resource;
 
 type DeviceResult<T> = Result<T, crate::DeviceError>;

wgpu-hal/src/gles/command.rs

@@ -279,7 +279,7 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
 
     unsafe fn clear_buffer(&mut self, buffer: &super::Buffer, range: crate::MemoryRange) {
         self.cmd_buffer.commands.push(C::ClearBuffer {
-            dst: buffer.clone(),
+            dst: *buffer,
             dst_target: buffer.target,
             range,
         });
@@ -300,9 +300,9 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
         };
         for copy in regions {
             self.cmd_buffer.commands.push(C::CopyBufferToBuffer {
-                src: src.clone(),
+                src: *src,
                 src_target,
-                dst: dst.clone(),
+                dst: *dst,
                 dst_target,
                 copy,
             })
@@ -346,7 +346,7 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
         for mut copy in regions {
             copy.clamp_size_to_virtual(&dst.copy_size);
             self.cmd_buffer.commands.push(C::CopyBufferToTexture {
-                src: src.clone(),
+                src: *src,
                 src_target: src.target,
                 dst: dst_raw,
                 dst_target,
@@ -372,7 +372,7 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
                 src: src_raw,
                 src_target,
                 src_format: src.format,
-                dst: dst.clone(),
+                dst: *dst,
                 dst_target: dst.target,
                 copy,
             })
@@ -409,7 +409,7 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
         let query_range = start as u32..self.cmd_buffer.queries.len() as u32;
         self.cmd_buffer.commands.push(C::CopyQueryResults {
             query_range,
-            dst: buffer.clone(),
+            dst: *buffer,
             dst_target: buffer.target,
             dst_offset: offset,
         });
@@ -450,12 +450,10 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
                         let attachment = glow::COLOR_ATTACHMENT0 + i as u32;
                         self.cmd_buffer.commands.push(C::BindAttachment {
                             attachment,
-                            view: cat.target.view.clone(),
+                            view: *cat.target.view,
                         });
                         if let Some(ref rat) = cat.resolve_target {
-                            self.state
-                                .resolve_attachments
-                                .push((attachment, rat.view.clone()));
+                            self.state.resolve_attachments.push((attachment, *rat.view));
                         }
                         if !cat.ops.contains(crate::AttachmentOps::STORE) {
                             self.state.invalidate_attachments.push(attachment);
@@ -471,7 +469,7 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
                     };
                     self.cmd_buffer.commands.push(C::BindAttachment {
                         attachment,
-                        view: dsat.target.view.clone(),
+                        view: *dsat.target.view,
                     });
                     if aspects.contains(crate::FormatAspects::DEPTH)
                         && !dsat.depth_ops.contains(crate::AttachmentOps::STORE)

wgpu-hal/src/gles/device.rs

@@ -1,11 +1,7 @@
 use super::conv;
 use crate::auxil::map_naga_stage;
 use glow::HasContext;
-use std::{
-    convert::TryInto,
-    iter, ptr,
-    sync::{Arc, Mutex},
-};
+use std::{convert::TryInto, iter, ptr, sync::Arc};
 
 #[cfg(not(target_arch = "wasm32"))]
 use std::mem;
@@ -89,15 +85,13 @@ impl super::Device {
     ///
     /// - `name` must be created respecting `desc`
     /// - `name` must be a texture
-    /// - If `drop_guard` is [`None`], wgpu-hal will take ownership of the texture. If `drop_guard` is
-    ///   [`Some`], the texture must be valid until the drop implementation
-    ///   of the drop guard is called.
+    /// - If `externally_owned` is `true`, the application must manually destroy the texture.
     #[cfg(any(not(target_arch = "wasm32"), feature = "emscripten"))]
     pub unsafe fn texture_from_raw(
         &self,
         name: std::num::NonZeroU32,
         desc: &crate::TextureDescriptor,
-        drop_guard: Option<crate::DropGuard>,
+        externally_owned: bool,
     ) -> super::Texture {
         let mut copy_size = crate::CopyExtent::map_extent_to_copy_size(&desc.size, desc.dimension);
 
@@ -108,8 +102,7 @@ impl super::Device {
                 raw: glow::NativeTexture(name),
                 target,
             },
-            drop_guard,
-            mip_level_count: desc.mip_level_count,
+            externally_owned,
             array_layer_count: if desc.dimension == wgt::TextureDimension::D2 {
                 desc.size.depth_or_array_layers
             } else {
@@ -126,24 +119,21 @@ impl super::Device {
     ///
     /// - `name` must be created respecting `desc`
     /// - `name` must be a renderbuffer
-    /// - If `drop_guard` is [`None`], wgpu-hal will take ownership of the renderbuffer. If `drop_guard` is
-    ///   [`Some`], the renderbuffer must be valid until the drop implementation
-    ///   of the drop guard is called.
+    /// - If `externally_owned` is `true`, the application must manually destroy the RBO.
     #[cfg(any(not(target_arch = "wasm32"), feature = "emscripten"))]
     pub unsafe fn texture_from_raw_renderbuffer(
         &self,
         name: std::num::NonZeroU32,
         desc: &crate::TextureDescriptor,
-        drop_guard: Option<crate::DropGuard>,
+        externally_owned: bool,
     ) -> super::Texture {
         let copy_size = crate::CopyExtent::map_extent_to_copy_size(&desc.size, desc.dimension);
 
         super::Texture {
             inner: super::TextureInner::Renderbuffer {
                 raw: glow::NativeRenderbuffer(name),
             },
-            drop_guard,
-            mip_level_count: desc.mip_level_count,
+            externally_owned,
             array_layer_count: if desc.dimension == wgt::TextureDimension::D2 {
                 desc.size.depth_or_array_layers
             } else {
@@ -439,12 +429,13 @@ impl crate::Device<super::Api> for super::Device {
                 .contains(super::PrivateCapabilities::BUFFER_ALLOCATION);
 
         if emulate_map && desc.usage.intersects(crate::BufferUses::MAP_WRITE) {
+            let data = vec![0; desc.size as usize];
             return Ok(super::Buffer {
                 raw: None,
                 target,
                 size: desc.size,
                 map_flags: 0,
-                data: Some(Arc::new(Mutex::new(vec![0; desc.size as usize]))),
+                data: ptr::NonNull::new(data.leak().as_mut_ptr()),
             });
         }
 
@@ -520,7 +511,8 @@ impl crate::Device<super::Api> for super::Device {
         }
 
         let data = if emulate_map && desc.usage.contains(crate::BufferUses::MAP_READ) {
-            Some(Arc::new(Mutex::new(vec![0; desc.size as usize])))
+            let data = vec![0; desc.size as usize];
+            ptr::NonNull::new(data.leak().as_mut_ptr())
         } else {
             None
         };
@@ -534,6 +526,11 @@ impl crate::Device<super::Api> for super::Device {
         })
     }
     unsafe fn destroy_buffer(&self, buffer: super::Buffer) {
+        if let Some(data) = buffer.data {
+            let _ = unsafe {
+                Vec::from_raw_parts(data.as_ptr(), buffer.size as usize, buffer.size as usize)
+            };
+        }
         if let Some(raw) = buffer.raw {
             let gl = &self.shared.context.lock();
             unsafe { gl.delete_buffer(raw) };
@@ -547,19 +544,23 @@ impl crate::Device<super::Api> for super::Device {
     ) -> Result<crate::BufferMapping, crate::DeviceError> {
         let is_coherent = buffer.map_flags & glow::MAP_COHERENT_BIT != 0;
         let ptr = match buffer.raw {
-            None => {
-                let mut vec = buffer.data.as_ref().unwrap().lock().unwrap();
-                let slice = &mut vec.as_mut_slice()[range.start as usize..range.end as usize];
-                slice.as_mut_ptr()
-            }
+            None => unsafe { buffer.data.unwrap().as_ptr().add(range.start as usize) },
             Some(raw) => {
                 let gl = &self.shared.context.lock();
                 unsafe { gl.bind_buffer(buffer.target, Some(raw)) };
                 let ptr = if let Some(ref map_read_allocation) = buffer.data {
-                    let mut guard = map_read_allocation.lock().unwrap();
-                    let slice = guard.as_mut_slice();
-                    unsafe { self.shared.get_buffer_sub_data(gl, buffer.target, 0, slice) };
-                    slice.as_mut_ptr()
+                    let ptr = unsafe { map_read_allocation.as_ptr().add(range.start as usize) };
+                    let slice = unsafe {
+                        std::slice::from_raw_parts_mut(
+                            ptr,
+                            range.end as usize - range.start as usize,
+                        )
+                    };
+                    unsafe {
+                        self.shared
+                            .get_buffer_sub_data(gl, buffer.target, range.start as _, slice)
+                    };
+                    ptr
                 } else {
                     unsafe {
                         gl.map_buffer_range(
@@ -737,8 +738,7 @@ impl crate::Device<super::Api> for super::Device {
 
         Ok(super::Texture {
             inner,
-            drop_guard: None,
-            mip_level_count: desc.mip_level_count,
+            externally_owned: false,
             array_layer_count: if desc.dimension == wgt::TextureDimension::D2 {
                 desc.size.depth_or_array_layers
             } else {
@@ -751,7 +751,7 @@ impl crate::Device<super::Api> for super::Device {
         })
     }
     unsafe fn destroy_texture(&self, texture: super::Texture) {
-        if texture.drop_guard.is_none() {
+        if !texture.externally_owned {
             let gl = &self.shared.context.lock();
             match texture.inner {
                 super::TextureInner::Renderbuffer { raw, .. } => {
@@ -763,33 +763,25 @@ impl crate::Device<super::Api> for super::Device {
                 }
             }
         }
-
-        // For clarity, we explicitly drop the drop guard. Although this has no real semantic effect as the
-        // end of the scope will drop the drop guard since this function takes ownership of the texture.
-        drop(texture.drop_guard);
     }
 
     unsafe fn create_texture_view(
         &self,
         texture: &super::Texture,
         desc: &crate::TextureViewDescriptor,
     ) -> Result<super::TextureView, crate::DeviceError> {
-        let end_array_layer = match desc.range.array_layer_count {
-            Some(count) => desc.range.base_array_layer + count.get(),
-            None => texture.array_layer_count,
-        };
-        let end_mip_level = match desc.range.mip_level_count {
-            Some(count) => desc.range.base_mip_level + count.get(),
-            None => texture.mip_level_count,
-        };
         Ok(super::TextureView {
             //TODO: use `conv::map_view_dimension(desc.dimension)`?
-            inner: texture.inner.clone(),
+            inner: texture.inner,
             sample_type: texture.format.describe().sample_type,
             aspects: crate::FormatAspects::from(texture.format)
                 & crate::FormatAspects::from(desc.range.aspect),
-            mip_levels: desc.range.base_mip_level..end_mip_level,
-            array_layers: desc.range.base_array_layer..end_array_layer,
+            base_mip_level: desc.range.base_mip_level,
+            base_array_layer: desc.range.base_array_layer,
+            array_layer_count: match desc.range.array_layer_count {
+                Some(count) => count.get(),
+                None => texture.array_layer_count,
+            },
             format: texture.format,
         })
     }
@@ -1008,7 +1000,7 @@ impl crate::Device<super::Api> for super::Device {
                 }
                 wgt::BindingType::Texture { .. } => {
                     let view = desc.textures[entry.resource_index as usize].view;
-                    if view.mip_levels.start != 0 || view.array_layers.start != 0 {
+                    if view.base_mip_level != 0 || view.base_array_layer != 0 {
                         log::error!("Unable to create a sampled texture binding for non-zero mipmap level or array layer.\n{}",
                             "This is an implementation problem of wgpu-hal/gles backend.")
                     }
@@ -1025,11 +1017,11 @@ impl crate::Device<super::Api> for super::Device {
                     let (raw, _target) = view.inner.as_native();
                     super::RawBinding::Image(super::ImageBinding {
                         raw,
-                        mip_level: view.mip_levels.start,
+                        mip_level: view.base_mip_level,
                         array_layer: match view_dimension {
                             wgt::TextureViewDimension::D2Array
                             | wgt::TextureViewDimension::CubeArray => None,
-                            _ => Some(view.array_layers.start),
+                            _ => Some(view.base_array_layer),
                         },
                         access: conv::map_storage_access(access),
                         format: format_desc.internal,

wgpu-hal/src/gles/egl.rs

@@ -1257,11 +1257,10 @@ impl crate::Surface<super::Api> for Surface {
             inner: super::TextureInner::Renderbuffer {
                 raw: sc.renderbuffer,
             },
-            drop_guard: None,
+            externally_owned: false,
             array_layer_count: 1,
-            mip_level_count: 1,
             format: sc.format,
-            format_desc: sc.format_desc.clone(),
+            format_desc: sc.format_desc,
             copy_size: crate::CopyExtent {
                 width: sc.extent.width,
                 height: sc.extent.height,