From 96fb99afe34e87e1897ba647a68f8d38582bfc8d Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 30 Oct 2025 12:09:47 -0400 Subject: [PATCH 1/3] refactor(gles): extract `Buffer::{raw,data}` into new `enum BufferBacking` --- wgpu-hal/src/gles/command.rs | 40 ++++++++++---- wgpu-hal/src/gles/device.rs | 95 ++++++++++++++++++-------------- wgpu-hal/src/gles/mod.rs | 34 +++++++++++- wgpu-hal/src/gles/queue.rs | 101 +++++++++++++++++++++-------------- 4 files changed, 178 insertions(+), 92 deletions(-) diff --git a/wgpu-hal/src/gles/command.rs b/wgpu-hal/src/gles/command.rs index 6136b4fd034..9fb355ed008 100644 --- a/wgpu-hal/src/gles/command.rs +++ b/wgpu-hal/src/gles/command.rs @@ -3,7 +3,7 @@ use core::{mem, ops::Range}; use arrayvec::ArrayVec; -use super::{conv, Command as C}; +use super::{conv, BufferBacking, Command as C}; #[derive(Clone, Copy, Debug, Default)] struct TextureSlotDesc { @@ -291,9 +291,13 @@ impl crate::CommandEncoder for super::CommandEncoder { if !bar.usage.from.contains(wgt::BufferUses::STORAGE_READ_WRITE) { continue; } + let buffer = match &bar.buffer.backing { + &BufferBacking::Gl { raw } | &BufferBacking::GlCachedOnHost { raw, .. } => raw, + BufferBacking::Host { .. } => unreachable!(), + }; self.cmd_buffer .commands - .push(C::BufferBarrier(bar.buffer.raw.unwrap(), bar.usage.to)); + .push(C::BufferBarrier(buffer, bar.usage.to)); } } @@ -1001,9 +1005,11 @@ impl crate::CommandEncoder for super::CommandEncoder { ) { self.state.index_offset = binding.offset; self.state.index_format = format; - self.cmd_buffer - .commands - .push(C::SetIndexBuffer(binding.buffer.raw.unwrap())); + let buffer = match &binding.buffer.backing { + &BufferBacking::Gl { raw } | &BufferBacking::GlCachedOnHost { raw, .. } => raw, + BufferBacking::Host { .. } => unreachable!(), + }; + self.cmd_buffer.commands.push(C::SetIndexBuffer(buffer)); } unsafe fn set_vertex_buffer<'a>( &mut self, @@ -1012,8 +1018,12 @@ impl crate::CommandEncoder for super::CommandEncoder { ) { self.state.dirty_vbuf_mask |= 1 << index; let (_, ref mut vb) = self.state.vertex_buffers[index as usize]; + let raw = match &binding.buffer.backing { + &BufferBacking::Gl { raw } | &BufferBacking::GlCachedOnHost { raw, .. } => raw, + BufferBacking::Host { .. } => unreachable!(), + }; *vb = Some(super::BufferBinding { - raw: binding.buffer.raw.unwrap(), + raw, offset: binding.offset, }); } @@ -1107,10 +1117,14 @@ impl crate::CommandEncoder for super::CommandEncoder { for draw in 0..draw_count as wgt::BufferAddress { let indirect_offset = offset + draw * size_of::() as wgt::BufferAddress; + let indirect_buf = match &buffer.backing { + &BufferBacking::Gl { raw } | &BufferBacking::GlCachedOnHost { raw, .. } => raw, + BufferBacking::Host { .. } => unreachable!(), + }; #[allow(clippy::clone_on_copy)] // False positive when cloning glow::UniformLocation self.cmd_buffer.commands.push(C::DrawIndirect { topology: self.state.topology, - indirect_buf: buffer.raw.unwrap(), + indirect_buf, indirect_offset, first_instance_location: self.state.first_instance_location.clone(), }); @@ -1130,11 +1144,15 @@ impl crate::CommandEncoder for super::CommandEncoder { for draw in 0..draw_count as wgt::BufferAddress { let indirect_offset = offset + draw * size_of::() as wgt::BufferAddress; + let indirect_buf = match &buffer.backing { + &BufferBacking::Gl { raw } | &BufferBacking::GlCachedOnHost { raw, .. } => raw, + BufferBacking::Host { .. } => unreachable!(), + }; #[allow(clippy::clone_on_copy)] // False positive when cloning glow::UniformLocation self.cmd_buffer.commands.push(C::DrawIndexedIndirect { topology: self.state.topology, index_type, - indirect_buf: buffer.raw.unwrap(), + indirect_buf, indirect_offset, first_instance_location: self.state.first_instance_location.clone(), }); @@ -1221,8 +1239,12 @@ impl crate::CommandEncoder for super::CommandEncoder { self.cmd_buffer.commands.push(C::Dispatch(count)); } unsafe fn dispatch_indirect(&mut self, buffer: &super::Buffer, offset: wgt::BufferAddress) { + let indirect_buf = match &buffer.backing { + &BufferBacking::Gl { raw } | &BufferBacking::GlCachedOnHost { raw, .. } => raw, + BufferBacking::Host { .. } => unreachable!(), + }; self.cmd_buffer.commands.push(C::DispatchIndirect { - indirect_buf: buffer.raw.unwrap(), + indirect_buf, indirect_offset: offset, }); } diff --git a/wgpu-hal/src/gles/device.rs b/wgpu-hal/src/gles/device.rs index e85809e6045..4f7ba60f78c 100644 --- a/wgpu-hal/src/gles/device.rs +++ b/wgpu-hal/src/gles/device.rs @@ -7,7 +7,7 @@ use arrayvec::ArrayVec; use glow::HasContext; use naga::FastHashMap; -use super::{conv, lock, MaybeMutex, PrivateCapabilities}; +use super::{conv, lock, BufferBacking, MaybeMutex, PrivateCapabilities}; use crate::auxil::map_naga_stage; use crate::TlasInstance; @@ -526,13 +526,16 @@ impl crate::Device for super::Device { .private_caps .contains(PrivateCapabilities::BUFFER_ALLOCATION); + let host_backed_bytes = || Arc::new(MaybeMutex::new(vec![0; desc.size as usize])); + if emulate_map && desc.usage.intersects(wgt::BufferUses::MAP_WRITE) { return Ok(super::Buffer { - raw: None, + backing: BufferBacking::Host { + data: host_backed_bytes(), + }, target, size: desc.size, map_flags: 0, - data: Some(Arc::new(MaybeMutex::new(vec![0; desc.size as usize]))), offset_of_current_mapping: Arc::new(MaybeMutex::new(0)), }); } @@ -560,8 +563,8 @@ impl crate::Device for super::Device { map_flags |= glow::MAP_WRITE_BIT; } - let raw = Some(unsafe { gl.create_buffer() }.map_err(|_| crate::DeviceError::OutOfMemory)?); - unsafe { gl.bind_buffer(target, raw) }; + let raw = unsafe { gl.create_buffer() }.map_err(|_| crate::DeviceError::OutOfMemory)?; + unsafe { gl.bind_buffer(target, Some(raw)) }; let raw_size = desc .size .try_into() @@ -614,33 +617,38 @@ impl crate::Device for super::Device { .private_caps .contains(PrivateCapabilities::DEBUG_FNS) { - let name = raw.map_or(0, |buf| buf.0.get()); + let name = raw.0.get(); unsafe { gl.object_label(glow::BUFFER, name, Some(label)) }; } } - let data = if emulate_map && desc.usage.contains(wgt::BufferUses::MAP_READ) { - Some(Arc::new(MaybeMutex::new(vec![0; desc.size as usize]))) + let backing = if emulate_map && desc.usage.contains(wgt::BufferUses::MAP_READ) { + BufferBacking::GlCachedOnHost { + cache: host_backed_bytes(), + raw, + } } else { - None + BufferBacking::Gl { raw } }; self.counters.buffers.add(1); Ok(super::Buffer { - raw, + backing, target, size: desc.size, map_flags, - data, offset_of_current_mapping: Arc::new(MaybeMutex::new(0)), }) } unsafe fn destroy_buffer(&self, buffer: super::Buffer) { - if let Some(raw) = buffer.raw { - let gl = &self.shared.context.lock(); - unsafe { gl.delete_buffer(raw) }; + match buffer.backing { + BufferBacking::Gl { raw } | BufferBacking::GlCachedOnHost { raw, cache: _ } => { + let gl = &self.shared.context.lock(); + unsafe { gl.delete_buffer(raw) }; + } + BufferBacking::Host { data: _ } => {} } self.counters.buffers.sub(1); @@ -656,33 +664,32 @@ impl crate::Device for super::Device { range: crate::MemoryRange, ) -> Result { let is_coherent = buffer.map_flags & glow::MAP_COHERENT_BIT != 0; - let ptr = match buffer.raw { - None => { - let mut vec = lock(buffer.data.as_ref().unwrap()); + let ptr = match &buffer.backing { + BufferBacking::Host { data } => { + let mut vec = lock(data); let slice = &mut vec.as_mut_slice()[range.start as usize..range.end as usize]; slice.as_mut_ptr() } - Some(raw) => { + &BufferBacking::Gl { 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 = lock(map_read_allocation); - let slice = guard.as_mut_slice(); - unsafe { self.shared.get_buffer_sub_data(gl, buffer.target, 0, slice) }; - slice.as_mut_ptr() - } else { - *lock(&buffer.offset_of_current_mapping) = range.start; - unsafe { - gl.map_buffer_range( - buffer.target, - range.start as i32, - (range.end - range.start) as i32, - buffer.map_flags, - ) - } - }; - unsafe { gl.bind_buffer(buffer.target, None) }; - ptr + *lock(&buffer.offset_of_current_mapping) = range.start; + unsafe { + gl.map_buffer_range( + buffer.target, + range.start as i32, + (range.end - range.start) as i32, + buffer.map_flags, + ) + } + } + &BufferBacking::GlCachedOnHost { raw, ref cache } => { + let gl = &self.shared.context.lock(); + unsafe { gl.bind_buffer(buffer.target, Some(raw)) }; + let mut guard = lock(cache); + let slice = guard.as_mut_slice(); + unsafe { self.shared.get_buffer_sub_data(gl, buffer.target, 0, slice) }; + slice.as_mut_ptr() } }; Ok(crate::BufferMapping { @@ -691,22 +698,23 @@ impl crate::Device for super::Device { }) } unsafe fn unmap_buffer(&self, buffer: &super::Buffer) { - if let Some(raw) = buffer.raw { - if buffer.data.is_none() { + match &buffer.backing { + &BufferBacking::Gl { raw } => { let gl = &self.shared.context.lock(); unsafe { gl.bind_buffer(buffer.target, Some(raw)) }; unsafe { gl.unmap_buffer(buffer.target) }; unsafe { gl.bind_buffer(buffer.target, None) }; *lock(&buffer.offset_of_current_mapping) = 0; } + &BufferBacking::Host { .. } | &BufferBacking::GlCachedOnHost { .. } => {} } } unsafe fn flush_mapped_ranges(&self, buffer: &super::Buffer, ranges: I) where I: Iterator, { - if let Some(raw) = buffer.raw { - if buffer.data.is_none() { + match &buffer.backing { + &BufferBacking::Gl { raw } => { let gl = &self.shared.context.lock(); unsafe { gl.bind_buffer(buffer.target, Some(raw)) }; for range in ranges { @@ -720,6 +728,7 @@ impl crate::Device for super::Device { }; } } + &BufferBacking::Host { .. } | &BufferBacking::GlCachedOnHost { .. } => {} } } unsafe fn invalidate_mapped_ranges(&self, _buffer: &super::Buffer, _ranges: I) { @@ -1261,7 +1270,11 @@ impl crate::Device for super::Device { wgt::BindingType::Buffer { .. } => { let bb = &desc.buffers[entry.resource_index as usize]; super::RawBinding::Buffer { - raw: bb.buffer.raw.unwrap(), + raw: match &bb.buffer.backing { + &BufferBacking::Gl { raw } + | &BufferBacking::GlCachedOnHost { raw, .. } => raw, + &BufferBacking::Host { .. } => unreachable!(), + }, offset: bb.offset as i32, size: match bb.size { Some(s) => s.get() as i32, diff --git a/wgpu-hal/src/gles/mod.rs b/wgpu-hal/src/gles/mod.rs index aeef3418694..3640cb57a23 100644 --- a/wgpu-hal/src/gles/mod.rs +++ b/wgpu-hal/src/gles/mod.rs @@ -342,14 +342,44 @@ impl Drop for Queue { #[derive(Clone, Debug)] pub struct Buffer { - raw: Option, + backing: BufferBacking, target: BindTarget, size: wgt::BufferAddress, map_flags: u32, - data: Option>>>, offset_of_current_mapping: Arc>, } +/// Storage backing a [`Buffer`]'s operations, possibly implemented with a host-side vector of +/// bytes. +/// +/// The [`Self::OnlyRaw`] variant is preferred, when supported. However, various workarounds for +/// lack of support are needed to implement some operations. See [`Device::create_buffer`] for more +/// details. +#[derive(Clone, Debug)] +enum BufferBacking { + /// A single [`glow::Buffer`] backing all operations. + Gl { raw: glow::Buffer }, + /// A synchronized vector of bytes on the host. When needed, a newly created buffer with the + /// contents of `emulated_map_data` will be used for copy operations. + /// + /// This variant is used for write-only buffers. + Host { data: Arc>> }, + /// A [`glow::Buffer`] that does not support byte access, and so requires whole copies + /// between a synchronized vector of bytes (`cache`) and a `raw` GL buffer. + GlCachedOnHost { + raw: glow::Buffer, + cache: Arc>>, + }, +} + +// #[derive(Clone, Debug)] +// enum MapState { +// Mapped { +// offset: Arc>, +// }, +// Unmapped, +// } + #[cfg(send_sync)] unsafe impl Sync for Buffer {} #[cfg(send_sync)] diff --git a/wgpu-hal/src/gles/queue.rs b/wgpu-hal/src/gles/queue.rs index 5ce0cac30b4..17d81feda17 100644 --- a/wgpu-hal/src/gles/queue.rs +++ b/wgpu-hal/src/gles/queue.rs @@ -5,7 +5,7 @@ use core::sync::atomic::Ordering; use arrayvec::ArrayVec; use glow::HasContext; -use super::{conv::is_layered_target, lock, Command as C, PrivateCapabilities}; +use super::{conv::is_layered_target, lock, BufferBacking, Command as C, PrivateCapabilities}; const DEBUG_ID: u32 = 0; @@ -323,8 +323,8 @@ impl super::Queue { ref dst, dst_target, ref range, - } => match dst.raw { - Some(buffer) => { + } => match &dst.backing { + &BufferBacking::Gl { raw } | &BufferBacking::GlCachedOnHost { raw, cache: _ } => { // When `INDEX_BUFFER_ROLE_CHANGE` isn't available, we can't copy into the // index buffer from the zero buffer. This would fail in Chrome with the // following message: @@ -341,7 +341,7 @@ impl super::Queue { if can_use_zero_buffer { unsafe { gl.bind_buffer(glow::COPY_READ_BUFFER, Some(self.zero_buffer)) }; - unsafe { gl.bind_buffer(dst_target, Some(buffer)) }; + unsafe { gl.bind_buffer(dst_target, Some(raw)) }; let mut dst_offset = range.start; while dst_offset < range.end { let size = (range.end - dst_offset).min(super::ZERO_BUFFER_SIZE as u64); @@ -357,17 +357,15 @@ impl super::Queue { dst_offset += size; } } else { - unsafe { gl.bind_buffer(dst_target, Some(buffer)) }; + unsafe { gl.bind_buffer(dst_target, Some(raw)) }; let zeroes = vec![0u8; (range.end - range.start) as usize]; unsafe { gl.buffer_sub_data_u8_slice(dst_target, range.start as i32, &zeroes) }; } } - None => { - lock(dst.data.as_ref().unwrap()).as_mut_slice() - [range.start as usize..range.end as usize] - .fill(0); + BufferBacking::Host { data } => { + lock(data).as_mut_slice()[range.start as usize..range.end as usize].fill(0); } }, C::CopyBufferToBuffer { @@ -392,10 +390,15 @@ impl super::Queue { glow::COPY_WRITE_BUFFER }; let size = copy.size.get() as usize; - match (src.raw, dst.raw) { - (Some(ref src), Some(ref dst)) => { - unsafe { gl.bind_buffer(copy_src_target, Some(*src)) }; - unsafe { gl.bind_buffer(copy_dst_target, Some(*dst)) }; + match (&src.backing, &dst.backing) { + ( + &BufferBacking::Gl { raw: src } + | &BufferBacking::GlCachedOnHost { raw: src, cache: _ }, + &BufferBacking::Gl { raw: dst } + | &BufferBacking::GlCachedOnHost { raw: dst, cache: _ }, + ) => { + unsafe { gl.bind_buffer(copy_src_target, Some(src)) }; + unsafe { gl.bind_buffer(copy_dst_target, Some(dst)) }; unsafe { gl.copy_buffer_sub_data( copy_src_target, @@ -406,8 +409,12 @@ impl super::Queue { ) }; } - (Some(src), None) => { - let mut data = lock(dst.data.as_ref().unwrap()); + ( + &BufferBacking::Gl { raw: src } + | &BufferBacking::GlCachedOnHost { raw: src, cache: _ }, + BufferBacking::Host { data }, + ) => { + let mut data = lock(data); let dst_data = &mut data.as_mut_slice() [copy.dst_offset as usize..copy.dst_offset as usize + size]; @@ -421,8 +428,12 @@ impl super::Queue { ) }; } - (None, Some(dst)) => { - let data = lock(src.data.as_ref().unwrap()); + ( + BufferBacking::Host { data }, + &BufferBacking::Gl { raw: dst } + | &BufferBacking::GlCachedOnHost { raw: dst, cache: _ }, + ) => { + let data = lock(data); let src_data = &data.as_slice() [copy.src_offset as usize..copy.src_offset as usize + size]; unsafe { gl.bind_buffer(copy_dst_target, Some(dst)) }; @@ -434,7 +445,7 @@ impl super::Queue { ) }; } - (None, None) => { + (&BufferBacking::Host { data: _ }, &BufferBacking::Host { data: _ }) => { todo!() } } @@ -756,14 +767,15 @@ impl super::Queue { let mut unbind_unpack_buffer = false; if !dst_format.is_compressed() { let buffer_data; - let unpack_data = match src.raw { - Some(buffer) => { - unsafe { gl.bind_buffer(glow::PIXEL_UNPACK_BUFFER, Some(buffer)) }; + let unpack_data = match &src.backing { + &BufferBacking::Gl { raw } + | &BufferBacking::GlCachedOnHost { raw, cache: _ } => { + unsafe { gl.bind_buffer(glow::PIXEL_UNPACK_BUFFER, Some(raw)) }; unbind_unpack_buffer = true; glow::PixelUnpackData::BufferOffset(copy.buffer_layout.offset as u32) } - None => { - buffer_data = lock(src.data.as_ref().unwrap()); + BufferBacking::Host { data } => { + buffer_data = lock(data); let src_data = &buffer_data.as_slice()[copy.buffer_layout.offset as usize..]; glow::PixelUnpackData::Slice(Some(src_data)) @@ -818,16 +830,17 @@ impl super::Queue { let offset = copy.buffer_layout.offset as u32; let buffer_data; - let unpack_data = match src.raw { - Some(buffer) => { - unsafe { gl.bind_buffer(glow::PIXEL_UNPACK_BUFFER, Some(buffer)) }; + let unpack_data = match &src.backing { + &BufferBacking::Gl { raw } + | &BufferBacking::GlCachedOnHost { raw, cache: _ } => { + unsafe { gl.bind_buffer(glow::PIXEL_UNPACK_BUFFER, Some(raw)) }; unbind_unpack_buffer = true; glow::CompressedPixelUnpackData::BufferRange( offset..offset + bytes_in_upload, ) } - None => { - buffer_data = lock(src.data.as_ref().unwrap()); + BufferBacking::Host { data } => { + buffer_data = lock(data); let src_data = &buffer_data.as_slice() [(offset as usize)..(offset + bytes_in_upload) as usize]; glow::CompressedPixelUnpackData::Slice(src_data) @@ -901,14 +914,15 @@ impl super::Queue { let read_pixels = |offset| { let mut buffer_data; - let unpack_data = match dst.raw { - Some(buffer) => { + let unpack_data = match &dst.backing { + &BufferBacking::Gl { raw } + | &BufferBacking::GlCachedOnHost { raw, cache: _ } => { unsafe { gl.pixel_store_i32(glow::PACK_ROW_LENGTH, row_texels as i32) }; - unsafe { gl.bind_buffer(glow::PIXEL_PACK_BUFFER, Some(buffer)) }; + unsafe { gl.bind_buffer(glow::PIXEL_PACK_BUFFER, Some(raw)) }; glow::PixelPackData::BufferOffset(offset as u32) } - None => { - buffer_data = lock(dst.data.as_ref().unwrap()); + BufferBacking::Host { data } => { + buffer_data = lock(data); let dst_data = &mut buffer_data.as_mut_slice()[offset as usize..]; glow::PixelPackData::Slice(Some(dst_data)) } @@ -991,11 +1005,17 @@ impl super::Queue { dst_target, dst_offset, } => { + let dst_raw_buffer = |buf: &super::Buffer| match &buf.backing { + &BufferBacking::Gl { raw } + | &BufferBacking::GlCachedOnHost { raw, cache: _ } => Some(raw), + &BufferBacking::Host { data: _ } => None, + }; + let dst_raw_buffer = dst_raw_buffer(dst); if self .shared .private_caps .contains(PrivateCapabilities::QUERY_BUFFERS) - && dst.raw.is_some() + && dst_raw_buffer.is_some() { unsafe { // We're assuming that the only relevant queries are 8 byte timestamps or @@ -1023,7 +1043,7 @@ impl super::Queue { query_size * i, ) } - gl.bind_buffer(dst_target, dst.raw); + gl.bind_buffer(dst_target, dst_raw_buffer); gl.copy_buffer_sub_data( glow::QUERY_BUFFER, dst_target, @@ -1062,9 +1082,10 @@ impl super::Queue { temp_query_results.push(result); } let query_data = bytemuck::cast_slice(&temp_query_results); - match dst.raw { - Some(buffer) => { - unsafe { gl.bind_buffer(dst_target, Some(buffer)) }; + match &dst.backing { + &BufferBacking::Gl { raw } + | &BufferBacking::GlCachedOnHost { raw, cache: _ } => { + unsafe { gl.bind_buffer(dst_target, Some(raw)) }; unsafe { gl.buffer_sub_data_u8_slice( dst_target, @@ -1073,8 +1094,8 @@ impl super::Queue { ) }; } - None => { - let data = &mut lock(dst.data.as_ref().unwrap()); + BufferBacking::Host { data } => { + let data = &mut lock(data); let len = query_data.len().min(data.len()); data[..len].copy_from_slice(&query_data[..len]); } From c3b4672d8c7a7ec56636c3cfffdfab2724ba0f62 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Fri, 24 Oct 2025 17:31:38 -0400 Subject: [PATCH 2/3] test(cts): add failing entry for `webgpu:api,operation,buffers,map:mapAsync,read:*` --- cts_runner/test.lst | 1 + 1 file changed, 1 insertion(+) diff --git a/cts_runner/test.lst b/cts_runner/test.lst index 9c8a4f31806..86c43a5ceb8 100644 --- a/cts_runner/test.lst +++ b/cts_runner/test.lst @@ -4,6 +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,command_buffer,basic:* webgpu:api,operation,command_buffer,copyBufferToBuffer:* fails-if(vulkan) webgpu:api,operation,command_buffer,copyTextureToTexture:copy_depth_stencil:format="depth24plus" From bf25b89e7166dd76cf45bc46ebd0c7329f861cd7 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 23 Oct 2025 16:49:21 -0400 Subject: [PATCH 3/3] 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`. --- cts_runner/test.lst | 2 +- tests/tests/wgpu-gpu/cloneable_types.rs | 3 --- wgpu-core/src/conv.rs | 5 +++-- wgpu-core/src/device/resource.rs | 7 ++++-- wgpu-hal/src/gles/device.rs | 30 ++++++++++++++++++++++--- wgpu-hal/src/gles/mod.rs | 1 + 6 files changed, 37 insertions(+), 11 deletions(-) diff --git a/cts_runner/test.lst b/cts_runner/test.lst index 86c43a5ceb8..1abb27fd4d2 100644 --- a/cts_runner/test.lst +++ b/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" diff --git a/tests/tests/wgpu-gpu/cloneable_types.rs b/tests/tests/wgpu-gpu/cloneable_types.rs index fd578f6cc92..ffae024ff3f 100644 --- a/tests/tests/wgpu-gpu/cloneable_types.rs +++ b/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(); diff --git a/wgpu-core/src/conv.rs b/wgpu-core/src/conv.rs index 0003c991d20..3c1a1bf4f5d 100644 --- a/wgpu-core/src/conv.rs +++ b/wgpu-core/src/conv.rs @@ -26,7 +26,7 @@ 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, @@ -34,7 +34,8 @@ pub fn map_buffer_usage(usage: wgt::BufferUsages) -> wgt::BufferUses { ); 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, diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 94834746c5a..bb171b6b933 100644 --- a/wgpu-core/src/device/resource.rs +++ b/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 { diff --git a/wgpu-hal/src/gles/device.rs b/wgpu-hal/src/gles/device.rs index 4f7ba60f78c..6580e3d8ae4 100644 --- a/wgpu-hal/src/gles/device.rs +++ b/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 { .. } => {} } } diff --git a/wgpu-hal/src/gles/mod.rs b/wgpu-hal/src/gles/mod.rs index 3640cb57a23..98ae66033f8 100644 --- a/wgpu-hal/src/gles/mod.rs +++ b/wgpu-hal/src/gles/mod.rs @@ -369,6 +369,7 @@ enum BufferBacking { GlCachedOnHost { raw: glow::Buffer, cache: Arc>>, + writeable_while_mapped: bool, }, }