Font bug when using multiple instances #591

Closed
opened 2016-04-15 19:39:38 +00:00 by Cthutu · 14 comments
Cthutu commented 2016-04-15 19:39:38 +00:00 (Migrated from github.com)

This is briefly mentioned in #586 but when I switch state via the ImGui::SetInternalState() API, the fonts that is the not most recently created instance get corrupted. There is a work around. I run this code before the draw loops (OpenGL renderer):

unsigned char* pixels;
int width, height;
io.Fonts->GetTexDataAsRGBA32(&pixels, &width, &height);

And immediately I bind the texture:

glBindTexture(GL_TEXTURE_2D, (GLuint)(intptr_t)drawCmd.TextureId);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, (GLint)GL_LINEAR);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, (GLint)GL_LINEAR);
glTexImage2D(GL_TEXTURE_2D, 0, (GLint)GL_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, pixels);

I am essentially reloading the OpenGL textures. Is this because ImGui::SetInternalState() is moving the font texure around in memory and the OpenGL texture is referencing old memory? I haven't analysed the behaviour between switching internal states yet, but I thought I'd let you be aware of it.

This is briefly mentioned in #586 but when I switch state via the `ImGui::SetInternalState()` API, the fonts that is the not most recently created instance get corrupted. There is a work around. I run this code before the draw loops (OpenGL renderer): ``` unsigned char* pixels; int width, height; io.Fonts->GetTexDataAsRGBA32(&pixels, &width, &height); ``` And immediately I bind the texture: ``` glBindTexture(GL_TEXTURE_2D, (GLuint)(intptr_t)drawCmd.TextureId); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, (GLint)GL_LINEAR); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, (GLint)GL_LINEAR); glTexImage2D(GL_TEXTURE_2D, 0, (GLint)GL_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, pixels); ``` I am essentially reloading the OpenGL textures. Is this because `ImGui::SetInternalState()` is moving the font texure around in memory and the OpenGL texture is referencing old memory? I haven't analysed the behaviour between switching internal states yet, but I thought I'd let you be aware of it.
ocornut commented 2016-04-17 08:23:06 +00:00 (Migrated from github.com)

SetInternalState() is only assigning to a pointer so isn't affecting anything font related.
Now, I believe you that may be a bug but I would need more detailed information and ideally a repro. What may be obvious/trivial in your context may not be obvious everywhere.

How are you creating the second state? Are you reassigning the font atlas pointer?

SetInternalState() is only assigning to a pointer so isn't affecting anything font related. Now, I believe you that may be a bug but I would need more detailed information and ideally a repro. What may be obvious/trivial in your context may not be obvious everywhere. How are you creating the second state? Are you reassigning the font atlas pointer?
itamago commented 2016-04-17 10:45:50 +00:00 (Migrated from github.com)

I had the same issue few time ago while using 2 different ImGui contexts with their own fonts. After reading the source code I understood that you need to have separate texture atlases too.
So, create an additional atlas for each additional context, and when you SetInternalState you must swap the atlases too. It works perfectly well.

I had the same issue few time ago while using 2 different ImGui contexts with their own fonts. After reading the source code I understood that you need to have separate texture atlases too. So, create an additional atlas for each additional context, and when you SetInternalState you must swap the atlases too. It works perfectly well.
ocornut commented 2016-04-17 11:08:28 +00:00 (Migrated from github.com)

This isn't as intended, you should be able to share the same atlas for two instances.

This isn't as intended, you should be able to share the same atlas for two instances.
Cthutu commented 2016-04-17 11:27:29 +00:00 (Migrated from github.com)

I do create an additional atlas for each context. In fact, I have a
different OpenGL context per ImGui context. I will look at the code on
Monday and show you how I manage the ImGui contexts.

On Sun, 17 Apr 2016 at 07:08 omar notifications@github.com wrote:

