fix(core): use BufferMapState::Active for any BufferUsages::MAP_* flags

For cases where a buffer is `mapped_at_creation`, our current
implementation of `Buffer::create` initializes the buffer's internal
state with `BufferMapState::Init` (which contains a staging buffer
underneath the hood) for a descriptor requesting `MAP_READ` that is
copied to a host-backed buffer . `MAP_WRITE` works a little differently,
starting from the beginning with a host-backed buffer.

`Init` does a buffer copy between the staging buffer and the host-backed
buffer in the device's queue when the buffer is `unmap`ped. However,
`Buffer::map_async` (correctly) assumes that a host-backed buffer need
not wait for anything in the queue. This results in a bug where
`map_async` doesn't actually wait long enough for the device queue to
complete its operations before resolving. Oops!

Up to the point where a buffer is unmapped after being mapped at
creation, `MAP_READ` and `MAP_WRITE` buffers' capabilities are the same.
That is, we should be able to get mutable slices for mapped ranges, no
matter what. So, make `MAP_READ` just initialize its internal state in
the same way as with `MAP_WRITE`.

Author: ErichDonGubler

cts_runner/test.lst

@@ -4,7 +4,7 @@
 // ```
 unittests:*
 webgpu:api,operation,buffers,createBindGroup:buffer_binding_resource:*
-fails-if(vulkan,dx12,metal) webgpu:api,operation,buffers,map:mapAsync,read:*
+webgpu:api,operation,buffers,map:mapAsync,read:*
 webgpu:api,operation,command_buffer,basic:*
 webgpu:api,operation,command_buffer,copyBufferToBuffer:*
 fails-if(vulkan) webgpu:api,operation,command_buffer,copyTextureToTexture:copy_depth_stencil:format="depth24plus"

tests/tests/wgpu-gpu/cloneable_types.rs

@@ -28,9 +28,6 @@ fn cloneable_buffers(ctx: TestingContext) {
 
     buffer.unmap();
 
-    // This is actually a bug, we should not need to call submit to make the buffer contents visible.
-    ctx.queue.submit([]);
-
     let cloned_buffer = buffer.clone();
     let cloned_buffer_contents = buffer_contents.clone();
 

wgpu-core/src/conv.rs

@@ -26,15 +26,16 @@ pub fn is_valid_external_image_copy_dst_texture_format(format: wgt::TextureForma
     }
 }
 
-pub fn map_buffer_usage(usage: wgt::BufferUsages) -> wgt::BufferUses {
+pub fn map_buffer_usage(usage: wgt::BufferUsages, mapped_at_creation: bool) -> wgt::BufferUses {
     let mut u = wgt::BufferUses::empty();
     u.set(
         wgt::BufferUses::MAP_READ,
         usage.contains(wgt::BufferUsages::MAP_READ),
     );
     u.set(
         wgt::BufferUses::MAP_WRITE,
-        usage.contains(wgt::BufferUsages::MAP_WRITE),
+        usage.contains(wgt::BufferUsages::MAP_WRITE)
+            || (usage.contains(wgt::BufferUsages::MAP_READ) && mapped_at_creation),
     );
     u.set(
         wgt::BufferUses::COPY_SRC,

wgpu-core/src/device/resource.rs

@@ -900,7 +900,7 @@ impl Device {
             }
         }
 
-        let mut usage = conv::map_buffer_usage(desc.usage);
+        let mut usage = conv::map_buffer_usage(desc.usage, desc.mapped_at_creation);
 
         if desc.usage.contains(wgt::BufferUsages::INDIRECT) {
             self.require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?;
@@ -990,7 +990,10 @@ impl Device {
 
         let buffer_use = if !desc.mapped_at_creation {
             wgt::BufferUses::empty()
-        } else if desc.usage.contains(wgt::BufferUsages::MAP_WRITE) {
+        } else if desc
+            .usage
+            .intersects(wgt::BufferUsages::MAP_WRITE | wgt::BufferUsages::MAP_READ)
+        {
             // buffer is mappable, so we are just doing that at start
             let map_size = buffer.size;
             let mapping = if map_size == 0 {

wgpu-hal/src/gles/device.rs

@@ -528,7 +528,10 @@ impl crate::Device for super::Device {
 
         let host_backed_bytes = || Arc::new(MaybeMutex::new(vec![0; desc.size as usize]));
 
-        if emulate_map && desc.usage.intersects(wgt::BufferUses::MAP_WRITE) {
+        if emulate_map
+            && (desc.usage.intersects(wgt::BufferUses::MAP_WRITE)
+                && !desc.usage.intersects(wgt::BufferUses::MAP_READ))
+        {
             return Ok(super::Buffer {
                 backing: BufferBacking::Host {
                     data: host_backed_bytes(),
@@ -626,6 +629,7 @@ impl crate::Device for super::Device {
             BufferBacking::GlCachedOnHost {
                 cache: host_backed_bytes(),
                 raw,
+                writeable_while_mapped: desc.usage.contains(wgt::BufferUses::MAP_WRITE),
             }
         } else {
             BufferBacking::Gl { raw }
@@ -644,7 +648,12 @@ impl crate::Device for super::Device {
 
     unsafe fn destroy_buffer(&self, buffer: super::Buffer) {
         match buffer.backing {
-            BufferBacking::Gl { raw } | BufferBacking::GlCachedOnHost { raw, cache: _ } => {
+            BufferBacking::Gl { raw }
+            | BufferBacking::GlCachedOnHost {
+                raw,
+                cache: _,
+                writeable_while_mapped: _,
+            } => {
                 let gl = &self.shared.context.lock();
                 unsafe { gl.delete_buffer(raw) };
             }
@@ -683,7 +692,11 @@ impl crate::Device for super::Device {
                     )
                 }
             }
-            &BufferBacking::GlCachedOnHost { raw, ref cache } => {
+            &BufferBacking::GlCachedOnHost {
+                raw,
+                ref cache,
+                writeable_while_mapped: _,
+            } => {
                 let gl = &self.shared.context.lock();
                 unsafe { gl.bind_buffer(buffer.target, Some(raw)) };
                 let mut guard = lock(cache);
@@ -706,6 +719,17 @@ impl crate::Device for super::Device {
                 unsafe { gl.bind_buffer(buffer.target, None) };
                 *lock(&buffer.offset_of_current_mapping) = 0;
             }
+            &BufferBacking::GlCachedOnHost {
+                raw,
+                ref cache,
+                writeable_while_mapped,
+            } if writeable_while_mapped => {
+                let gl = &self.shared.context.lock();
+                let data = lock(cache);
+                unsafe { gl.bind_buffer(buffer.target, Some(raw)) };
+                unsafe { gl.buffer_sub_data_u8_slice(buffer.target, 0, &data) };
+                unsafe { gl.bind_buffer(buffer.target, None) };
+            }
             &BufferBacking::Host { .. } | &BufferBacking::GlCachedOnHost { .. } => {}
         }
     }

wgpu-hal/src/gles/mod.rs

@@ -369,6 +369,7 @@ enum BufferBacking {
     GlCachedOnHost {
         raw: glow::Buffer,
         cache: Arc<MaybeMutex<Vec<u8>>>,
+        writeable_while_mapped: bool,
     },
 }