Skip to content

Commit 1ee2372

Browse files
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`, `MAP_WRITE`, and even _non_-`MAP_*` 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`.
1 parent fe5a256 commit 1ee2372

File tree

4 files changed

+9
-8
lines changed

4 files changed

+9
-8
lines changed

cts_runner/test.lst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// ```
55
unittests:*
66
webgpu:api,operation,buffers,createBindGroup:buffer_binding_resource:*
7-
fails-if(vulkan,dx12,metal): webgpu:api,operation,buffers,map:mapAsync,read:*
7+
webgpu:api,operation,buffers,map:mapAsync,read:*
88
webgpu:api,operation,command_buffer,basic:*
99
webgpu:api,operation,command_buffer,copyBufferToBuffer:*
1010
fails-if(vulkan) webgpu:api,operation,command_buffer,copyTextureToTexture:copy_depth_stencil:format="depth24plus"

tests/tests/wgpu-gpu/cloneable_types.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@ fn cloneable_buffers(ctx: TestingContext) {
2828

2929
buffer.unmap();
3030

31-
// This is actually a bug, we should not need to call submit to make the buffer contents visible.
32-
ctx.queue.submit([]);
33-
3431
let cloned_buffer = buffer.clone();
3532
let cloned_buffer_contents = buffer_contents.clone();
3633

wgpu-core/src/conv.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,16 @@ pub fn is_valid_external_image_copy_dst_texture_format(format: wgt::TextureForma
2626
}
2727
}
2828

29-
pub fn map_buffer_usage(usage: wgt::BufferUsages) -> wgt::BufferUses {
29+
pub fn map_buffer_usage(usage: wgt::BufferUsages, mapped_at_creation: bool) -> wgt::BufferUses {
3030
let mut u = wgt::BufferUses::empty();
3131
u.set(
3232
wgt::BufferUses::MAP_READ,
3333
usage.contains(wgt::BufferUsages::MAP_READ),
3434
);
3535
u.set(
3636
wgt::BufferUses::MAP_WRITE,
37-
usage.contains(wgt::BufferUsages::MAP_WRITE),
37+
usage.contains(wgt::BufferUsages::MAP_WRITE)
38+
|| (usage.contains(wgt::BufferUsages::MAP_READ) && mapped_at_creation),
3839
);
3940
u.set(
4041
wgt::BufferUses::COPY_SRC,

wgpu-core/src/device/resource.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,7 @@ impl Device {
900900
}
901901
}
902902

903-
let mut usage = conv::map_buffer_usage(desc.usage);
903+
let mut usage = conv::map_buffer_usage(desc.usage, desc.mapped_at_creation);
904904

905905
if desc.usage.contains(wgt::BufferUsages::INDIRECT) {
906906
self.require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?;
@@ -990,7 +990,10 @@ impl Device {
990990

991991
let buffer_use = if !desc.mapped_at_creation {
992992
wgt::BufferUses::empty()
993-
} else if desc.usage.contains(wgt::BufferUsages::MAP_WRITE) {
993+
} else if desc
994+
.usage
995+
.intersects(wgt::BufferUsages::MAP_WRITE | wgt::BufferUsages::MAP_READ)
996+
{
994997
// buffer is mappable, so we are just doing that at start
995998
let map_size = buffer.size;
996999
let mapping = if map_size == 0 {

0 commit comments

Comments
 (0)