gfx-rs / wgpu-rs

Rust bindings to wgpu native library
https://wgpu.rs
Mozilla Public License 2.0
1.69k stars 186 forks source link

Panic while trying to write buffer contents to file #104

Closed sotrh closed 5 years ago

sotrh commented 5 years ago

I'm creating a tutorial site for wgpu at sotrh.github.io/learn-wgpu. As part of my research, I've been trying to write a program that does some rendering and compute work without a window. It works up unto the point ehrn I try to pull the data out of the resulting buffer.

fn main() {
    let instance = wgpu::Instance::new();
    let adapter = instance.request_adapter(&Default::default());
    let mut device = adapter.request_device(&Default::default());

    let texture_size = 32u32;
    let texture_desc = wgpu::TextureDescriptor {
        size: wgpu::Extent3d {
            width: texture_size,
            height: texture_size,
            depth: 1,
        },
        array_layer_count: 1,
        mip_level_count: 0,
        sample_count: 1,
        dimension: wgpu::TextureDimension::D2,
        format: wgpu::TextureFormat::Rgba8UnormSrgb,
        usage: wgpu::TextureUsage::COPY_SRC 
            | wgpu::TextureUsage::OUTPUT_ATTACHMENT,
    };

    let texture = device.create_texture(&texture_desc);
    let texture_view = texture.create_default_view();

    let row_pitch = std::mem::size_of::<u32>() as u32;
    let output_buffer_size = (row_pitch * texture_size * texture_size) as wgpu::BufferAddress;
    let output_buffer_desc = wgpu::BufferDescriptor {
        size: output_buffer_size,
        usage: wgpu::BufferUsage::COPY_DST,
    };
    let output_buffer = device.create_buffer(&output_buffer_desc);

    let mut encoder = device.create_command_encoder(&wgpu::CommandEncoderDescriptor {
        todo: 0,
    });

    {
        let render_pass_desc = wgpu::RenderPassDescriptor {
            color_attachments: &[
                wgpu::RenderPassColorAttachmentDescriptor {
                    attachment: &texture_view,
                    resolve_target: None,
                    load_op: wgpu::LoadOp::Clear,
                    store_op: wgpu::StoreOp::Store,
                    clear_color: wgpu::Color::BLACK,
                }
            ],
            depth_stencil_attachment: None,
        };
        let mut render_pass = encoder.begin_render_pass(&render_pass_desc);
    }

    encoder.copy_texture_to_buffer(
        wgpu::TextureCopyView {
            texture: &texture,
            mip_level: 0,
            array_layer: 1,
            origin: wgpu::Origin3d::ZERO,
        }, 
        wgpu::BufferCopyView {
            buffer: &output_buffer,
            offset: 0,
            row_pitch,
            image_height: texture_size,
        }, 
        texture_desc.size,
    );

    device.get_queue().submit(&[encoder.finish()]);

    output_buffer.map_read_async(0, output_buffer_size, move |result: wgpu::BufferMapAsyncResult<&[u8]>| {
        println!("Testing 1, 2, 3");
        let mapping = result.unwrap();
        let data = mapping.data;

        use image::{ImageBuffer, Rgba};
        let buffer = ImageBuffer::<Rgba<u8>, _>::from_raw(
            texture_size,
            texture_size,
            data,
        ).unwrap();

        buffer.save("image.png").unwrap();
    });

    device.poll(true);
}

