Check both resolveImageView and resolveMode before referencing the view

[werman]

Vulkan spec says:

 If resolveMode is not VK_RESOLVE_MODE_NONE, and resolveImageView is not
 VK_NULL_HANDLE, a render pass multisample resolve operation is defined
 for the attachment subresource.

So both of them should be valid for resolve to happen.

---

Happens with Zink.

[baldurk]

This is a bit surprising that it works like that. The spec language is not explicitly clear - I'm assuming that if resolveImageView is VK_NULL_HANDLE then it acts as if no resolve is happening? i.e. resolveMode behaves like it's implicitly set to VK_RESOLVE_MODE_NONE?

This isn't the only place in the code that it's assumed resolve modes will be valid - can you check and update the other places that the resolveMode is checked to also handle it being implicitly disabled by an omitted resolveImageView?

[werman]

The spec language is not explicitly clear - I'm assuming that if resolveImageView is VK_NULL_HANDLE then it acts as if no resolve is happening?

I don't think it's valid for anything else to happen beside not resolving. Some context that it is intentionally in the spec:

• https://gitlab.khronos.org/vulkan/vulkan/-/issues/4877
• https://gitlab.khronos.org/vulkan/vulkan/-/merge_requests/8375

This isn't the only place in the code that it's assumed resolve modes will be valid - can you check and update the other places that the resolveMode is checked to also handle it being implicitly disabled by an omitted resolveImageView?

I see two kind of places:

• Non-custom-resolve mods handling - possibility of resolveImageView being NULL is handled in all other cases.
• VK_RESOLVE_MODE_CUSTOM_BIT_EXT - I think Vulkan spec does not permit NULL resolve view. For vkCmdBeginCustomResolveEXT spec says:

Following this call, any resolveImageView with resolveMode set to VK_RESOLVE_MODE_CUSTOM_BIT_EXTwill be written by outputs which would otherwise have written to the imageView image until the end of the current render pass instance.

[baldurk]

Digging again into the VUs there are in fact VUs about when resolveImageView can be NULL and it's to do with VkMultisampledRenderToSingleSampledInfoEXT which fakes MSAA temporarily without an actual MSAA image, so the resolve does happen:

VUID-VkRenderingAttachmentInfo-imageView-06862
If all of the following are true:

• imageView is not VK_NULL_HANDLE
• resolveMode is not VK_RESOLVE_MODE_NONE
• the pNext chain of VkRenderingInfo does not include a VkMultisampledRenderToSingleSampledInfoEXT structure with the multisampledRenderToSingleSampledEnable field equal to VK_TRUE

resolveImageView must not be VK_NULL_HANDLE

VUID-VkRenderingAttachmentInfo-imageView-06863
If imageView is not VK_NULL_HANDLE, resolveMode is not VK_RESOLVE_MODE_NONE, the pNext chain of VkRenderingInfo includes a VkMultisampledRenderToSingleSampledInfoEXT structure with the multisampledRenderToSingleSampledEnable field equal to VK_TRUE, and imageView has a sample count of VK_SAMPLE_COUNT_1_BIT, resolveImageView must be VK_NULL_HANDLE

VUID-VkRenderingAttachmentInfo-None-12256
If all of the following are true:

• imageView is not VK_NULL_HANDLE
• imageView has a sample count of VK_SAMPLE_COUNT_1_BIT
• the pNext chain of VkRenderingInfo includes a VkMultisampledRenderToSingleSampledInfoEXT structure with the multisampledRenderToSingleSampledEnable field equal to VK_TRUE

then resolveMode must not be VK_RESOLVE_MODE_NONE

So this applies equally to any resolveMode including custom resolves. The code will need to handle that case where the resolve view is NULL and it doesn't mean no resolve is happening.

[werman]

Looking at MRTSS interaction with custom resolve, it doesn't even work in Turnip...

Looking through places to handle it in RenderDoc - I'm totally not sure how to handle them correctly so that UI would display correct images, my first attempt just shows "undefined img" for every image in RP. And there are no even tests in CTS exercising this.

So I don't think I would be able to fully solve this at the moment. The current change works with non-custom-resolve case and I see correctly resolved attachments in the output.

[baldurk]

I would rather have the complete fix that handles custom resolves as well. It sounds like maybe the discard pattern handling needs to be updated to check whether there's a real resolve target or not. Undefined image is an odd pattern because that only comes from image transitions and not from renderpass load/store, but with automatic renderpass resolves it can behave in unexpected ways.

@Zorro666 will be back next week and he implemented the custom resolve support, so he can take a look at this and see what would be needed.

[werman]

I think that VUID-VkRenderingAttachmentInfo-imageView-06863 together with the following text for vkCmdBeginCustomResolveEXT:

Following this call, any resolveImageView with resolveMode set to VK_RESOLVE_MODE_CUSTOM_BIT_EXT will be written by outputs which would otherwise have written to the imageView image until the end of the current render pass instance.

Effectively prevents MRTSS and custom resolve to be both applicable to the same attachment, since for custom resolve to happen it requires resolveImageView to be non-NULL, but MRTSS requires resolveImageView to be NULL.

[baldurk]

It's not very well specified because the meaning of "any resolveImageView with resolveMode set to VK_RESOLVE_MODE_CUSTOM_BIT_EXT" is unclear given the other ways these parameters are talked about - is a NULL parameter not included as "[any] resolveImageView" because it is NULL? I don't know what the intention is but I don't see any particular reason why you couldn't have a custom resolve the same as any fixed resolve, from the fake MSAA to real image. It'd be consistent with the other bits of the spec there.

This will need to get resolved inside khronos, it can't be resolved here when the language needs clarification to know what is actually allowed and not allowed.

[werman]

Fair enough. This edge case definitely needs an explicit statement in the spec.

[Zorro666]

Thank you for finding this problem at capture time. I have been looking at the other areas of the code that would need to be changed. Most of it is in the replay of the custom resolve feature.

Two Khronos Vulkan specification issues have been opened to get clarification:

1. if resolveMode is not None then when can resolveImageView be NULL (I believe it is only possible with MRTSS but need Khronos clarification)
2. the interaction of the custom resolve feature and MRTSS (it could/should work but I doubt there is CTS coverage and unsure which drivers have implemented it correctly).

When both of those issues are answered then the complete RenderDoc fix can be implemented.