Wgpu: InvalidImageLayout when updating texture between render passes

[hecrj]

First of all, congratulations on the release! :tada: It's really great to see all the progress here.

I have migrated wgpu_glyph to the latest release in https://github.com/hecrj/wgpu_glyph/pull/33. During the process, I have noticed a validation error was being triggered under some circumstances:

[2020-04-07T20:28:50Z ERROR gfx_backend_vulkan] 
    VALIDATION [UNASSIGNED-CoreValidation-DrawState-InvalidImageLayout (0)] : Submitted command buffer expects VkImage 0x100000000010[wgpu_glyph::Cache] (subresource: aspectMask 0x1 array layer 0, mip level 0) to be in layout VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL--instead, current layout is VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL.
    object info: (type: COMMAND_BUFFER, hndl: 94720332266560)

More specifically, the error seems to happen when the glyph cache (a texture) is updated (using a buffer copy) between two render passes. I have modified one of the examples in wgpu_glyph to reproduce the issue consistently. It is the clipping example in the wgpu-validation-error branch.

It may be relevant to point out that the error does not happen if the texture is _also_ updated before the first render pass. I needed to add a frame_count condition and separate the cache updates in different frames to reproduce it.

If the example is too convoluted, I can try to make an SSCCE.

[kvark]

Looks like part of the problem here is that you aren't initializing this resource properly (i.e. using OpLoad::Load before anything is written to it instead of OpLoad::Clear). I filed https://github.com/gfx-rs/wgpu/issues/563 for this. You can see the warning by RUST_LOG=gfx_backend_vulkan=warn environment on your test case.

It could be that once you fix this, the original error also goes away. Please try to find where you are doing this, while we have a task to validate this on our side properly.

[hecrj]

You can see the warning by RUST_LOG=gfx_backend_vulkan=warn environment on your test case.

I have investigated a bit and this warning seems to be unrelated to the glyph cache. I can reproduce it if I try to reuse a SwapchainOutput in multiple passes, like this:

let frame = swap_chain
    .get_next_texture()
    .expect("Timeout when acquiring next swap chain texture");

let mut encoder =
    device.create_command_encoder(&wgpu::CommandEncoderDescriptor { label: None });

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

{
    let _ = encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
        color_attachments: &[wgpu::RenderPassColorAttachmentDescriptor {
            attachment: &frame.view,
            resolve_target: None,
            load_op: wgpu::LoadOp::Load,
            store_op: wgpu::StoreOp::Store,
            clear_color: wgpu::Color::BLACK,
        }],
        depth_stencil_attachment: None,
    });
}

queue.submit(&[encoder.finish()]);

Is this forbidden? If I use wgpu 0.4 there are no warnings produced. You can find the full SSCCE here.

In any case, if I set all the load operations to Clear in wgpu_glyph then the warning disappears but the validation error I originally reported stays there.

[kvark]

Thank you for that important detail! I'll have another look.
In general, this is not expected because it's not efficient. You should try to do as few passes as possible.

[hecrj]

In general, this is not expected because it's not efficient. You should try to do as few passes as possible.

Yes, I am aware of this :sweat_smile: Ideally, I should prepare everything upfront and then record everything in a single render pass, but some of the dependencies of wgpu_glyph need to change considerably in order to allow this (texture updates and draw calls are coupled).

I have plans to work on an alternative relatively soon, as this is the main issue with iced_wgpu currently.

[kvark]

texture updates and draw calls are coupled

You could consider providing 2 things in your API to the outsiders:

• a command encoder used for prep work, like the transfers of data
• a render pass used to render things

And then you'd submit them together, prep first and the one with the pass later.

[hecrj]

The issue is that the dependencies of wgpu_glyph assume each draw call is immediately performed after the cache is updated.

This means that I can't batch all the updates together and then draw everything because some of the updates may overwrite previous glyphs that the dependency assumes are already drawn. This can happen easily if you are combining text with other kinds of primitives.

The issue is that I am using a dependency that doesn't fit the use case completely. I've been planning to create an alternative to wgpu_glyph for a while anyways, as it does not support text shaping.

[kvark]

@hecrj that will be an anti-pattern. I understand what you are doing, and the problem is that you have to break out of a render pass for before each draw call, if you need to update the data. Render passes are heavy, especially on mobile, you'd want to do everything at once.

So the proper way to do this would be writing out the data sequentially somewhere through the frame, and use the data offset when binding it for shader use as a buffer with dynamic offset.

Of course, if it's a texture the story will be a tiny bit different. You could provide the shaders with something like base Y coordinate, so that they add it when sampling from the texture.

[hecrj]

@kvark Yeah, as I said I have plans to rewrite the caching strategy (and other parts) so it is possible to batch everything in a single render pass. It's just very involved because it implies rewriting a bunch of dependencies that deal with fonts and text layouting. The ecosystem is still not very mature when it comes to this.