DestroyContext should set GImDefaultContext

[sonoro1234]

Hi,

actually DestroyContext is:

void ImGui::DestroyContext(ImGuiContext* ctx)
{
    void (*free_fn)(void*) = ctx->IO.MemFreeFn;
    ctx->~ImGuiContext();
    free_fn(ctx);
    if (GImGui == ctx)
        SetCurrentContext(NULL);
}

this makes imgui crash unless I set GImDefaultContext as the current context from the implementation.
perhaps it would be better to have:

void ImGui::DestroyContext(ImGuiContext* ctx)
{
    void (*free_fn)(void*) = ctx->IO.MemFreeFn;
    ctx->~ImGuiContext();
    free_fn(ctx);
    if (GImGui == ctx)
        SetCurrentContext(&GImDefaultContext);
}

or to avoid some extrange behaviour of free_fn changing ctx value

void ImGui::DestroyContext(ImGuiContext* ctx)
{
    void (*free_fn)(void*) = ctx->IO.MemFreeFn;
    ctx->~ImGuiContext();
    if (GImGui == ctx)
        SetCurrentContext(&GImDefaultContext);
    free_fn(ctx);
}

[ocornut]

Both very good points, looking into it.
I might also assert on ctx not being GImDefaultContext?

[ocornut]

1. Hmm, sort of going backward on this. Setting to GImDefaultContext again may promote not understanding what's going on with contexts so it's debatable what's right. If you are creating contexts then maybe you don't need the default context at all? Or you could manage to retrieve its pointer?
   In the spirit of ImGui instance working out of the box without an initialisation, I think it be worth doing it but I'm not totally sure.

2. I misread you on the second suggestion. free_fn() can't possibility change the ctx pointer it is a function.
   In fact calling free_fn() before is correct in case the free function rely on the current context (which in practice is unlikely as it's hard to track multiple allocators).

[sonoro1234]

In my glfwgl3 implementation with contexts I get default context

https://github.com/sonoro1234/cimgui/blob/lua_build/extras/impl_glfw3/imgui_impl_glfw_gl3.h#L38

for being able to set it before destroying mine to avoid crashing (that's why i need it)

https://github.com/sonoro1234/cimgui/blob/lua_build/extras/impl_glfw3/imgui_impl_glfw_gl3.h#L52

I think it would be simpler if DestroyContext does this.

[ocornut]

I don't understand, what exactly is crashing? I thought you meant that using ImGui:: functions after destroying current context crashed, which is normal. If it's anything else please clarify :)

[sonoro1234]

I am doing Shutdown afterwards, perhaps Shutdown is trying to access context.

[thedmd]

Without looking closely at the issue, but basing on my experience. Crashing could be caused by GImDefaultFontAtlas which if initialized require valid context to be present during destruction. Otherwise ImVector it contains cannot access (valid) free function (see: ImGui::MemFree).

Bottom line is, if you destroy default context but keep using second one, when closing application crash is guaranteed.

To avoid such issues I modified my copy of ImGui to not create default context nor default atlas at all.
This way context setup is little more complicated than before, in return I will not have to deal with static destructors.

[sonoro1234]

I prefer not to modify ImGui and only modify the implementation

[ocornut]

Note that you can #define GImGui in imconfig.h and among other things it will disable the default context.

I'm however interested in fixing that mess with the allocators. I think we should move the allocators outside of the context, into globals GImGuiMalloc GImGuifFree?

[sonoro1234]

I dont mind having a default context. Just want to do destroycontext followed by shutdown. Solutions can be fixing allocators or just changing destroycontext to set default context.

[thedmd]

Ability to override default malloc/free pair in imconfig.h looks like a good idea.

This will however not solve problem with finalisation order. GDefaultFontAtlas still has a ImVector and will be destructed after main(). By this time user allocator may be long gone.

I added options to disable creation of default context and global font atlas. I can prepare a PR with those changes if you like.

[sonoro1234]

It is the same happenning in #992

[aiekick]

do you have a fix for now ? please

[ocornut]

@aiekick you shouldn't be reliant on a fix since you can always set the context yourself afterward?
I will solve the malloc/free situation first! (again I'm completely overwhelmed with shipping my game now so all of this has been put aside for now)

[aiekick]

i say that because, i failed to fix by the solution upper. but ok i search more. no problem. i understand :)

[aiekick]

ok i just gain understanding of the principle of the imgui context. its ok, it work. thanks :)

[sonoro1234]

@aiekick in #1007 (comment)
you can see how I solve it in my implementation: Save the default context in a variable and set it before destroying my context.

[ocornut]

@sonoro1234 The issues discussed here should now be clarified and fixed by the changes discussed in #1565. Context creation are now explicit, and have a parameter to pass in a font atlas created by the user.
Read #1565 for details.