Multiple instances of Dear ImGui in the same program

[nsf]

It's not a bug report but more like a question/discussion topic.

So, I have this engine of mine where I have a rendering thread which runs C++ code and scripting environment which runs C# code. They talk to each other via queue and triple buffers to pass rendering lists. Of course I want ImGui to be available on both sides and I can't really just lock it with a mutex, because it will block quite a lot. So what I did:

Converted GImGui to a thread local variable, initialize ImGui from main thread before running mono scripts, also force creation of a font atlas via GetTexDataAsRGBA32 and... it kind of works. Mono renders stuff when it wants to, passes the contents of ImDrawData via triple buffer to the rendering thread, rendering thread renders both GUIs, its own and the one from mono scripting environment.

But I've noticed there are a few places where functions don't look reentrant, for example ImHash function has a static LUT which is initialized on first access. Perhaps there are some other places where implicit global state is used?

In general what do you think about that kind of usage (or should I say abusage :D)?

[nsf]

Yeah, it segfaults somewhere. I guess my other option is to just copy & paste imgui into two translation units, maybe wrap them into different namespaces and use it this way. After all I need only 2 instances of it.

[ocornut]

I can't really just lock it with a mutex, because it will block quite a lot.

Depending on how/where you use it it may not be that bad.

Perhaps there are some other places where implicit global state is used?

I don't know! You'd have to list them and we can see if it is reasonable or out of reach to fix all of them and maintain it. I would suppose there aren't so many.

Yeah, it segfaults somewhere.

Where/what?
A very quick skimming shows one for the CRC32 lut, one static const array in RoundScalar (const may not be a problem?), and one in the default clipboard implementation for Win32. Off-hand I don't think there's anything else?

In general what do you think about that kind of usage (or should I say abusage :D)?

If it works for you I don't mind :) I have never tried to do that.
Locking mixed with SetInternalState() so far.

[nsf]

Yeah, sorry, this segfault was actually my fault. But anyways I decided to split imgui into two translation units. This way I'm 100% sure the instances have nothing shared and it does work well.

Thanks for awesome lib.

[ocornut]

Some discussions about this on twitter today. Adding copies here for reference.

https://twitter.com/Donzanoid/status/720548646601797633

Don Williamson ‏@Donzanoid 11h11 hours ago
@ocornut @andrewwillmott @daniel_collin worth checking out how CUDA API handles this without explicit context param http://docs.nvidia.com/cuda/cuda-c-programming-guide/#context

https://twitter.com/Donzanoid/status/720552966957199360

Rest of the discussion was about the possibility of having an explicit state API (e.g. c-style first parameter passed as state) which could be handled by a script generating code. I have no issue with that as long as it doesn't have pollute or add noise to the basic API, so it should be invisible to the casual user (different file/folder?).

Frankly I don't feel we need a stack but I would like to modify the code to allow the user to make the global pointer a TLS variable.

Also naming needs to be fixed see #269

[Cthutu]

What is the problem with having a ImGui object? The instance can keep
track of it's own IO etc.

On Thu, 14 Apr 2016 at 17:06 omar notifications@github.com wrote:

Some discussions about this on twitter today. Adding copies here for
reference.

https://twitter.com/Donzanoid/status/720548646601797633

Don Williamson ‏@Donzanoid 11h11 hours ago
@ocornut https://github.com/ocornut @andrewwillmott
https://github.com/andrewwillmott @daniel_collin worth checking out how
CUDA API handles this without explicit context param
http://docs.nvidia.com/cuda/cuda-c-programming-guide/#context

https://twitter.com/Donzanoid/status/720552966957199360

Rest of the discussion was about the possibility of having an explicit
state API (e.g. c-style first parameter passed as state) which could be
handled by a script generating code. I have no issue with that as long as
it doesn't have pollute or add noise to the basic API, so it should be
invisible to the casual user (different file/folder?).

Frankly I don't feel we need a stack but I would like to modify the code
to allow the user to make the global pointer a TLS variable.

Also naming needs to be fixed see #269
#269

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#586 (comment)

[nsf]

Well, if you ask me, I would definitely prefer explicit context variable. But when it comes to having both APIs, global style and the one with context variable, it's a tricky question.

Maybe you can even do that with macros, i.e. make it a compile-time feature. But not sure if you would like that solution or not.

Something like:

IMGUI_API bool          RadioButton(IMGUI_CONTEXT const char* label, bool active);

And in implementation:

GImGui *g = IMGUI_USE_CONTEXT;

Probably pretty ugly though. So one can leave IMGUI_CONTEXT empty and say that IMGUI_USE_CONTEXT simply refers to global var. Or fill it with the right content.

Or the same, but wrap all the API into a class conditionally with a macro. But again some macro pressure on the implementation side, because funcs/methods look different: void X::Foo() vs void Foo().

Whatever works I'm happy with all variants. I don't like much TLS usage though. On my side it was a quickest hack possible.

[ocornut]

That's actually a neat solution from a technical perspective, would save us a lot of work.
It's mainly ugly because of the missing comma, but it also mean that within functions the parameter has to be passed around which adds another layer of missing-comma-fugliness inside the functions themselves.

I really don't know what's the best solution. Ultimately it isn't a much desired feature, people sort of prefer if it was there but nobody absolutely needs it so I may put that in the backburner until someone comes up with a better idea. I'd really love it if some macro trick could save us from that missing comma.

Should still fix that CRC32 static table too.

EDIT Not helping much, but another solution for an alternative reality would be to always have context pointer and allow 0/NULL as a shortcut for global state pointer. ImGui::Button(0, "blah").

[emoon]

For me personally I would rather have this:

void RadioButton(context, ...)

(or, like talked about in the twitter thread)

void RadioButton(...); // Global Edition

void RadioButton(...)
{ 
  RadioButton(g_context, ...); // defaults to global context, defined in imgui_context.h or such
}

I know this would result in lots of cut'n'paste of all the functions but I really think it's the cleanest way to do it. Also as @nsf points out I really don't like the TLS stuff. I would personally see that dear imgui only deals with a context and and let the application deal with anything threading.

As always threading is a data issue and not a code one so as long as dear imgui would only play with a given context (and global in the causal use-case) everything would be fine.

[ocornut]

TLS is the crap workaround obviously but as long as it's not enabled by default (because of performance) it's probably a 2-lines source change (probably some optional macro in imconfig.h) and not causing harm.

Now on discussing proper solution. There's about 330 functions in imgui.h
Potentially the "no-context" versions could be inlined in a header file (If the global pointer can be declared extern there?) meaning the overhead would be 1 line per function and in a single place.
@nsf how did you "split imgui into two translation units" without duplicates at link time?

They would perhaps need to be in different namespace in order to avoid end-user seeing 2 versions of each.

[nsf]

@ocornut wrapped the second copy into a different namespace

[emoon]

Yes all of the non-context functions would be inlined as you say so there wouldn't be any extra overhead to the change. Also the TLS change doesn't help if you want multiple instances on the same thread (I haven't used TLS much but is it possible to have more instances per thread? Then it would work also)

[nsf]

No, with TLS - one instance per thread, unless you do that push/pop trick from cuda API. Also TLS is bad for coroutine-like situation where your jobs can be scheduled onto different threads over time. Whether it's bad or not is a different topic, but it's possible. E.g. languages like Go and coroutines will come into C++ sooner or later.

[ocornut]

@emoon In that case the thread could always call e.g. SetCurrentContext() or some equivalent Push/Pop to change context.

Looks like adding the non-context inline function sounds fairly reasonable, just need to swallow the bullet of making imgui.h less good looking with extra arg everywhere. Or, I replace namespace ImGui by struct ImGuiContext for the context version and make them method? Would that be any less friendly, have subtle side-effects?

[nsf]

Struct is okay, but the main problem with structs is that even private methods have to be in the header file. Or you'll have to pass the context around as an argument for private functions.

[DanielGibson]

probably for migrating code, search and replace on ImGUi:: to imguiCtx. (or however one calls their local context) is a bit easier than adding a parameter on every call?

[emoon]

Yeah for my use-case I think I will be fine with SetCurrentContext as I don't run my UI stuff multi-threaded (yet)

I assume you still want to keep your namespace ImGui::Function(...) syntax? Otherwise the other approach would be to actually use a struct/class like g_imgui->Function(...) I much prefer the current style though.

[emoon]

@ocornut remember that you can still keep the current layout exactly as in the header file. All you need is to include a "some.inl" that actually does the inline implementation of the non-context functions.

[ocornut]

Struct is okay, but the main problem with structs is that even private methods have to be in the header file. Or you'll have to pass the context around as an argument for private functions.

I hate this, but tempted to still do it if I cold. I was mainly wondering if using method would make a slight difference to people working on bindings or other languages. Perhaps explicit parameters can be easier to bind to other languages without adding another set of wrapper functions? In term of how mangled names can be accessed?

(..Second paragraph completely backtracking on the first one..)

However, if there is an actual struct + methods now we need to also shove members into header OR have an additional indirection (bad) OR have a private derived type and cast this in the private code. None of those solution are exciting. Any better idea that doesn't suck?

I don't actually care the ImGui:: namespace too much (personally mostly because of the two uppercase letters :) tho changing it may be more of a v2.0 thing? DanielGibson has a point that at least it could be search-and-replaced safely, so it's not out of consideration and may be part of solution.

Now clearly:
ctx.Button()
is shorter than
ImGui::Button(ctx)

But later also feels a little less C++, perhaps more bindings-friendly, more search-and-replace friendly, and I can't find a satisfactory solution to use member functions/variables (paragraph above).

[nsf]

Well, you can pass context as an argument to private functions. I agree there is no pretty solution in C++. As for bindings, I don't think any C++ solution works there. Name mangling is already bad enough and if you have it, only brave people use C++ .so/.dll directly. So, one way or another there will be a C wrapper also and it's typically easier to use for bindings. As for me, for my C# bindings I use wrapper functions, so the C++ API isn't a big deal.

I would say that passing context argument explicitly is a true procedural way, but also it's not pretty at all. But that's the only way how C api will look like at the end. So, eh, I don't know what's the best solution here.

[Cthutu]

I would definitely prefer the ctx.Button() way that ImGui::Button(ctx). I
don't agree with bindings-friendly as every scripting language generally
deals with C++ object bindings. It would get rid of the SetInternalState()
calls that I am currently doing to support multiple instances (on the same
thread) of ImGui. It would also solve the multi-threaded issue too.

On Thu, 14 Apr 2016 at 19:00 nsf notifications@github.com wrote:

Well, you can pass context as an argument to private functions. I agree
there is no pretty solution in C++. As for bindings, I don't think any C++
solution works there. Name mangling is already bad enough and if you have
it, only brave people use C++ .so/.dll directly. So, one way or another
there will be a C wrapper also and it's typically easier to use for
bindings. As for me, for my C# bindings I use wrapper functions, so the C++
API isn't a big deal.

I would say that passing context argument explicitly is a true procedural
way, but also it's not pretty at all. But that's the only way how C api
will look like at the end. So, eh, I don't know what's the best solution
here.

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#586 (comment)

[emoon]

I do all my wrapping in C++ to a C API in the end also so I really don't care that much how the context is being sent. Regarding the data in the header. I would just go with

struct PrivateData;

struct ImGui {
  void RadioButton(...);
  ...
  PrivateData* private_data;
}

And have the PrivateData declared in the C++ code. If you really want to hide it you have can of course have a void pointer here but then you would need to do more casting inside you other code to use it.

Problem with this approach is if the user wants to modify/get data from the private struct it has to be done using functions which may be a bit annoying. Of course the private data can be stuffed in another header also to keep the regular header a bit clear.

[Pagghiu]

I am doing multi context in my apps as well as multi-thread and I'm now using locks to avoid undefined behaviour/data races.
I am also trying to do some fancy experiments with multiple concurrent UI "Sessions", one for each user connected through the browser ...
My preference is to make everything ctx.Method(...) or at the very minimum ImGui::Method(&ctx,...)

I don't see so much of a value to save any casual newbie programmer from thinking where he should declare this state/context variable to access where he needs it.
Any real c++ coder will know where to declare it 😆

You can also expose the "default static context" as global variable from imgui.h
If you call this variable "ImGui" and you transform ImGui namespace to a struct ImGuiContext,

in imgui.cpp

ImGuiContext ImGui;

in imgui.h

extern ImGuiContext ImGui

everything that the casual programmer will need to do is to replace

ImGui::Button()

with

ImGui.Button()

Would that be acceptable as a minimal search&replace update to a potential 2.0 Api?

[andrewwillmott]

Another vote for context->Blah() and so on here, rather than Blah(context). If there's a backwards compatibility header with inline wrappers, it's not going to affect users who don't change their code either way, and I think that's a cleaner more C++ approach. (Though sadly the meaning of 'more C++ approach' has degenerated into insanity these days so maybe ignore that comment :P.)

The changes on the ImGui side can be pretty minimal, thanks to the state already being well encapsulated. Daniel's already alluded to this but

struct ImGuiContext
{
    void NewFrame();
    // ...


    ImGuiContext();
    ~ImGuiContext();
protected:
    ImGuiState& g;
};

would reduce changes in many cases to just taking out the dereference lines:

void ImGui::NewFrame()
{
    ImGuiState& g = *GImGui;

    if (!g.Initialized)
    {
    }
}

// Becomes

void ImGuiContext::NewFrame()
{
    if (!g.Initialized)
    {
    }
}

The constructor/destructor is just

ImGuiContext::ImGuiContext() : g(*new ImGuiState)
{
}
ImGuiContext::~ImGuiContext()
{
    delete &g;
}

// or better

ImGuiContext::ImGuiContext() : g(*new(MemAlloc(sizeof(ImGuiState))) ImGuiState)
{
}
ImGuiContext::~ImGuiContext()
{
    g.~ImGuiState();
    MemFree(&g);
}

// or even better avoid the double-alloc by having CreateContext()/DestroyContext() that allocate one block.

One thing that would need thinking about is memory allocation. This either has to be separated out of the context, or the code needs to be updated to call per-context allocs rather than MemAlloc(), and pass the memory allocator into the context constructor or creation function.

Some of the functions don't reference the context internally at all of course. I guess the easiest thing would be for these to remain as ImGui::ColorConvertRGBtoHSV (say) and hence not needed in the optional backwards compat. header.

[ocornut]

Some extra ideas

• Would break cases of people adding their own functions in ImGui namespace, which is a rather convenient feature to extend the library in a way that feels natural. I'll have to look into some big codebases using ImGui so gauge the side-effects. If you can't add to an existing namespace one workaround would be to allocate your own derived instance but it is nowhere are convenient.
• May need to move MemAlloc etc. function pointers, perhaps to be define or use another system?

or even better avoid the double-alloc by having CreateContext()/DestroyContext() that allocate one block.

This is desirable to avoid be touching 1 extra cache line on every call accessing the context. Will probably use a scheme like that. That and perhaps changing the way allocators are specified might lead us to require an explicit one-liner Initialization of ImGui.

I will let those ideas simmer for a bit, afaik there is not urgency.
I would like to point out that it is a topic where everyone would naturally want to say "of course it is better if there is an explicitly context" because obviously it is better, but it's not a requirement, it has a cost of change, and I don't think it is blocking many people to not have it soon.

1-thread 1-context : not affected by change. (most common)
1-thread 2-contexts : that change will help to avoid calling SetInternalState/SetContext so code will be a little nicer, but it's not total a deal-breaker to not have it.
N-threads 1-context : that change won't help at all, user still need a lock.
N-threads 1-context per-thread : that change will allow that situation. note that the TLS workaround also allows it, albeit with extra cost of TLS access.

In other words, there's no situation that's 100% blocked right now.
I want to do this but I don't think it is urgent.

[ocornut]

(Note that for a known number of instances,e.g. 2, the namespacing trick with 2 copies of ImGui code in different namespaces also works)

Soonish tasks:

• Move alloc handlers to macros and out of context so we can have a function that actually allocates a context.
• Rename existing context functions to use sane names/patterns Multiple instances of GUIs and viewports #269

[ocornut]

Cleaned up the messy/confusing Set/GetInternalState functions, and added

ImGuiContext* CreateContext(void* (*malloc_fn)(size_t) = NULL, void (*free_fn)(void*) = NULL);
void          DestroyContext(ImGuiContext* ctx);
ImGuiContext* GetCurrentContext();
void          SetCurrentContext(ImGuiContext* ctx);

Decided against changing io.MemAllocFn etc. to imconfig.h IM_MALLOC because editing anything in imconfig.h has big mental and practical barrier cost for many. Instead, to solve the fact that CreateContext() needs to do an allocation the functions are passed here and stored immediately within the context.

This is breaking API for users who were using SetInternalState GetInternalState but they are very rare and those users are expert users so they will figure it out. Added the information in the API Breaking Changes section of the documentation.

I also also renamed the internal ImGuIState struct to ImGuiContext.

[FunMiles]

This thread hasn't seen any activity in a while, but I want to also vote for having a C++ approach of the type context.Button(...) and it seems to me that from the point of refactoring the code to have this would be easier than passing the context as an argument. The reason being that, roughly speaking, one can substitute class ImGui for namespace ImGui and the GImGui global variable becomes a member of the class and compilation goes on without trouble for all the functions that have been neatly defined with a fully qualified name ImGui::function(...). Mostly a few issues exist. All the static functions will have to be declared in the include file and the word static removed. They can be declared as private methods.

For context, I am new to ImGui, but I have a scientific computing app where it is the perfect type of quick-to-implement interface to the visualization. I do need to be able to open multiple windows each with their own GUI attached to the object they contain and not a global object. That is the reason I looked for a thread discussing global variables and found this one.

@ocornut , this is a great and impressive library!

For my immediate use, I will create my own branch where I will make a class.

[FunMiles]

I'm back on this subject because I started doing work with C++20 coroutines. The reason coroutines have an impact on this subject is due to the use of TLS. A coroutine can switch thread any time it co_await something. Thus there is a danger of calling some ImGui functions on one particular thread and then more functions on another thread from the same coroutine function. In such a case, the context associated with the ImGui calls would have been switched in a hard-to-notice manner.

The flip side is that one could also use something like work_thread = co_await gui_thread; to bring the GUI work to a specific thread with the context and then co_await work_thread; to continue on the original work thread.

Has anyone used ImGui in a coroutine context? I think it is one more reason to really have the context passed to all ImGui calls.

[falkTX]

Hi there!
I was just told of this issue, didnt know it was open.

I have been using imgui for audio plugin GUIs for quite a while now, with 2 published projects making use of it so far:

• https://github.com/DISTRHO/Ildaeil/
• https://github.com/trummerschlunk/master_me

The code for imgui stuff is part of https://github.com/DISTRHO/DPF-Widgets, nothing special about it.
I convert the events coming from DPF (the framework that deals with audio plugin stuff) into imgui ones.
There is some seemingly odd/custom code in regards to the final drawing, but only because I wanted imgui-based widgets to be reusable as opengl-child things. Basically having a top-level widget based on opengl where we can draw anything, then in a subsection of the window draw the imgui stuff (as done for master_me histogram).

For this to work it assumes the GUI uses a single thread. Which so far has been the case for all hosts.

Besides the small changes on the rendering side (related to viewport), I am using imgui releases as-is.

[falkTX]

Regarding multiple imgui instances (better named contexts in my case), things go indeed just work.
Here is with a similar setup but in the Cardinal project.

The outside Rack and menus are based on NanoVG + blendish, with NanoSVG for SVG background handling.
The modules on the screenshot are written for imgui:

• audio file player list of files (embedded into some other non-imgui widget)
• sassy scope (taken from the sassy spreadsheet project)
• text editor (from https://github.com/BalazsJako/ImGuiColorTextEdit)
• Ildaeil, as described in my previous post

[Dragnalith]

Regarding this issue, I have made a PR #5856 as a tentative to tackle the problem.

[FunMiles]

Regarding this issue, I have made a PR #5856 as a tentative to tackle the problem.

@Dragnalith I am happy that you started what looks to me like a good attempt at this issue. I am commenting here for now because I don't want to add noise to the PR discussion. I will be trying out your code with Vulkan, as it is now my rendering platform of choice.

[perkele1989]

Code references this issue by "Future development aims to make this context pointer explicit to all calls" in imgui.cpp:1147 (latest docking branch)

Is this still the plan?

[ocornut]

Is this still the plan?

Probably but not soon as it'll be a largely breaking change, perhaps for 2.0.
In the meanwhile #5856 offers a way to convert the codebase with a script.
Note that you can also #define GImGui to be a TLS variable which makes it possible to run multiple parallel contexts.

[NostraMagister]

The 'context argument per function' will indeed be an ABI breaking change, and maybe it doesn't have to be.

As an example, a main function with a context as an argument could become:

void RadioButton(context, ...)

Those that want to do the effort, new user or new code could use that new style.

To maintain backwards ABI compatibility with the current function style, the following could be envisaged.

#ifdef IM_KEEP_OLD_CONTEXT_ABI
void RadioButton(...)
{
// Retrieve the context here and pass it as an argument to the new function.
RadioButton(ctx, ...);
}
#endif

Existing code should be able to run unmodified, IMO, with only the above extra function call as overhead. Existing code could even be partially upgraded only focusing on functions that are called at very high frequency if overhead would be a problem, which I doubt. IMO, when one starts to upgrade they will just undefine IM_KEEP_OLD_CONTEXT_ABI and the compiler will bring up all lines that can be upgraded to the new style.

This is also easy to maintain by the ImGui team because all the new changes are made in the new-style function, and only
if an extra function argument is needed, the old-style function needs 30 seconds extra work.

The TLS technique that is currently in place should still work 100%, unaltered, with this approach.

Furthermore, this should continue to work with SetCurrentContext() because the ctx is retrieved inside the, now wrapper, old function, hence when the application already called SetCurrentContext() on the outside if it uses multi-context.

As far as I studied the code, there may be some issue to be looked at related to SetAllocatorFunctions(). Normally this function is called in pairs with SetCurrentContext() and it is the application that knows when it changes context and related allocator functions. But the new function style, with a context in each function, is actually a kind of more performant and straight forward SetCurrentContext() alternative, isn't it. Hence the allocator functions should also be set.

The solution could be (an opinion) to associate allocator functions with the context. With that concept the application is still in control of what allocator functions it wants to associate with each context. And that will be a performance gain versus calling SetAllocatorFunctions() all the time.

The above would also allow all ImGui demo code and ImPlot code to remain unchanged if there would be a resource problem, because I can imagine that upgrading those would be a serious effort. Now, defining IM_KEEP_OLD_CONTEXT_ABI would be all it takes to keep them running.

From a documentation perspective this needs only a few lines of explanation, as well for existing as for new users.

Well, that is as far as I understood the ImGui code correctly. I am studying it from an SDL3/Vulkan back-end perspective.

With the above approach I am under the impression that ImGui would be fully multi-treaded, multi-context, multi-dll and docking and multi-viewport enabled in a relatively simple way. OK, I know, simple is easily said if you need to do the above for many hundreds of functions, but you get the picture.

That would, IMO, make Dear ImGui one of the most complete, performant and versatile ImGui implementation.

My 5 cent, for what its worth.

[FunMiles]

The 'context argument per function' will indeed be an ABI breaking change, and maybe it doesn't have to be.

As an example, a main function with a context as an argument could become:

void RadioButton(context, ...)

Those that want to do the effort, new user or new code could use that new style.

To maintain backwards ABI compatibility with the current function style, the following could be envisaged.

#ifdef IM_KEEP_OLD_CONTEXT_ABI void RadioButton(...) { // Retrieve the context here and pass it as an argument to the new function. RadioButton(ctx, ...); } #endif

Existing code should be able to run unmodified, IMO, with only the above extra function call as overhead. Existing code could even be partially upgraded only focusing on functions that are called at very high frequency if overhead would be a problem, which I doubt. IMO, when one starts to upgrade they will just undefine IM_KEEP_OLD_CONTEXT_ABI and the compiler will bring up all lines that can be upgraded to the new style.

Using macros and defines is last century's solution 😝. First this is C++ where overloading exists, which means that both

void RadioButton(Context* ctx, Arg1 arg1, Arg2 arg2);

and

void RadioButton(Arg1 arg1, Arg2 arg2) {
   context = getGlobalContext();
   RadioButton(context, arg1, arg2);
}

can coexist without conflict. Secondly there are namespaces to offer a cleaner approach, whereby a using namespace back_compat could bring in the function without context with a single line of code modification. Omitting the using line for users of the context aware API would avoid any danger of accidental misuse of an undefined global context.

[NostraMagister]

Using macro's is adhering with what ImGui uses currently (see imgui.h and imgui.c). I suggested something within the style of the library.

ImGui is in a single namespace and introducing using namespace back_compat would, IMO, break the current easy model where, per docs, users are even invited to extend the ImGui namespace.

The macro also allows to keep the old style code completely out of the compile unit (many hundreds of functions) for new users and new code. ImGui is compiled into most applications as source code.

The new-style being the default has IMO only advantages. See the many questions about multi-context, multi-thread, multi-dll that all in someway relate to that context pointer (as mentioned by the authors in in-line docs).

Yet, as written, backwards compatibility must be available for large projects such as the ImGui and ImPlot Demo and existing user projects, allowing their authors to evaluate on a case per case basis if and when to upgrade them.

About your suggested 'misuse'. Being able to use the old and new style intermixed is IMO a big advantage and there is no real misuse if everything works in symphony anyway. It is a no brainer for the users and it allows for partial upgrades.

[FunMiles]

Using macro's is adhering with what ImGui uses currently (see imgui.h and imgui.c). I suggested something within the style of the library.

Macros can be useful but they are inherently broken. They are not part of the language and can create headaches. It is, IMHO best to avoid them and only use them for things the language cannot provide. In this case, namespaces are a perfect fit.

ImGui is in a single namespace and introducing using namespace back_compat would, IMO, break the current easy model where, per docs, users are even invited to extend the ImGui namespace.

using namespace ImGui::V1;

The macro also allows to keep the old style code completely out of the compile unit (many hundreds of functions) for new users and new code. ImGui is compiled into most applications as source code.

You can have a compile header that does not include the compatibility definitions if that is your issue, but reading and parsing all such simple functions as the one I have shown takes no time at all. Not a concern in my book.

The new-style being the default has IMO only advantages. See the many questions about multi-context, multi-thread, multi-dll that all in someway relate to that context pointer (as mentioned by the authors in in-line docs).

I don't see how that goes against the idea of a back-compatibility namespace. The default is the new-style. Just do not use the compatibility namespace.

Yet, as written, backwards compatibility must be available for large projects such as the ImGui and ImPlot Demo and existing user projects, allowing their authors to evaluate on a case per case basis if and when to upgrade them.

Easy with namespace. If, indeed the new style is the default, just add using namespace ImGui::V1; to those projects. You're done. And it is safer than the define. Why? That line can located at a logical place, after the include, before use of old-style functions.
With a macro style #define, that definition has to be put before before including the ImGui files. In my experience, having to make sure a #define before a specific #include is asking for problem, particularly in large complex projects. One day, some other programmer will introduce some file that will include directly without the define and you'll be wondering for hours where things went wrong and how to fix it.

About your suggested 'misuse'. Being able to use the old and new style intermixed is IMO a big advantage and there is no real misuse if everything works in symphony anyway. It is a no brainer for the users and it allows for partial upgrades.

You misunderstood me. There are cases where you want to mix, and the namespace approach does not impeded it but there are places where you don't: E.g. You can do partial upgrades: when you upgrade some part of your code, it would be best to make sure that within the upgraded parts of your code, only strictly explicit context functions are being used (that's the avoiding misuse comment). In such cases, not having a call to the old interface compile is a significant advantage because if you forget, by accident, to pass the context, the compiler will stop you.
In fact you gave me the greatest advantage of the namespace approach: you can, WITHIN A SINGLE FILE, have the old style function visible only within a certain scope while in other places, they are not visible. That is because the using namespace... can be limited to a scope (within a function, or starting at a certain line, or within the scope of another namespeace). With the macro approach, the declaration is either on or off for the whole file.

All in all, my opinion are stylistic, and I am not imposing them. They are, however, educated by 36 years of C++ programming on large, complex codes.

[GamingMinds-DanielC]

In fact you gave me the greatest advantage of the namespace approach: you can, WITHIN A SINGLE FILE, have the old style function visible only within a certain scope while in other places, they are not visible. That is because the using namespace... can be limited to a scope (within a function, or starting at a certain line, or within the scope of another namespeace). With the macro approach, the declaration is either on or off for the whole file.

Many true things here and namespaces are great, but they do have their limits. While you can import definitions from another (nested or not) namespace into a namespace by using it within the scope of the target namespace, you can't declare or extend a namespace from within the scope of a function. You can't declare a namespace alias to refer to a combination of more than one namespace. So if you need or want to avoid using a namespace, you can't get ImGui::* to refer to ImGui::* and ImGui::compat::* declarations combined. Not in function scope at least.

Just some nitpicking, not an endorsement of macros. ;)

[ocornut]

The solution could be (an opinion) to associate allocator functions with the context. With that concept the application is still in control of what allocator functions it wants to associate with each context.

Allocators cannot conveniently be associated to a context as it would mean e.g. ImVector<> or other leaf helpers, would need a context, or we could decide to need to drop them using our allocators.

And that will be a performance gain versus calling SetAllocatorFunctions() all the time.

???

With the above approach I am under the impression that ImGui would be fully multi-treaded, multi-context, multi-dll and docking and multi-viewport enabled in a relatively simple way.

To clarify. This would only allow different contexts to be used in parallel in multiple threads. Which is already possible with a TLS variable. In no case there is even remotely a possibility that multiple threads would be able to submit or interact simultaneously with a same Dear ImGui context. People seems to be mostly fighting for a feature that's already there, while misunderstanding the actual benefits. The main benefit of explicit context is that a few things would be "neater" (by some vague ideal software engineering definition of neater), it will not enable anything new.

The proposal in #5856 already offers a way to keep an implicit context API which are basically those inline wrappers you suggest, and this is all generated.

The new-style being the default has IMO only advantages.

I personally see one strong disadvantage, which is that it will make Dear ImGui accessible from many less leaf points in a codebase. Technically people could have a global function to retrieve their main context, but I know that in reality that many people are resisting this and would therefore reduce their own access to Dear ImGui. I may personally have the urge to help people fight this by keeping a ImGui::GetCurrentContext() function solely for this purpose. I also noticed while debugging third party codebase that when GImGui was defined to be a function, it would make debugging noticeably more difficult since the pointer is not accessible at all time.

Macros are sometimes a better fit for the need of language bindings generators and use from C, but I haven't explored this particular topic in depth so I don't know what's best. It's a minor detail either way that may be decided when the time has come, but I appreciate the different ideas exposed there.

I think our most likely bet is that we facilitate making #5856 an official thing, and steer toward making backends more usable with multiple contexts.

[NostraMagister]

Yes, (#5856) is even better then just only adding the ctx to every function. Thanks for attracting attention on that post. I will follow that thread instead as I see thought and effort have been put in.

[FunMiles]

Many true things here and namespaces are great, but they do have their limits. While you can import definitions from another (nested or not) namespace into a namespace by using it within the scope of the target namespace, you can't declare or extend a namespace from within the scope of a function.

True, but you cannot add any outwardly visible functions from within the scope of a function. Do you have a concrete example of what you are thinking of? I am not seeing an actual limitation but I may be not thinking of some of your usage cases.

You can't declare a namespace alias to refer to a combination of more than one namespace. So if you need or want to avoid using a namespace, you can't get ImGui::* to refer to ImGui::* and ImGui::compat::* declarations combined. Not in function scope at least.

Right but you can mimic the same effect with something like this:

namespace A {
   int f(int) { return 1; }
}

namespace B {
   int f(int, double) { return 2; }
}

namespace C {
using namespace A;
using namespace B;
}

void user() {
  using namespace C;
  f(3);
  f(3, 1.0);
}

Just some nitpicking, not an endorsement of macros. ;)

Healthy dialog :)

I think our most likely bet is that we facilitate making #5856 an official thing, and steer toward making backends more usable with multiple contexts.

Totally agree. I have fully embraced the code from #5856 but had to tune the backend. So for me the last part of your statement is the currently incomplete part for a smooth use of multiple contexts in a codebase. I only worked on the backend for Vulkan/gflw and am unable to work on/test all the other combinations.

Could we have (maybe in a branch) #5856 merged into the main line code and with having the non-explicit-context function be accessible via namespace or any other approach and then start an evolution of the backend code base for a clean context-aware API?

[adrianboyko]

To clarify. This would only allow different contexts to be used in parallel in multiple threads. Which is already possible with a TLS variable. In no case there is even remotely a possibility that multiple threads would be able to submit or interact simultaneously with a same Dear ImGui context. People seems to be mostly fighting for a feature that's already there, while misunderstanding the actual benefits. The main benefit of explicit context is that a few things would be "neater" (by some vague ideal software engineering definition of neater), it will not enable anything new.

I have high hopes that I can one day make Dear ImGui the go-to GUI for the Pony programming language. Since Pony is actor-oriented, its threading story is a bit different. Pony actors are single threaded but the thread comes from a pool and can differ from invocation to invocation. From the perspective of a high-performance actor-oriented system, I suspect that it would be better to avoid TLS if possible. Instead, a Pony actor that has GUI would save its context in its private state and would simply specify it in calls to Dear ImGui.

But, as you've said, this probably doesn't enable anything new. I suppose that the actor could, instead, assign its context to the TLS variable at the beginning of each invocation and nullify it at the end (to prevent the possibility of it leaking out to some other actor). I'm just concerned about the performance impact of frequent TLS assignments that could be avoided.

Just wanted to add this perspective to the discussion in case it makes any difference.

[FunMiles]

To clarify. This would only allow different contexts to be used in parallel in multiple threads. Which is already possible with a TLS variable. In no case there is even remotely a possibility that multiple threads would be able to submit or interact simultaneously with a same Dear ImGui context. People seems to be mostly fighting for a feature that's already there, while misunderstanding the actual benefits. The main benefit of explicit context is that a few things would be "neater" (by some vague ideal software engineering definition of neater), it will not enable anything new.

I have high hopes that I can one day make Dear ImGui the go-to GUI for the Pony programming language. Since Pony is actor-oriented, its threading story is a bit different. Pony actors are single threaded but the thread comes from a pool and can differ from invocation to invocation. From the perspective of a high-performance actor-oriented system, I suspect that it would be better to avoid TLS if possible. Instead, a Pony actor that has GUI would save its context in its private state and would simply specify it in calls to Dear ImGui.

I am glad you bring up the issues of TLS for your usage. TLS is also not appropriate in some C++ coroutine systems. A coroutine can easily be moved from thread to thread and thus there is no direct binding of any TLS variable to a running coroutine.

@ocornut is partially right if you can have several contexts via TLS, you can work with different contexts in parallel. But he totally miss the point for many usages in fully multi-threaded codes. GUI operations may need to be done on any thread of a pool and this is where explicit contexts are the reliable conceptually clear (that's more than neater or vague) approach, while TLS is not. But even with explicit contexts, you eventually have to render the data from those contexts. That is where clear, easy to use API must be introduced for the backends. He's mentioned the backends and I am glad for that. I just hope we can move forward with a bit more enthusiasm. The explicit context PR he mentioned (and which I use) has been in purgatory way too long. Unless it is brought in (with all the care needed) soon, the work on the backend will never take place. It's a chicken and egg situation.

[ocornut]

I doesn't matter if it's "conceptually clear" if it also comes with another set of problems or challenges.

There are hundreds of open useful topics that have been open for many years. I keep them open because I am interested in them. I would happily merge more third-party PR if they were without side-effects or faults but guess what they almost always are coming with problems and it almost always take me more time to finish a PR than for the submitter to make the initial PR. This is what it takes. It's already my life everyday to wade through literally one thousand open topics (not counting another thousands in my own notes) competing for attention. I only have one instance of myself and I don't know how to do better. The endless commentaries are not necessary helping.

[FunMiles]

@ocornut I appreciate all the work you do and understand the difficulty. Wouldn't there be some possible ways to ease this problem? Could others help you do that work? Could bringing into a side branch, like there is the docking branch, what I think is such an important feature for many and for the future of the project, avoid the side-effects or faults for users who do not need multiple context be a viable approach?

[ocornut]

Could bringing into a side branch, like there is the docking branch, what I think is such an important feature for many and for the future of the project, avoid the side-effects or faults for users who do not need multiple context be a viable approach?

That's close to what #5856 is (and it is a great polished PR which I appreciate very much, which is why I haven't ignored it), if it gets finished and well tested I think we can consider making it an official in-repo script and tested on CI. But all of this need further polishing work (e.g. backends, demos), as you stated yourself. But honestly it seems a little bit low-priority to me to move small mountains to avoid what's in most cases is a function call. I don't think it is realistic use case to use dear imgui from coroutines with one dear imgui context per thread allocated to process coroutines, you probably still need some association logic in place. Either you have a single thread running the coroutines and then there's no need to do anything. Either you have multiple threads running the coroutines and then it means you have multiple context and you are likely to want to associate coroutines to a selected context based on some high-level semantic/classification, at which point it may be affordable to just call a TLS setter if using a manually implemented coroutine system. Either way I suspect at this point this is all a theoretical problem.

