Issues with Vulkan image layout transitions

[mitchmindtree]

One of the final blockers before landing v0.9 is the state of automatic layout transitions in vulkano. Before explaining the issue, it's important to understand 1. image layouts, 2. image memory barriers and 3. image layout transitions.

Image Layouts

The ImageLayout type describes the layout of an image (e.g. AttachmentImage, StorageImage or SwapchainImage - anything that implements ImageAccess) in GPU memory at each stage within a vulkan program. The thing that determines what ImageLayout an image should have at each stage is the way it is used. I believe this is specified by ImageUsage, but there could be other factors involved in determining required layout - I'm unclear on this. This is highlighted nicely by a passage from the Vulkan cookbook:

For example, sampling data from images inside shaders may need them to be cached in such a
way that neighbor texels are also neighbors in memory. But, writing data to images may be
performed faster when a memory is laid out linearly. That's why image layouts were
introduced in Vulkan. Each image usage has its own, designated layout.

You can find a list of all possible image layouts supported by the spec along with descriptions here. Side note: Vulkano currently seems to enumerate only a subset of these in its ImageLayout type.

Image Memory Barriers and Layout Transitions

Let's say we have a SwapchainImage and we want to clear it with a single colour and then present it via the swapchain.

To clear the image, we can use the vkCmdClearColorImage command (via the clear_color_image commandbuffer builder method in vulkano). The vkCmdClearColorImage operation requires that the image is in the VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL layout (ImageLayout::TransferDstOptimal in vulkano).

VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL must only be used as a destination image of a transfer command. This layout is valid only for image subresources of images created with the VK_IMAGE_USAGE_TRANSFER_DST_BIT usage bit enabled.

To present an image via the swapchain, the image must be in the VK_IMAGE_LAYOUT_PRESENT_SRC_KHR image layout (ImageLayout::PresentSrc in vulkano).

VK_IMAGE_LAYOUT_PRESENT_SRC_KHR must only be used for presenting a presentable image for display. A swapchain’s image must be transitioned to this layout before calling vkQueuePresentKHR, and must be transitioned away from this layout after calling vkAcquireNextImageKHR.

To ensure that the image has the TransferDstOptimal layout while clearing and the PresentSrc layout while presenting, we must use an Image Memory Barrier. On the barrier, the oldLayout is the layout the image should have for commands precending the barrier (in our case, TransferDstOptimal) and newLayout is the layout the image will have after the barrier (in our case, PresentSrc). As far as I understand it is up to the vulkan implementation to actually perform this transition at runtime - as a result, transitions can incur a cost and unnecessary transitions should be avoided if possible.

The Bug

Vulkano aims to make life easier by inferring the necessary image layout transitions (and in turn, insert the necessary image memory barriers) for the user (as noted in the ImageLayout docs) which I think is a highly worthy goal due to the tedious and error-prone nature of manually specifying the barriers. Unfortunately, it seems there are cases where vulkano does not (or can not?) have the necessary transition implemented just yet. E.g. tomaka mentions a missing transition here, a comment in clear_color_image has a TODO, etc. As a result, it is not uncommon to run into cryptic assert_neq!(Undefined, Undefined) panic!s or AccessError { ImageNotInitialized } runtime errors. Some of these can be worked around by manually specifying the initial_layout and final_layout fields for attachments within renderpass! macros (I'm not 100% clear yet how exactly these fields are passed along to the barriers), however in other cases there is no clear option for manually specifying the necessary transition manually (e.g. transitioning for a clear_color_image command).

Here are some issues/PRs I believe are related:

vulkano-rs/vulkano#1117
vulkano-rs/vulkano#974
vulkano-rs/vulkano#977
vulkano-rs/vulkano#1061

The Temporary Patch

Currently in order to work with v0.9 we are using a branch of vulkano called nannou-patches which contains a small commit that changes the initial_layout of the SwapchainImage and AttachmentImage ImageAccess implementations to use the Undefined layout. The Vulkan spec indicates that it is OK to use Undefined as the oldLayout, though it is unclear what the performance cost is compared to specifying the image's actual layout.

oldLayout must be VK_IMAGE_LAYOUT_UNDEFINED or the current layout of the image subresources affected by the barrier

The docs on the Undefined layout:

VK_IMAGE_LAYOUT_UNDEFINED does not support device access. This layout must only be used as the initialLayout member of VkImageCreateInfo or VkAttachmentDescription, or as the oldLayout in an image transition. When transitioning out of this layout, the contents of the memory are not guaranteed to be preserved.

Solution?

While this branch serves as a temporary workaround, it is obviously not a correct, long-term solution. I think we can improve the situation by surveying cases where vulkano fails to infer the correct image layout transition and try and find the necessary fix within vulkano itself.

An alternative solution might be to ensure the user always has the ability to manually specify the layout transition themselves. However, I imagine this might lead to safety issues due to the potential for invalid transitions when vulkano has the potential to remove these issues altogether if it could only correctly infer all the necessary transitions itself.

To find a starting point, the bugs mentioned above can be easily reproduced by changing the [replace] vulkano instances in nannou's cargo.toml from my nannou-patches branch back to the vulkano-rs master branch and removing all the instances in which we manually specify initial_layout and final_layout within renderpass! macro invocations (frame/mod.rs, the draw vulkano.rs backend, ui.rs, the vk_*.rs examples).

It might be worth filling in some of the knowledge gaps above and posting a similar issue about this to vulkano as discussion on the topic there is currently pretty scattered.

@freesig I'm going to stop digging for now and keep hacking on the mirrors, but thought I'd consolidate my thoughts/info here for when I come back or in case you wanted to have a dig yourself after the mac installer stuff.

[mitchmindtree]

This link has a really good overview on image layouts.

http://vulkan-spec-chunked.ahcox.com/ch11s04.html

[freesig]

I'm still a little bit confused on exactly what's the bug is but I think one possible solution is setting up subpasses that that describe the beginning and end of the render pass. I'm just doing some more reading on it now.

Is there a known situation that triggers the bug?
Sorry just saw you mention at the bottom.

Am I missing something here or isn't it impossible for vulkano to infer the initial and dest layout transitions because it would require vulkano to know what you were intending to use the attachments for?

[freesig]

I'm just reading though this and I it states:

The old layout must always be equal to the current or undefined layout.

One theory is that however the transition is occurring it is not setting the correct current layout and by setting it to undefined layout then this requirement is no longer need.
If this is the case maybe we need to somehow find out what the current layout is set to so we can correctly set the transition wherever it is happening.

Sorry if this what you have already said, I'm pretty sick today so might be repeating stuff

[mitchmindtree]

Am I missing something here or isn't it impossible for vulkano to infer the initial and dest layout transitions because it would require vulkano to know what you were intending to use the attachments for?

It does the inference at the time of command buffer building/submission I believe - at that point it should be able to know everything as it has all the of the commands submitted by the user including render passes, image copies, etc.

One theory is that however the transition is occurring it is not setting the correct current layout and by setting it to undefined layout then this requirement is no longer need.

Yeah this is what the bug is, and this is why my patch of changing the initial_layout for the image types we're using fixes the bug for us (even though it's not a proper solution as having the actual correct layout should be much more performant).

