Support multiple Vulkan contexts with custom loader.

[lstalmir]

This PR allows applications to create multiple contexts with Vulkan backend for different devices with custom loader.

Currently all functions are globally defined in imgui_impl_vulkan.cpp and shared between instances of the backend. This leads to a spec violation if two backends are created for two different logical devices and the custom loader loads functions using vkGetDeviceProcAddr.

To resolve this issue, the dynamically-loaded function pointers are moved to ImGui_ImplVulkan_Data, so that each instance of backend has its own set of pointers. With this change, all functions are now invoked with IMGUI_VULKAN_CALL macro that evaluates to 'bd->pfn_vkFunction' if VK_NO_PROTOTYPES is defined, and to 'vkFunction' otherwise.

[ocornut]

Hello,
This feels rather noisy. It’d be better if the loader version used a #define so backend code can refer to short vulkan function names directly, and if structures weren’t moved as much.

Are you actually genuinely in this case, aka multi-context but with different loader ?

[lstalmir]

I'm working on a Vulkan profiling layer which creates a context for each device to display the results in the application's window. The number of contexts depends on the application, so usually there is only 1, but I have to handle the other cases too.

For now I had a custom version of backend that used function tables with pointers to the next layer/icd, but it quickly outdated so I couldn't use the latest features. I want to use the library's backend instead but I can't use vkGetInstanceProcAddr because it points to the Vulkan loader's loader_gpa_instance_terminator. It doesn't return core device functions, so the only way to get pointers to the next layer or icd is to use vkGetDeviceProcAddr.

To avoid the IMGUI_VULKAN_CALL I could add a macro for each function like in OpenGL:

#ifdef VK_NO_PROTOTYPES
#define vkAllocateMemory (bd->pfn_vkAllocateMemory)
...
#endif

or always use the pointers in code and initialize them with the exported functions if VK_NO_PROTOTYPES is not defined:

struct ImGui_ImplVulkan_Data {
#ifndef VK_NO_PROTOTYPES
  static constexpr PFN_vkAllocateMemory pfn_vkAllocateMemory = ::vkAllocateMemory;
#else
  PFN_vkAllocateMemory pfn_vkAllocateMemory = nullptr;
#endif
};

[ocornut]

I want to use the library's backend instead but I can't use vkGetInstanceProcAddr because it points to the Vulkan loader's loader_gpa_instance_terminator. It doesn't return core device functions, so the only way to get pointers to the next layer or icd is to use vkGetDeviceProcAddr.

I am currently mostly away from keyboard so I don’t fully understand this, but if there’s an alternate way to specify a loader function that could be wired in backend i am open to that.

To avoid the IMGUI_VULKAN_CALL I could add a macro for each function like in OpenGL:
#ifdef VK_NO_PROTOTYPES
#define vkAllocateMemory (bd->pfn_vkAllocateMemory)
...
#endif

This i would accept.

I’d prefer if the way it was implemented didn’t move too much code around but I imagine at minimum the function list and macros may need moving to create fields inside the structure.

[lstalmir]

I added the list of aliases to the functions.

I also found an issue with ImplVulkanH functions that don't require the backend to be initialized (according to the comment), so there must be a list of pointers always available at any time after calling ImGui_ImplVulkan_LoadFunctions. I added a g_Functions that is initialized by LoadFunctions to fix that.

Now the requirement for the aliases to work is to provide an imvkFunctions that resolves to ImGui_ImplVulkan_Functions before using an alias, so in the file looks like this:

#define imvkFunctions bd->Functions
vkAllocateMemory // Resolves to bd->Functions.pfn_vkAllocateMemory if VK_NO_PROTOTYPES

#define imvkFunctions g_Functions
vkAllocateMemory // Resolves to g_Functions.pfn_vkAllocateMemory if VK_NO_PROTOTYPES

I moved DestroyWindowRenderBuffers and DestroyFrameRenderBuffers to the top as it is used with the backend and not the global functions.

[ocornut]

I thought we could rely on existing IMGUI_VULKAN_FUNC_MAP() to avoid listing all functions again but I suppose this cannot be used to emit additional #defines.

I moved DestroyWindowRenderBuffers and DestroyFrameRenderBuffers to the top as it is used with the backend and not the global functions.

I am not sure I understand this? I guess if any vkXXXX functions can use bd-> then everything else in the ImGui_ImplVulkanH_XXXX list is affected?

[ocornut]

I have pushed a small commit 6f10cef on master which is designed to simplify the branch diff/merge with this.
I have also updated this PR by merging latest master + applying some minor syntax changes also aimed at reducing the diff.
The patch is vastly more readable now.

Above question still stand, I am not sure I understand why two VulkanH_xxx functions were moved.

[lstalmir]

Hi!

The functions at the bottom of the file are mostly helpers that don't require the backend. They are providing initialization and clean-up of ImGui_ImplVulkanH_Window. In those functions the bd may be null, so they have to use g_Functions to be backwards compatible.

The 2 functions I moved, on the other hand, are called from the ImGui_ImplVulkan_DestroyDeviceObjects, so the backend data is available and should be used instead of g_Functions.

[ocornut]

Right. I pushed 6228c2e and rebased+merged latest into your PR reducing the diff further.

It's now a single commit. For now I'm not sure I want to merge the remaining bits just yet, and would ask that you try using/running with this for a while: I suspect it will be very simple to merge/rebase occasionally.