(That said, a possible sponsor will soon bring me in to investigate similar issues so it may justify liberating the time to investigate it)

[FunMiles]

I don't think it is realistic use case to use dear imgui from coroutines with one dear imgui context per thread allocated to process coroutines, you probably still need some association logic in place. Either you have a single thread running the coroutines and then there's no need to do anything. Either you have multiple threads running the coroutines and then it means you have multiple context and you are likely to want to associate coroutines to a selected context based on some high-level semantic/classification, at which point it may be affordable to just call a TLS setter if using a manually implemented coroutine system. Either way I suspect at this point this is all a theoretical problem.

I am using coroutines with imgui. It is unreasonable to make a coroutine system aware of some imgui TLS variable and forcing the coroutine system to set TLS variables for any library (not just imgui) when re-starting a coroutine on a thread.
A coroutine system and any library use are orthogonal principles.
A given coroutine is associated to a context simply by having that context as a variable of the coroutine.
The following skeletal example how the context aware code can be used cleanly and comments show where TLS cause trouble:

thread_local ImGuiContext* tls_context;
task<void> visualization(Rendering &rendering) {
    auto ctx = ImGui::CreateContext(); // On some thread B
   
    while(not window_closed) {
       tls_context = ctx; //  Has not effect in the explicit-context imgui code, just for illustration of TLS vs coroutines.
       [...]
       // run calls to the backend on the main thread and wait for it to complete 
       co_await on_main_thread([&]) {
            // This is running on the main thread
            ImGui_ImplVulkan_NewFrame(ctx);
            ImGui_ImplGlfw_NewFrame(ctx);
            ImGui::NewFrame(ctx);
        });
       // the coroutine can now be running on another thread B than the original A. 
       assert(tls_context == ctx); // This assert will fail any time the coroutine is moved between threads.
       // We don't want to have to set TLS variables after every `co_await` it is error prone and unnecessary if the context
       // is explicit in function calls that need it.
       [...] 
   }
}

[Dragnalith]

I am not sure why there are long debates: - everyone seems already convinced by the approach of having an explicit context as first argument. That's what the PR implements - it has already been decided not pursue a version where both the legacy implicit API and the new explicit API coexist - it has already been decided the PR won't be merged until a V2 of Dear Imgui accepting breaking change. So whatever people opinion on TLS being good enough or not, it has not impact on the current course of action.

…

On Sat, Jun 8, 2024, 02:35 FunMiles ***@***.***> wrote: I don't think it is realistic use case to use dear imgui from coroutines with one dear imgui context per thread allocated to process coroutines, you probably still need some association logic in place. Either you have a single thread running the coroutines and then there's no need to do anything. Either you have multiple threads running the coroutines and then it means you have multiple context and you are likely to want to associate coroutines to a selected context based on some high-level semantic/classification, at which point it may be affordable to just call a TLS setter if using a manually implemented coroutine system. Either way I suspect at this point this is all a theoretical problem. I am using coroutines with imgui. It is unreasonable to make a coroutine system aware of some imgui TLS variable and forcing the coroutine system to set TLS variables for any library (not just imgui) when re-starting a coroutine on a thread. A coroutine system and any library use are orthogonal principles. A given coroutine is associated to a context simply by having that context as a variable of the coroutine. The following skeletal example how the context aware code can be used cleanly and comments show where TLS cause trouble: thread_local ImGuiContext* tls_context; task<void> visualization(Rendering &rendering) { auto ctx = ImGui::CreateContext(); // On some thread B while(not window_closed) { tls_context = ctx; // Has not effect, just for illustration. [...] // run calls to the backend on the main thread and wait for it to complete co_await on_main_thread([&]) { // This is running on the main thread ImGui_ImplVulkan_NewFrame(ctx); ImGui_ImplGlfw_NewFrame(ctx); ImGui::NewFrame(ctx); }); // the coroutine can now be running on another thread B than the original A. assert(tls_context == ctx); // This assert will fail any time the coroutine is moved between threads. // We don't want to have to set TLS variables after every `co_await` it is error prone and unnecessary if the context // is explicit in function calls that need it. [...] } } — Reply to this email directly, view it on GitHub <#586 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHSSFE7YUGZTMU5YBTFGSDZGHVOJAVCNFSM4CAXWF6KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJVGUZDKMZZGE3A> . You are receiving this because you were mentioned.Message ID: ***@***.***>