Read-mapped buffer content incorrect unless a submission happens between write and read

[nical]

Description

This is visible during CTS runs, but it was already filed against wgpu-native a while back in https://github.com/gfx-rs/wgpu-native/issues/305.

See the test below. It creates a mapped buffer, writes data into it, unmaps it, maps it again and checks that the content matches what was written.

Repro steps

#[gpu_test]
static MAP_WITHOUT_SUBMIT: GpuTestConfiguration = GpuTestConfiguration::new().run_async(|ctx| async move {
    let buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor {
        label: None,
        size: 12,
        usage: wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::COPY_DST,
        mapped_at_creation: true,
    });

    {
        let mut mapped = buffer.slice(0..12).get_mapped_range_mut();
        assert!(mapped.len() == 12);
        for (i, elt) in mapped.iter_mut().enumerate() {
            *elt = (i %255) as u8;
        }    
    }

    buffer.unmap();

    // Uncommenting this makes the test pass
    //ctx.queue.submit([]);

    buffer.slice(0..12).map_async(wgpu::MapMode::Read, Result::unwrap);

    ctx.async_poll(wgpu::Maintain::wait())
        .await
        .panic_on_timeout();

    {
        let mapped = buffer.slice(0..12).get_mapped_range();
        assert!(mapped.len() == 12);
        for (i, elt) in mapped.iter().enumerate() {
            assert_eq!(*elt, (i %255) as u8); // Boom.
        }
    }

    buffer.unmap();
});

Expected vs observed behavior

The above test should pass but currently doesn't unless the submit call is uncommented.

Extra materials

The failing cts test is webgpu:api,operation,buffers,map:mapAsync,read:mapAsyncRegionLeft="default-expand";mapAsyncRegionRight="default-expand"

Platform Information about your OS, version of wgpu, your tech stack, etc.

[nical]

Looks like we should detect when a buffer that has pending writes is mapped again and either:

• trigger an empty submission
• not map it until the next submission

[teoxoy]

I think we can defer unmapping & putting the staging buffer in pending writes to the beginning of a submission that uses the buffer.

Consequently this would also fix point 1 of https://github.com/gfx-rs/wgpu/issues/6053.

[ErichDonGubler]

A similar Firefox downstream bug, though it's for a buffer initialized with write_buffer instead: bug 1994733

[teoxoy]

My suggestion above is not spec compliant.

Looks like we should detect when a buffer that has pending writes is mapped again and either:

• trigger an empty submission
• not map it until the next submission

Triggering an empty submission on every unmap call is going to be costly. Not mapping it until the next submission is also not spec compliant.

I think the ideal solution is to always use BufferMapState::Active variant for CPU-backed buffers. We currently only do so for buffers with MAP_WRITE usage; I think that's incomplete and should include MAP_READ as well.

https://github.com/gfx-rs/wgpu/blob/13a9c1b308ffc369ab37a95a72138e9b0ccb4e71/wgpu-core/src/device/resource.rs#L993

[teoxoy]

A similar Firefox downstream bug, though it's for a buffer initialized with write_buffer instead: bug 1994733

A possible solution for Bug 1994733#c6 might also fix this issue.