KhronosGroup / Vulkan-Samples

One stop solution for all Vulkan samples
Apache License 2.0
4.34k stars 648 forks source link

Add a new plugin for command line GPU selection #829

Closed SaschaWillems closed 1 year ago

SaschaWillems commented 1 year ago

Description

This PR adds a new plugin that adds GPU selection via command line. With this plugin, a user can now override the default GPU selection and explicitly request a GPU by a given index. This is esp. useful if you want to debug/test samples on GPUs other than your main GPU. I currently use this to test and fix things using swiftshader as a second ICD on my system. This PR also removes the get_first_gpu instance function that wasn't used anywhere.

The plugin itself is pretty simple, and GPU selection is done as follows:

sample instancing --gpu 1

General Checklist:

Please ensure the following points are checked:

asuessenbach commented 1 year ago

This plugin seems to be pretty much straight forward... but I'm a bit concerned about the direct access to the [HPP]Instance. Wouldn't it be possible, to carry that information to the sample via the ApplicationOptions?

SaschaWillems commented 1 year ago

Tbh I've had a hard time find out the optimal way of passing plugin data to other classes and just took the same approach as an already existing plugin.

asuessenbach commented 1 year ago

Instead of directly coupling vkb::Instance and vkb::core::HPPInstance to this plugin, you could as well modify Platform::start_app like this:

    auto gpu_selection_it = std::find_if(active_plugins.begin(), active_plugins.end(), [](Plugin *plugin) { return plugin->get_name() == "GPU selection"; });

    std::optional<uint32_t> gpu_selection;
    if (gpu_selection_it != active_plugins.end())
    {
        gpu_selection = reinterpret_cast<::plugins::GpuSelection *>(*gpu_selection_it)->get_gpu_index();
    }

    if (!active_app->prepare({false, window.get(), gpu_selection}))
    {

extending the ApplicationOptions by some std::optional<uint32_t> gpu_selection. And vkb::Instance::get_suitable_gpu (and vkb::core::HPPInstance::get_suitable_gpu) could then take an additional argument std::optional<uint32_t> const &gpu_selection

SaschaWillems commented 1 year ago

Yeah, that would be a different way. But the way plugins pass data and how that data is accessed is all over the place, and every plugin seems to do this differently.

I'd prefer to have this merged (as I need it to fix some open issues) and then at some point talk about how to rework/improve the whole framework situation, esp. for those that work on it.

tomadamatkinson commented 1 year ago

@SaschaWillems we can look at simplifying plugins and allowing them to be enabled pragmatically as well as the CLI. I image we can drop quite a bit of the complexity there