aclysma / rafx

Multi-backend renderer with asset pipeline. The objective of this repo is to build a scalable, flexible, data driven renderer.
Apache License 2.0
641 stars 32 forks source link

Framework vulkan RenderGraph misses layout transition from attachment to present_khr if no renderpass callback is registered #239

Closed RDambrosio016 closed 2 years ago

RDambrosio016 commented 2 years ago

Can be reproduced by just commenting out the entirety of the graph_builder.set_renderpass_callback in framework_triangle. This logs a vulkan validation error:

Validation Error: [ VUID-VkPresentInfoKHR-pImageIndices-01296 ] Object 0: handle = 0x5a29a210160, type = VK_OBJECT_TYPE_QUEUE; | MessageID = 0xc7aabc16 | vkQueuePresentKHR(): 
pSwapchains[0] images passed to present must be in layout VK_IMAGE_LAYOUT_PRESENT_SRC_KHR or VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR but is in VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL. The Vulkan spec states: Each element of pImageIndices must be the index of a presentable image acquired from the swapchain specified by the corresponding element of the pSwapchains array, and the presented image subresource must be in the VK_IMAGE_LAYOUT_PRESENT_SRC_KHR layout at the time the operation is executed on a VkDevice (https://github.com/KhronosGroup/Vulkan-Docs/search?q=)VUID-VkPresentInfoKHR-pImageIndices-01296)

I think the lack of actual attachment usage within the node triggers the resource allocator to try and use the color attachment directly for presentation and it forgets to transition the resource for some reason.

aclysma commented 2 years ago

I'm going to look at and think about this more, but what's happening is that we have two kinds of render graph nodes - "render" and "callback". Callback is usually compute operations but there's no reason one couldn't do other GPU operations, so I use the more general naming of a "callback" node because it's literally just giving you a callback. The way the render graph code knows which type of node you want is based on if you called set_renderpass_callback or set_callback. If you don't call either, it defaults to just a callback (i.e. no renderpass is created.)

At a code level, we're checking image states as we progress through nodes to determine where barriers need to be placed. Most of the time, we visit a node and look at all the transitions needed to do the work before the node performs work. The exception is output images - their transition is after all nodes are visited. So that case is a different (older) codepath. Right now it only handles image attachments, which should only ever be placed on renderpasses. So the code is reasonably skipped for non-renderpasses.

So some things to resolve:

A potential bandaid would be to fail loudly if no callback is provided, and probably also fail loudly if a color attachment is used with a node that's not a renderpass. This would make it more obvious that the graph in this repro is not legal (because under current API design it describes using a color attachment without a renderpass.)

aclysma commented 2 years ago

242 should address all three issues I mentioned in my previous comment