-
Notifications
You must be signed in to change notification settings - Fork 591
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dynamic state3 multisample rasterization #856
base: main
Are you sure you want to change the base?
Dynamic state3 multisample rasterization #856
Conversation
Signed-off-by: Pawel Jastrzebski <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
We should wait until validation is fixed. The error is per draw call, so with validation the sample right now will spam lots of (the same) validation errors. |
VkSampler get_sampler() const; | ||
|
||
VkImageView get_font_image_view() const; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would stay on the same abstraction level as this class, and return a vkb::core::Sampler const &
, and vkb::core::ImageView const &
, respectively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since sampler
and image_view
are defined as std::unique_ptr
's it would mean that it's either returned by std::move
- not a good idea to change ownership of a pointer of internal class member. Or alternatively, in my opinion even worse to return raw pointer. It could be also defined as std::shared_ptr
and this means a design change. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, returning a vkb::core::Sampler const &
and a vkb::core::ImageView const &
would be right.
As as they both don't support copy construction or copy assignment, you would not be able to accidentally assign or copy them.
samples/extensions/dynamic_multisample_rasterization/README.adoc
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.h
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.h
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.cpp
Show resolved
Hide resolved
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
Signed-off-by: pawel-jastrzebski-mobica <pawel.jastrzebski@mobica.com>
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.cpp
Show resolved
Hide resolved
Status of resolving VVL issues:
About second VL It have issue when sample count is changed (by creating new renderpass/framebuffer and invoking vkCmdSetRasterizationSamplesEXT) to other from value which have been set on pipeline creation. Is my thoughts make sense? |
After implementing dynamic rendering into that Sample I solved all previous VL issues. Unfortunately I found new one related to necessary image transition (same as was presented in dynamic_rendering sample). I checked Dynamic Rendering sample and the same issue is visible there. After resolving that on this PR I will create new one to fix it in Dynamic Rendering sample. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On startup, I get this VL error:
Validation Error: [ VUID-vkDestroyImageView-imageView-parameter ] Object 0: handle = 0x2647997b740, type = VK_OBJECT_TYPE_INSTANCE; | MessageID = 0xe0208096 | vkDestroyImageView(): imageView Invalid VkImageView Object 0xcdcdcdcdcdcdcdcd. The Vulkan spec states: If imageView is not VK_NULL_HANDLE, imageView must be a valid VkImageView handle (https://vulkan.lunarg.com/doc/view/1.3.275.0/windows/1.3-extensions/vkspec.html#VUID-vkDestroyImageView-imageView-parameter)
=> Would you please initialize the members of ImageData in api_vulkan_sample.h with VK_NULL_HANDLE?
When resizing or maximizing the window, I get this one:
Validation Error: [ VUID-VkFramebufferCreateInfo-pAttachments-00881 ] Object 0: handle = 0xab64de0000000020, type = VK_OBJECT_TYPE_RENDER_PASS; Object 1: handle = 0x947e98000000126c, type = VK_OBJECT_TYPE_IMAGE_VIEW; Object 2: handle = 0x4549ee000000126a, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x2ff52eec | vkCreateFramebuffer(): pCreateInfo->pAttachments[1] has VK_SAMPLE_COUNT_4_BIT samples that do not match the VK_SAMPLE_COUNT_1_BIT samples used by the corresponding attachment for VkRenderPass 0xab64de0000000020[]. The Vulkan spec states: If flags does not include VK_FRAMEBUFFER_CREATE_IMAGELESS_BIT, each element of pAttachments must have been created with a samples value that matches the samples value specified by the corresponding VkAttachmentDescription in renderPass (https://vulkan.lunarg.com/doc/view/1.3.275.0/windows/1.3-extensions/vkspec.html#VUID-VkFramebufferCreateInfo-pAttachments-00881)
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.h
Outdated
Show resolved
Hide resolved
samples/extensions/dynamic_multisample_rasterization/dynamic_multisample_rasterization.h
Outdated
Show resolved
Hide resolved
Fix for synchronization issue was provided. |
samples/extensions/dynamic_multisample_rasterization/README.adoc
Outdated
Show resolved
Hide resolved
|
||
== Using the extension | ||
|
||
To use the extension: | ||
1) `VK_DYNAMIC_STATE_RASTERIZATION_SAMPLES_EXT` must be added to `VkPipelineDynamicStateCreateInfo`. | ||
2) Method `void vkCmdSetRasterizationSamplesEXT(VkCommandBuffer commandBuffer, VkSampleCountFlagBits rasterizationSamples)` should be called between `vkCmdBeginRenderPass` and `vkCmdEndRenderPass`. | ||
2) Method `void vkCmdSetRasterizationSamplesEXT(VkCommandBuffer commandBuffer, VkSampleCountFlagBits rasterizationSamples)` should be called inside active command buffer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe: with the active command buffer
?
|
||
== Known issues | ||
|
||
The extension always reports the following validation errors when enabled: | ||
.... | ||
VUID-vkCmdDraw-rasterizationSamples-07474(ERROR / SPEC): msgNum: -2007336333 - Validation Error: [ VUID-vkCmdDraw-rasterizationSamples-07474 ] Object 0: handle = 0x1ab5b866f50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; | MessageID = 0x885a7a73 | vkCmdDraw(): Render pass attachment 1 samples VK_SAMPLE_COUNT_1_BIT does not match samples VK_SAMPLE_COUNT_4_BIT set with vkCmdSetRasterizationSamplesEXT(). The Vulkan spec states: If the bound graphics pipeline state was created with the VK_DYNAMIC_STATE_RASTERIZATION_SAMPLES_EXT state enabled, and neither the VK_AMD_mixed_attachment_samples nor the VK_NV_framebuffer_mixed_samples extensions are enabled, then the rasterizationSamples in the last call to vkCmdSetRasterizationSamplesEXT must be the same as the current subpass color and/or depth/stencil attachments (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-vkCmdDraw-rasterizationSamples-07474) | ||
VUID-vkCmdDraw-renderPass-02684: Validation Error: [ VUID-vkCmdDraw-renderPass-02684 ] Object 0: handle = 0x7533b600000006c9, type = VK_OBJECT_TYPE_RENDER_PASS; Object 1: handle = 0x30f5a50000000020, type = VK_OBJECT_TYPE_RENDER_PASS; | MessageID = 0x50685725 | vkCmdDraw(): RenderPasses incompatible between active render pass w/ VkRenderPass 0x7533b600000006c9[] and pipeline state object w/ VkRenderPass 0x30f5a50000000020[] Attachment 0 is not compatible with 0: They have different samples.. The Vulkan spec states: The current render pass must be compatible with the renderPass member of the VkGraphicsPipelineCreateInfo structure specified when creating the VkPipeline bound to VK_PIPELINE_BIND_POINT_GRAPHICS (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-vkCmdDraw-renderPass-02684) | ||
SYNC-HAZARD-WRITE-AFTER-READ: Validation Error: [ SYNC-HAZARD-WRITE-AFTER-READ ] Object 0: handle = 0x555556f0b520, type = VK_OBJECT_TYPE_QUEUE; | MessageID = 0x376bc9df | vkQueueSubmit(): Hazard WRITE_AFTER_READ for entry 0, VkCommandBuffer 0x555558e50aa0[], Submitted access info (submitted_usage: SYNC_IMAGE_LAYOUT_TRANSITION, command: vkCmdPipelineBarrier, seq_no: 1, VkImage 0xcb3ee80000000007[], reset_no: 6). Access info (prior_usage: SYNC_PRESENT_ENGINE_SYNCVAL_PRESENT_ACQUIRE_READ_SYNCVAL, read_barriers: VK_PIPELINE_STAGE_2_COLOR_ATTACHMENT_OUTPUT_BIT|VK_PIPELINE_STAGE_2_BOTTOM_OF_PIPE_BIT, , batch_tag: 158, vkAcquireNextImageKHR aquire_tag:158: VkSwapchainKHR 0xf56c9b0000000004[], image_index: 2image: VkImage 0xcb3ee80000000007[]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some explanation on this validation error? Is it wrong, or just not yet resolved, or what?
} | ||
VkPhysicalDeviceExtendedDynamicState3FeaturesEXT requested_feature = {VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EXTENDED_DYNAMIC_STATE_3_FEATURES_EXT}; | ||
requested_feature.extendedDynamicState3RasterizationSamples = VK_TRUE; | ||
gpu.request_extension_features<VkPhysicalDeviceExtendedDynamicState3FeaturesEXT>(VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EXTENDED_DYNAMIC_STATE_3_FEATURES_EXT) = requested_feature; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahm, no... this code sequence was supposed to be inside the (now removed) if-clause, checking if that feature is supported.
When I start this sample, I get this error message: That is, you would need to |
Thanks for letting me know about that VL error. On which platform do you test? I didn't see that issue on ubuntu, but I will add that line and please give me feedback if that resolve your VL issue. |
Strange, I can use structs from this extension without add but when I add that line I get error because of not having that extension available on my GPU. I think something is not right, because I obviously can use features of that extension without adding it to application. |
I see that on Win10, using an NVIDIA GPU, Vulkan SDK 1.3.280. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works fine on my setup (Win 11, NV RTX 4070). Validation is clean, and changing MSAA modes has a clear visual effect. 👍🏻
I only noticed a few minor issues not directly related to the rendering part itself.
Those should be easy to fix, and once done I'll approve this :)
samples/extensions/dynamic_multisample_rasterization/README.adoc
Outdated
Show resolved
Hide resolved
{ | ||
title = "DynamicState3 Multisample Rasterization"; | ||
|
||
set_api_version(VK_API_VERSION_1_2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we lower this to VK_API_VERSION_1_1
? None of the extension used requires 1.2, and the readme also states 1.1 as the base line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about Dynamic Rendering? In spec I read that extension was introduced in Vulkan 1.2.197.
Also if you check dynamic rendering sample it also have Vulkan version set on 1.2.
When I lower version to 1.1 bunch of new VL errors appear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the actual VL error you get? Dynamic rendering works fine with 1.1 if you meed the criteria described under https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_dynamic_rendering.html#_extension_and_version_dependencies. Maybe you were missing some of the extensions required when using 1.1?
If that's too big of a change, I'm fine with the 1.2 requirement.
Overview
This sample demonstrates one of the functionalities of VK_EXT_extended_dynamic_state3 related to rasterization samples.
The extension can be used to dynamically change sampling without any need to restart the application.
Enabling the extension
To be able to use this extension in Vulkan API:
VkPhysicalDeviceExtendedDynamicStateFeaturesEXT
,VkPhysicalDeviceExtendedDynamicState2FeaturesEXT
,VkPhysicalDeviceExtendedDynamicState3FeaturesEXT
must be added to the feature chain of
VkPhysicalDeviceProperties2
andVkPhysicalDeviceExtendedDynamicState3PropertiesEXT
must be added toVkPhysicalDeviceProperties2
.Using the extension
To use the extension:
VK_DYNAMIC_STATE_RASTERIZATION_SAMPLES_EXT
must be added toVkPipelineDynamicStateCreateInfo
.void vkCmdSetRasterizationSamplesEXT(VkCommandBuffer commandBuffer, VkSampleCountFlagBits rasterizationSamples)
should be called betweenvkCmdBeginRenderPass
andvkCmdEndRenderPass
.Known issues
The extension always reports the following validation error when enabled:
This implies several other validation errors during runtime:
Resources
https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_extended_dynamic_state3.html
https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCmdSetRasterizationSamplesEXT.html
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including: