v1.60: Remove global DockContext, require CreateDockContext()

[justas-d]

Since dear imgui v1.60, calling CreateContext is required as the static global context has been removed. The purpose of this PR is to reflect that design decision in the imguidock addon.

No new API calls were added. I have reused the existing CreateDockContext SetCurrentDockContext and DestroyDockContext functions:

// A dock context needs to be present.
ImGui::DockContext *dockCtx = ImGui::CreateDockContext();
ImGui::SetCurrentDockContext(dockCtx);

// Context cleanup (will also set the current context to null):
ImGui::DestroyDockContext(dockCtx);

I have added this same example to the header usage code.

In case anybody forgets to set up a context, I have sprinkled IM_ASSERT before all g_dock dereferences so that any null pointer dereference issues easier to debug.

This would benefit the people who are hotloading code.

[Flix01]

Since dear imgui v1.60, calling CreateContext is required as the static global context has been removed. The purpose of this PR is to reflect that design decision in the imguidock addon.

That would be a breaking change... I'm not sure if this is good or not... let me see...

Basically, as far as I can see, you want to prevent imguidock to have its static DockContent that is used when the user doesn't call CreateDockContext() explicitly, right?

static DockContext g_default_dock;
static DockContext *g_dock = &g_default_dock;

Why do you want to get rid of g_default_dock? Are you experiencing some problems with it? If not, I'm not sure if breaking existent code is a good thing or not... just to reflect a change in the global setup of Dear ImGui.

What do you think about it?

P.S. Any other comment from people that are using imguidock in their projects is welcome.

P.S. If g_default_dock is a problem, maybe there's a way to create a fallback DockContent on the heap only if none is created, so that existing code won't break. What about it?

[Edit] In any case I've not tested imguidock very much... so if this change is really necessary we can go ahead and merge it... I just need some feedback to be sure about it :smiley:

[Flix01]

P.S. If g_default_dock is a problem, maybe there's a way to create a fallback DockContent on the heap only if none is created, so that existing code won't break. What about it?

OK, I think I'll try this approach soon and propose an alternative fix.

[Flix01]

OK, I think I'll try this approach soon and propose an alternative fix.

Mmmmh, main problem with this approach is that I don't know when it's safe to delete the default DockContent that now is allocated on the heap only if the user does not create one himself...

The user should call some kind of static method to delete it... and this would be a breaking change...

In short I'm not sure this approach is good anymore.

[justas-d]

Basically, as far as I can see, you want to prevent imguidock to have its static DockContent that is used when the user doesn't call CreateDockContext() explicitly, right?

I want to completely remove the static DockContext and force the user to call CreateDockContext() and SetCurrentDockContext()

Why do you want to get rid of g_default_dock? Are you experiencing some problems with it?

This creates issues when trying to hotload C++ code. I'm not sure about windows but on linux static variables get reset when reloading code via shared libraries. Since imguidock creates a default static DockContext, dock state will be lost upon a code reload unless the user expliticly creates their own DockContext.

If not, I'm not sure if breaking existent code is a good thing or not... just to reflect a change in the global setup of Dear ImGui.

Generally I think breaking changes should be avoided but in this case I think this would be ok as this draws a parallel to the breaking change in the main library.

P.S. If g_default_dock is a problem, maybe there's a way to create a fallback DockContent on the heap only if none is created, so that existing code won't break. What about it?

I think something like this could work:

struct DockContext
{
    bool isImplicit = false;
};

...

static DockContext *g_dock_implicit = NULL;

static inline void ensureDockContextExists()
{
    if(!g_dock)
    {
        if(!g_dock_implicit)
        {
            g_dock_implicit = new DockContext;
            g_dock->isImplicit = true;
        }

        g_dock = g_dock_implicit;
    }
}

Call ensureDockContextExists() before deferencing g_dock and if it's null, an implicit DockContext will be allocated.

Mmmmh, main problem with this approach is that I don't know when it's safe to delete the default DockContent that now is allocated on the heap only if the user does not create one himself... The user should call some kind of static method to delete it... and this would be a breaking change...

void DestroyImplicitContext()
{
    if(g_dock_implicit) delete g_dock_implicit;
}

Maybe even call it in ShutdownDock?

[Flix01]

OK, but we're editing fixes at the same time!

Can you please see if my new version works for you, or there's some other fix we should apply?

[Flix01]

[OT] Interesting this 'hotload stuff' you've explained here: https://ssstormy.github.io/dlfcn-and-cpp/ (I'm a Linux user too, and most articles on this topic are for Windows users).

I think I'll came back tomorrow with imguidock...

[justas-d]

Sorry for the wait.

Just tried out the experimental change and there is still a problem with it and my proposed implicit dock context solution. The issue is that there still is a static pointer to a DockContext present. After a reload, the pointer's heap memory might be untouched but the pointer itself will be cleared to 0's.

The only solution I see is to ask the used to create a context via CreateDockContext(), and then have the user reinitialize g_dock after a reload by calling SetCurrentDockContext()

[Flix01]

OK. I'll prepare a commit for it (with my update now there are conflicts with your pull request).

Basically you need to get rid of g_default_dock, right ? And you can allow g_dock to be static. OK?

I was thinking that we could just remove the static keywords, if static variables are a problem. [Even if we don't expose g_default_dock and g_dock, if they are not static anymore, they can be declared extern and be set from any other cpp file]

However I'm not sure this would fix your issue, wouldn't it?

[Flix01]

OK. It should work now.

[justas-d]

Works fine 👌

[Flix01]

Thanks for your feedback :smiley: !