gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.75k stars 935 forks source link

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

Open nical opened 9 months ago

nical commented 9 months ago

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 commented 9 months ago

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

teoxoy commented 3 months ago

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.