Skip to content
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

Optional Features/Extensions should be deprecated/removed. #103

Open
kumihoclub opened this issue Jan 4, 2022 · 3 comments
Open

Optional Features/Extensions should be deprecated/removed. #103

kumihoclub opened this issue Jan 4, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@kumihoclub
Copy link

While built-in optional/features impl would seem helpful on the surface, there is no simple/friendly way to to reflect what features/extensions pass or fail back to the user both in internals and api-wise. I feel that the impls should be deprecated and eventually removed in favor of promoting a "feature levels" pattern, which is a common pattern ive seen where users configure multiple vkb::PhysicalDeviceSelector's as different conceptual "feature levels". if the best case doesn't pass, the next best is attempted and so on.

@charles-lunarg
Copy link
Owner

Agreed.

An example of this pattern in the docs would be good.

It would break API but well, I think its for the better. And its still not 1.0 so that's fine.

@charles-lunarg
Copy link
Owner

Finally got around to adding a deprecation notice.
No plan on how long the deprecated API calls will be available, but best to give some notice.
#212

@DexterHaxxor
Copy link

DexterHaxxor commented Oct 14, 2023

These feature levels are definitely a great idea. but there should be way to do these in the API itself:

auto phys_ret = selector.set_surface(surface)
        .set_minimum_version(1, 2)
        .add_required_extension(VK_KHR_DYNAMIC_RENDERING_EXTENSION_NAME)
        .add_required_extension_features(VkPhysicalDeviceDynamicRenderingFeatures{
                .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DYNAMIC_RENDERING_FEATURES,
                .dynamicRendering = true,
        })
        .feature_level(1) // 0 is the default feature level, omitted above
        .add_required_extensions(VK_EXT_MESH_SHADER_EXTENSION_NAME)
        .add_required_extension_features(...)
        .select();
// handle errors
auto physical_device = phys_ret.value();
// reading physical_device.feature_level tells the application which features were enabled
fmt::print("device {} feature level {}\n", physical_device.name, physical_device.feature_level);

This should allow for supplying new VkPhyisicalDeviceXXXFeatures with on each feature level, should also allow setting the minimum Vulkan API version, generally everything that you can do in the default feature level should be doable in any feature level, they should be thought of as overrides, and the VkBootstrap library will attempt to create and select the device with the highest feature level.

We also need to create an alternative to feature levels, feature flags, that are not mutually exclusive, e.g. feature flags X & Y and feature level 2 could be enabled at the same time.

bool memory_budget;
physical_device_selector
        .feature_flag(&memory_budget)
        .add_required_extension(VK_EXT_MEMORY_BUDGET_EXTENSION_NAME);

// when initialising VMA
if (memory_budget) {
    vma_flags |= VMA_ALLOCATOR_CREATE_EXT_MEMORY_BUDGET_BIT;
}

These feature flags do not have to support every operation that feature levels support, for example, set_minimum_version does not make sense inside a feature flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants