Skip to content

WIP: Vulkan Context and Builders - Experimental Early Feedback Wanted :) #576

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

Merged

Conversation

tomadamatkinson
Copy link
Collaborator

Based on #574 and #575.

Description

This is a larger PR that aims to address the foundation of the Vulkan Framework. Every sample requires and instance, GPU selection, and device creation. Most samples require some form of queue management (currently this is done in the device itself).

Currently, we have a few ways of communicating with the Instance and Device class upfront to be able to pass the correct information to their constructor. However, these classes are fixed functions and are not easily adjusted to a sample's use-case. Enabled extensions and layers are not transparent to the user. Device features can not be easily toggled.

Solution

I propose that we simplify the entire process. Allow users to configure the Instance, GPU Selection Criteria, Device, and Queues upfront using a nested builder pattern. Handles and objects will then be accessible via the Context object.

I also aim to duplicate these builders using VulkanHPP so that VulkanHPP features can be developed in the future. The ContextHPP object will have a method to retrieve a Context& so that it is interoperable with Vulkan C components.

Examples

There are a few patterns to interact with the builder. The most powerful is the ability to pass BuilderFunctions.

void apply_default_configuration(ContextBuilder &builder)
{
	builder
	    .configure_instance()
              .application_info([]() -> VkApplicationInfo {
                  return {
                      VK_STRUCTURE_TYPE_APPLICATION_INFO,
                      nullptr,
                      "Vulkan Samples",
                      VK_MAKE_VERSION(1, 0, 0),
                      "Vulkan Samples Framework",
                      VK_MAKE_VERSION(1, 0, 0),
                      VK_API_VERSION_1_3,
                  };
              })
              .done()
	    .build();
}

This function can be later used by calling builder.apply(apply_default_configuration). We can compound these functions in different ways to achieve different effects. For instance, we may have a single builder.apply(default_config); which populates the builder with a minimal set of features to render to the screen.

With this pattern we can also remove functionality that normally clutters classes. One example of this is creating a VkSurfaceKHR. Currently, we need to create objects in specific orders so that the Surface is ready before device creation. This adds a strict requirement to a samples application flow. Now, there is no strict requirement and the builder itself does not need to know the existence of VkSurfaceKHR.

PhysicalDeviceBuilderFunction surface_must_be_compatable(VkSurfaceKHR surface)
{
	return [=](PhysicalDeviceBuilder &builder) -> uint32_t {
		builder.apply_custom([=](VkPhysicalDevice gpu, Queues queues) {
			for (auto family : queues)
			{
				VkBool32 supported;
				vkGetPhysicalDeviceSurfaceSupportKHR(gpu, family.index(), gpu, &supported);
				if (supported)
				{
					return 1000U;        // large score for supported surfaces
				}
			}

			return 0U;        // no score if the surface is not supported by any queues
		});
	}
}

There are other methods we can use to configure queues in the future.

TODO: flesh out this concept a little more

@tomadamatkinson tomadamatkinson added the framework This is relevant to the framework label Dec 11, 2022
@tomadamatkinson tomadamatkinson self-assigned this Dec 11, 2022
Copy link
Contributor

@asuessenbach asuessenbach left a comment

Choose a reason for hiding this comment

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

This seems to be a very interesting approach. I kind of like it.
But I think, requiring that a sample has to derive from some Sample base class is easier to digest. And following a couple of virtual functions is probably also easier to do for the vulkan newbies looking at the samples here.
After all, I think, the framework should be as simple as possible, but not simpler. Using your builder approach seems to hide more than it makes visible.


// forward to the caller for configuration
func(*target);

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add the new element into the chain right here and omit the build function:

if ( !_chain.empty())
{
    _chain.back()->pNext = static_cast<VkBaseInStructure *>(target);
}

This would require some function void * get_head() const, or so.

}

private:
VkInstance Build();
Copy link
Contributor

Choose a reason for hiding this comment

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

Build -> build ?

BUILDER(PhysicalDeviceBuilder)

public:
Self &AcceptType(VkPhysicalDeviceType type);
Copy link
Contributor

Choose a reason for hiding this comment

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

AcceptType -> accept_type ?
And so all the other functions here.

private:
VkPhysicalDevice SelectPhysicalDevice(VkInstance instance);

ContextBuilder &_parent;
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't _parent already part of the BUILDER macro?

#include <algorithm>
#include <cstring>

#define KHORNOS_VALIDATION_LAYER_NAME "VK_LAYER_KHRONOS_validation"
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

@tomadamatkinson
Copy link
Collaborator Author

This seems to be a very interesting approach. I kind of like it.
But I think, requiring that a sample has to derive from some Sample base class is easier to digest. And following a couple of virtual functions is probably also easier to do for the vulkan newbies looking at the samples here.
After all, I think, the framework should be as simple as possible, but not simpler. Using your builder approach seems to hide more than it makes visible.

I am still going to retain the sample base class approach - in a sense. It will just look a little bit different. There will likely be a component for api_sample, performance_sample and hpp_sample. Here we can put all of the Vulkan components together at a higher level specific to samples.

Somewhere in the sample main we will then have

// populate builder with a bunch of exterior configs not provided by a sample
vulkan::ContextBuilder builder;
builder
    .apply(api_samples::default_configuration)
    .apply(platform_context->surface_configuration)

// populate the builder with sample specific items
    .apply(sample_setup);

auto context = builder.build();

The benefit of going modular at the Vulkan framework level is the adaptability it gives to make samples that don't have to follow the normal approach. This can still be achieved with the class inheritance approach, but over time that seems to be the most coupling and confusing.

I guess the thing to remember here is that, when we get into samples past hello triangle. The developer is less focussed on the setup of the context and more focused on the configuration of further vulkan objects and rendering

@tomadamatkinson
Copy link
Collaborator Author

@asuessenbach thanks for your review, I will address your comments after #575 merges

@tomadamatkinson
Copy link
Collaborator Author

We are going a little bit more dynamic with the framework/v2.0 approach. The idea that a PlatformContext coming from a host application can contain information that adapts a sample at runtime was not possible before. Now it isn't too hard to imagine how the caller would set up this scenario

This is hypothetical but it follows the same line of logic as the example above.

// samples launcher
PlatformContext context;
context.vulkan_context_func= [](){} // some vulkan context builder implementation
launcher.launch(sample, context); // some wrapper which simplifies the desktop launcher i wrote a while back

// sample.cpp
SAMPLE_MAIN(PlatformContext & context)
{
vulkan::ContextBuilder builder;
builder
    .apply(context.get_vulkan_context_func());
}

This would allow the platform to maintain the window used for a sample. It could dictate if the sample is headless or rendering to a glfw child window. On android the preamble android_main.cpp would look after the surface lifecycle without the sample having to deal with it.

@tomadamatkinson tomadamatkinson merged commit 4e06838 into KhronosGroup:framework/v2.0 Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework This is relevant to the framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants