Skip to content

Refactor device extension handling#1552

Open
asuessenbach wants to merge 3 commits into
KhronosGroup:mainfrom
asuessenbach:request_device_extensions
Open

Refactor device extension handling#1552
asuessenbach wants to merge 3 commits into
KhronosGroup:mainfrom
asuessenbach:request_device_extensions

Conversation

@asuessenbach

Copy link
Copy Markdown
Contributor

Description

Similar to what we've done in #1455, I refactored the device extension handling.
Instead of the add_device_extension and get_device_extensions functions, we now have a request_device_extensions, exactly like we already have a request_instance_extensions.

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 force-pushed the request_device_extensions branch 2 times, most recently from 3abef20 to a82f6ef Compare July 9, 2026 06:55
@asuessenbach asuessenbach requested a review from a team July 9, 2026 08:46
tomek-brcm
tomek-brcm previously approved these changes Jul 10, 2026
@tomek-brcm

Copy link
Copy Markdown

Nit: VK_EXT_DEBUG_MARKER_EXTENSION_NAME is now requested unconditionally (as Optional) in the base request_device_extensions(), whereas before it was only added once the code had already confirmed debug_utils wasn't set via VK_EXT_DEBUG_UTILS. This is harmless (Optional, so no failure) but can enable an unused device extension when debug-utils is already active via the instance extension — a minor cleanliness nit, not a bug.


void OITLinkedLists::request_device_extensions(std::unordered_map<std::string, vkb::RequestMode> &requested_extensions) const
{
vkb::VulkanSampleC::request_device_extensions(requested_extensions);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there a way to implicitly call that base class function? Otherwise people might forget to call this and wonder why things might not work as expected. Not a fan of enabling extensions this way.

@asuessenbach asuessenbach Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to implicitly call that base class function?

Sure, would not be hard to do.
But then request_instance_extensions, request_layers, request_layer_settings, request_validation_feature_enables should be changed accordingly as well.

Note: currently, the only function that does not follow that call-the-base-class-function principle is request_gpu_features. Probably just because the base class' implementation is empty.

That is, if requested (pun intended), I could change it, but would prefer to do so in a separate PR for all those functions.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good point. If you ask me, consistency should be preferred. So if we want to go with the explicit way of calling the base class function, we should make all similar places use that and also update the sample templates accordingly to make things easier for maintainers ;)

@asuessenbach

Copy link
Copy Markdown
Contributor Author

Nit: VK_EXT_DEBUG_MARKER_EXTENSION_NAME is now requested unconditionally (as Optional) in the base request_device_extensions(), whereas before it was only added once the code had already confirmed debug_utils wasn't set via VK_EXT_DEBUG_UTILS.

Good point.
Changed code to request VK_EXT_DEBUG_MARKER_EXTENSION_NAME (as Required) only when debug_utils points to a vkb::core::HPPDebugMarkerExtDebugUtils.

@asuessenbach asuessenbach changed the title Refactored device extension handling Refactor device extension handling Jul 14, 2026
@asuessenbach asuessenbach force-pushed the request_device_extensions branch from 38a7260 to 643ed09 Compare July 14, 2026 12:21
@asuessenbach asuessenbach force-pushed the request_device_extensions branch from 643ed09 to 236be2a Compare July 14, 2026 15:18
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.

3 participants