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.

[ocornut]

I've added some quick comments from skimming through the first commits. Please don't take them too seriously (may be quite wrong on a few things). I'm not sure when I'll be able to make sensible progress on this.

[ocornut]

Seems to be a typo using GImGui here?

[Dragnalith]

Yes, it is a typo, I will fix it

[Dragnalith]

Actually, I don't know why it displays GImGui because on the real diff it correctly shows ctx.

It is possible you commented the diff of the first commit only, but actually on the diff of the branch there is no problem.

PS: On the first commit GImGui still exist and is still used everywhere

[ocornut]

I was never happy with those functions being in ImGuiWindow (unusual as per our coding style), so I may do a slight refactor earlier to change that.

[Dragnalith]

@ocornut
Understood. Let me know if you do it and I will adapt.

[ocornut]

Feels like adding ImGuiContext* inside ImGuiInputTextState would be better here?

[Dragnalith]

@ocornut
I agree, it would prevent numerous signature change. I felt I could not take this decision myself because having ImGuiInputTextState pointing to an ImGuiContext* feels conceptually wrong regarding my interpretation of ImGuiInputTextState. But if you are comfortable with that choice, it definitely makes things easier.

[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: #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 a5e96ff

• 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 #5856 (comment)

• 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
I have created imgui-explicit repository. It contains two branches: master-explicit, and docking-explicit, as well as two tags: v1.90-explicit and v1.90-docking-explicit.

Let me know your thoughts.

PS: only one thing bothers me, I will likely update v1.90-explicit tags and v1.90-docking-explicit tags when I will add compatibility for more backends. But I am uncomfortable, because it seems bad git practice.

[ocornut]

I have merged 16af144 as e07663d (minor modifications: fixed some spacing in other statements).

[Dragnalith]

I have rebased the branches master-explicit and docking-explicit on top of v1.90.1 and v1.90.1-docking and I have adding 2 new tags v1.90.1-explicit and v1.90.1-docking-explicit.

[FunMiles]

I've taken v1.90.1-explicit and added support for glfw3/vulkan. How do I pass on my changes? I will also work on the docking branch.

PS: I have done similar changes to the docking branch as well.

[Dragnalith]

Hi, Thank you for contributing. If you open a PR in the imgui-explicit repo, I will cherry pick the commit of your branch. Does it sound okay for you?

…

On Wed, Jan 17, 2024, 05:12 FunMiles ***@***.***> wrote: I've taken v1.90.1-explicit and added support for glfw3/vulkan. How do I pass on my changes? I will also work on the docking branch. — Reply to this email directly, view it on GitHub <#5856 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHSSFH263NJ4IXHQ2EZSYDYO3NLHAVCNFSM6AAAAAARXMYAX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJUGQ2DENJRGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[FunMiles]

Hi, Thank you for contributing. If you open a PR in the imgui-explicit repo, I will cherry pick the commit of your branch. Does it sound okay for you?

It sure does. Though the mechanics of it on github may be a bit tricky because my fork is based on the original repo. But I can handle it.

[Dragnalith]

If somehow you succeed in creating a PR adding just two commits with can be fast-forward maybe on top of the existing branch, I will merge it from github interface. Ano

…

On Thu, Jan 18, 2024, 00:25 FunMiles ***@***.***> wrote: Hi, Thank you for contributing. If you open a PR in the imgui-explicit repo, I will cherry pick the commit of your branch. Does it sound okay for you? It sure does. Though the mechanics of it on github may be a bit tricky because my fork is based on the original repo. But I can handle it. — Reply to this email directly, view it on GitHub <#5856 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHSSFHYTTSL4CDKSM3PHODYO7UPTAVCNFSM6AAAAAARXMYAX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJWGA2DEMJSGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[FunMiles]

After successfully doing the explicit context docking branch modifications for glfw/vulkan, I started trying to make a demo of multiple contexts. A new issue derives from this and I just want to mention it here:

Vulkan allows to do all the commands from multiple threads, and presumably other 3D libraries must offer similar features. From Vulkan's use point of view, two threads can fully work in parallel and never synchronize. Though on some platforms, vkQueuePresentKHR may better be handled simultaneously for all the windows for performance reasons. (I've had the experience on Mac's MoltenVK where frame rates would drop to by half when two windows were doing it on their own threads)

Leaving that performance issue aside, the new issue is that the processing of events by glfw happens only on the main thread due to some OSs constraints (e.g. MacOS). I think I one solution is to buffer the events associated with ImGui contexts individually and call the ImGui callbacks with a call similar to glfw3's glfwPollEvents so that each can handle them on their own threads. I hope the docking part can adapt to this.
@ocornut do you see that as something that can be attached to the context object? Importantly, once the events are buffered in the context, there is nothing glfw specific to them, they become imgui objects and other backends can use the same mechanism.

[FunMiles]

If somehow you succeed in creating a PR adding just two commits with can be fast-forward maybe on top of the existing branch, I will merge it from github interface. Ano

Great.

There's one thing incomplete in the docking. I do not know if you had something similar with Windows/DX12: there is an event when new monitors are (dis-)connected. That event is global, but yet the original event handler calls a function that now needs a context. It seems that the event should essentially be dispatched to all existing contexts. I am not sure how to really handle that. I have currently commented that part of the code. Here is the original code:

void ImGui_ImplGlfw_MonitorCallback(GLFWmonitor*, int)
{
    ImGui_ImplGlfw_Data* bd = ImGui_ImplGlfw_GetBackendData();
    bd->WantUpdateMonitors = true;
}

One solution would be to have a global variable with a timestamp for when that even happens. Then contexts can check that vs their creation time/last check time instead of the flag in their backend data object.
I dislike having a global variable (hence my being in this discussion 😄 , so maybe I need to find a way to attach it to the glfw instance.

Any other idea?

[ocornut]

See #7186 #7155 #5671.
#7186 is written in a way inadequate for merging, and IMHO unnecessarily complex but basically some backends are lacking multi-context supports in the first place (not even parallel multi-contexts).

Leaving that performance issue aside, the new issue is that the processing of events by glfw happens only on the main thread due to some OSs constraints (e.g. MacOS). I think I one solution is to buffer the events associated with ImGui contexts individually and call the ImGui callbacks with a call similar to glfw3's glfwPollEvents so that each can handle them on their own threads. I hope the docking part can adapt to this.
@ocornut do you see that as something that can be attached to the context object? Importantly, once the events are buffered in the context, there is nothing glfw specific to them, they become imgui objects and other backends can use the same mechanism

When answering to #6895 (comment)
I realized that effectively - and it wasn't really planned! - all the io.AddXXX() functions could nearly be called from another thread after ImGui::NewFrame(). May need more thorough validation but in essence they are the only function writing to io.InputEventsQueue and the only function reading from it are within NewFrame().

( Monitor callback) Any other idea?

My initial idea was that multi-contexts support in eg. GLFW backend would rely on having a global map (ImGuiStorage) in imgui_impl_glfw.cpp, so the monitor callback could simply iterate them, but it was not considering parallel multi-contexts. The global map would also mean some constraints on when it is legal to create/destroy a context (maybe an ok constraints).

I don't have much bandwidth to help on this, but moving toward multi-contexts support in standard backend would be nice, and obviously parallel multi-context support obviously nicer. Please consider create dedicated issue # to work on narrower topics as this issue is becoming too unwieldy and has a bunch on its plate already to add backend refactors.

---

I would however need you @FunMiles to clarify precisely what is your intended use case, because I don't want us to move mountains and add too much complexity for too-esoteric use cases. While I can see use for backends supporting multi-contexts, them supporting parallel multi-contexts is another thing and I expect people running parallel multi-context imgui to be a very specific kind of user base (e.g. AAA) who most likely have custom backends in place anyway.

(And amusingly, I realized when meeting and discussing with Marc that his utter-motive for this PR is one that I consider absolutely unjustified :P but because I think the change is ultimately desirable I don't mind if his motives don't match mine :)

[FunMiles]

I would however need you @FunMiles to clarify precisely what is your intended use case, because I don't want us to move mountains and add too much complexity for too-esoteric use cases. While I can see use for backends supporting multi-contexts, them supporting parallel multi-contexts is another thing and I expect people running parallel multi-context imgui to be a very specific kind of user base (e.g. AAA) who most likely have custom backends in place anyway.

My motivation is that modern development does require multi-threading. With it comes also the need for better encapsulation/isolation of entities, components, objects, whatever your favorite term of the day is. I also abhor global variables like many programmers. 😄 You may thus understand my interest in this change.
One of my application is in scientific visualization. Right now, I can only view one model at a time. I have to close that visualization to be able to add another one. I do not want the creation of the visualization objects to be done on the main thread via some form of time slice on the main thread. It's not practical nor desirable.

When I do a google search with imgui multiple context or imgui multi-threading I find enough results to know my use is not esoteric. We may agree to disagree on this, but moving to something that can be multi-contexts and multi-threaded is a necessity in modern codes.
After having seen the branch from @Dragnalith I believe it is within reach. So I am adopting v1.90.1-docking-explicit as the base for my use. With my current changes, and after I look at the event queue in more details (thanks for pointing to #7186 ) I think I will have all I need. I'll definitely continue to contribute my modifications.

[FunMiles]

If somehow you succeed in creating a PR adding just two commits with can be fast-forward maybe on top of the existing branch, I will merge it from github interface. Ano

I tried doing a PR to your explicit repo, but, I presume because you have another repo that is a fork of imgui, it will not show in the PR interface on github. If you want to look at what I have done, it is in https://github.com/FunMiles/imgui/tree/docking-explicit

I have also successfully reached my multi-threading, multi-context goal, albeit with a more costly method than would be achievable with a lot more changes. I will paire down some of the extra fluff I have in there and then put it in another branch.

[Dragnalith]

@FunMiles About the branch: You can fork my imgui-explicit repo vs push your branch in this repo then you will be able to open a PR. About your need for multithreaded multicontext, I could not understand the fundamental problem you are encountering with glfw?

…

On Sun, Jan 21, 2024, 02:00 FunMiles ***@***.***> wrote: If somehow you succeed in creating a PR adding just two commits with can be fast-forward maybe on top of the existing branch, I will merge it from github interface. Ano I tried doing a PR to your explicit repo, but, I presume because you have another repo that is a fork of imgui, it will not show in the PR interface on github. If you want to look at what I have done, it is in https://github.com/FunMiles/imgui/tree/docking-explicit I have also successfully reached my multi-threading, multi-context goal, albeit with a more costly method than would be achievable with a lot more changes. I will paire down some of the extra fluff I have in there and then put it in another branch. — Reply to this email directly, view it on GitHub <#5856 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHSSFBQWPMPGSGUPXPPXU3YPPZZXAVCNFSM6AAAAAARXMYAX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBSGE3TKNBTG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[FunMiles]

@FunMiles About the branch: You can fork my imgui-explicit repo vs push your branch in this repo then you will be able to open a PR.

I will do that as soon as I can.

About your need for multithreaded multicontext, I could not understand the fundamental problem you are encountering with glfw?

Some glfw functions must only be called from the main UI thread. That is the fundamental issue.
The NewFrame function as well as the creation and destruction of windows for the docking make such calls.
On the other hand, doing rendering preparation, whether for the actual app or from imgui does not need to be done on the main thread and in most cases where the app rendering takes some time, it should be kept off the main thread (general best practice for any UI app).

[Dragnalith]

@FunMiles I understand serious application require a more complex threading model but I would to confirm one thing: the glfw example is monothreaded right? So no special multithreading work is required to port it to explicit context, right? Also, my 2 cents. I don't think it should be the responsibility of imgui backend to be multithreaded. If you have to isolate the your window system in its own thread likely it's not only for ImGui, likely you have custom backend needs anyway.

…

On Sun, Jan 21, 2024, 02:40 FunMiles ***@***.***> wrote: @FunMiles <https://github.com/FunMiles> About the branch: You can fork my imgui-explicit repo vs push your branch in this repo then you will be able to open a PR. I will do that as soon as I can. About your need for multithreaded multicontext, I could not understand the fundamental problem you are encountering with glfw? Some glfw functions must only be called from the main UI thread. That is the fundamental issue. The NewFrame function as well as the creation and destruction of windows for the docking make such calls. On the other hand, doing rendering preparation, whether for the actual app or from imgui does not need to be done on the main thread and in most cases where the app rendering takes some time, it should be kept off the main thread (general best practice for any UI app). — Reply to this email directly, view it on GitHub <#5856 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHSSFBKJYBRNDPPCJHGGETYPP6RJAVCNFSM6AAAAAARXMYAX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBSGE4TCNJSHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[FunMiles]

@FunMiles I understand serious application require a more complex threading model but I would to confirm one thing: the glfw example is monothreaded right? So no special multithreading work is required to port it to explicit context, right? Also, my 2 cents. I don't think it should be the responsibility of imgui backend to be multithreaded. If you have to isolate the your window system in its own thread likely it's not only for ImGui, likely you have custom backend needs anyway.

I have both a regular mono threaded example and a multi-threaded one. Doing the multi-threaded has to be done. Without it I had missed a bug in reuse of some Vulkan variables in the single-threaded. Further it also gives an example on how to properly do it, showing which part have to be sent to the main thread. And yes, it does use standard library includes, because there is no way around it. However it is only in the demo code, so I do not think it is a problem.

[FunMiles]

@FunMiles I understand serious application require a more complex threading model but I would to confirm one thing: the glfw example is monothreaded right? So no special multithreading work is required to port it to explicit context, right? Also, my 2 cents. I don't think it should be the responsibility of imgui backend to be multithreaded. If you have to isolate the your window system in its own thread likely it's not only for ImGui, likely you have custom backend needs anyway.

I have made the PR to your repo. The demo is single threaded. I have also a demo in multi-threaded that does not touch the backend, but I have not yet provided it. That demo loses performance because most likely due to the lag introduced by awaking threads.

My 2 cents: writing a backend for imgui is not a trivial thing. Providing the tools to be able to quickly add correct multi-threading to the existing backends is not as difficult and would be a good thing for the community in general. There is a difference between making a backend multi-threaded and making it multi-thread safe. It is only the latter I would like to see. I really do not understand the heavy drag on this.

[ocornut]

This is tangential to the task discussed here, but FYI i have released Dear ImGui Multi-Context Compositor, a simple helper to facilitate displaying & interacting with multiple contexts simultaneously (e.g. update vs render domains contexts)
https://github.com/ocornut/imgui_club -> imgui_multicontext_compositor
Manages z-order, mouse/keyboard routing, cross-context drag and drop.

(Posting here for visibility for please don't reply here, you can post to imgui_club if you have questions)