It might be worth first making a big table of all the operations in which images require a specific layout, and then use this to make a list of all places in which layout transitions can occur and use this to dig in and see what's missing in vulkano?

[freesig]

This seems to imply setting it to UNDEFINED is the correct choice. I don't think we ever need to care about what was in the previous frame yeh?
This is just for the swapchain

[mitchmindtree]

Yeah so I think is actually the case for images in general, not just the swapchain image. If they have just been initialized then the initial_layout should always be UNDEFINED (or PREINITIALIZED if the Host is yet to write to the memory), but after being used (whether for presentation, or after being cleared, or used as a shader sampler texture, etc) the image should have a known layout which can be used for future transitions rather than UNDEFINED.

From my understanding, while UNDEFINED is always valid as an initial layout, specifying the actual known layout can be much more efficient as the GPU can optimise transitions more easily.

It seems to me like tomaka's goal with vulkano was to take advantage of the fact that we can know what layout an image should be in from the way it is used (by looking at the commands) and infer the optimal transitions. Right now, it just seems to be missing a lot of cases which cause it to fail at this. You should be able to find the parts where transitions are handled in vulkano by finding the places in which image memory barriers are created (I'm pretty sure this is during command buffer building).

[freesig]

From the docs:

Transitions can happen with an image memory barrier, included as part of a vkCmdPipelineBarrier or a vkCmdWaitEvents command buffer command (see Section 6.5.6, “Image Memory Barriers”), or as part of a subpass dependency within a render pass (see VkSubpassDependency).

I will have a dig through vulkano now and see if I can see where these are happening

[freesig]

This is a little complex so I'm just going to put stuff here as I find it and hopefully it will become a bit clearer

• This is set to true when it is created but never accessed again in the entire crate until here which means it's always true
• The images are transitioned here and I wonder if it's just a matter of changing that undefined_layout bool from here
• Ok so setting this line to false makes nannou work again. The question is, is this just a matter of the bool not being set and the transitions are happening or maybe it's not being set because not all transitions are implemented yet

[mitchmindtree]

Yeah in the original patch I did I removed that field altogether along with the check as I also noticed it was never getting set to false as well! vulkano-rs/vulkano#1117

I think you're right though that ensuring it actually does get set to false after the first time it is transitioned would be the correct fix.

Also when testing changes with nannou make sure to test at least:

1. an example that uses the Draw API,
2. an example that draws a UI,
3. vk_triangle.rs,
4. vk_quad_warp.rs and
5. vk_teapot.rs

as each of these require different combinations of images and layout transitions.

[freesig]

I think you're right though that ensuring it actually does get set to false after the first time it is transitioned would be the correct fix.

Yeh I'm having trouble seeing the link between the ImageEntry and the SyncCommandBuffer::build() There doesn't seem to be anyway for that function to know that it's resources are swapchain images and set the boolean. I'm a little confused as to why the state of the layout of the swapchain images is even tracked in that boolean. None of the other types of Images seem to be tracking it. Actually AttachmentImage has initialized which seems to be doing a similar thing but also never being set.

Even more odd is why this doesn't happen in the vulkan examples. They don't seem to set that variable to true. Unless they are just always using undefined.
Yeh it appears they are always undefined.

