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

Allow for Vulkan functions to use the results of clients call to vkGetDeviceProcAddr #120

Closed
ghost opened this issue Aug 24, 2017 · 13 comments

Comments

@ghost
Copy link

ghost commented Aug 24, 2017

I'm referencing this stack-overflow question (look at the accepted answers where it talks about Vulkan.hpp)

https://stackoverflow.com/questions/45868990/should-i-use-the-vulkan-loader/45871076?noredirect=1#comment78702774_45871076

which talks about using vkGetDeviceProcAddr in order to avoid using the loader trampoline/terminator for efficiency reasons as described in this link

https://vulkan.lunarg.com/doc/view/1.0.57.0/windows/LoaderAndLayerInterface.html#user-content-best-application-performance-setup

Currently, I don't see a way to accomplish this using Vulkan.hpp

@mtavenrath
Copy link
Contributor

We're working on a solution which will allow using multiple dispatch tables retrived through vkGetDeviceProcAddr or vkGetInstanceProcAddr. In addition to this we plan to provide the required utility functions to build the dispatch tables to mimimize work on the developer side.

@ghost ghost closed this as completed Sep 6, 2017
@ghost ghost reopened this Oct 3, 2017
@ioquatix
Copy link
Contributor

ioquatix commented Dec 4, 2017

Just wondering if there is an update to this?

I'm currently having to write chunks of code falling back to the C interface when I'm fetching functions.

It seems like it would make sense to be able to supply a v-table of functions as required, e.g. perhaps something like the following:

struct MyFunctions {
	PFN_vkGetPhysicalDeviceSurfaceSupportKHR vkGetPhysicalDeviceSurfaceSupportKHR;
	PFN_vkGetPhysicalDeviceSurfaceCapabilitiesKHR vkGetPhysicalDeviceSurfaceCapabilitiesKHR; 
	PFN_vkGetPhysicalDeviceSurfaceFormatsKHR vkGetPhysicalDeviceSurfaceFormatsKHR;
	PFN_vkGetPhysicalDeviceSurfacePresentModesKHR vkGetPhysicalDeviceSurfacePresentModesKHR;
	PFN_vkCreateSwapchainKHR vkCreateSwapchainKHR;
	PFN_vkDestroySwapchainKHR vkDestroySwapchainKHR;
	PFN_vkGetSwapchainImagesKHR vkGetSwapchainImagesKHR;
	PFN_vkAcquireNextImageKHR vkAcquireNextImageKHR;
	PFN_vkQueuePresentKHR vkQueuePresentKHR;
}

class PhysicalDevice {
	template <typename FunctionsT>
	auto getPhysicalDeviceSurfaceFormatsKHR(..., const FunctionsT & functions) {
		// ...
		functions.vkGetPhysicalDeviceSurfaceFormatsKHR(...)
		// ...
	}
};

@mtavenrath
Copy link
Contributor

I've started to implement something like an extension loader and proxy handles in a new namespace tbd. The idea was to generate a proxy vk handle out of a vk handle and a functions table like this

vk::ext::PhysicalDevice pdext(pd, functions);
```
Those proxy objects would dispatch all vk functions through a function table.

Looking at your example it might an interesting solution to have a forwarding VkFunctions class which forwards all vk*Calls directly to Vulkan and make it a default template argument for FunctionsT like this

```
struct VkFunctionForwarder{
	FORCE_INLINE someType vkGetPhysicalDeviceSurfaceFormatsKHR(... arguments ...) ::vkGetPhysicalDeviceSurfaceFormatsKHR(... arguments ...);
};

	template <typename FunctionsT=VkFunctionForwarder>
	auto getPhysicalDeviceSurfaceFormatsKHR(..., const FunctionsT & functions = FunctionsT()) {
		// ...
		functions.vkGetPhysicalDeviceSurfaceFormatsKHR(...)
		// ...
	}
