Make Vulkan implementation distinguish the user's render pass to its own created render pass in un-docked windows.

[FunMiles]

When a new window is created for a GUI component going out of the current window, a fresh render-pass is created that is not likely to be compatible with the user's render pass used when doing the GUI rendering in her/his window.
This PR addresses this issue by associating a pipeline with the created window that is compatible with the new render-pass.

Two more minor commits due to newer versions of Vulkan on Mac not liking the old validation layer. Also the CMake finding/linking with Vulkan is slightly updated to the more modern way.

[ocornut]

Thank you @FunMiles for the PR!
FYI I have merged the first two commits (the minor ones) into master and made the equivalent changes to the SDL+Vulkan example (see b25756b), so if you ever have to rebase to may remove those two commits.
Looking at the big one now.

[ocornut]

Hello,

As pushed this PR seems to crashes both Vulkan examples, as main.cpp calls SetupVulkanWindow() which via ImGui_ImplVulkan_MakeShaderModules() now relies on the InitInfo which haven't been submitted yet.

(I would also recommend keeping the ImGui_ImplVulkan_InitInfo parameter of all those functions as a pointer as it is currently to minimize noise in the diff.)

Thank you!

[FunMiles]

Hello,

As pushed this PR seems to crashes both Vulkan examples, as main.cpp calls SetupVulkanWindow() which via ImGui_ImplVulkan_MakeShaderModules() now relies on the InitInfo which haven't been submitted yet.

(I would also recommend keeping the ImGui_ImplVulkan_InitInfo parameter of all those functions as a pointer as it is currently to minimize noise in the diff.)

Thank you!

Sorry about this. It's rooted in my state of confusion last night. I wasn't sure if a line was supposed to be there or not and did a force push. However it seems a bit of a conceptual twist. The example is calling ImGui_ routines that use Vulkan calls before it sets up the ImGui_ImplVulkan_InitInfo which is where the Vulkan Instance is set. I am going to review the logic of this ordering. In my own code, I call ImGui_ImplVulkan_Init upstream, thus there is no issue.

[ocornut]

Btw I'm not too fussy with small breakage in the Vulkan back-ends, people who use Vulkan tends to be quite knowledgable and flexible with those, as long as they are documented (e.g. nice asserts).

[FunMiles]

I'd rather avoid doing a breakage because of the sdl_vulkan code. I don't know how I can test it.

On the other hand, I am now hitting some issues related global variables. (global RenderPass, PipelineLayout etc...)
Is there going to be a move for Dear ImGui away from globals ? 😄 It's a subject that is also present in trying to have different state for different windows... 😉

[FunMiles]

I have pushed one more commit that restores the working demo. Hopefully it should not break any other code. It is more robust and allows to keep the call to ImGui_ImplVulkan_InitInfo either early or after creating the window. I've tested it with my code, where I create a full Vulkan state myself and pass it to ImGui_ImplVulkan_InitInfo before other calls. So hopefully it keeps working for this issue: #3455

[ocornut]

Thank you @FunMiles.
The issue is that imgui_impl_vulkan contains code that are both helper to the examples and used by the multi-viewports system, and for the later they are not supposed to depend on the back-end initialization, hence the spaghetti you want through.

I have now merged this with various changes (actually replaced some of your added static with globals, made the changes work in master and then merged in docking). I agree we should move away from globals in eventually. We can probably start by gathering everything in a single struct, one bit of annoyance ihmo was lack of "default value in member variable declaration" which is part of C++11 but we're moving to C++11 this year (might make it one target for 1.80). The refactor of back-ends with perhaps a more explicit context is a separate topic we could tackle more globally.

[ocornut]

Having add another look at this, it seems that there was dead code (unused shader modules), some leaks, and an extraneous pipeline was created for the main window in the example. I fixed those with 770c995

EDIT I broke things with this. See #6325 (comment)

[FunMiles]

Having add another look at this, it seems that there was dead code (unused shader modules), some leaks, and an extraneous pipeline was created for the main window in the example. I fixed those with 770c995

I had not used this last commit in my running code. However, having decided to upgrade the Vulkan version on my Mac, I was hit with several issues that made me come back. Not creating the pipeline each time caused me issues. So I want to revisit this in a clean way. The pipeline can be lazily created. However, the created pipeline can be shared with other windows. Is there an aversion to using shared_ptr ? Maybe this is a more general question linked with the lack of RAII in the code. If using a shared_ptr, the destructor should take care of destroying the pipeline. And, because of Vulkan's order of destruction requirements, it seems some other mechanism has to be implemented if some other objects on which the pipeline depends need to be destroyed earlier.

[ocornut]

There was indeed an issue, I regret not spotting it earlier. See resolution here:
#6325 (comment)