Skip to content

Unify vkb::ShaderModule and vkb::core::HPPShaderModule into vkb::core::ShaderModule<bindingType> - #1556

Open
asuessenbach wants to merge 15 commits into
KhronosGroup:mainfrom
asuessenbach:unify_ShaderModule
Open

Unify vkb::ShaderModule and vkb::core::HPPShaderModule into vkb::core::ShaderModule<bindingType>#1556
asuessenbach wants to merge 15 commits into
KhronosGroup:mainfrom
asuessenbach:unify_ShaderModule

Conversation

@asuessenbach

Copy link
Copy Markdown
Contributor

Description

Next unified class on C- and C++-bindings.
Comes together with unification of vkb::ShaderResourceType, vkb::ShaderResourceMode, vkb::ShaderResourceQualifiers, vkb::ShaderResource, vkb::ShaderVariant, vkb::ShaderSource, and vkb::SPIRVReflection.

Build tested on Win11 with VS2026. Run tested on Win11 with NVidia GPU.

General Checklist:

Please ensure the following points are checked:

  • My code follows the coding style
  • I have reviewed file licenses
  • I have commented any added functions (in line with Doxygen)
  • I have commented any code that could be hard to understand
  • My changes do not add any new compiler warnings
  • My changes do not add any new validation layer errors or warnings
  • I have used existing framework/helper functions where possible
  • My changes do not add any regressions
  • I have tested every sample to ensure everything runs correctly
  • This PR describes the scope and expected impact of the changes I am making

Note: The Samples CI runs a number of checks including:

  • I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
  • My changes build on Windows, Linux, macOS and Android. Otherwise I have documented any exceptions

If this PR contains framework changes:

  • I did a full batch run using the batch command line argument to make sure all samples still work properly

Sample Checklist

If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist:

  • I have tested the sample on at least one compliant Vulkan implementation
  • If the sample is vendor-specific, I have tagged it appropriately
  • I have stated on what implementation the sample has been tested so that others can test on different implementations and platforms
  • Any dependent assets have been merged and published in downstream modules
  • For new samples, I have added a paragraph with a summary to the appropriate chapter in the readme of the folder that the sample belongs to e.g. api samples readme
  • For new samples, I have added a tutorial README.md file to guide users through what they need to know to implement code using this feature. For example, see conditional_rendering
  • For new samples, I have added a link to the Antora navigation so that the sample will be listed at the Vulkan documentation site

@asuessenbach
asuessenbach requested a review from a team July 22, 2026 08:53
SaschaWillems and others added 2 commits August 10, 2026 08:28
* Add sample for descriptor heaps

* First working version

* Trying to fix clang format

* Clang format

* Add Slang and HLSL shaders

* No longer pre-record command buffers

* Cleanup

* Cleanup

* UBOs per Frame

* Add precompiled SPIR-V

* Override render pass / framebuffer creation

Both not required, as the sample uses dynamic rendering

* Slight restructuring

* Slight restructuring

* Clang format

* Cleanup

* Remove extension not required
Core since 1.1

* Reserved range offsets

* Variable naming

* Minor fixes

* Adjust offset and size calculations

* Add tutorial for descriptor heap sample

* Add compiled shaders

* Add new sample to navigation

* Clean up, push data documentation

* Fix clang format

* Variable naming

* Spelling and grammar

* Add comparison to traditional descriptor bindings

* Minor fix

* Add some key benefits of descriptor heaps

* Address PR feedback

* Adjust heap size calculations

* Address PR feedback

* Add and use non-const memory access

* Use non-uniform indexing to access textures

* Add note on where to find other binding mapping options
* Add simple API sample for VK_EXT_present_timing.

* Address review comments

- Fix "times domains" typo in README
- Add missing blank lines before bullet lists in README
- Rename desired_swapchain_images to desired_swapchain_image_count
- Simplify can_use_present_timing boolean assignment

* Remove double-assignment of time domains / timing properties counters
@tomek-brcm

Copy link
Copy Markdown

I'm hitting 2 problems:

