Vulkan function loader feature might conflict with app's similar functionality

[kennyalive]

Version/Branch of Dear ImGui: Version: 1.87 WIP Branch: master

Renderer: imgui_impl_vulkan.cpp

Compiler: VS 2022

My concern: Crash when ImGui calls vulkan function when volk library is used to load function pointers.

~~ImGui defines vulkan function pointers using official names if VK_NO_PROTOTYPES is defined as part of custom function loader feature. For example, vkAllocateCommandBuffers name will be defined if VK_NO_PROTOTYPES is defined.~~

~~This might create a conflict with application's own vulkan function loader functionality. For example, volk library defines VK_NO_PROTOTYPES and it also defines function pointers using official names. vkAllocateCommandBuffers function will be defined both by the application and by the ImGui. This leads to violation of single definition rule and in my use case it leads to runtime crash.~~

~~For my use case I fixed the issue by disabling the part of ImGui code that defines/initialize function pointers. I'm not sure what's the best way to ensure that ImGui vulkan function loader coexist with app's similar functionality and if it's in the scope of ImGui library at all, so I'm writing this mostly with informative purpose. One possible solution is not to declare function pointers globally with official names or to put those pointers into a structure.~~

[ocornut]

The function pointers are defined static so they shouldn’t interfer with equivalent ones used by other compilation unit. Are you trying to include both this .cpp file and eg: volk headers in a same compilation unit ?

[kennyalive]

Here what I see in the debugger.

At first the function from my app is called

And later we get to ImGui initialization. The compiler does not try to dereference vkCreateSampler pointer but calls it directly.

vkCreateSampler pointer value is the same in both screenshots, so compiler does not try to use static vkCreateSampler from ImGui compilation unit. Could it be the problem that even if ImGui compilation unit uses static but the volk version of vkCreateSampler is regular global variable so it does not make any promises and the compiler is free to use it. I'm wondering if there is some shadowing rules for this situation, so 'static' hides other names? It smells like undefined behavior too, non-static variables leak form other modules and we get back to single definition rule violation.

UPDATE: I was wrong in my initial message that VK_NO_PROTOTYPES defined by volk is visible in ImGui compilation units - it does not. Sorry, it took me some time to figure out what's going on. ImGui code sees only vulkan/vulkan.h definitions and because VK_NO_PROTOTYPES is not defined it generates code for statically linked function call - using vkCreateSampler value directly without dereference. But it uses pointer of global variable (vkCreateSampler) which leaks from another compilation unit and violates single definition rule and we end up with code generated for static function that tries to call pointer to a function.

UPDATE2: Volk library also warns about scenario when both volk.h (app logic) and vulkan.h (imgui) are used. Here's a piece from volk documentation:

In summary I can not say that ImGui does something wrong, it's more like inconvenience you encounter when using custom loader. I remember that in the past I had the same problem with volk in my code when if forgot to replace vulkan.h with volk,h in some files. It's easy to fix when it's your own app but harder when you have to make changes in 3rd party code. Again for my personal use I fixed the problem, that's mostly if this will be the issue for other users.

[yelibumeng]

@kennyalive hi i get the same error，how to fix this question(also use volk), help ！！

[kennyalive]

https://github.com/kennyalive/vulkan-base/commit/65113bfffffb317b8c355b7abd13bf3ad361e03b

[yelibumeng]

thanks i will try it

[yelibumeng]

@kennyalive thanks!!!!!, it run now

[artyomd]

I just stumbled into this problem and I think I found a very simple solution. IMGUI_IMPL_VULKAN_NO_PROTOTYPES this definition enables VK_NO_PROTOTYPES for the vulkan.h include of imgui. (Obviously, you can add the VK_NO_PROTOTYPES define for the entire project, but it is a nice touch if you want something specific for imgui). After that, if you try to compile you will get a message like this