This isn't as intended, you should be able to share the same atlas for two
instances.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/ocornut/imgui/issues/591#issuecomment-211001352

I do create an additional atlas for each context. In fact, I have a different OpenGL context per ImGui context. I will look at the code on Monday and show you how I manage the ImGui contexts. On Sun, 17 Apr 2016 at 07:08 omar notifications@github.com wrote: > This isn't as intended, you should be able to share the same atlas for two > instances. > > — > You are receiving this because you authored the thread. > Reply to this email directly or view it on GitHub > https://github.com/ocornut/imgui/issues/591#issuecomment-211001352
ocornut commented 2016-04-17 12:50:40 +00:00 (Migrated from github.com)

Thanks!

Similarly, you should be able to reuse the same atlas if they are going to be the same (tho obviously you need to upload texture to your two opengl context and find a way to pass textureid that works for both).

This looks fishy so I'll need to give it a closer look.

Thanks! Similarly, you should be able to reuse the same atlas if they are going to be the same (tho obviously you need to upload texture to your two opengl context and find a way to pass textureid that works for both). This looks fishy so I'll need to give it a closer look.
ocornut commented 2016-04-17 18:11:55 +00:00 (Migrated from github.com)

This worked for me:

    // Setup ImGui binding
    void* state1 = ImGui::GetInternalState();
    ImGui_ImplGlfw_Init(window, true);

    void* state2 = (void*)new char[ImGui::GetInternalStateSize()];
    ImGui::SetInternalState(state2, true);
    ImGui_ImplGlfw_Init(window, true);

    [...] Main Loop

        ImGui::SetInternalState(state2);

        ImGui_ImplGlfw_NewFrame();

        ImGui::Begin("HELLO");
        ImGui::Text("Hello");
        ImGui::End();

        ImGui::SetInternalState(state1);

        ImGui_ImplGlfw_NewFrame();
        [...] 

        ImGui::SetInternalState(state1);
        ImGui::Render();

        ImGui::SetInternalState(state2);
        ImGui::Render();

BUT I had to fix code in ImGui_ImplGlfw_NewFrame(), added the else statement else:

    ImGuiIO& io = ImGui::GetIO();
    if (!g_FontTexture)
        ImGui_ImplGlfw_CreateDeviceObjects();
    else
        io.Fonts->TexID = (void *)(intptr_t)g_FontTexture;

Maybe the sample code should be more aware of multiple contexts? Are you using this code as is, or something that made more sense for your own use of multiple contexts?

Also the InternalState function names/API sucks (will be renamed).

This worked for me: ``` // Setup ImGui binding void* state1 = ImGui::GetInternalState(); ImGui_ImplGlfw_Init(window, true); void* state2 = (void*)new char[ImGui::GetInternalStateSize()]; ImGui::SetInternalState(state2, true); ImGui_ImplGlfw_Init(window, true); [...] Main Loop ImGui::SetInternalState(state2); ImGui_ImplGlfw_NewFrame(); ImGui::Begin("HELLO"); ImGui::Text("Hello"); ImGui::End(); ImGui::SetInternalState(state1); ImGui_ImplGlfw_NewFrame(); [...] ImGui::SetInternalState(state1); ImGui::Render(); ImGui::SetInternalState(state2); ImGui::Render(); ``` BUT I had to fix code in ImGui_ImplGlfw_NewFrame(), added the else statement else: ``` ImGuiIO& io = ImGui::GetIO(); if (!g_FontTexture) ImGui_ImplGlfw_CreateDeviceObjects(); else io.Fonts->TexID = (void *)(intptr_t)g_FontTexture; ``` Maybe the sample code should be more aware of multiple contexts? Are you using this code as is, or something that made more sense for your own use of multiple contexts? Also the InternalState function names/API sucks (will be renamed).
Cthutu commented 2016-04-17 21:22:41 +00:00 (Migrated from github.com)

I will compare what you have to what I have on Monday and will report in!