[error] Error Message: unordered_map::at
[error] Failed when running application Compute N-Body simulation
[info] Benchmark for Compute N-Body simulation completed in 0 seconds (ran 0 frames, averaged nan fps)
[error] -1363947098 - VUID-vkDestroyPipeline-pipeline-parameter: vkDestroyPipeline(): pipeline Invalid VkPipeline Object 0x2c895a0.
The Vulkan spec states: If pipeline is not VK_NULL_HANDLE, pipeline must be a valid VkPipeline handle (https://docs.vulkan.org/spec/latest/chapters/pipelines.html#VUID-vkDestroyPipeline-pipeline-parameter)

followed by:
vulkan_samples: Vulkan-ValidationLayers/layers/./thread_tracker/thread_safety_validation.h:122: std::shared_ptrthreadsafety::ObjectUseData threadsafety::Counter::FindObject(T, const Location&) [with T = VkPipeline_T*]: Assertion `object_table.contains(object)' failed.
Aborted (core dumped)

@asuessenbach

asuessenbach commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@tomek-brcm Thanks for spotting this failure. Fix was easy, I've no idea how that could have slept through.

Besides that, this PR now fixes an error I get with 2 samples on main:

hpp_pipeline_cache:

[error] Error Message: Failed to open file for reading at path: shaders/
[error] Failed when running application HPP Pipeline Management

pipeline_cache:

[error] Error Message: Failed to open file for reading at path: shaders/
[error] Failed when running application Pipeline Management

tomek-brcm
tomek-brcm previously approved these changes Aug 24, 2026

@tomek-brcm tomek-brcm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Works for me now.

@asuessenbach
asuessenbach dismissed tomek-brcm’s stale review August 24, 2026 14:11

The merge-base changed after approval.

asuessenbach and others added 7 commits August 24, 2026 08:34
* Add sample demonstrating VK_EXT_layer_settings usage

* - Update README with GitHub repository link for sample

* Add local debug messenger and UI overlay for validation messages in LayerSettingsSample

* Adjust code formatting in LayerSettingsSample for improved readability

* Add interactive validation scenarios with toggleable UI and message caching

Transforms layer_settings sample into an interactive demonstration with three toggleable validation scenarios (wrong buffer flags, suboptimal transitions, small allocations). Executes validation-triggering operations once during setup using one-time command buffers to work with message caching. Adds per-scenario warning/error counters, message caching, and resource cleanup. Updates UI with checkboxes, statistics display, and scrollable message viewer with improved styling.

* Update copyright year to 2026

* Refactor scenario state tracking from array to unordered_map and modernize C++ usage

Replaces `std::array` with `std::unordered_map<Scenario, ScenarioState>` for scenario state storage, removing the `Scenario::Count` sentinel. Adds custom hash functor for the enum key. Modernizes code with `std::format`, `std::ranges`, and direct use of `vkCreateDebugUtilsMessengerEXT`. Updates README list formatting, adds `validation_features_supported()` helper, and initializes scenario map entries in constructor. Improves code consistency with better whitespace and comment cleanup.

* Fix indentation to use tabs consistently throughout layer_settings files

* Replace std::format with fmt::format and refactor loop variable naming

Removes unused `<format>` include and switches to `fmt::format` for formatting. Renames loop variable from `kv` to `scenario` for better clarity when iterating over scenario states map.

* Add note requested by comments.

* Add instance extension and layer setting registration to VulkanSample base class

Adds `add_instance_extension()` and `add_layer_setting()` methods to allow samples to register instance-level extensions and layer settings. Introduces `instance_extensions` map and `layer_settings` vector as member variables. Integrates these into `request_instance_extensions()` and `request_layer_settings()` to automatically apply sample-specific configurations during instance creation.

* Fix whitespace alignment in vulkan_sample.h and correct method name in layer_settings

Aligns `present_mode` and `present_mode_priority_list` declarations with consistent spacing. Replaces incorrect `is_enabled()` call with `is_extension_enabled()` for debug utils extension check.

* Revert the CRLF to Unix-style line endings that caused the whole file change inadvertently.

* Address comments from Andreas' latest feedback.  replaced constructor-time add_instance_extension/add_layer_setting calls with proper virtual overrides.

* address requested changes.
tomek-brcm
tomek-brcm previously approved these changes Aug 24, 2026
@asuessenbach
asuessenbach dismissed tomek-brcm’s stale review August 24, 2026 17:05

The merge-base changed after approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants