Make Dear ImGui support explicit multiple contexts used in parallel

[Dragnalith]

Goal

The goal of this PR is to make Dear ImGui support multiple contexts used at the same time whatever the concurrency model. This PR does that by making Dear ImGui context be explicit by introducing a new set of APIs without breaking the previous APIs.

Problem

The problem is discussed in the issue #586 open 6 years ago.

Problem 1: Running ImGui in parallel

It not possible to run ImGui in parallel without introducting race condition. Indeed, today running ImGui in parallel imply concurrent access to the global context. You can store the global context in thread local storage, this solves simple scenario where parallel code run on distinct threads. But for more complex concurrency model thread local storage is not enough.

Example:

• thread pool where job can run on any thread
• Fiber-based job system
• Go's gorountine
• C# Async/Await

Problem 2: Separation of concern

Today, even when not running ImGui in parallel, two modules statically link in the same program using ImGui cannot be independent. Such modules need to be aware of each other existence, and they need to agree on how they will share the use of ImGui (at least they will need to figure out when to save and restore their context). Dealing with this constraint in large projects adds communication burden, and it can become very tricky when integrating external code using ImGui in a code base already using ImGui.

Solution

Making ImGui context explicit to all calls when necessary. This is actually what is mentioned in imgui.cpp comment around line 1053:

//   - Future development aims to make this context pointer explicit to all calls.

Approach

No breaking change

Dear ImGui is too well established in the industry to be allowed to introduce breaking change for each API. Instead, I have prefered a two layer approach. API from ImGui:: namespace remain with no breaking change but are implemented using a new set of API defined in the ImGuiEx:: namespace. The ImGuiEx:: are new APIs. they behave exactly as their ImGui:: equivalent except you need to provide an ImGuiContext pointer as first argument (when necessary).

Note: I choose ImGuiEx name because Ex is usual to mean "Extended" and in this case it can also mean "Explicit".

Minimal difference with master

In order to minimize the complexity of future merge, this PR try to keep the difference with master as minimal as possible.

The code for ImGuiEx:: is actually the old code of ImGui:: with some rename and insert:

• The namespace ImGui has been renamed namespace ImGuiEx
• GImGui has been renamed ctx
• ImGuiContext* ctx has been inserted at the beginning of signature when necessary
• ctx has been inserted as first argument of function call when necessary

The new code for ImGui:: has been generated at the end of imgui.h in order not to disturb any current work made in the middle of imgui.h.

Automatic conversion from implicit to explicit APIs using libclang parser

Chances are I will have to re-do the refactoring again taking into account some comments and suggestions. Because I do not want to refactor manually again and again, I have written a Python script call make_explicit_imgui.py to do it for me. This script is based on libclang. It still needs some manually work, but finding function which need an explicit context parameter and the work of modifying function signature and function call have been automated.

This script is available here: https://github.com/Dragnalith/make_explicit_imgui

A two steps transformation

In order for maintainer and reviewer to easily understand the transformation I have applied to Dear ImGui code base, my changes can be split in two steps

Step 1: Preparing the code base

The first five commits are pure refactoring and do not change or introduce any new API. Those commits decouple some classes and function from the global GImGui context. It prepares the code base in a way so that the only remaining job is to modify API signature and function call. This job can easily be automated.

You can find detail explanation in the commit message of those five commits.

Step 2: Add new API with explicit context

The last commit is mainly the result of make_explicit_imgui.py script:

• It renames the old ImGui namespace to ImGuiEx,
• It removes GImGui and insert ImGuiContext where it is required
• It generates new ImGui api declarations at the end of imgui.h
• It generates new ImGui api definitions in imgui_implicit.cpp

ImGui::CreateContext, ImGui::DestroyContext, ImGui::CreateContext, ImGui::DestroyContext have been manually rewritten.

Design Details

Function Style vs Method Style

We can imagine two call styles to forward the context to APIs.

Function Style:

ImGuiEx::Checkbox(context, "My Checkbox", &state);

Method Style:

context->Checkbox("My Checkbox", &state);

This PR chose the the function style because it was less disruptive. Indeed, today APIs are split between imgui.h and imgui_internal.h. To keep this separation, we cannot have one context class with both imgui.h and imgui_internal.h APIs. Dealing with this constraint is a bit more complex than just chosing the function style.

IMGUI_DISABLE_IMPLICIT_API

This PR introduces the IMGUI_DISABLE_IMPLICT_API flag. If defined, all implicit context APIs (i.e the historical APIs) are disabled and the global GImGUI is disabled too.

ImGuiTextFilterEx and ImGuiListClipperEx

ImGuiTextFilter and ImGuiListClipper were both helper class applying to an implicit context. Now that the context need to be explicit those classes need to take context pointer as constructor or as method parameter.

To keep backward compatibility, this PR introduces two new classes: ImGuiTextFilterEx and ImGuiListClipperEx which are the explicit version of ImGuiTextFilter and ImGuiListClipper. Implemenation-wise, ImGuiTextFilter and ImGuiListClipper have been renamed ImGuiTextFilterEx and ImGuiListClipperEx and slightly modified to handle explicit context. ImGuiTextFilter and ImGuiListClipper have been recreated by subclassing ImGuiTextFilterEx and ImGuiListClipperEx and forwarding the implicit global context to the parant class.

ImGuiInputTextState::OnKeyPressed

It was not obvious but ImGuiInputTextState::OnKeyPressed was depending on the global context because it needed to know the font and font size for to update the state. The solve the problem, this PR forward the font and the font size as parameters of OnKeyPressed(...). On the implementation side it makes font and font size be forwarded to most of the imstb_textedit functions.

ImGuiOnceUponAFrame

ImGuiOnceUponAFrame is a helper API which does not really make sense in code using explicit context, so it does not have explicit equivalent. The definition of ImGuiOnceUponAFrame has been moved at the bottom of imgui.h with the new ImGui:: declarations.

MetricsActiveAllocation

ImGui::MemAlloc and ImGui::MemFree implementation should not depend on ImGui context, but it does today because it is using the global context to store active allocation metrics. This PR change this by introducing a global atomic integer named GImAllocatorMetricsActiveAllocations to implement active allocation metrics. This global is defined next to GImAllocatorAllocFunc and GImAllocatorUserData globals

Other

• The automatic build workflow has been very useful to find subtle issue related to platform-specific code or related to IMGUI_DISABLE_* flags.
• This branch can be complicated to merge because it will easily conflict with every new version of master. If approved, and once approved, I can make myself available to synchronize with reviewer and maintainer on when to prepare the final rebase.

[ocornut]

Thank you for the thoughtful and detailed PR. Automating the last step is definitively the right thing to do and I appreciate this as well. While I imagine it is unlikely I can merge this fully soon, I am willing to put some effort merging the first few commits earlier to facilitate this moving forward.

[Dragnalith]

Hi @ocornut

Thank you for your prompt reply and showing interest in my work.

While I imagine it is unlikely I can merge this fully soon, (...)

I did not mention it in the PR description but, despite the effort made to be as few disruptive as possible, I am aware this PR is huge and quite disruptive. Just to understand the change enough to feel you own it and that there won't be unexpected problem in the future, I can imagine it is already lot of work.

I see this PR as a proof of concept to show the result of my research, and the beginning of a public discussion about some practical details.

I am willing to put some effort merging the first few commits earlier to facilitate this moving forward.

I am glad you are willing to merge the first few commits as a first step. It will simplify future rebase.

I will update my work to take into account suggestions and issues you raised, then I will prepare a separated PR with just with the first commits

[Dragnalith]

@ocornut I have pushed a new version of the first 5 commits:

• No inclusion of anymore if compiler is MSVC, Clang ou GCC
• No breaking change anymore in imgui_demo.cpp
• ImGuiTextInputState now contains a pointer to its parent context, so text edit function signature don't have to be modified anymore

I have created a subsidiary PR including those 5 commits: #5859

[ocornut]

I have created a subsidiary PR including those 5 commits: https://github.com/ocornut/imgui/pull/5859

thank you! It would be better to keep this in same PR. Feel free to keep reworking history and force-pushing into existing PR. It’s easy to cherry pick certain commits so potentially first commits can be merged earlier.

[Dragnalith]

@OmarEmaraDev

Understood, I will close the other PR

[FunMiles]

How will this PR affect the docking branch? (And as a corollary, any reason to keep the docking separate? It is such a great feature.) It seems that it may not be easy to merge the two with the deep reorganization this PR is doing under the hood.

PS: Your first posts details are great. I would add one aspect to the thread local storage, it is for C++20 coroutines. I've been using them and indeed that causes issue because a task can switch threads. One can always get back to a given thread but with no timing guarantees.

[Dragnalith]

@FunMiles

This PR do very very few reorganization. It does have the potential to generate conflict anywhere, but those conflit are simple to solve, it's just a matter of forwarding the context. Once the code compile, it's unlikely that remain any issue.

the docking branch will conflict at the location where the code is different from the main branch in imgui.h and imgui_*.cpp file. If the docking branch do not diverge too much from main it should not be a big issue.

[FunMiles]

@Dragnalith You are right. ~~A test of a rebase on the latest docking branch only creates unresolved conflicts in one file imgui_internal.h.~~ A merge does create a lot more conflicts. I'll try the rebase and see if I can get it working.

Did you add any demonstration the use of multiple contexts in any of the demo? I am running the glfw/vulkan demo and it does not have any threading.

PS: After just fixing two conflicts that kdiff3 didn't fix automatically, it works. The docking is active. If @Dragnalith does not have an actual demo with multiple context, I'll create one and test it with the docking.

[Dragnalith]

@FunMiles there is no demo for multicontext, the demo code is exactly the same as before in order to prove the PR does not introduce any breaking change.

What prove it works with multiple context is the fact the PR remove the GImGui global variable from imgui.cpp, so it's technically impossible for ImGui code to produce any race condition as long as the context given as first argument is different.

[FunMiles]

I was wrong for the merge. It's only one commit merge. Here are two questions on this PR coming from handling the merges:

• Windows have to be associated with a context. Couldn't they have the context as a member rather than passing it around?
• The context is being passed as a pointer, but it should never be null. Good practice is to use references when the object has to exist. It would also avoid the constant use of ImGuiContext &g = *ctx;. Is there a reason I missed to have a pointer?

[Dragnalith]

@FunMiles

Windows have to be associated with a context. Couldn't they have the context as a member rather than passing it around?

ImGuiWindow already have a context as a member, it is used in by ImGuiWindow methods. When is it passed around?

The context is being passed as a pointer, but it should never be null. Good practice is to use references when the object has to exist. It would also avoid the constant use of ImGuiContext &g = *ctx;. Is there a reason I missed to have a pointer?

I do prefer the reference too. I have used pointer to follow the style of some already existing function, changing for reference is an easy find & replace. I don't mind it. I let to decision to the maintainers.

[FunMiles]

@FunMiles

Windows have to be associated with a context. Couldn't they have the context as a member rather than passing it around?

ImGuiWindow already have a context as a member, it is used in by ImGuiWindow methods. When is it passed around?

In ImGui.cpp there are 45 places where ctx and window are the first two arguments. Example:

static ImVec2 CalcWindowSizeAfterConstraint(ImGuiContext* ctx, ImGuiWindow* window, const ImVec2& size_desired)

I do see that indeed you added ImGuiContext* Context; to ImGuiWindow. Making use of the window's context everywhere would remove all those extra redundant arguments.

[Dragnalith]

@FunMiles Got it. Indeed, in this case you don't need the context. It is like this just because of the script doing the automated refactoring.

• [ ] I will fix it manually

[ocornut]

I think it is fine to make all functions take a context parameters, more consistent.

[FunMiles]

I think it is fine to make all functions take a context parameters, more consistent.

It makes merging docking harder. It can also make it possible to send an inconsistent context by mistake.

[Dragnalith]

Okay, I will not make the change. Let me know if you change your mind.

I think for all the code inside imgui.cpp which is not part of the API it does not matter much for now. It could be changed easily later with no impact.

[Dragnalith]

It makes merging docking harder. It can also make it possible to send an inconsistent context by mistake.

Actually it makes sense.

Let me know the decision, I will modified the PR if necessary.

[tksuoran]

I think it is fine to make all functions take a context parameters, more consistent.

It makes merging docking harder. It can also make it possible to send an inconsistent context by mistake.

I am also concerned about possible (accidental) inconsistent use. Ideally at least the debug build would check for such inconsistencies. Even better if the API does not allow the inconsistency in the first place.

This is a great PR, and I can't wait to try it. I would need this to work with the docking branch though. Any thoughts on which could in theory be merged first, this or docking?

[Dragnalith]

I don't think docking after or before does make a huge difference. It will conflict in any but with conflicts being very straightforward to solve

[Dragnalith]

What does prevent the docking branch to merge?

[ocornut]

I am not sure I understand the problem with docking and Dragnalith's answer "Actually it make senses" is ambiguous to me. Can you clarify?

Every change getting merged in master will be merged in docking, docking is maintained and synchronized about every week and when there a major change we merge immediately.

On using reference vs pointer arg:

• references makes internal code simpler since we already use them.
• references may make user code less terse, as many people will have pointers based on how/when they perform their context initialization, and may be more likely to keep doing stuff like ImGui::Button(*m_ImGui, xxx);. But we could alleviate that by providing examples suggesting users to cache a local reference when they do UI code, encouraging people to follow that pattern.

IMHO the reference/pointer thing is a minor detail we can decide on later and not worth going back and forth now. Priority will be to merge the first few commits.

Here's my suggested strategy:

• Should focus on merging the first "manual" commits earlier
• Meaning they should not expose problematic API changes in current codebase. To take the ImGuiListClipper example. One approach for the early merge is e.g. to not change constructor signature, but doing construction copy GImGui in a member, then only use the member. So "most" of clipper code is updated in the commit merged sooner, and some later commit can decide whether to add the parameter (default to NULL) or how to process the two variations, but that will change few lines.
• Same logic as clipper for other similar structures.
• When majority of a set of function needs context and some function in the set doesn't, it seems better to add context even unused so signatures are consistent and functions contents can evolve without breakage (what's where I don't understand what you meant about docking?).
• Once the early/manual commit are merged, if this works well we can contemplate treating this as a maintained extension/feature, e.g. officially maintained conversion script, tested by CI. That's to stay, even if we don't envision applying the switch until, say 2.0, it can be made available earlier and with some guaranteed.
• It may make sense to rework the script to have 2 output variants: A) emit both versions B) emit only explicit version. Perhaps in (B) we don't need to change the namespace at all. I envision that users of the explicit version will likely simply want one API, especially at this will be an opt-in feature/script for a while. And perhaps by the time we decide this can be merged in mainline we'll decide we'll simply want to bite the bullet and provide B) as default and A) as an optional thing.
• I'd be curious to see the repository diff with B) as it is likely to "feel" more attractive in order to move toward both. Not emitting implicit version is a trivial change, but of course it means extra parsing work to make e.g. Demo use explicit version.

I'll see if there are early commit I can consider merging soon.

[Dragnalith]

@ocornut

Clarification about my "It makes sense" answer.

I meant: I understand that having an API taking both a ImGuiContext* and a ImGuiWindow* as parameter is ambiguous. Since ImGuiWindow contains a context, it maybe be different than the context given as parameter of the API. In that case what should happen?

[Dragnalith]

@ocornut

About the script, it's not yet possible to use it automatically. The script does 99% of the work automatically, but it still requires some manuel touch afterwards for some corner cases.

Automating the remaining 1% is not impossible, but have a significant cost compared to just do the manual. I wish to pay for that cost only when we are sure we go for the "automated CI" solution.

[ocornut]

As an experiment I have now merged your second commit (shorter than 1st one), reworked as a5e96ff9

• Named the stored field Ctx (i suggest doing the same in your branch before rebasing on master, will prevent conflict, EDIT or simply drop from yours when rebasing). *
• Focus on more consistently assigning whatever source to a local ImGuiContext& g in function header.
• Focus on more consistently adding context as parameter instead of individual fields (see InputTextCalcTextSizeW()).
• (*) (Note that I considering renaming ImGuiContext to ImGuiCtx in the future to make this feature take less horizontal space. But ignore this idea for now. If/when we do it it'll be after finishing merging the first commits)

Separate topic, about the allocation count discussed in https://github.com/ocornut/imgui/pull/5856#discussion_r1014621242

• This is merely a "vanity debug feature" and has no incidence on anything but display in order to be able to do a quick sanity check. As such, and especially considering how little we allocate (hence the "vanity debug feature" label) I would - perhaps surprisingly - consider it acceptable if we used a non-thread safe integer for this, and side-stepped the problem. But I don't know if the rare concurrent access could trigger some zealous debug tools? (is that possible to detect?)
• We can provide an opt-in way for user to "correctly" make it atomic.
• "If we do not want to include , I do not think we can accept to include platform specific header." Yes and no. C++ headers are notoriously badly behaved in the sense that one may wake up one day and find out that <atomic> decides to include <regex> and link with OpenSSL and a webserver on some setup ;) ... OBVIOUSLY exaggerating but C++ headers tend to be this way because of C++ and C++committee nature. Platform-specific systems C headers don't.
• Your current solution seems however sane to me. I appreciate your logic and investigation and implementation. I would tweak it as is: (1) try to confirm required versions for Clang/GCC/MSVC so we are safe, (2) Fallback to a dumb wrapper using a non-atomic int instead of <atomic>, (3) Provide an opt-in define to use <atomic> or somehow configure it, (4) Further dumbify/compact the dear-imgui way: struct, all-public, assign _value = 0; in declaration line, solely to compact and make this a "less visible" complication.

[Dragnalith]

@ocornut Sorry, I have been a bit busy during Christmas time. Thank you for starting to merge this branch.

I have updated my branch:

• I have rebased it to the last version of master
• I have renamed all Context class attributes to Ctx
• I have updated the ImAtomicCounter implementation according to suggestion
  • It become a struct
  • I have replaced the construct with _value = 0; assignment
  • the fallback is a simple integer
• the Android pipeline is failing on my branch, but I don't think it is a problem due to my branch. Gradle has just been updated to 8.0 last week, I guess the pipeline would fail for master if run today.

About the atomic counter, I have checked ImAtomicCounter implementation using this godbolt, it works from gcc 4.7.1 and clang 3.2 (both released is 2012). Also for MSVC I have foundation documentation of _InterlockIncrement back to VS2008. Should Dear ImGui be compatible with compiler older than 2012? Otherwise I don't think we need to guard the implementation with compiler version.

Finally do you have an idea of the step and decision we need to go through in order to merge this branch? Do you think it's likely this branch will take years to be merged like the docking branch?

[ocornut]

Commit 1 "Make ImGui classes not depend on the implicit GImGui context" Mostly merged as 10ace22 and c8ad25c (with changes). I'm e-mailing you with details of small "left overs" which I suggest you carry in your branch for now. I pushed privately merely to reduce public noise until your branch carries them.

Basically:

• As a style thing I made most Ctx setups explicit and not part of constructors. They are generally not affecting users.
• The approach is that classes like ImGuiListClipper can always hold on a ctx. That change is merged. The only unmerged change is avoid the explicit parameter to constructor. This is part of the "left overs" commits and consists in only a few lines of changes, the main one being change from:

ImGuiListClipper::ImGuiListClipper()
{
    memset(this, 0, sizeof(*this));
    Ctx = ImGui::GetCurrentContext();
    IM_ASSERT(Ctx != NULL);
    ItemsCount = -1;
}

to

ImGuiListClipper::ImGuiListClipper(ImGuiContext* ctx)
{
    memset(this, 0, sizeof(*this));
    Ctx = ctx ? ctx : ImGui::GetCurrentContext();
    IM_ASSERT(Ctx != NULL);
    ItemsCount = -1;
}

I think your version of the lib after patching to support explicit-context can perfectly implement GetCurrentContext() as returning NULL if desired. The patch may also decide to default the default = NULL in constructor signature. We avoid xxxEx classes.

• I haven't merged the changes for ImGuiTextFilter. The reasoning is:
  • although currently only Draw() requires the context, one could argue this is an "implementation detail" and could change.
  • unlike other structs it is designed to be persistently stored by user. Therefore setting context field in constructor is even LESS advised as the ui context may not be ready or accessible during construction.
  • as such I think the best and most consistent solution is to add Ctx member and let user set it up (and we can decide whether Draw fallback can use GetCurrentContext() or not). Since in those preliminary commits the Ctx is not used yet, I chose to leave that stuff in the "left over" commit. What's left is basically 1 unused field and 1 IM_UNUSED() macros, which I suspect can become used after your full patch.

The call has been remove, and ImeWindowHandle is forwarded by setting ImGuiViewport::PlatformHandleRaw to ImeWindowHandle when PlatformHandleRaw is null.

Commit 2: Make MetricsActiveAllocations thread-safe and independent from GImGui context I am a little hesitant to merge this right now considering it adds a little complexity and expectation (due to the fallback) for a vanity debug counter. Maybe a name like ImDebugAtomicCounter can lower expectation that it is actually "probably ok" if this counter is not atomic. Anyhow, in my private branch I've carried all the small/shallow changes in this commit (code has been moved), so if you cherry-pick the version of my branch (will e-mail details) then I likely won't need to make other changes to it and it's closer to be merged.

Commit 3: Make ImGui::StyleColors function not depend on GImGui context* I'm not sure I understand the need for this and doubling entries points. In the explicit-context version the "default" target style structure may be inferred from explicit-context? What more do we need? The "final" explicit-context signature would be StyleColorsDark(ImGuiContext* ctx, ImGuiStyle* dst = NULL) with null style using context one?

Commit 4: Make win32 clipboard text and platform ime functions not depend on GImGui I have rewritten and pushed this as b5f9381: it does the same for the 3 handlers needing it (vs 1 in yours) + your removed the legacy fallback for ImeWindowHandle with a comment saying it could be set in PlatformHandleRaw but there was no code to do that, so I added the code. (I'll be tempted to remove that legacy block anyhow in a few months, but for now it works)

Commit 5: Introduce an ImGuiEx:: set of APIs where the ImGuiContext is explicit Untouched yet, but I will make an effort to merge some of it earlier.

TL;DR; Can you recreate your branch with:

• Commit 1: picking suggested "left overs" commit from my branch (assuming you agree)
• Commit 2: picking shallow tweaked commit from my branch
• Commit 3: imho this is not necessary but I may be misunderstanding what's needed to prep for the script.
• Commit 4: should not solved now
• Commit 5: picking whatever is left in your branch.

[Dragnalith]

@ocornut Thank you for the update. I'll take care of my branch in the coming days.

About Commit 3:

• The practical problem is that ImGuiStyle is calling ImGui::StyleColorsDark in its constructor. If ImGui::StyleColorDark depends on ImGuiContext then you are forced to forward a context to ImGuiStyle constructor. It's a pity because the context is not used.
• The theoretical issue with StyleColorsDark(ImGuiContext* ctx, ImGuiStyle* dst = NULL) is that either you want to apply the style to an explicit ImGuiStyle or you wanna apply the style to the implict style of ImGuiContext, but you never need both parameter at the same time. Trying to have a signature doing both is weird, isn't?
• My commit introduce StyleColorsDark() and StyleColorsDark(ImGuiStyle* dst) which become StyleColorsDark(ImGuiContext* ctx) and StyleColorsDark(ImGuiStyle* dst) once the context become explicit with my last commit.

About Commit 4:

• I am not sure to understand "should not solved now". My understanding is I should delete the Commit 4 from my branch because you have already merged a commit doing something equivalent.

[ocornut]

The practical problem is that ImGuiStyle is calling ImGui::StyleColorsDark in its constructor.

I think it would be ok to move that call to ImGui::Initialize().

The theoretical issue with StyleColorsDark(ImGuiContext* ctx, ImGuiStyle* dst = NULL) is that either you want to apply the style to an explicit ImGuiStyle or you wanna apply the style to the implict style of ImGuiContext, but you never need both parameter at the same time. Trying to have a signature doing both is weird, isn't?

I feel it is better to keep the Ctx param in all/majority of ImGui:: calls regardless of them needing it in current version This being an ImGui call we can treat it as such. Thus StyleColorsDark(ImGuiContext* ctx, ImGuiStyle* dst = NULL) in the explicit-context version seems consistent? And simpler.

I am not sure to understand "should not solved now". My understanding is I should delete the Commit 4 from my branch because you have already merged a commit doing something equivalent.

Correct, I made a typo and wanted to say "should BE solved now".

[Dragnalith]

@ocornut

I think it would be ok to move that call to ImGui::Initialize().

It is a breaking change. Anyone creating ImGuiStyle will now have to explicitly initialize it, or at minimum the style after construction won't have the same value. If you agree with this breaking change, I will implement this solution.

[adrianboyko]

Do the stb__ variables in imgui_draw.cpp also need to be dealt with?

static unsigned char *stb__barrier_out_e, *stb__barrier_out_b;
static const unsigned char *stb__barrier_in_b;
static unsigned char *stb__dout;