On Sun, 17 Apr 2016 at 14:11 omar notifications@github.com wrote:

This worked for me:

// Setup ImGui binding
void* state1 = ImGui::GetInternalState();
ImGui_ImplGlfw_Init(window, true);

void* state2 = (void*)new char[ImGui::GetInternalStateSize()];
ImGui::SetInternalState(state2, true);
ImGui_ImplGlfw_Init(window, true);

[...] Main Loop

    ImGui::SetInternalState(state2);

    ImGui_ImplGlfw_NewFrame();

    ImGui::Begin("HELLO");
    ImGui::Text("Hello");
    ImGui::End();

    ImGui::SetInternalState(state1);

    ImGui_ImplGlfw_NewFrame();
    [...]

    ImGui::SetInternalState(state1);
    ImGui::Render();

    ImGui::SetInternalState(state2);
    ImGui::Render();

Apart from the fact that the function names/API sucks (will be fixed!).

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/ocornut/imgui/issues/591#issuecomment-211074666

I will compare what you have to what I have on Monday and will report in! On Sun, 17 Apr 2016 at 14:11 omar notifications@github.com wrote: > This worked for me: > > ``` > // Setup ImGui binding > void* state1 = ImGui::GetInternalState(); > ImGui_ImplGlfw_Init(window, true); > > void* state2 = (void*)new char[ImGui::GetInternalStateSize()]; > ImGui::SetInternalState(state2, true); > ImGui_ImplGlfw_Init(window, true); > > [...] Main Loop > > ImGui::SetInternalState(state2); > > ImGui_ImplGlfw_NewFrame(); > > ImGui::Begin("HELLO"); > ImGui::Text("Hello"); > ImGui::End(); > > ImGui::SetInternalState(state1); > > ImGui_ImplGlfw_NewFrame(); > [...] > > ImGui::SetInternalState(state1); > ImGui::Render(); > > ImGui::SetInternalState(state2); > ImGui::Render(); > ``` > > Apart from the fact that the function names/API sucks (will be fixed!). > > — > You are receiving this because you authored the thread. > Reply to this email directly or view it on GitHub > https://github.com/ocornut/imgui/issues/591#issuecomment-211074666
Cthutu commented 2016-04-17 21:25:07 +00:00 (Migrated from github.com)

Immediately I notice that you're only using one OpenGL context, but if that
would affect anything, I couldn't tell you.

On Sun, 17 Apr 2016 at 17:22 Matt Davies mattie.davies@gmail.com wrote:

I will compare what you have to what I have on Monday and will report in!

On Sun, 17 Apr 2016 at 14:11 omar notifications@github.com wrote:

This worked for me:

// Setup ImGui binding
void* state1 = ImGui::GetInternalState();
ImGui_ImplGlfw_Init(window, true);

void* state2 = (void*)new char[ImGui::GetInternalStateSize()];
ImGui::SetInternalState(state2, true);
ImGui_ImplGlfw_Init(window, true);

[...] Main Loop

    ImGui::SetInternalState(state2);

    ImGui_ImplGlfw_NewFrame();

    ImGui::Begin("HELLO");
    ImGui::Text("Hello");
    ImGui::End();

    ImGui::SetInternalState(state1);

    ImGui_ImplGlfw_NewFrame();
    [...]

    ImGui::SetInternalState(state1);
    ImGui::Render();

    ImGui::SetInternalState(state2);
    ImGui::Render();

Apart from the fact that the function names/API sucks (will be fixed!).

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/ocornut/imgui/issues/591#issuecomment-211074666

