Backends: Vulkan: Modifiable depth and stencil formats

[rwf93]

Hello,

When using Dear ImGui with Vulkan's dynamic rendering extensions, depthAttachmentFormat and stencilAttachmentFormat are unset. This may lead to problematic errors if your depth image or stencil has differing formats.

The solution I have considered is creating a new pipeline and setting the required values, but that seems quite unnecessary for simple use of Dear ImGui.

For users (like myself) who just want to use dynamic rendering without worrying about creating a new pipeline for ImGui, this seems like the best solution.

[ocornut]

depthAttachmentFormat and stencilAttachmentFormat are unset. This may lead to problematic errors if your depth image or stencil has differing formats.

Could you clarify this?

It wasn't mentioned in #5037 and #5446 and we had a test bed that worked without them.
Could you suggest a modification to this patch #5037 (comment) that showcases an issue?

[ocornut]

I'd like to understand this better and then we can also design if perhaps it is a better idea that we add a VkPipelineRenderingCreateInfoKHR field in the init structure, dedicated to user passing the info required for dynamic rendering.

[rwf93]

Could you clarify this?

It's typically better to set the format of the depth image to a format that the GPU supports. In the case of non-dynamic rendering this isn't really an issue as the depth image is "bound" to the VkFramebuffer object as an attachment and not the pipeline. In dynamic rendering, because of the depth image is being bound dynamically at draw time, if the formats of the pipeline and the formats of the depth image don't match, it results in a VUID-vkCmdDrawIndexed-pDepthAttachment-06181 validation error.

I think a passable VkPipelineRenderingCreateInfoKHR would actually be a pretty good decision. It also considers the cases where you might need to extend that struct with the pNext member.

The idea I had was just a hack to get around this issue relating to depth images and dynamic rendering.

[ocornut]

Could you suggest a modification to this patch #5037 (comment) that showcases an issue?

[ocornut]

I think the PR could be reworked by adding VkPipelineRenderingCreateInfoKHR PipelineRenderingCreateInfo; // Required for dynamic rendering in ImGui_ImplVulkan_InitInfo.
In ImGui_ImplVulkan_CreatePipeline() we would assert that its sType is set, its pNext is NULL, and then use that structure directly.

But this change would need to account for 6695006 in docking branch (issue #6999).

[rwf93]

I've currently lost interest/am not working on ImGUI at the moment as I am busy, I may revisit this PR in the future. I could turn this into a draft or close it outright.

[shawnhatori]

@ocornut I'm having this exact issue actually. And I think the suggestion of providing VkPipelineRenderingCreateInfoKHR instead of just the color format is a good one.

Disclaimer: I'm still relatively new to Vulkan. To give an example of what causes the issue in code:

// inside update loop
cImGui_ImplVulkan_NewFrame();
cImGui_ImplWin32_NewFrame();
ImGui_NewFrame();

VkRenderingAttachmentInfo color_rai
    = {.sType = VK_STRUCTURE_TYPE_RENDERING_ATTACHMENT_INFO,
        .imageView = vk->swapchain_iv[swapchain_img_idx],
        .imageLayout = VK_IMAGE_LAYOUT_ATTACHMENT_OPTIMAL,
        .loadOp = VK_ATTACHMENT_LOAD_OP_CLEAR,
        .storeOp = VK_ATTACHMENT_STORE_OP_STORE,
        .clearValue = {.color = {.float32 = {0.0f, 0.0f, 0.0f, 1.0f}}}};
VkRenderingAttachmentInfo depth_rai
    = {.sType = VK_STRUCTURE_TYPE_RENDERING_ATTACHMENT_INFO,
        .imageView = vk->depth_stencil_iv,
        .imageLayout = VK_IMAGE_LAYOUT_ATTACHMENT_OPTIMAL,
        .loadOp = VK_ATTACHMENT_LOAD_OP_CLEAR,
        .storeOp = VK_ATTACHMENT_STORE_OP_STORE,
        .clearValue = {.depthStencil = {.depth = 1.0f}}};
VkRenderingInfo ri
    = {.sType = VK_STRUCTURE_TYPE_RENDERING_INFO,
        .renderArea = {.extent = {.width = (u32)render_res.width,
                                    .height = (u32)render_res.height}},
        .layerCount = 1,
        .colorAttachmentCount = 1,
        .pColorAttachments = &color_rai,
        .pDepthAttachment = &depth_rai};
vkCmdBeginRendering(vk->cb, &ri);

// Draw.

// Draw ImGui UI.

ImGui_Render();
cImGui_ImplVulkan_RenderDrawData(ImGui_GetDrawData(), vk->cb); // <- Validation Error here

vkCmdEndRendering(vk->cb);

The full error is as follows:
Validation Error: [ VUID-vkCmdDrawIndexed-dynamicRenderingUnusedAttachments-08914 ] Object 0: handle = 0x11e6eecb0210, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xea035600000000a7, type = VK_OBJECT_TYPE_PIPELINE; Object 2: VK_NULL_HANDLE, type = VK_OBJECT_TYPE_RENDER_PASS; | MessageID = 0x1cb6ab3d | vkCmdDrawIndexed: VkRenderingInfo::pDepthAttachment->imageView format (VK_FORMAT_D32_SFLOAT) must match corresponding format in VkPipelineRenderingCreateInfo::depthAttachmentFormat (VK_FORMAT_UNDEFINED) The Vulkan spec states: If current render pass instance was begun with vkCmdBeginRendering, the dynamicRenderingUnusedAttachments feature is not enabled, and VkRenderingInfo::pDepthAttachment->imageView was not VK_NULL_HANDLE, the value of VkPipelineRenderingCreateInfo::depthAttachmentFormat used to create the currently bound graphics pipeline must be equal to the VkFormat used to create VkRenderingInfo::pDepthAttachment->imageView (https://vulkan.lunarg.com/doc/view/1.3.261.1/windows/1.3-extensions/vkspec.html#VUID-vkCmdDrawIndexed-dynamicRenderingUnusedAttachments-08914)

[shawnhatori]

I'll try to get a PR up for some of this too. I was thinking of doing one anyway based on the discussion in #7140

[shawnhatori]

Created the PR here: #7166

[ocornut]

Should be merged with #7166, thanks both!

Please note I've been asking for a repro based on the existing patch to enable dynamic rendering in examples, and wasn't provided one: #6855 (comment), many times is the main reason those PR are not merged. I still don't have such repro so I merged this blindly but this is only adding technical debt to my ability to react to Vulkan changes.