renderdoc icon indicating copy to clipboard operation
renderdoc copied to clipboard

Add QCOM Multiview Per View Viewport extension support

Open mazvalente opened this issue 1 year ago • 5 comments

Add support for the extension VK_QCOM_multiview_per_view_viewports which allows for multiple viewports to be submitted in a single view. RenderDoc already supports this viewport/scissor configuration, and this change will allow applications which submit multiple viewports and scissors using the extension to render correctly.

mazvalente avatar Dec 22 '23 02:12 mazvalente

was it marked as ready for review accidentally and it should still be a draft?

It was marked as ready for review intentionally, but definitely should have received another pass before doing so, my apologies. I've cleaned up the suggested changes and will will reopen for review once I verify that everything is working properly.

mazvalente avatar Jan 10 '24 23:01 mazvalente

You haven't explained why the difference - does this extension really need no modification whatsoever of the replay handling? If so, why were there changes before and how have you tested this to make sure that the replay handling is robust even when the extension is used?

You're right, this does require some clarification. All that multiviewperviewviewports does is give permission for Vulkan to use multiple viewports and scissors, which is already supported by the current VkCmdSetViewports/VkCmdSetScissors functions. From my testing with an app which submits multiple viewports and scissors it looks like there is no change needed on the replay side to enable the feature. Your mentioning that the feature should not be automatically enabled if the struct is attached suggests that I should add a check when submitting viewports to ensure that we have a form of multiviewport support enabled and otherwise either only accept the first submission or throw an error.

mazvalente avatar Jan 23 '24 23:01 mazvalente

Something sounds wrong here. Your description of what the extension does doesn't match up with its description in the spec or my understanding of it. Unextended vulkan absolutely does allow the use of multiple viewports and scissor regions and there is no extension needed for that, though there are extensions that expand its use like VK_EXT_shader_viewport_index_layer. This extension seems to change the behaviour of multiview passes to automatically use the multiview view index as a viewport index if no explicit viewport index is selected.

I don't understand what you're saying about only accepting 'the first submission' or throwing any kind of error. Can you explain exactly what you mean by that?

baldurk avatar Jan 24 '24 12:01 baldurk

You're right, I was misunderstanding the description. After testing the implementation by making shader changes to selectively enable multiview in shaders through VK_EXT_shader_viewport_index_layer, it appears that no replay changes are necessary. Without shader changes the multiview functionality follows the spec implementation of using the multiview index as viewport index, and if I set the viewportIndex in the shader I override that behavior instead following the VK_EXT_shader_viewport_index_layer logic.

I don't understand what you're saying about only accepting 'the first submission' or throwing any kind of error. Can you explain exactly what you mean by that?

I did not properly understand what I should have been doing to handle non-spec compliant situations, and was making bad assumptions, both on what the correct spec requirements were, and how to handle them. Please ignore it.

I've also uncovered one issue that I'll need to fix before putting this back in review, the debug overlay for Viewports/Scissors does not recognize the second viewport properly and is currently not displaying any overlay when selected for slice 1. I'll reopen this when I have a fix.

mazvalente avatar Feb 09 '24 20:02 mazvalente

This resolves the issue with vk_overlay not displaying the proper viewport/scissors region. If you know of a better way to retrieve the active slice value without modifying the RenderOverlay signature please advise me, but I don't believe it's retrievable without either changing the Overlay value to not be the enum or adding a new value in.

mazvalente avatar Feb 27 '24 23:02 mazvalente

I'm not meaning to be rude but I think there's still a misunderstanding here about how this extension works, and maybe how extensions work on Vulkan in general.

For that reason I'm closing this PR for now - could you please open this as a feature request issue for extension support and then it can be tracked that way?

baldurk avatar Mar 05 '24 17:03 baldurk