Immediately I notice that you're only using one OpenGL context, but if that would affect anything, I couldn't tell you. On Sun, 17 Apr 2016 at 17:22 Matt Davies mattie.davies@gmail.com wrote: > I will compare what you have to what I have on Monday and will report in! > > On Sun, 17 Apr 2016 at 14:11 omar notifications@github.com wrote: > > > This worked for me: > > > > ``` > > // Setup ImGui binding > > void* state1 = ImGui::GetInternalState(); > > ImGui_ImplGlfw_Init(window, true); > > > > void* state2 = (void*)new char[ImGui::GetInternalStateSize()]; > > ImGui::SetInternalState(state2, true); > > ImGui_ImplGlfw_Init(window, true); > > > > [...] Main Loop > > > > ImGui::SetInternalState(state2); > > > > ImGui_ImplGlfw_NewFrame(); > > > > ImGui::Begin("HELLO"); > > ImGui::Text("Hello"); > > ImGui::End(); > > > > ImGui::SetInternalState(state1); > > > > ImGui_ImplGlfw_NewFrame(); > > [...] > > > > ImGui::SetInternalState(state1); > > ImGui::Render(); > > > > ImGui::SetInternalState(state2); > > ImGui::Render(); > > ``` > > > > Apart from the fact that the function names/API sucks (will be fixed!). > > > > — > > You are receiving this because you authored the thread. > > Reply to this email directly or view it on GitHub > > https://github.com/ocornut/imgui/issues/591#issuecomment-211074666
ocornut commented 2016-05-08 15:01:47 +00:00 (Migrated from github.com)

@Cthutu Any news on that?

I still have to admit that contexts are confusing because a) imgui_impl_xxx files aren't very context aware. b) Font Atlas being shared by default makes things confusing.

Would definitively need to clarify the situation with font atlas.

    // Setup ImGui binding
    ImGuiContext* ctx1 = ImGui::GetCurrentContext();
    ImGui_ImplGlfw_Init(window, true);

    ImGuiContext* ctx2 = ImGui::CreateContext();
    ImGui::SetCurrentContext(ctx2);
    ImGui_ImplGlfw_Init(window, true);

...

        ImGui::SetCurrentContext(ctx1);
        ImGui_ImplGlfw_NewFrame();
        ImGui::Text("Hello");

        ImGui::SetCurrentContext(ctx2);
        ImGui_ImplGlfw_NewFrame();

...

        ImGui::SetCurrentContext(ctx1);
        ImGui::Render();

        ImGui::SetCurrentContext(ctx2);
        ImGui::Render();
@Cthutu Any news on that? I still have to admit that contexts are confusing because a) imgui_impl_xxx files aren't very context aware. b) Font Atlas being shared by default makes things confusing. Would definitively need to clarify the situation with font atlas. ``` // Setup ImGui binding ImGuiContext* ctx1 = ImGui::GetCurrentContext(); ImGui_ImplGlfw_Init(window, true); ImGuiContext* ctx2 = ImGui::CreateContext(); ImGui::SetCurrentContext(ctx2); ImGui_ImplGlfw_Init(window, true); ... ImGui::SetCurrentContext(ctx1); ImGui_ImplGlfw_NewFrame(); ImGui::Text("Hello"); ImGui::SetCurrentContext(ctx2); ImGui_ImplGlfw_NewFrame(); ... ImGui::SetCurrentContext(ctx1); ImGui::Render(); ImGui::SetCurrentContext(ctx2); ImGui::Render(); ```
Cthutu commented 2016-05-08 16:22:47 +00:00 (Migrated from github.com)

I haven't been able to check this out yet. I'm currently busy at ToJam but
when I get to work I will figure it out. But first I have to port my code
to use your tree nodes :) Sorry I can't be more timely on this but things
are a bit crazy at the moment. Expect a reply towards the end of the week.

On Sun, 8 May 2016 at 11:01 omar notifications@github.com wrote:

@Cthutu https://github.com/Cthutu Any news on that?

I still have to admit that contexts are confusing because a)
imgui_impl_xxx files aren't very context aware. b) Font Atlas being shared
by default makes things confusing.

Would definitively need to clarify the situation with font atlas.