As you can see, all I'm doing is create a texture to render to, clearing it with the color black, copying the texture to a buffer, and trying to save that buffer to a file as a png. Everything seems to work until device.poll(true). I get a panic with the following backtrace.

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `Ok(false)`,
 right: `Ok(true)`: GPU got stuck :(', /home/benjamin/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-native-0.3.3/src/device.rs:204:13
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:47
   3: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:36
   4: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:200
   5: std::panicking::default_hook
             at src/libstd/panicking.rs:214
   6: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:477
   7: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:384
   8: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:339
   9: wgpu_native::device::PendingResources<B>::cleanup
             at /home/benjamin/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-native-0.3.3/src/device.rs:204
  10: wgpu_native::device::Device<gfx_backend_vulkan::Backend>::maintain
             at /home/benjamin/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-native-0.3.3/src/device.rs:550
  11: wgpu_device_poll
             at /home/benjamin/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-native-0.3.3/src/device.rs:2037
  12: wgpu::Device::poll
             at /home/benjamin/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.3.0/src/lib.rs:602
  13: windowless::main
             at code/src/intermediate/windowless/main.rs:86
  14: std::rt::lang_start::{{closure}}
             at /rustc/eae3437dfe991621e8afdc82734f4a172d7ddf9b/src/libstd/rt.rs:64
  15: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:49
  16: std::panicking::try::do_call
             at src/libstd/panicking.rs:296
  17: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:82
  18: std::panicking::try
             at src/libstd/panicking.rs:275
  19: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  20: std::rt::lang_start_internal
             at src/libstd/rt.rs:48
  21: std::rt::lang_start
             at /rustc/eae3437dfe991621e8afdc82734f4a172d7ddf9b/src/libstd/rt.rs:64
  22: main
  23: __libc_start_main
  24: _start
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace

This seems odd, as the example for wgpu-native uses a similar strategy (though it doesn't do anything with the buffer). I'm pretty sure I'm missing something.

I'm on version 3.0.0 from crates.io.

kvark commented 5 years ago

You can't map a buffer that was created without MAP_READ usage. You are only creating it with BufferUsage::COPY_DST as far as I see. We have it checked on master branch, so no fixes are needed on our side.

sotrh commented 5 years ago

So I added the MAP_READ flag, but it's still crashing. I'm going to try pointing to the repository directly instead of pulling from crates.io

sotrh commented 5 years ago

So I tried adding a cargo-patch, but it's still crashing.

[patch.crates-io]
wgpu = { git = "https://github.com/gfx-rs/wgpu-rs", branch = "master"}

I'm going to try making the output_buffer really small, and having the thread sleep.

sotrh commented 5 years ago

I reduced texture_size to 2u32, and have the thread sleep for 10 secs, but it's still crashing. Another thing to note that is that it seems the map block isn't getting run at all.

sotrh commented 5 years ago

I've added a device.poll(true) in a few places, and it looks like the following line is the problem.

device.get_queue().submit(&[encoder.finish()]);

I'm not sure why. I'm going to check my encoder code.

sotrh commented 5 years ago

Looks like the problem is in my encoding step.

{
    let render_pass_desc = wgpu::RenderPassDescriptor {
        color_attachments: &[
            wgpu::RenderPassColorAttachmentDescriptor {
                attachment: &texture_view,
                resolve_target: None,
                load_op: wgpu::LoadOp::Clear,
                store_op: wgpu::StoreOp::Store,
                clear_color: wgpu::Color::BLACK,
            }
        ],
        depth_stencil_attachment: None,
    };
    let mut render_pass = encoder.begin_render_pass(&render_pass_desc);
}

encoder.copy_texture_to_buffer(
    wgpu::TextureCopyView {
        texture: &texture,
        mip_level: 0,
        array_layer: 1,
        origin: wgpu::Origin3d::ZERO,
    }, 
    wgpu::BufferCopyView {
        buffer: &output_buffer,
        offset: 0,
        row_pitch,
        image_height: texture_size,
    }, 
    texture_desc.size,
);

If comment out these lines, there's no crash.

sotrh commented 5 years ago
encoder.copy_texture_to_buffer(
    wgpu::TextureCopyView {
        texture: &texture,
        mip_level: 0,
        array_layer: 1,
        origin: wgpu::Origin3d::ZERO,
    }, 
    wgpu::BufferCopyView {
        buffer: &output_buffer,
        offset: 0,
        row_pitch,
        image_height: texture_size,
    }, 
    texture_desc.size,
);

This part here seems to be the problem. My guess is that it's row_pitch, image_height: texture_size.

kvark commented 5 years ago

Great investigation, @sotrh ! So this falls under the category of validating all the inputs, which is fairly straightforward for copy operations.

sotrh commented 5 years ago

I read the docs for BufferCopyView, and it turns out I misunderstood what row_pitch meant. I thought it meant something akin to stride, but it's the total bytes in a row. The following code fixes things.

encoder.copy_texture_to_buffer(
    wgpu::TextureCopyView {
        texture: &texture,
        mip_level: 0,
        array_layer: 0,
        origin: wgpu::Origin3d::ZERO,
    }, 
    wgpu::BufferCopyView {
        buffer: &output_buffer,
        offset: 0,
        row_pitch: u32_size * texture_size,
        image_height: texture_size,
    }, 
    texture_desc.size,
);