imgui-src/backends/imgui_impl_vulkan.cpp:1033: bool ImGui_ImplVulkan_Init(ImGui_ImplVulkan_InitInfo*, VkRenderPass): Assertion `g_FunctionsLoaded && "Need to call ImGui_ImplVulkan_LoadFunctions() if IMGUI_IMPL_VULKAN_NO_PROTOTYPES or VK_NO_PROTOTYPES are set!"' failed.

So I added a call to ImGui_ImplVulkan_LoadFunctions

  ImGui_ImplVulkan_LoadFunctions([](const char *function_name, void *vulkan_instance) {
    return vkGetInstanceProcAddr(*(reinterpret_cast<VkInstance *>(vulkan_instance)), function_name);
  }, &vulkan_instance_);

In this compilation unit, all the vulkan functionality is provided by volk (yes, I am using volk, too :) but this approach is applicable for all the loaders), so we can easily delegate all the loaded function to imgui, without changing anything in imgui sources. Hope this helps somebody, and pls let me know if I missed anything :raised_hands:

[zhivkob]

@artyomd Much more elegant than what I initially did, which was fiddle with the original implementation. Thanks for pointing me in the right direction!

[tim-rex]

The function pointers are defined static so they shouldn’t interfer with equivalent ones used by other compilation unit. Are you trying to include both this .cpp file and eg: volk headers in a same compilation unit ?

In my case, that is precisely what I'm doing and how I landed on this issue just now (unity/jumbo build). Though this approach seems to be creating more friction in other areas.. simple option for me is to revert to a regular build style at least where ImGui is concerned.

[Markyparky56]

To expand on @artyomd's solution, if you are using volk with "optimised device calls" (volkLoadInstanceOnly + volkLoadDevice) you might want to use vkGetDeviceProcAddr rather than vkGetInstanceProcAddr for all requested functions. Except vkGetDeviceProcAddr can't fetch certain functions that are instance-specific!

I found passing in a method to fetch both Instance and Device handles and fetching for both, then selecting the device proc address if available works well enough.

ex:

auto funcLoader = [](const char* funcName, void* engine) 
  {
    FCEngine* fcEngine = reinterpret_cast<FCEngine*>(engine);
    PFN_vkVoidFunction instanceAddr = vkGetInstanceProcAddr(fcEngine->GetVulkanInstance(), funcName);
    PFN_vkVoidFunction deviceAddr = vkGetDeviceProcAddr(fcEngine->GetVulkanDevice(), funcName);
    return deviceAddr ? deviceAddr : instanceAddr;
  };
const bool funcsLoaded = ImGui_ImplVulkan_LoadFunctions(funcLoader, this);

Perhaps there is a more elegant way, but since this is a one-and-done function with a very limited number of functions being requested, I'm happy.

[azer89]

I also got the same error because I volk did not vibe with imgui 🤔

access violation executing location when executing VkCreateSampler()

Fixed using the solution described by @artyomd

Thanks!

[ocornut]

@kennyalive, @adalsteinnh can you clarify why you cannot use the solutions highlighted above, aka using IMGUI_IMPL_VULKAN_NO_PROTOTYPE + calling ImGui_ImplVulkan_LoadFunctions() to make it use Volk?

It seems that this would work out of the box.

[ocornut]

I have pushed b720c0f which adds support for #define IMGUI_IMPL_VULKAN_USE_VOLK Both GLFW+Vulkan and SDL+Vulkan also now supports Volk optionally, which will make it easier to test forward.

See my reasoning in https://github.com/ocornut/imgui/pull/6582#issuecomment-2066582450 after why I didn't merge the IMGUI_IMPL_VULKAN_CUSTOM_HEADER version.

Thanks for your help!

[kennyalive]

can you clarify why you cannot use the solutions highlighted above, aka using IMGUI_IMPL_VULKAN_NO_PROTOTYPE + calling ImGui_ImplVulkan_LoadFunctions() to make it use Volk?

@ocornut I probably never tried it, since my ad-hoc solution worked for me. I updated ImGui with the latest code and IMGUI_IMPL_VULKAN_USE_VOLK option works great for me. Thank you for implementing it.