```

This way there wouldn't be a special case for function pointers in the header itself. A possible issue is that the default arguments might interfere with existing default arguments. I'll check this.

@ioquatix
Copy link
Contributor

ioquatix commented Dec 4, 2017

My first attempt is to use a prototype class like so:

struct Prototype {
	Prototype(vk::Instance & instance, vk::Device & device) {
		instance.getProcAddr("vkGetPhysicalDeviceSurfaceSupportKHR", vkGetPhysicalDeviceSurfaceSupportKHR);
		instance.getProcAddr("vkGetPhysicalDeviceSurfaceCapabilitiesKHR", vkGetPhysicalDeviceSurfaceCapabilitiesKHR);
		instance.getProcAddr("vkGetPhysicalDeviceSurfaceFormatsKHR", vkGetPhysicalDeviceSurfaceFormatsKHR);
		instance.getProcAddr("vkGetPhysicalDeviceSurfacePresentModesKHR", vkGetPhysicalDeviceSurfacePresentModesKHR);
		
		device.getProcAddr("vkCreateSwapchainKHR", vkCreateSwapchainKHR);
		device.getProcAddr("vkDestroySwapchainKHR", vkDestroySwapchainKHR);
		device.getProcAddr("vkGetSwapchainImagesKHR", vkGetSwapchainImagesKHR);
		device.getProcAddr("vkAcquireNextImageKHR", vkAcquireNextImageKHR);
		device.getProcAddr("vkQueuePresentKHR", vkQueuePresentKHR);
	}
	
	PFN_vkGetPhysicalDeviceSurfaceSupportKHR vkGetPhysicalDeviceSurfaceSupportKHR;
	PFN_vkGetPhysicalDeviceSurfaceCapabilitiesKHR vkGetPhysicalDeviceSurfaceCapabilitiesKHR;
	PFN_vkGetPhysicalDeviceSurfaceFormatsKHR vkGetPhysicalDeviceSurfaceFormatsKHR;
	PFN_vkGetPhysicalDeviceSurfacePresentModesKHR vkGetPhysicalDeviceSurfacePresentModesKHR;
	
	PFN_vkCreateSwapchainKHR vkCreateSwapchainKHR;
	PFN_vkDestroySwapchainKHR vkDestroySwapchainKHR;
	PFN_vkGetSwapchainImagesKHR vkGetSwapchainImagesKHR;
	PFN_vkAcquireNextImageKHR vkAcquireNextImageKHR;
	PFN_vkQueuePresentKHR vkQueuePresentKHR;
};

I added another version of getProcAddr for both device and instance. Not sure if it should throw or return false.

template <typename FunctionT, typename StringT>
bool getProcAddr(const StringT & name, FunctionT & function)
{
  function = static_cast<FunctionT>(this->getProcAddr(name));
  
  return static_cast<bool>(function);
}

Perhaps throw would be better - fail early. If you need more complex logic perhaps you should implement it using the basic getProcAddr.

@ioquatix
Copy link
Contributor

ioquatix commented Dec 4, 2017

It would also probably make sense to use std::string_view once that becomes available.

@mtavenrath
Copy link
Contributor

We discussed the solution passing an object to forward all Vulkan functions and came to the conclusion that it's a good solution besides some template ugliness.

With this change each (member) function will become a template function with the forwarder class as new template argument

void vkXyz(arguments) ;

will become

template <typename Dispatcher = DefaultDispatcher>
void vkXyz(arguments, Dispatcher const & dispatcher = Dispatcher());

VkDefaultDispatcher will just call the corresponding vk* functions without using function pointers. With the default argument for the Dispatcher existing code will just compile with any change.

If people want to use their own dispatch mechanism they have to pass a dispatch object to each function call.

Another feature we can integrate is a constexpr (or enum because of vs2013) for each function which specifies whether the function is supported by the dispatcher. This way developers will get a failure at compile time and they know that they've to go the route through the function pointers.

@mtavenrath
Copy link
Contributor

I've submitted a first version of Vulkan-Hpp with support for a dispatch class to https://github.com/mtavenrath/Vulkan-Hpp/tree/extension_loader. All calls into Vulkan now go through a dispatcher class.

The default dispatcher class is DispatchLoader which delegates all functions to the Vulkan loader. Since we have a Vulkan ABI now this class should fail compilation if functions are not exported by the Vulkan loader. To enabled this feature we need some more information in the spec file which symbols are exported on which platform.

Existing code will continue to work. Calling device.acquireNextImage2KHX(...);``` defaults to calling device.acquireNextImage2KHX(..., vk::DispatchLoader());``` at no additional cost.

This feature enables interesting possibilities, i.e. one does not have to specify all Vulkan functions in a class, but just a subset. E.g. this piece of code just works fine (and of course doesn't make much sense):

  class DummyDispatch {
    VkResult vkAcquireNextImage2KHX(const VkAcquireNextImageInfoKHX* pAcquireInfo, uint32_t* pImageIndexd) const { return VK_SUCCESS; }
  };
  device.acquireNextImage2KHX({}, Y());

Besides some cleanup the missing part is implementing a class which gets all functions through vk*GetProcAddress.

@ioquatix
Copy link
Contributor

That's great

@ghost
Copy link
Author

ghost commented Jan 23, 2018

What is the situation on this officially?

@mtavenrath
Copy link
Contributor

We have merged a PR a day ago which provides the basic infrastructure: e97e346 as described in my comment from Dec 11th including a dispatch class for the Vulkan loader. What's missing is a dispatch class with function pointers for each Vulkan function and two functions to initialize those, one for instance functions and one for device functions.

@mtavenrath
Copy link
Contributor

@ioquatix, @UPS7kYkHEPbZIucaqjje Can you give https://github.com/mtavenrath/Vulkan-Hpp/tree/extension_loader a try to see if it fits your needs?

Usage:

vk::DispatchLoaderDynamic dld(<optional instance>, <optional device>);
device.someFunction(..., dld);

It's also possible to initialize the dispatch loader deferred by calling dld.init(instance, );

@ioquatix
Copy link
Contributor

That's awesome, I'll try it out.

@mtavenrath
Copy link
Contributor

The feature has been merged. Closing.

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

No branches or pull requests

2 participants