// Setup ImGui binding
ImGuiContext* ctx1 = ImGui::GetCurrentContext();
ImGui_ImplGlfw_Init(window, true);

ImGuiContext* ctx2 = ImGui::CreateContext();
ImGui::SetCurrentContext(ctx2);
ImGui_ImplGlfw_Init(window, true);

...

    ImGui::SetCurrentContext(ctx1);
    ImGui_ImplGlfw_NewFrame();
    ImGui::Text("Hello");

    ImGui::SetCurrentContext(ctx2);
    ImGui_ImplGlfw_NewFrame();

...

    ImGui::SetCurrentContext(ctx1);
    ImGui::Render();

    ImGui::SetCurrentContext(ctx2);
    ImGui::Render();

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
https://github.com/ocornut/imgui/issues/591#issuecomment-217725412

I haven't been able to check this out yet. I'm currently busy at ToJam but when I get to work I will figure it out. But first I have to port my code to use your tree nodes :) Sorry I can't be more timely on this but things are a bit crazy at the moment. Expect a reply towards the end of the week. On Sun, 8 May 2016 at 11:01 omar notifications@github.com wrote: > @Cthutu https://github.com/Cthutu Any news on that? > > I still have to admit that contexts are confusing because a) > imgui_impl_xxx files aren't very context aware. b) Font Atlas being shared > by default makes things confusing. > > Would definitively need to clarify the situation with font atlas. > > ``` > // Setup ImGui binding > ImGuiContext* ctx1 = ImGui::GetCurrentContext(); > ImGui_ImplGlfw_Init(window, true); > > ImGuiContext* ctx2 = ImGui::CreateContext(); > ImGui::SetCurrentContext(ctx2); > ImGui_ImplGlfw_Init(window, true); > ``` > > ... > > ``` > ImGui::SetCurrentContext(ctx1); > ImGui_ImplGlfw_NewFrame(); > ImGui::Text("Hello"); > > ImGui::SetCurrentContext(ctx2); > ImGui_ImplGlfw_NewFrame(); > ``` > > ... > > ``` > ImGui::SetCurrentContext(ctx1); > ImGui::Render(); > > ImGui::SetCurrentContext(ctx2); > ImGui::Render(); > ``` > > — > You are receiving this because you were mentioned. > > Reply to this email directly or view it on GitHub > https://github.com/ocornut/imgui/issues/591#issuecomment-217725412
Cthutu commented 2016-05-13 13:21:41 +00:00 (Migrated from github.com)

FYI, things have been delayed to Monday so I will look at updating my imgui
repo on Tuesday.

On Sun, 8 May 2016 at 12:22 Matt Davies mattie.davies@gmail.com wrote:

I haven't been able to check this out yet. I'm currently busy at ToJam
but when I get to work I will figure it out. But first I have to port my
code to use your tree nodes :) Sorry I can't be more timely on this but
things are a bit crazy at the moment. Expect a reply towards the end of
the week.

On Sun, 8 May 2016 at 11:01 omar notifications@github.com wrote:

@Cthutu https://github.com/Cthutu Any news on that?

I still have to admit that contexts are confusing because a)
imgui_impl_xxx files aren't very context aware. b) Font Atlas being shared
by default makes things confusing.

Would definitively need to clarify the situation with font atlas.

// Setup ImGui binding
ImGuiContext* ctx1 = ImGui::GetCurrentContext();
ImGui_ImplGlfw_Init(window, true);

ImGuiContext* ctx2 = ImGui::CreateContext();
ImGui::SetCurrentContext(ctx2);
ImGui_ImplGlfw_Init(window, true);

...

    ImGui::SetCurrentContext(ctx1);
    ImGui_ImplGlfw_NewFrame();
    ImGui::Text("Hello");

    ImGui::SetCurrentContext(ctx2);
    ImGui_ImplGlfw_NewFrame();

...

    ImGui::SetCurrentContext(ctx1);
    ImGui::Render();

    ImGui::SetCurrentContext(ctx2);
    ImGui::Render();

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
https://github.com/ocornut/imgui/issues/591#issuecomment-217725412

FYI, things have been delayed to Monday so I will look at updating my imgui repo on Tuesday. On Sun, 8 May 2016 at 12:22 Matt Davies mattie.davies@gmail.com wrote: > I haven't been able to check this out yet. I'm currently busy at ToJam > but when I get to work I will figure it out. But first I have to port my > code to use your tree nodes :) Sorry I can't be more timely on this but > things are a bit crazy at the moment. Expect a reply towards the end of > the week. > > On Sun, 8 May 2016 at 11:01 omar notifications@github.com wrote: > > > @Cthutu https://github.com/Cthutu Any news on that? > > > > I still have to admit that contexts are confusing because a) > > imgui_impl_xxx files aren't very context aware. b) Font Atlas being shared > > by default makes things confusing. > > > > Would definitively need to clarify the situation with font atlas. > > > > ``` > > // Setup ImGui binding > > ImGuiContext* ctx1 = ImGui::GetCurrentContext(); > > ImGui_ImplGlfw_Init(window, true); > > > > ImGuiContext* ctx2 = ImGui::CreateContext(); > > ImGui::SetCurrentContext(ctx2); > > ImGui_ImplGlfw_Init(window, true); > > ``` > > > > ... > > > > ``` > > ImGui::SetCurrentContext(ctx1); > > ImGui_ImplGlfw_NewFrame(); > > ImGui::Text("Hello"); > > > > ImGui::SetCurrentContext(ctx2); > > ImGui_ImplGlfw_NewFrame(); > > ``` > > > > ... > > > > ``` > > ImGui::SetCurrentContext(ctx1); > > ImGui::Render(); > > > > ImGui::SetCurrentContext(ctx2); > > ImGui::Render(); > > ``` > > > > — > > You are receiving this because you were mentioned. > > > > Reply to this email directly or view it on GitHub > > https://github.com/ocornut/imgui/issues/591#issuecomment-217725412
ocornut commented 2016-09-23 10:59:08 +00:00 (Migrated from github.com)

@Cthutu Any news of that? Still looking for a repro :)

@Cthutu Any news of that? Still looking for a repro :)
ocornut commented 2017-08-09 12:31:05 +00:00 (Migrated from github.com)

Could we get to the bottom of this topic?

AFAIK:

  • You can share a ImFontAtlas between two ImGuiContext (I might be wrong, maybe there is an issue in which case let me know)
  • If you have 2 OpenGL contexts sharing texture data is a solvable problem (out of the scope of imgui) but if for some reason it is causing issues you can effectively have 2 ImFontAtlas, or share 1 ImFontAtlas and somehow reinterpret the atlas TextureId to refer to a different OpenGL texture according to your OpenGL context.
Could we get to the bottom of this topic? AFAIK: - You can share a ImFontAtlas between two ImGuiContext (I might be wrong, maybe there is an issue in which case let me know) - If you have 2 OpenGL contexts sharing texture data is a solvable problem (out of the scope of imgui) but if for some reason it is causing issues you can effectively have 2 ImFontAtlas, or share 1 ImFontAtlas and somehow reinterpret the atlas TextureId to refer to a different OpenGL texture according to your OpenGL context.
ocornut commented 2018-02-06 20:54:12 +00:00 (Migrated from github.com)

@Cthutu The issues discussed here should now be clarified and fixed by the changes discussed in #1565.
Context creation is now explicit, and have a parameter to pass in a font atlas created by the user.
Read #1565 for details.

@Cthutu The issues discussed here should now be clarified and fixed by the changes discussed in #1565. Context creation is now explicit, and have a parameter to pass in a font atlas created by the user. Read #1565 for details.
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ocornut/imgui#591
No description provided.