• What would be good is a way to confirm what the actual ImageLayout is of the swapchain images. They we could confirm that the transition is actually happening.
• The I need to find a way to update these booleans to match the state of the ImageLayout. I might do a git blame and see what they were ment to do. Perhaps they used to be used and are no longer needed or maybe they are half implemented.

[mitchmindtree]

Actually AttachmentImage has initialized which seems to be doing a similar thing but also never being set.

Yeah AttachmentImage is the other culprit which we had to patch in nannou_patches. Whatever the fix is for SwapchainImage we'll probably need something similar for AttachmentImage.

or maybe they are half implemented

I get the feeling this is probably the case as tomaka mentions here, but defs worth checking blame just in case 👍

[freesig]

I'm a bit stuck with where to go from here. The git blame doesn't shed any light and based on the issue it seems that its not an easy issue.

I get the feeling that the current state of vulkano is that you aren't eve ment to specify an ImageLayout and vulkano is ment to just do that under the hood. However the current implementation seems to be just using undefined.

I think it's worth noting that this bug can also be solved by just not ever specifying ImageLayout in nannou. Perhaps this is the best strategy because then we can just work on vulkanos transitions without needing to touch nannou

[mitchmindtree]

I think it's worth noting that this bug can also be solved by just not ever specifying ImageLayout in nannou.

