jvcleave / ofxImGui

Use ImGui in openFrameworks
292 stars 123 forks source link

ofxImGui::Gui::exit() released context, can't Gui::setup() again #121

Closed zhangew closed 9 months ago

zhangew commented 10 months ago

Hi jvcleave, I'm using your ofxImGui, it is convenient, thanks. I want to create ofxImGui::Gui and destruct the instance in each 'screen' of my application. I mean spawn and destroy cycle, multiple times. I got an error, when create ofxImGui::Gui instance for the second time, in its setup() method. the assert failed: ImGuiIO& ImGui::GetIO() { IM_ASSERT(GImGui != NULL && "No current context. Did you call ImGui::CreateContext() and ImGui::SetCurrentContext() ?"); return GImGui->IO; }

I traced the code of setup() and exit() of ofxImGui::Gui. In exit(), it destroyed the context: // Destroy ImGui::DestroyContext(context->imguiContext);

            // Set context to null so that slaves can know the context is gone.
            context->imguiContext = nullptr;

and the 'context' is a shortcut pointer to the static map: static std::unordered_map<ofAppBaseWindow*, ofxImGuiContext> imguiContexts; // Window/MasterContext map

when run into setup again after created an new instance of ofxImGui::Gui, the existance of static map will route into the branch: auto foundWindowContext = imguiContexts.find( _ofWindow.get() ); if( foundWindowContext != imguiContexts.end() ) existingOfWindowContext = &foundWindowContext->second ;

    if( existingOfWindowContext != nullptr ){
        existingOfWindowContext->slaveCount++; // tells master that the context became shared

mean existingOfWindowContext != nullptr.

Thus, will not create the destroyed imguiContext again, and the assertion(mentioned before) will then fail: ImGuiIO& io = ImGui::GetIO();

I feel the effect is like a dangling pointer problem.

Would you like to have a look on it?

BTW, my solution from Application's view is only create and exit once through the whole life of my application. Keep an instance in the longest survived object.

Thank you!

Daandelange commented 10 months ago

Hi, From your pieces of code I deduce that you're using the new develop version which brings (beta?) support for multiple instances, I have been working on this feature (and I still need to clean it up before merging it).

First of all, before going further into some details, I'd like to point out that ImGui discourages to use multiple GUI instances and switching them (event with destruct() etc.), and I also remember reading that it's mostly a "design mistake" in the sense that the GUI is meant to be used from within 1 single host system window; we -openFrameworkers- of course can not always comply with this and I tried to come up with a solution for this.

As you pointed out, the solution that I implemented tries to setup 1 ImGui context per ofAppBaseWindow and destroy it on close. In between, you can just render or not, and ofxImGui automatically handles switching the imgui context when you gui.begin()+end() from within each ofAppBaseWindow. The drawback of this solution is that only the first call to setup() from an ofWindow has "master" control over the gui instance, the next ones will setup() as "slaves" and have less control over some settings. (In your ofApp you can check this by checking the return flag of setup().)

So I'd say, what you're trying to achieve is probably already baked in, except that we tested it only for the whole ofWindow lifetime, not for occasional use, but I can understand the need for it. On the other side, I also remember reading advice from the ImGui community that it's recommended (in terms of design principles) to keep that GUI context alive for the whole window lifetime and omit rendering to it if you don't need it and let it "sleep in memory", so we're probably adventuring into unsupported usage of the underlying library.

Do you really wish to kill that instance until its next use ? (Could you use a dummy instance to keep it alive ?)
ImGui has got a very tiny footprint and chances are that you lose more then you gain by periodically destroying/reconstructing it.
What are your application windows lifetimes ?

Btw, it could be great to throw a more informative ofxImGui assert before the ImGui assert throws.

zhangew commented 10 months ago

Hi @Daandelange, Thanks for your detailed explanation. Yes, I'm using the code downloaded from code page as zip directly, I might want trying it ASAP at that time. My design is in one glfw window there is one guimanager and it manages several gui 'pages' in an application. Because I'm new to Dear ImGui and ofxImGui, I didn't understand the mechanism of them well at first. so I put the instance of ofxImGui::Gui into the 'page' level, and thus it's natural to release and recreate instance of the class each time when switching between 'pages'. So I found the assertion fail.

And after I posted here, as you have mentioned, now I indeed moved the only ofxImGui::Gui instance into my guimanager instance, whose life is as long as the whole application. So the problem is gone for me. I just need to manager the state of some 'controls' which keep alive between 'pages'. It worked well so far.

And thank you for the knowledge you told me. I understand it better.

Best regards.