Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GImDefaultFontAtlas causes problems with custom allocator #396

Closed
bkaradzic opened this issue Nov 3, 2015 · 11 comments
Closed

GImDefaultFontAtlas causes problems with custom allocator #396

bkaradzic opened this issue Nov 3, 2015 · 11 comments

Comments

@bkaradzic
Copy link
Contributor

Problem with this code:
https://github.com/ocornut/imgui/blob/master/imgui.cpp#L623

Is that allocations might happen with different allocators. It uses default allocator before custom allocator is set and afterwards it uses custom allocator. On shutdown even if default allocator is set there still might be lingering block allocated with custom allocator.

@ocornut
Copy link
Owner

ocornut commented Nov 3, 2015

What is the problem exactly?

The constructor for GImDefaultFontAtlas shouldn't allocate anything. The only requirement is that you set the memory functions before calling NewFrame or any of the font functions such as AddFont or GetTexData*

@bkaradzic
Copy link
Contributor Author

The problem is that static destructor is clearing it, and data contained is allocated with custom allocator that is gone by the time static destructors run. ImGui::Shutdown() should clean it instead. If you need repro case just run 20-nanovg from bgfx and it will crash on shutdown.

@ocornut
Copy link
Owner

ocornut commented Nov 3, 2015

OK so the problem is in Shutdown(). Will look into that.

@ocornut
Copy link
Owner

ocornut commented Nov 3, 2015

I couldn't find the problem. Unless you are manipulating and changing the g.IO.Fonts pointer (which should point to GImDefaultFontAtlas), Shutdown() clears it and afaik MemFree() is called everywhere there. Which field is causing a problem?

Can you confirm that you are doing things in this order?

  • set custom allocators pointers
  • call GetTextData, NewFrame, ImGui functions
  • cal Shutdown() (custom allocators still set at this point)

@bkaradzic
Copy link
Contributor Author

Ok, found it, in this particular case I initialized ImGui because stb TTF is used between ImGui and NanoVG, but I never called ImGui::NewFrame because there is no ImGui.

Maybe, adding ImGui::NewFrame into ImGui::Shutdown would be good to prevent this kind of bug?

@ocornut
Copy link
Owner

ocornut commented Nov 4, 2015

I'd rather make it fullproof without calling NewFraame(). Was exactly was misfreed or crashing? (of the two bug reports i keep not knowing what it is precisely :)

@bkaradzic
Copy link
Contributor Author

It was freed after ImGui::Shutdown. I intentionally NULL allocator in my code to catch this kind of bugs.

@ocornut
Copy link
Owner

ocornut commented Nov 5, 2015

I'll go and update/build bgfx tonight to try to get a repro. Nobody wants to tell me WHAT is freed, WHICH FIELD(S) ? it sounds like a conspiracy that I can't get an answer.

@bkaradzic
Copy link
Contributor Author

Here is info:

* thread #1: tid = 0x104062, 0x00000001000f99f0 example-20-nanovgDebug`ImGuiStyle::ImGuiStyle(this=0x0000000000000000) + 420656 at imgui.cpp:629, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x00000001000f99f0 example-20-nanovgDebug`ImGuiStyle::ImGuiStyle(this=0x0000000000000000) + 420656 at imgui.cpp:629
626     // User facing structures
627     //-----------------------------------------------------------------------------
628
-> 629  ImGuiStyle::ImGuiStyle()
630     {
631         Alpha                   = 1.0f;             // Global alpha applies to everything in ImGui
632         WindowPadding           = ImVec2(8,8);      // Padding within a window
(lldb) bt
* thread #1: tid = 0x104062, 0x00000001000f99f0 example-20-nanovgDebug`ImGuiStyle::ImGuiStyle(this=0x0000000000000000) + 420656 at imgui.cpp:629, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
* frame #0: 0x00000001000f99f0 example-20-nanovgDebug`ImGuiStyle::ImGuiStyle(this=0x0000000000000000) + 420656 at imgui.cpp:629
    frame #1: 0x0000000100107146 example-20-nanovgDebug`OcornutImguiContext::memFree(_ptr=0x000000010e9bb000) + 38 at ocornut_imgui.cpp:674
    frame #2: 0x0000000100096377 example-20-nanovgDebug`ImGui::MemFree(ptr=0x000000010e9bb000) + 71 at imgui.cpp:1775
    frame #3: 0x00000001000ca2b9 example-20-nanovgDebug`ImFontAtlas::ClearInputData(this=0x00000001001d19d8) + 137 at imgui_draw.cpp:972
    frame #4: 0x00000001000ca1e9 example-20-nanovgDebug`ImFontAtlas::Clear(this=0x00000001001d19d8) + 25 at imgui_draw.cpp:1008
    frame #5: 0x00000001000ca1a9 example-20-nanovgDebug`ImFontAtlas::~ImFontAtlas(this=0x00000001001d19d8) + 25 at imgui_draw.cpp:964
    frame #6: 0x00000001000ca225 example-20-nanovgDebug`ImFontAtlas::~ImFontAtlas(this=0x00000001001d19d8) + 21 at imgui_draw.cpp:963
    frame #7: 0x00007fff8c4800ff libsystem_c.dylib`__cxa_finalize_ranges + 345
    frame #8: 0x00007fff8c480403 libsystem_c.dylib`exit + 55
    frame #9: 0x00007fff9686e5b4 libdyld.dylib`start + 8

@ocornut
Copy link
Owner

ocornut commented Nov 7, 2015

Alright :) it all makes sense now. If you don't call NewFrame(), ImGui is marked as not initialized and Shutdown() doesn't clear the FontAtlas either. I'll fix that!

@ocornut
Copy link
Owner

ocornut commented Nov 7, 2015

Thanks for sorry for taking so long to figure that out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants