-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Support multiple Vulkan contexts with custom loader. #6616
base: master
Are you sure you want to change the base?
Conversation
Hello, Are you actually genuinely in this case, aka multi-context but with different loader ? |
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 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
}; |
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.
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. |
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 Now the requirement for the aliases to work is to provide an #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. |
I thought we could rely on existing
I am not sure I understand this? I guess if any vkXXXX functions can use |
This is mostly to facilitate maintainance of ocornut#6616.
I have pushed a small commit 6f10cef on master which is designed to simplify the branch diff/merge with this. Above question still stand, I am not sure I understand why two VulkanH_xxx functions were moved. |
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 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. |
06d2005
to
1adfcb5
Compare
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. |
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.