Skip to content

Refactor chaining of DeviceCreateInfo - #1588

Open
asuessenbach wants to merge 1 commit into
KhronosGroup:mainfrom
asuessenbach:extend_device_create_info
Open

asuessenbach wants to merge 1 commit into
KhronosGroup:mainfrom
asuessenbach:extend_device_create_info

Conversation

@asuessenbach

Copy link
Copy Markdown
Contributor

Description

Analogous to the structure chain handling for instance creation on InstanceCreateInfo, this PR uses the StructureChainBuilder to chain the handling for device creation on DeviceCreateInfo, introducing a dedicated function where and when to do that:

  • introduces the virtual function vkb::VulkanSample::extend_device_create_info
  • that function is called in the vkb::core::Device constructor
  • thus slightly simplifies vkb::core::PhysicalDevice, as it's not used any more to gather and store that structure chain
  • the member functions request_optional_feature and request_required_feature of the vkb::core::PhysicalDevice have changed and renamed to enable_optional_feature and enable_required_feature, respectively
  • the helper macros REQUEST_OPTIONAL_FEATURE and REQUEST_REQUIRED_FEATURE are renamed to ENABLE_OPTIONAL_FEATURE and ENABLE_REQUIRED_FEATURE
  • for clarity, the member function vkb::PhysicalDevice::get_features is renamed to vkb::PhysicalDevice::get_supported_features, being a better companion to vkb::PhysicalDevice::get_requested_features
  • the member function vkb::StructureChainBuilder::set_feature_flag is added that handles setting a specific flag in a specific class in the structure chain, potentially adding that struct if needed.

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 extend_device_create_info branch 5 times, most recently from 6cdbddf to 251d24c Compare October 8, 2026 22:47
@SRSaunders

SRSaunders commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Apple builds are failing because several REQUEST_OPTIONAL_FEATURE calls have not been converted and are being guarded by VKB_ENABLE_PORTABILITY in the following two samples: async_compute, and multithreading_render_passes, as follows:

#ifdef VKB_ENABLE_PORTABILITY
	// Since sampler_info.compareEnable = VK_TRUE, must enable the mutableComparisonSamplers feature of VK_KHR_portability_subset
	REQUEST_OPTIONAL_FEATURE(gpu, VkPhysicalDevicePortabilitySubsetFeaturesKHR, mutableComparisonSamplers);
#endif

ApiVulkanSample::submit_frame();
}

void BufferDeviceAddress::extend_device_create_info(vkb::StructureChainBuilderC<VkDeviceCreateInfo> &scb)

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.

This reads much harder then the old code :/

E.g. why is that assert(create_info) needed now?

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.

E.g. why is that assert(create_info) needed now?

I think I don't get what you're asking for...
Before I use a pointer, I tend to check its validity. If it's allowed to be a nullptr, I'm using a runtime-check. Otherwise, when a nullptr would be a hint for a programming error, I'm using an assert.

This reads much harder then the old code :/

That may be true. But I think this way is safer.
Previously, the structure chain was controlled by vkb::core::PhysicalDevice just to create the vkb::core::Device. Now this chain is built as needed, making its handling more localized. And there’s now exactly one place in the code where you can extend it. This way, there’s no room for ambiguity.
And as a side effect—but this is exactly why I initiated this PR—vkb::core::PhysicalDevice is now a bit less complex.

@asuessenbach
asuessenbach force-pushed the extend_device_create_info branch from 251d24c to 2b3d208 Compare October 12, 2026 09:08
@asuessenbach
asuessenbach force-pushed the extend_device_create_info branch from 2b3d208 to 6fd5496 Compare October 12, 2026 10:00
@asuessenbach
asuessenbach requested a review from a team October 12, 2026 13:27

This branch has not been deployed

No deployments
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