Vk ext shader object optional layer #780
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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.
…_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.
SaschaWillems
previously approved these changes
May 28, 2024
gpx1000
commented
Dec 21, 2024
| // destroy created sampler before re-creating | ||
| vkDestroySampler(get_device().get_handle(), heightmap_texture.sampler, nullptr); | ||
| vkDestroySampler(get_device().get_handle(), terrain_array_textures.sampler, nullptr); | ||
|
|
||
| // Setup a mirroring sampler for the height map |
There was a problem hiding this comment.
looks like the calling load_assets multiple times bug has been fixed.
Updated validation layer mechanisms to use `unordered_map` for better clarity and flexibility in distinguishing required and optional layers. Adjusted related structures, methods, and logging to align with the new approach. Simplifies enabling and validation of layers across the framework and samples.
gpx1000
requested review from
marty-johnson59,
SaschaWillems and
asuessenbach
Aligned indentation and formatting in multiple files to maintain code consistency. Changes include correcting spacing, adjusting alignment in function signatures, and fixing misplaced inline elements where necessary.
Introduce the get_validation_layers() method in ShaderObject to return validation layer information. Additionally, make a minor formatting adjustment to vk::DynamicLoader initialization for readability.
20 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This fixes the remaining issues in the newly accepted/merged VK_EXT_shader_object PR.