-
Notifications
You must be signed in to change notification settings - Fork 728
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
WIP: Vulkan Context and Builders - Experimental Early Feedback Wanted :) #576
Conversation
There was a problem hiding this 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); | ||
|
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo
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 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 |
@asuessenbach thanks for your review, I will address your comments after #575 merges |
We are going a little bit more dynamic with the framework/v2.0 approach. The idea that a 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. |
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 aContext&
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
.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 singlebuilder.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.There are other methods we can use to configure queues in the future.
TODO: flesh out this concept a little more