Crash after creating second viewport (docking branch, Vulkan+GLFW impl) about imgui HOT 13 CLOSED

@PathogenDavid

Well, well, well.

I also only had it enabled via. the device in the VkPhysicalDeviceVulkan13Features struct.
It should be in both the extensions and the features, those are NOT equivalent.

Doing so solves the issue for me. Thanks for your help on this!

from imgui.

@PathogenDavid Update: adding dynamic rendering as extension indeed helped and solved the issue. Since it is mentioned in ImGui's source code, no fixes / updates are needed, so I close this issue.

Thanks everyone for contribution and help!

from imgui.

From your screenshot it looks like ImGuiImplVulkanFuncs_vkCmdBeginRenderingKHR is null. I'm not a regular Vulkan user, but my understanding is that with your combination of preprocessor flags you're responsible for indirectly initializing this pointer via ImGui_ImplVulkan_LoadFunctions.

It looks like you're probably doing that correctly:

https://github.com/daniilvinn/omniforce-engine/blob/695a69ddc55c524eedc401ace51d9a74358fe95d/Omniforce/src/Platform/Vulkan/Private/VulkanImGuiRenderer.cpp#L135-L138

and there's asserts to check that those functions were loaded before initializing the backend:

So either:

1. Asserts are disabled (and the functions aren't successfully loaded in the first place)
2. You're inadvertently calling ImGui_ImplVulkan_LoadFunctions a second time in a way that causes those functions to load as null
3. You have multiple copies of the backend accessible in your binary (IE: one copy in your EXE and another in a DLL -- perhaps due to some hot reloading mechanism you've implemented)

1 is the easiest to check, just comment out your call to ImGui_ImplVulkan_LoadFunctions and ensure the asserts in ImGui_ImplVulkan_Init fail as expected.

2 and 3 are best checked with a debugger.

from imgui.

I believe @PathogenDavid is correct for this specific case.
I also have a similar issue without using volk and using the default prototypes.
I think this is from a mismatch in the start and end rendering commands from my implementation compared to that of ImGui. "vkCmdBeginRenderingKHR" vs. "vkCmdBeginRendering". Since when I swap this on my side or ImGui's side it is resolved.

vkCmdBeginRenderingKHR is the correct command to use in this case,- though as of 1.3 not using the KHR should be equivalent. This does throw the same error however.

from imgui.

@PathogenDavid checked every case.

1. Asserts are enabled, since they are triggered when I comment out ImGui_ImplVulkan_LoadFunctions function call.
2. I setted up a breakpoint in ImGui_ImplVulkan_LoadFunctions function and it is triggered only once, but crash still happens.
3. I checked all my CMakeLists and imgui_impl_vulkan.cpp is compiled only once.

Also I checked through debugger and ImGuiImplVulkanFuncs_vkCmdBeginRenderingKHR is not null, it actually has address (screenshot provided). The only thing is that some __formal variable is null (visible on screenshot)

Screenshot:

from imgui.

Note: as I said, ImGui doesn't crash unless I drag ImGui window out of engine window. I can provide a video if needed.

So it only crashes after creating second viewport

from imgui.

Hmm. Maybe we did have the same error with the latest Vulkan SDK and volk is loading something similar to the default prototype.
Converting the default prototype to "vkCmdBeginRendering" solved this same issue for me. A different issue was popping up when I loaded and ran everything with the KHR ending that I just found.

As of 1.3 this is supposed to all be equivalent,- but maybe there is something else going on. I'd be interested to see if this ends up being the same issue and if there is a nicer way to solve it.

from imgui.

@AustinPeps what do you mean with "Converting the default prototype to vkCmdBeginRendering"?

If I understood correctly, I should remove KHR suffix in imgui_impl_vulkan.cpp here.

#ifdef IMGUI_IMPL_VULKAN_HAS_DYNAMIC_RENDERING
static PFN_vkCmdBeginRenderingKHR   ImGuiImplVulkanFuncs_vkCmdBeginRenderingKHR;
static PFN_vkCmdEndRenderingKHR     ImGuiImplVulkanFuncs_vkCmdEndRenderingKHR;
#endif

Am I right? If yes, then I tried to remove those KHR suffixes from there and it didn't help, it still crashes.

from imgui.

@AustinPeps Update: yes, we have same issues.
My comment above is not fully correct.
I fixed it so ImGui loads vkCmdBeginRendering (without KHR suffix) now viewports work as expected - issue is solved. But I still think it shouldn't work like that.

In my opinion this issue requires further investigation, so I don't close this issue

from imgui.

Yes and I agree.
I think it is likely a Vulkan specification difference that isn't immediately apparent and/or hasn't been well documented yet.

A fix would likely require more branching on ImGui's side based on the specification version to choose which rendering function to use. (It's silly to load the already available function) Vulkan is still changing though and this may end up NOT an issue in a later SDK if the equivalency between the two functions is reinforced. It's a bit of a hard place to be to make changes.

from imgui.

@daniilvinn Thanks for checking all those! Another possibility came to mind: It might be that the implementation of ImGuiImplVulkanFuncs_vkCmdBeginRenderingKHR is chain calling to a null pointer (or something similar) and as a result the actual faulting stack frame is missing. (You could confirm this by stepping into the call to ImGuiImplVulkanFuncs_vkCmdBeginRenderingKHR, but based on what you all are saying this sounds like it's the case and wouldn't really give us more info.)

It seems odd to me that vkCmdBeginRenderingKHR wouldn't just effectively become an alias for vkCmdBeginRendering when you're on 1.3. The wording of the spec implies this should be the case, so I wonder if this is a bug somewhere upstream.

Just to confirm, are you all still explicitly enabling the extension as noted in the below comment?

Note: as I said, ImGui doesn't crash unless I drag ImGui window out of engine window. I can provide a video if needed.

This is to be expected, ImGuiImplVulkanFuncs_vkCmdBeginRenderingKHR is only invoked when when rendering secondary viewports.

from imgui.

@PathogenDavid no, I didn't explicitly enabled that extransion, I just enabled dynamic rendering feature in VkPhysicalDeviceVulkan13Features. I will check if it was the case later.

from imgui.

No problem, glad I could help!

It's a shame that the extension functions still resolve even when the extension isn't enabled. Based on the vkGetInstanceProcAddr documentation and the blurb quoted below this is apparently expected:

it is undefined behavior to use extended Vulkan if the extensions are not enabled.

I was looking to see if we could just add an assert that the extension was enabled when the device was created, but as far as I could find Vulkan doesn't actually let you query that.

from imgui.