Vk ext shader object optional layer #780
Conversation
…yer, and it's not found, continue to load the application instead of erroring out. Fix the validation layer discovered issues in shader object. The render_pass gets created twice (once in the super class prepare and once after the child class recreates the frame buffer. Destroying the first created renderpass will solve this.). Also the heightmap_texture and terrain_array_texture both get their sampler's default created and then explicitly recreated. So adding the destroy sampler just before recreation solves the second and third validation issue.
…it can be downloaded after the run and applied as a patch.
There was a problem hiding this comment.
This PR works.
But instead of improving the somewhat hacky usage of VulkanSample::get_validation_layers, I would completely remove that function, and instead introduce some VulkanSample::add_instance_layer (or maybe just VulkanSample::add_layer, as there's nothing like a device layer), similar to VulkanSample::add_instance_extension, and handle it analogously.
| @@ -230,6 +230,11 @@ void ShaderObject::setup_framebuffer() | |||
| // Create render pass for UI drawing | |||
| void ShaderObject::setup_render_pass() | |||
| { | |||
| // delete existing render pass | |||
| if (render_pass != VK_NULL_HANDLE) | |||
There was a problem hiding this comment.
Is setup_render_pass called multiple times?
If so, you would need to initialize render_pass with VK_NULL_HANDLE to make this check work in debug mode. If not, you don't need to destroy the render pass here.
There was a problem hiding this comment.
Hi @asuessenbach , per discussion today, can you open an issue so we don't lose the context? We're going to merge this one soon w/o capturing this - but want to circle back later
There was a problem hiding this comment.
Sorry, I missed the meeting where this was discussed...
Why should this issue not be resolved right here?
There was a problem hiding this comment.
The idea is to fix this after this PR has been merged. The PR is kinda urgent, and the line you highlighted is not new to the PR but already present in the base sample.
There was a problem hiding this comment.
@SaschaWillems so it seems, you know about the issue to be resolved later. Would you be so kind and file that issue, instead of me guessing what's meant to be done here?
There was a problem hiding this comment.
Sorry, I was under the impression that this change was already present beforehand. But as this code is new to this PR you're right to mention this and it indeed needs fixing in this PR.
There was a problem hiding this comment.
I did this change because renderpass was being created by the vulkansample in the framework automatically. I can take this change out, but then validation layers correctly identifies a render_pass object not getting released. The rest of the changes are updated to conform with your requests.
There was a problem hiding this comment.
That makes sense. We should keep that change then :)
…_shader_object_optional_layer # Conflicts: # framework/core/hpp_instance.h # framework/core/instance.h # framework/hpp_vulkan_sample.cpp # framework/hpp_vulkan_sample.h # framework/vulkan_sample.cpp # samples/extensions/shader_object/README.adoc # third_party/volk # third_party/vulkan
Related functions and variables have been updated to accommodate the change. This ensures consistency and optimizes the performance for layer searches and validations.
Description
This fixes the remaining issues in the newly accepted/merged VK_EXT_shader_object PR.