Does this actually work for all the nannou examples I mentioned above though? If this was the case, I never would have run into the bug (when I first ran into the bug I didn't even know you could specify the initial_layout and final_layout in the renderpass). The only reason I started specifying the layouts is because sometimes they seemed to help get around the cases that vulkano would fail at inferring - I don't remember specific cases though, you'd have to try running those examples and see what you run into.

[freesig]

It works for everything except ui and vk_quad_warp and they both fail on the DepthStencilAttachmentOptimal

[freesig]

I have another idea, although I'm not sure how easy it will be to do.
According to the spec:

layout is a VkImageLayout value specifying the layout the attachment uses during the subpass. The implementation will automatically perform layout transitions as needed between subpasses to make each subpass use the requested layouts.

Subpasses can have VK_SUBPASS_EXTERNAL in the srcSubpass or dstSubpass. So one for the start and the other for the end. These are added implicitly if we don't add them but I have read there can be issues with that.

What I'm thinking is that we add an explicit subpass at the start and the end of the renderpass and it should do the transitions for us without any memory barriers needed. Vulkano currently doesn't set these so I definitely think it's worth a try.

Edit:
Ah I just remembered this won't help with clear calls

[freesig]

Notes

• We should be minimizing the amount of pipeline barriers.
• Any layout transitions in side the render pass scope will be more efficient if they are done through sub pass dependencies.
• For transitions outside the render pass scope like clears or copys we need to use pipeline barriers.
• They should be batched because multiple calls to create barriers is slow.
• It's really important that the barriers are waiting on the correct stage or it will really slow down the whole pipeline.
• The takeaway from what I've read so far is that if we get this wrong it will be slower then using undefined. It will probably take some time to really get this correct so I'm not sure if we should block v0.9 on it but if we do push v0.9 without solving it then we will need to depend on a custom vulkano branch that we use to actively work on this issue. It would be nice to get this strong though.
• It's probably also worth considering if this is a goal that vulkano should try and achieve. If tomaka is finding it extremely complicated then perhaps it's not something that should be done. I might be the case that only the application developer has enough information to know what transitions should take place. I don't think we will know the answer to this until we try to implement this a bit more. However if that does turn out to be the case then it could be better to make it super easy and obvious how to do these transitions through vulkano.

Resources

• Video on pipeline barriers
• Video on keeping the gpu fed

Plan

• Research pipeline barriers and layout transitions. I'll edit this comment with any resources I find.
• Make a table of all valid transitions and how they relate to the calls we are making.
• Research subpass transitions. So far I've read conflicting information that the implicit external subpasses at the start and end of the render pass do or don't take care of the layout transition.
• Find a way to check the current layout of an image. This would be really handy in testing if what we are doing is correct.
• Run render doc to see what is happening
  Renderdoc doesn't show memory barriers.

[mitchmindtree]

Yeah I'm still leaning towards the original idea of signaling to the images somehow that they've been transitioned out of the undefined state (I'm guessing this was the original plan and that is why they use AtomicBools) but this might require adding some method or trait so that they can be signalled. Like you mentioned with the clear call, it's important we address cases outside of the graphics pipeline too.

Also, I forgot to mention that the simple_window.rs and multi_window.rs examples should both be tested as well - they are unique in that they only clear and don't do any other drawing, which is a image layout transition case that the other examples I mentioned don't handle.

[freesig]

Ok I can give this a go aswell.
I guess my concern is that even if we get it working by transitioning and signaling the bool that if we don't understand the transitions correctly then its very possible that we end up with a slower pipeline then with just transitioning from undefined. It will also be hard to test because it might just be slower in some edge cases.

• Find a way to in the transition code to tell which type of image it is transitioning.
  This is probably a good spot to say that the image has been initialized / not undefined. However we only get the ImageAccess trait object here. So maybe it would be worth adding a function like ImageAccess::initialized() that sets the swapchain.image.undefined_layout = false or the image.initialized = true
• If it's transitioning out of undefined then signal either the undefined_layout or initialized bool

[freesig]

So here is an attempt at implementing the above comment. It works with all the examples that are listed here but I'm not sure if it's safe or fast. Nothing is noticeably slower though. The next step might be to use some profiling tools to look for pipleline bubbles.

[freesig]

This is the PR to master vulkano-rs/vulkano#1171

[freesig]

Command	Valid Layouts
vkCmdCopyBufferToImage	VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_GENERAL, VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR
vkCmdCopyImageToBuffer	VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL or VK_IMAGE_LAYOUT_GENERAL
vkCmdCopyImage	srcImageLayout must be VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, VK_IMAGE_LAYOUT_GENERAL, or VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR
	dstImageLayout must be VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_GENERAL, or VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR
vkCmdResolveImage	srcImageLayout must be VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL or VK_IMAGE_LAYOUT_GENERAL
	dstImageLayout must be VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL or VK_IMAGE_LAYOUT_GENERAL
vkCmdClearColorImage	VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_GENERAL, or VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR

[mitchmindtree]

Here's a slightly simplified form of the table above:

Vulkan Command - vkCmd*	Valid Layouts - VK_IMAGE_LAYOUT_*
Clear Commands - vkCmdClearColorImage - vkCmdClearDepthStencilImage	All (Dst Only) - VK_IMAGE_LAYOUT_GENERAL - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL - VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR
Copy Commands - vkCmdCopyImage - vkCmdCopyBufferToImage - vkCmdCopyImageToBuffer - vkCmdBlitImage - vkCmdResolveImage	All - VK_IMAGE_LAYOUT_GENERAL Src Only - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL Dst Only - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL - VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR

I believe that's all the transitions for commands outside of the render pass. I'll try and put something together for the required layouts for inside the render pass, though it seems quite a bit more complicated.

First I might just try and collect all image layout constraints locally in a massive big list, then see if there's a way to simplify it. You almost need graph resolution or a relational programming lang like prolog to work all of this out 🕵️‍♀️

[freesig]

I might be wrong but I think anything inside the renderpass is already handle by the subpass dependacies

[mitchmindtree]

The following are all of the mentions of requirements and restraints on image layouts I could find throughout the spec along with links to header of the relevant section. I'll have a go at cleaning this up into a more digestible format tomorrow.

Shading Rate Image
vkCmdBindShadingRateImageNV requires VK_IMAGE_LAYOUT_GENERAL or VK_IMAGE_LAYOUT_SHADING_RATE_OPTIMAL_NV if imageView is not null.

WSI Swapchain

Image Presentation
Before an application can present an image, the image’s layout must be
transitioned to the VK_IMAGE_LAYOUT_PRESENT_SRC_KHR layout, or for a shared
presentable image the VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR layout

Render Pass Creation

vkRenderPassCreateInfo valid usage:

• For any member of pAttachments with a loadOp equal to VK_ATTACHMENT_LOAD_OP_CLEAR, the first use of that attachment must not specify a layout equal to VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL or VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL.
• For any member of pAttachments with a stencilLoadOp equal to VK_ATTACHMENT_LOAD_OP_CLEAR, the first use of that attachment must not specify a layout equal to VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL or VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL.
• For any member of pAttachments with a loadOp equal to VK_ATTACHMENT_LOAD_OP_CLEAR, the first use of that attachment must not specify a layout equal to VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL.
• For any member of pAttachments with a stencilLoadOp equal to VK_ATTACHMENT_LOAD_OP_CLEAR, the first use of that attachment must not specify a layout equal to VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL.

VkRenderPassFragmentDensityMapCreateInfoEXT valid usage:

• If fragmentDensityMapAttachment is not VK_ATTACHMENT_UNUSED, layout must be equal to VK_IMAGE_LAYOUT_FRAGMENT_DENSITY_MAP_OPTIMAL_EXT, or VK_IMAGE_LAYOUT_GENERAL

Attachment Aliases - If a set of attachments alias each other, then all except the first to be used in the render pass must use an initialLayout of VK_IMAGE_LAYOUT_UNDEFINED, since the earlier uses of the other aliases make their contents undefined. Once an alias has been used and a different alias has been used after it, the first alias must not be used in any later subpasses. However, an application can assign the same image view to multiple aliasing attachment indices, which allows that image view to be used multiple times even if other aliases are used in between.

vkAttachmentReference - If attachment is not VK_ATTACHMENT_UNUSED, layout must not be VK_IMAGE_LAYOUT_UNDEFINED or VK_IMAGE_LAYOUT_PREINITIALIZED

An attachment used as both an input attachment and a color attachment must be in the VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR or VK_IMAGE_LAYOUT_GENERAL layout. An attachment used as an input attachment and depth/stencil attachment must be in the VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR, VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL, or VK_IMAGE_LAYOUT_GENERAL layout. An attachment must not be used as both a depth/stencil attachment and a color attachment.

vkCreateRenderPass2KHR valid usage:

• For any member of pAttachments with a loadOp equal to VK_ATTACHMENT_LOAD_OP_CLEAR, the first use of that attachment must not specify a layout equal to VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL, or VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL
• For any member of pAttachments with a stencilLoadOp equal to VK_ATTACHMENT_LOAD_OP_CLEAR, the first use of that attachment must not specify a layout equal to VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL, or VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL.

Framebuffers

For depth/stencil attachments, each aspect can be used separately as attachments and non-attachments as long as the non-attachment accesses are also via an image subresource in either the VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL layout or the VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL layout, and the attachment resource uses whichever of those two layouts the image accesses do not. Use of non-attachment aspects in this case is only well defined if the attachment is used in the subpass where the non-attachment access is being made, or the layout of the image subresource is constant throughout the entire render pass instance, including the initialLayout and finalLayout.

Note - These restrictions mean that the render pass has full knowledge of all uses of all of the attachments, so that the implementation is able to make correct decisions about when and how to perform layout transitions, when to overlap execution of subpasses, etc.

Render Pass Commands

Valid Usage:

• If any of the initialLayout or finalLayout member of the VkAttachmentDescription structures or the layout member of the VkAttachmentReference structures specified when creating the render pass specified in the renderPass member of pRenderPassBegin is VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL then the corresponding attachment image view of the framebuffer specified in the framebuffer member of pRenderPassBegin must have been created with a usage value including VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT
• If any of the initialLayout or finalLayout member of the VkAttachmentDescription structures or the layout member of the VkAttachmentReference structures specified when creating the render pass specified in the renderPass member of pRenderPassBegin is VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, or VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL then the corresponding attachment image view of the framebuffer specified in the framebuffer member of pRenderPassBegin must have been created with a usage value including VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT
• If any of the initialLayout or finalLayout member of the VkAttachmentDescription structures or the layout member of the VkAttachmentReference structures specified when creating the render pass specified in the renderPass member of pRenderPassBegin is VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL then the corresponding attachment image view of the framebuffer specified in the framebuffer member of pRenderPassBegin must have been created with a usage value including VK_IMAGE_USAGE_SAMPLED_BIT or VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT
• If any of the initialLayout or finalLayout member of the VkAttachmentDescription structures or the layout member of the VkAttachmentReference structures specified when creating the render pass specified in the renderPass member of pRenderPassBegin is VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL then the corresponding attachment image view of the framebuffer specified in the framebuffer member of pRenderPassBegin must have been created with a usage value including VK_IMAGE_USAGE_TRANSFER_SRC_BIT
• If any of the initialLayout or finalLayout member of the VkAttachmentDescription structures or the layout member of the VkAttachmentReference structures specified when creating the render pass specified in the renderPass member of pRenderPassBegin is VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL then the corresponding attachment image view of the framebuffer specified in the framebuffer member of pRenderPassBegin must have been created with a usage value including VK_IMAGE_USAGE_TRANSFER_DST_BIT
• If any of the initialLayout members of the VkAttachmentDescription structures specified when creating the render pass specified in the renderPass member of pRenderPassBegin is not VK_IMAGE_LAYOUT_UNDEFINED, then each such initialLayout must be equal to the current layout of the corresponding attachment image subresource of the framebuffer specified in the framebuffer member of pRenderPassBegin

Descriptor Types

Storage Image

• The image subresources for a storage image must be in the VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR or VK_IMAGE_LAYOUT_GENERAL layout in order to access its data in a shader.

Sampled Image

• The image subresources for a sampled image must be in the VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR, VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, or VK_IMAGE_LAYOUT_GENERAL layout in order to access its data in a shader.

Combined Image Sampler

• The image subresources for a combined image sampler must be in the VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR, VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, or VK_IMAGE_LAYOUT_GENERAL layout in order to access its data in a shader.

Input Attachment

• The image subresources for an input attachment must be in the VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR, VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, or VK_IMAGE_LAYOUT_GENERAL layout in order to access its data in a shader.

[mitchmindtree]

Here is everything above simplified and tabled:

https://docs.google.com/spreadsheets/d/1GMIHZaktFYiGk6NBFJtHyWROZ0cycCGfXOzsebz8Lhs/edit?usp=sharing

Perhaps we can add another column for checking what we have reviewed in vulkano and another for what we can currently check with our custom validation layer?

[freesig]

Layout Transition Error 1

UNASSIGNED-CoreValidation-DrawState-InvalidImageLayout(ERROR / SPEC): msgNum: 0 - Submitted command buffer expects image 0x8 (subresource: aspectMask 0x1 array laye      r 0, mip level 0) to be in layout VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL--instead, image 0x8's current layout is VK_IMAGE_LAYOUT_UNDEFINED.

I can confirm (as @mitchmindtree suggested) this is caused by the intermediary frame image being created with the layout VK_IMAGE_LAYOUT_UNDEFINED but then the first time it is used in a pipeline vulkan tries to transition from VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL.

This only happens the first time because at the end of the pipeline the frame image is transitioned to VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL.

Possible fixes

There should be a memory barrier transition form UNDEFINED to OPTIMAL before the first render pass command buffer is submitted. However it is not clear if this is something we should be doing in nannou or it's something vulkano should be taking care of automatically but isn't

log with details:
api_standard_single.log

[freesig]

Layout Transition Error 2

VUID-vkCmdCopyBufferToImage-dstImageLayout-00180(ERROR / SPEC): msgNum: 0 - vkCmdCopyBufferToImage(): Cannot use image 0x14 (layer=0 mip=0) with specific layout VK_      IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL that doesn't match the actual current layout VK_IMAGE_LAYOUT_GENERAL. The Vulkan spec states: dstImageLayout must specify the layo      ut of the image subresources of dstImage specified in pRegions at the time this command is executed on a VkDevice (https://www.khronos.org/registry/vulkan/specs/1.1      -extensions/html/vkspec.html#VUID-vkCmdCopyBufferToImage-dstImageLayout-00180)

and

VUID-VkDescriptorImageInfo-imageLayout-00344(ERROR / SPEC): msgNum: 0 - vkCmdDraw(): Cannot use image 0x14 (layer=0 mip=0) with specific layout VK_IMAGE_LAYOUT_GENE      RAL that doesn't match the actual current layout VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL. The Vulkan spec states: imageLayout must match the actual VkImageLayout of ea      ch subresource accessible from imageView at the time this descriptor is accessed as defined by the image layout matching rules (https://www.khronos.org/registry/vul      kan/specs/1.1-extensions/html/vkspec.html#VUID-VkDescriptorImageInfo-imageLayout-00344)

UNASSIGNED-CoreValidation-DrawState-DescriptorSetNotUpdated(ERROR / SPEC): msgNum: 0 - Descriptor set 0x21 bound as set #0 encountered the following validation erro      r at vkCmdDraw() time: Image layout specified at vkUpdateDescriptorSet* or vkCmdPushDescriptorSet* time doesn't match actual image layout at time descriptor is used      . See previous error callback for specific details.

This is happening in the osc_receiver example. I'm pretty sure this is linked to the UI not regular nannou. For some reason the descriptor set is being created with VK_IMAGE_LAYOUT_GENERAL. However the image being used as a destination for the buffer transfer is not being transitioned to that layout.

Solutions

I think we need to examine the UI code for this one. I think the descriptor set should probably not be GENERAL. Again I'm not sure if this is a vulkano bug or us miss using it.

Side Note

I noticed another possible issue. There is one point in this issue where a pipeline is submitted with 35 memory barriers. They are all look like this:

pImageMemoryBarriers[3]:        const VkImageMemoryBarrier = 0x556c355103d0:
            sType:                          VkStructureType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER (45)
            pNext:                          const void* = NULL
            srcAccessMask:                  VkAccessFlags = 4096 (VK_ACCESS_TRANSFER_WRITE_BIT)
            dstAccessMask:                  VkAccessFlags = 4096 (VK_ACCESS_TRANSFER_WRITE_BIT)
            oldLayout:                      VkImageLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL (7)
            newLayout:                      VkImageLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL (7)
            srcQueueFamilyIndex:            uint32_t = 4294967295
            dstQueueFamilyIndex:            uint32_t = 4294967295
            image:                          VkImage = 0x556c355138d0
            subresourceRange:               VkImageSubresourceRange = 0x556c35510400:
                aspectMask:                     VkImageAspectFlags = 1 (VK_IMAGE_ASPECT_COLOR_BIT)
                baseMipLevel:                   uint32_t = 0
                levelCount:                     uint32_t = 1
                baseArrayLayer:                 uint32_t = 0
                layerCount:                     uint32_t = 1

As you can see nothing is being changed. This is happening because there are 35 buffer transfer and vulkano is sticking a barrier between each one regardless of it's current layout.
Perhaps this is correct because you just need it to wait for the transfer to complete however I think it might be worth investigating this incase it's a bug or there is a better way

Log with details:
api_standard_osc_receiver.log

[freesig]

Ok Re 1
Looks like this https://github.com/vulkano-rs/vulkano/blob/3518268b7f8080f44c06daf33c90c5b8cb4986d4/vulkano/src/image/attachment.rs#L396 is causing the attachment image to get the layout of oldLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL.
This causes here https://github.com/vulkano-rs/vulkano/blob/3518268b7f8080f44c06daf33c90c5b8cb4986d4/vulkano/src/command_buffer/synced/base.rs#L652 to create a memory barrier transition from OPTIMAL
I think maybe either have a barrier inserted that converts images to this OPTIMAL (or whatever this is set to) or be a bit smarter about how we assign that attachment_layout

[freesig]

I think instead of setting just checking img.layout_initialized(); in fn prev_cmd_resource we should check if it's false and then add a pipeline barrier that goes from ImageLayout::Undefined to whatever initial_layout is set to.

Issues:

• How do we insert a barrier here when it will just get overwritten by the next barrier that's added below. Barriers only seem to be flushed on Entry::Occupied. We could this initial barrier as soon as we create it (Need to confirm that we aren't overwriting some previous buffer?).
  Actually this could be solved by just creating and passing the barrier straight to self.inner.pipeline_barrier(...)
  This is now fixed on this branch https://github.com/freesig/vulkano/tree/fix_initial_layout
• The image can also be in ImageLayout::Preinitialized. I can't see any obvious way to check this?
  I just noticed this function that looks like it was meant for this exact purpose. But it's never used in the whole library.
  I can't seem to find anywhere in vulkano where ImageLayout::Preinitialized is ever assigned to an image. But maybe we should still figure out a way to check this because it's still possible for a user to implement a custom image type that does this. I'm struggling to find a way to actually see what layout an image is in though. As far as I can tell the only way to convey this is through the previously mentioned trait

[freesig]

RE 2
This problem is a bit more complex. I can see why the image is transitioned to GENERAL because it's used as a StorageImage.
The issue is coming from the Side Note section above.
Either the barriers are being submitted in the wrong order or a transition to OPTIMAL that should be put on the end isn't happening.

The Cause

This is the line in conrod_vulkano that is causing the issue.
Also I don't think a storage image is ment to be sampled, however this could be a seperate issue

let glyph_cache_tex = StorageImage::with_usage(
                device.clone(),
                Dimensions::Dim2d { width, height },
                R8G8B8A8Unorm,
                ImageUsage {
                    transfer_destination: true,
                    sampled: true,
                    ..ImageUsage::none()
                },
                vec![graphics_queue_family],
)?;

From the vulkano docs:

Storage images. Gives read and/or write access to individual pixels in an image. The image cannot be sampled. In other words, you have exactly specify which pixel to read or write.

Ideas

• Find out why all these barriers from OPTIMAL to OPTIMAL are occurring. Perhaps this will shed some light on the whole issue.
  This is happening because there is 34 vkCmdCopyBufferToImage following the barrier. So it needs a barrier between each command. Maybe they shouldn't all be submitted in the same pipeline barrier?
  I could be wrong but it looks like all these barriers will wait on VK_PIPELINE_STAGE_TRANSFER_BIT which means they won't be inserted between the copys but all before. This isn't what we want
  This seem relevant:

If vkCmdPipelineBarrier is called outside a render pass instance, then the first set of commands is all prior commands submitted to the queue and recorded in the command buffer and the second set of commands is all subsequent commands recorded in the command buffer and submitted to the queue. If vkCmdPipelineBarrier is called inside a render pass instance, then the first set of commands is all prior commands in the same subpass and the second set of commands is all subsequent commands in the same subpass.

• Follow the code in the lead up to vkCmdCopyBufferToImage and try to find out if the barrier is missing or in the wrong order. (It's possible that it's not added because it's unaware of the transition back to GENERAL

I might need some help on this one.

I think it's worth noting that nowhere in this code is GENERAL ever needed. The only reason it's there is because that's what StorageImage is set to. It's basically a naive type that says I'll always go back to general in order to make sure you can use me anywhere. So you end up with a lot more barriers and in this case an error because something wasn't expecting the transition back.
There should be a smarter way to handle this sort of situation however maybe this is beyond the scope of this issue and v0.9. It's becoming apparent to me though that there is alot of these less then optimal cases in vulkano. I think we should definitely make it a priority to improve this after v0.9.

[freesig]

Ok I think that another possible explanation is that vulkano is trying to batch the PipelineBarrier commands which is a good idea but that last one:

        pImageMemoryBarriers[34]:       const VkImageMemoryBarrier = 0x556c35510c88:
            sType:                          VkStructureType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER (45)
            pNext:                          const void* = NULL
            srcAccessMask:                  VkAccessFlags = 4096 (VK_ACCESS_TRANSFER_WRITE_BIT)
            dstAccessMask:                  VkAccessFlags = 32 (VK_ACCESS_SHADER_READ_BIT)
            oldLayout:                      VkImageLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL (7)
            newLayout:                      VkImageLayout = VK_IMAGE_LAYOUT_GENERAL (1)
            srcQueueFamilyIndex:            uint32_t = 4294967295
            dstQueueFamilyIndex:            uint32_t = 4294967295
            image:                          VkImage = 0x556c355138d0
            subresourceRange:               VkImageSubresourceRange = 0x556c35510cb8:
                aspectMask:                     VkImageAspectFlags = 1 (VK_IMAGE_ASPECT_COLOR_BIT)
                baseMipLevel:                   uint32_t = 0
                levelCount:                     uint32_t = 1
                baseArrayLayer:                 uint32_t = 0
                layerCount:                     uint32_t = 1

Shouldn't be in that batch and should be in a barrier that occurs after the copys

I can't actually reproduce this by stepping through the code so it must be a timing thing. Maybe a bunch of copy's are being submitted faster then the gpu. I stills don't see how the last command back to GENERAL is being added to the same PipelineBarrier though.

[freesig]

I have spent the last few days walking through the debugger and I'm really struggling to gain any clarity on how this whole process works.
Here's what I have so far:

• This is the function that checks over all previous commands in a command buffer and all resources (buffers / images) that are associated with it.
• It tries to insert the correct sequence of barriers in order to make sure that images are in the correct layout and commands like vkCmdCopyBufferToImage are waited on.
• This function is called after each command is added
• It will only flush (send the actual commands to vulkan) if there is a collision ie:

1. The entry (essentially a command / resource pair) is already present.
2. exclusive || entry.exclusive || entry.current_layout != start_layout
   This entry is marked as exclusive or the collided entry is marked exclusive or the current and start layout do not match. Note that start_layout is a misleading name, it is not the actual starting layout but a layout determined by the type of image. In this case GENERAL and is sort of like a return to default layout.
3. The command hasn't been flushed before. So if the command id matches an already flushed command id then it will not flush the barrier

• This is not the only place in vulkano that pipeline barriers can be sent.
• It don't think it makes sense to submit all the pipeline barriers then all then all the commands. What is the point of having a barrier for each vkCmdCopyBufferToImage if they are all going to wait on the exact same pipleline stage (not in between each copy). I'm going to read more about pipe line barriers because this is pretty unclear to me.
  After digging into pipline barriers a bit more (see this and part 2.) I think this is pretty much an issue of the last layout transition being combined with this barrier. It really shouldn't be because right now it saying:
  Any transfer or shader commands after this must wait for all transfer commands before here to finish. And in between them I'm going to transition to GENERAL (which is invalid for a copy command).
  The next step is to figure out why vulkano is doing this
• I guess the picture I'm trying to get here is how sets of commands are grouped in vulkano and how it's decided to break them up into set A before a barrier and set B after the barrier.

[freesig]

When a command is submitted to this above function it checks if there is a conflict.
A conflict is just another command that uses the same image or buffer.
This line checks whether or not the command associated has been flushed and if not flushes it.

if collision_cmd_id >= first_unflushed_cmd_id {

The collision_cmd_id comes from the first command that uses that image or buffer entry.
So if we have a situation like:

CmdA uses ImgA
CmdA is flushed
CmdB uses ImgA
CmdC uses ImgA

Then CmdB will not be flushed before CmdC.
In some cases this is fine (although probably not optimal) like when CmdA, CmdB CmdC are all the same command type with the same requirements and layout.
In other cases like this one where the layout changes then the CmdC could change the layout before CmdB is flushed and therefor CmdB ends up with the wrong layout.

Ideas

• I could update collision_cmd_id each type a collision occurs but this would result in a new barrier for every command (ie. no batching) and result in a much slower pipeline.
• Write some smarter logic that checks all commands associated with a resource and can tell whether a flush is needed or if the command can be batched. This is the better option but is not trivial.

[freesig]

Ok I've solved this GENERAL issue!!!

The last thing left to do is check if an image is set to ImageLayout::Preinitialized and then I'll push the changes and we can run the tests again to see if anymore layout issue show up.

Preinitialized

I'll dig into this now but one thing I know is we can't have a Tiling::Optimal and Preinitialized.
Infact there really isn't many uses for it as it's not recommended to ever use Linear. If you need to upload data from the cpu, it's much more efficient to use a buffer and then copy it to an Tiling::Optimal image.

[freesig]

Ok everything is fixed now on mitchmindtree/vulkano#6
It's a bit rough and I need to add comments etc but it could be a good time to run all the nannou example tests and collect the output for layout issues. I don't have enough space on my computer to run the tests.

VK_INSTANCE_LAYERS=VK_LAYER_LUNARG_standard_validation
// Run all nannou example tests > output.log

[knappador]

I'm tagging in. My initial instinct is to say that all of the valid transitions can be described by an FSM and likely handled by a simple typestate system under the hood and validate at compile time.

[freesig]

Hey I'm trying to sort out this image layout pr but I thought I'd post here so that I don't pollute the PR.
I have managed to get it working without an extra Pipeline Barrier except for one thing.
This line

if initial_layout_requirement != start_layout || unsafe { !img.is_layout_initialized() } {
  actually_exclusive = true;

I'm not sure why in the original code actually_exclusive is set to true.
If I change it to this it works:

let is_layout_initialized = unsafe { img.is_layout_initialized() }; 
if initial_layout_requirement != start_layout || !is_layout_initialized {
  actually_exclusive = is_layout_initialized;

But I'm not sure if this is correct, although it doesn't trigger any problems that I can see

[freesig]

I've pushed this version to the PR. Maybe @mitchmindtree you can run it on linux and check for errors in the validation layers. It seems to be working on MacOS.

[mitchmindtree]

I'll add your branch to my local nannou repo as a patch, run cargo test with all examples and standard validation layers and record the output 👍

[mitchmindtree]

OK, I just ran all examples! There are no new validation layer errors, but the vk_image.rs and vk_images.rs seemed to present their first frame and then panic!.

vk_image_panic.txt
vk_images_panic.txt

Do you get this too?

Here's the full output from the run_all_examples test in case you wanna have a look:

cargo_test_stdout_stderr.txt

[freesig]

Ah dam I missed this because I was testing in release.
I'll do some digging into why val.final_layout != val.initial_layout is true.

[freesig]

Ok I have fixed this and tested it on all examples in the top level example folder and the vulkan examples. Feel free to run your more complete tests but I'm going to suggest the PR is merged now because I'm confident that its working

[mitchmindtree]

Awesome work!! I'l give this another run locally now just in case.

[freesig]

Ok the never ending issue continues.
I recently discovered and fixed a bug with the framebuffer expecting the swapchain images as UNDEFINED.
However now the problem is that because swapchain images do not pass through the command buffer layout check until they are actually used so they are not actually initialized until first use. This didn't show up before because they were expected as UNDEFINED anyway.
So we need a way to initialize the swapchain images before they are actually used.
I don't mean to delay this PR but if we didn't fix this now then we would have been in a position where we couldn't use the swapchain images without triggering a panic and then need to push for another vulkano PR so it's probably best to happen now.

[freesig]

Ok fixed it lol

[freesig]

Done!