1994733 - `mapAsync` buffer content incorrect on `MAP_READ` buffers unless a submission happens between write and read

Status: NEW

(Core :: Graphics: WebGPU, defect, P1)

Description

[Andy Leiserson [:aleiserson]]

const adapter = await navigator.gpu.requestAdapter();
const device = await adapter.requestDevice();
const buffer = device.createBuffer({ size: 1024, usage: GPUBufferUsage.MAP_READ | GPUBufferUsage.COPY_DST });

device.queue.writeBuffer(buffer, 0, new Uint8Array([0, 1, 2, 3]));
await buffer.mapAsync(GPUMapMode.READ);
new Uint8Array(buffer.getMappedRange(0, 4))

This should return [0, 1, 2, 3], but instead returns [0, 0, 0, 0], because we don't flush pending queue operations until a submission.

This is also covered by the CTS test webgpu:api,operation,buffers,map:mapAsync,read:mapAsyncRegionLeft="default-expand";mapAsyncRegionRight="default-expand". For a rust test case, see https://github.com/gfx-rs/wgpu/issues/5173.

Comment 1

[Andy Leiserson [:aleiserson]]

Reported by a user at https://matrix.to/#/!MFogdGJfnZLrDmgkBN:matrix.org/$pXN63TXGOqiId1b36QXBJTVzL0nEUbUjYMe6e8C-Z4c

Comment 2

[Jim Blandy :jimb]

P1: Probably has widespread impact.

Comment 3

[Erich Gubler [:ErichDonGubler] (he/him)]

Note: This bug is slightly different from wgpu#5173 because that bug uses mapOnCreation: true to initialize the buffer's contents, rather than this bug's writeBuffer call. There are some subtle differences, but both should work and are broken.

Comment 4

[Teodor Tanasoaia [:teoxoy]]

We should probably make writeBuffer calls to CPU-backed buffers write the data immediately.

Comment 5

[Teodor Tanasoaia [:teoxoy]]

Attachment: submit-writeBuffer-submit.png

I thought about the solution I proposed a bit more, if we want to write the data immediately we need to check that the buffer is not currently used by an active submission. If it is, we must defer writing to the buffer (via map+memcpy) until the submission finished; but then another problem arises: the user could have submitted another set of commands using that buffer before the first submission even finished, and so there won't be a good time to map and memcpy to the buffer.

This means that we are constrained to using a staging buffer + copy buffer to buffer operation (the current implementation of write_buffer in wgpu-core) even for CPU-backed buffers if they are used by an active submission.

Comment 6

[Teodor Tanasoaia [:teoxoy]]

A solution that would avoid that issue would be to submit work that accumulated in PendingWrites (if the buffer the user wants to map is in PendingWrites) as the first step in our map_async impl.

We could still implement the map+memcpy strategy if there are no active submissions but it would only be an optimization (it might be relevant for Bug 1968102).

Comment 7

[Jim Blandy :jimb]

The fix here will interact with the fix for bug 1870699 (poll from thread).

The fix in that bug requires that all operations that submit work to the GPU submit a poll call to the background I/O thread pool to wait for that work to be submitted. It seems to me that fixing this bug will cause queue-based write operations to, under some circumstances, actually submit work rather than just buffering it for the next submission. In that case, such operations will need to be covered by polling too, the way bug 1870699 does it for mapAsync and onSubmittedWorkDone.

Comment 8

[Jim Blandy :jimb]

A general thought on the problem here:

I understand that there's a lot of resistance from the wgpu side to making queue-based writes cause a submission, since those are expensive. Would it make everyone happy if queue-based writes continue to buffer the operations, but buffer mapping flushes those buffered operations, if any?

Comment 9

[Teodor Tanasoaia [:teoxoy]]

I think so; that's how I was going to try to resolve this and the other mapping issue (https://github.com/gfx-rs/wgpu/issues/5173). It seemed like the better compromise.