-
Notifications
You must be signed in to change notification settings - Fork 618
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
Add Layer Settings API to framework, improve batch mode error recovery, fix macOS issues #1084
base: main
Are you sure you want to change the base?
Conversation
…. create_instance() custom override
… settings and work-around variable descriptor counts
…ot DEFINED Vulkan_dxc_EXECUTABLE
…ility_subset extension and features
… permit VK_SUBOPTIMAL_KHR
…settings to work with Release builds
…) vs. ExitCode::FatalError
…_barycentric, and texture_mipmap_generation samples
…setup platform default search paths for dxc
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.
Thank you very much for this PR. Very much appreciated 👍🏻
I did a quick first look, but with so many changes crammed into one PR it may take some time to properly review this ;)
Though I won't be able to test any Mac OS / iOS related changes due to a lack of such hardware.
…ckwards compatibilty with older SDKs
The remaing CI issues (Doxygen, clang-format) appear to be upstream issues, not related to this PR. |
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.
The VulkanSample class is one of the few classes (by now) that are unified with regards to C- and C++-bindings of vulkan. To keep it consistent, some changes to your PR would be needed, that you can find in the attached patch.
0001-Adjust-LayerSetting-handling-in-VulkanSample-class.patch
samples/performance/multithreading_render_passes/multithreading_render_passes.cpp
Outdated
Show resolved
Hide resolved
…rror handling and logging
…older Vulkan SDKs
I don't plan to make any more changes here unless there are additional review comments or issues found. I will submit changes in a separate PR to remove macOS workarounds within the descriptor_indexing sample once improvements are made to MoltenVK in this area (currently in development). |
…tability subset features
Hopefully all issues raised by @asuessenbach and @SaschaWillems are now resolved. Again, I don't plan any more updates and re-review of recent changes / resolutions can now proceed. |
… sample_helper.cmake
Description
This PR provides solutions to #1080, #1081, #1082, and #1083.
add_layer_setting(VkLayerSettingEXT layerSetting)
, andget_layer_settings()
API calls in vulkan_sample.h. ModifiedInstance()
andHPPInstance()
parameter lists to includerequired_layer_settings
so they can be activated during instance creation. Protected all changes with#if defined(VK_EXT_layer_settings)
for backwards compatibility with Vulkan SDKs earlier than 1.3.272 where the extension may not be defined.get_validation_layers()
and use layer settings to activate the Validation Layer'sVK_VALIDATION_FEATURE_ENABLE_DEBUG_PRINTF_EXT
feature using the standard framework. The sample's customcreate_instance()
is now only used ifVK_EXT_layer_settings
is not available at runtime.MVK_CONFIG_USE_METAL_ARGUMENT_BUFFERS
feature which is necessary to increase the available descriptors for the sample. Also worked around a macOS/MoltenVK limitation in the variable descriptor count feature. This may be fixed in future versions of MoltenVK, but the workaround will function for past, current and future MoltenVK versions.VK_ENABLE_BETA_EXTENSIONS
on Apple platforms for access to theVK_KHR_portability_subset
extension and features. ActivatingVK_KHR_portability_subset
is required on all portability implementations where the extension is defined and available. Also activated themutableComparisonSamplers
portability feature for the async_compute and multithreading_render_pass samples.throw std::runtime_error()
vs.ExitCode::FatalError
inmain_loop()
andmain_loop_frame()
, b) modifyingVK_CHECK
to usethrow std::runtime_error()
to gracefully recover from more subtle issues such as pipeline creation problems (e.g. SPIR-V to Metal cross-compilation errors due to feature limitations) as well as queue submission errors, c) resetting the shader compiler's target environment settings prior to each sample to avoid stickiness from earlier samples in batch mode, d) allowingVK_SUBOPTIMAL_KHR
which avoids early termination of some samples when exiting while in batch mode, and e) fixing specific issues in async_compute, texture_mipmap_generation, and fragment_shader_barycentric samples to permit graceful failure and continuation while in batch mode../build/mac/app/bin/Release/<x86_64|arm64>/vulkan_samples
vs../build/mac/app/bin/Release/<x86_64|arm64>/vulkan_samples.app/Contents/MacOS/vulkan_samples
). App bundles are needed for iOS, but get in the way on macOS for command line applications like vulkan_samples.I have other improvements pending for Xcode project generation for macOS/iOS, running on iOS and the iOS simulator (x86_64 and arm64 hosts), and standalone MoltenVK support (i.e. without the vulkan loader, useful when working on the MoltenVK dev stream), but will submit those in a separate PR.
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
I have tested using batch runs on Windows and macOS.
Depending on CI for now on linux - will test laterBatch mode testing on Manjaro now complete. Cannot test on Android - don't have the environment. I have also tested on iOS but have additional changes to add in a separate PR.If this PR contains framework changes:
batch
command line argument to make sure all samples still work properly