Backends: Vulkan: reset render state with the rendering context's backend data

Open

MilosLordwants to merge intofrom

This PR fixes an issue where changing the ImGui context inside a callback could cause a crash. The renderer could use data from the wrong context. Now it uses the correct backend data when resetting the render state.

static void SwitchContext(const ImDrawList*, const ImDrawCmd* cmd) { ImGui::SetCurrentContext((ImGuiContext*)cmd->UserCallbackData); }

ImGuiContext* other = ImGui::CreateContext(); // no renderer backend
ImGui::SetCurrentContext(ctx);

// in a frame of ctx, rendered with ImGui_ImplVulkan_RenderDrawData() while ctx is current:
draw_list->AddCallback(SwitchContext, other);
draw_list->AddCallback(ImGui::GetPlatformIO().DrawCallback_ResetRenderState, NULL);
draw_list->AddCallback(SwitchContext, ctx);
draw_list->AddRectFilled(ImVec2(8, 8), ImVec2(56, 40), IM_COL32(200, 100, 50, 255));

v1.93.0-docking, lavapipe:

received signal SIGSEGV, Segmentation fault.
#0  vkCmdPushConstants<VkCommandBuffer_T*&, VkPipelineLayout_T*&, VkShaderStageFlagBits, int, unsigned int, float (&) [4]> (...)
#1  ImGui_ImplVulkan_SetupRenderState (draw_data=..., pipeline=..., command_buffer=..., rb=..., fb_width=128, fb_height=64) at imgui_impl_vulkan.cpp:603
#2  ImGui_ImplVulkan_RenderDrawData (draw_data=..., command_buffer=..., pipeline=...) at imgui_impl_vulkan.cpp:726

With this change: no crash, same pixels as the frame without the context switch.

Activity

Backends: Vulkan: ImDrawCallback_ResetRenderState sets up the render state with the backend data of the context being rendered, not the current one.

4c8b24bOct 30

ocornut14d ago

This seems a bit of an esoteric use case. Can you confirm that you were actually in this situation and needed a fix? (aka this is not just something found by an LLM)

We can absolutely fix this but i'm not sure I can guarantee all backends or features would support it.

ocornut14d ago

I'm not comfortable with adding support in ImDrawCallback_ResetRenderState because the other callbacks e.g. DrawCallback_SetSamplerLinear wouldn't work the same way, and the current ImDrawCallback signature unfortunately doesn't allow bypassing the global context access for them (#6969).

This is why I need to understand precisely why you want to do that, considering the above snippet seems a like esoteric.

One possible workaround would be to double-down on adding ImDrawList::GetOwnerGontext(), but it could interfere with some other esoteric use of manually-creating-ImDrawListSharedData.