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

Using multiple merged fonts -> crash on ClearFonts() ? #544

Closed
MINIONBOTS opened this issue Mar 3, 2016 · 10 comments
Closed

Using multiple merged fonts -> crash on ClearFonts() ? #544

MINIONBOTS opened this issue Mar 3, 2016 · 10 comments

Comments

@MINIONBOTS
Copy link

Hey,
I am not sure if this is just me using the DefaultFont wrong or if this is a bug. I have 4 Language files, 1 which is the default Latin one, being loaded from memory, and 3 other ones which are loaded from a seperate file. Since these 4 languages need the default Latin one as well, I created 4 languages which each are a merged Latin + other language - Font.
It all works fine, I have 4 separate languages to choose from and I can switch without problems in between.
Now the crash occurs in the Dtor which calls ClearFonts(), where it iterates through the allocated Fonts and due to the Default Latin language being the same memory area, it obviously rashes on the 2nd iteration.

My probably wrong way to get 4 merged language files (?)

ImFontConfig config;
strcpy(config.Name, "English");
config.MergeMode = true;
io.Fonts->AddFontFromFileTTF(spath + "\\LangPackRJ.ttf").c_str()), 16.0f, &config, io.Fonts->GetGlyphRangesCyrillic());


// 2nd Font
ImFontConfig config2;
config2.OversampleH = 1;
config2.OversampleV = 1;
config2.PixelSnapH = true;
config2.MergeMode = false;
strcpy(config2.Name, "<default>");
io.Fonts->AddFontDefault(&config2);
strcpy(config2.Name, "Chinese");
config2.MergeMode = true;
io.Fonts->AddFontFromFileTTF(spath + "\\LangPackC.ttf").c_str()), 14.0f,&config2,io.Fonts->GetGlyphRangesChinese());
...

If that is the wrong way to do what I want, any advice how to load multiple languages which each have the default one merged into them ?

@MINIONBOTS
Copy link
Author

Well, I merged all into one big texture now, that circumvents my problem.

@ocornut
Copy link
Owner

ocornut commented Mar 4, 2016

I would like to look at this. It isn't clear what you were doing, could you elaborate? Were you creating multiple ImFontAtlas?

@ocornut ocornut reopened this Mar 4, 2016
@MINIONBOTS
Copy link
Author

I did not change any imgui code in terms of ImFontAtlas, so no, I did not create multiple instances of it.
In the IMGUI Demo window, under Window Options, there is the "Fonts" group, where all loaded fonts are shown. My goal was to have 4 separate fonts here to select/switch in-between. Since the default fallback language should be English (and the additional language ttf files I loaded did not contain any Latin/English characters), I created 4 "merged" languages (English + one language file). This somehow worked and I had 4 languages in the Demo Window.
The code to load 4 merged languages looked like this:
...
ImFontConfig config;
strcpy(config.Name, "English");
config.MergeMode = true;
io.Fonts->AddFontFromFileTTF(spath + "\LangPackRJ.ttf").c_str()), 16.0f, &config, io.Fonts->GetGlyphRangesCyrillic());

// 2nd Font
ImFontConfig config2;
config2.OversampleH = 1;
config2.OversampleV = 1;
config2.PixelSnapH = true;
config2.MergeMode = false;
strcpy(config2.Name, "");
io.Fonts->AddFontDefault(&config2);
strcpy(config2.Name, "Chinese");
config2.MergeMode = true;
io.Fonts->AddFontFromFileTTF(spath + "\LangPackC.ttf").c_str()), 14.0f,&config2,io.Fonts->GetGlyphRangesChinese());

// 3rd Font
ImFontConfig config3;
config3.OversampleH = 1;
config3.OversampleV = 1;
config3.PixelSnapH = true;
config3.MergeMode = false;
strcpy(config3.Name, "");
io.Fonts->AddFontDefault(&config3);
strcpy(config3.Name, "Korean");
config3.MergeMode = true;
io.Fonts->AddFontFromFileTTF(spath + "\LangPackK.ttf").c_str()), 14.0f,&config3,io.Fonts->GetGlyphRangesKorean());

// 4th Font
.. same as above
...

I do have to say that it was not really obvious how to get where I wanted, so I may have done it the wrong way. But this way, the Fonts[] would hold the pointer to the default English language multiple times which then crashed on ClearFonts() ... but I am not 100% sure why this happened....

@ocornut
Copy link
Owner

ocornut commented Mar 4, 2016

Calling AddFontDefault() multiple times shouldn't be a problem.
I have copied your code (only replaced the TTF filenames) and couldn't get a crash out of it.

@ocornut
Copy link
Owner

ocornut commented Mar 4, 2016

By the way what you were doing sounds right to me (it is just unfortunately a little tedious to fill all those fields).

So it isn't clear what you have changed that have fixed the problem for you:
When you say:

Well, I merged all into one big texture now, that circumvents my problem.

ImFontAtlas is always one big texture, that's the entire point of the atlas. So if you hadn't created different font atlas before then that statement doesn't appear to make sense.

There's no such thing as the Fonts[] would hold the pointer to the default English language multiple times which then crashed on ClearFonts(). Each of those copy of the default font info held within the ImFont should be unique.

It would be good to have more information about your program state/from your debugger at the time of calling ClearFonts().

@MINIONBOTS
Copy link
Author

Ohh, maybe this was the problem: Since ImGui is using a vector for the Fonts and the "switch language" was always switching the entries in that vector, I never knew which entry belonged to which language. So I added actually 5 languages:
0 = default
1 = default again
2 = japanese
3 = korean
4 = chinese

this way I could switch the language by just moving the one I wanted to show into place 0.
...
ImFontConfig config;
strcpy(config.Name, "English");
config.MergeMode = true;
io.Fonts->AddFontFromFileTTF(spath + "\LangPackRJ.ttf").c_str()), 16.0f, &config, io.Fonts->GetGlyphRangesCyrillic());

// 1st Font
ImFontConfig config2;
config2.OversampleH = 1;
config2.OversampleV = 1;
config2.PixelSnapH = true;
config2.MergeMode = false;
strcpy(config2.Name, "");
io.Fonts->AddFontDefault(&config2);
strcpy(config2.Name, "Chinese");
config2.MergeMode = true;
io.Fonts->AddFontFromFileTTF(spath + "\LangPackRJ.ttf").c_str()), 16.0f, &config2, 14.0f,&config2,io.Fonts->GetGlyphRangesCyrillic());

// 2nd Font...

...

Could it be because of me using the SAME AddFontFromFileTTF file twice ?

@MINIONBOTS
Copy link
Author

I had trouble with a huge texture font before on smaller GPUs (Onboard stuff), that's why I came up with that idea to split them into 4 textures. But it seems you changed the default max texture size in one of the recent commits and now it seems to work fine with one big texture :) , that is what I meant with "I solved it by merging all into one texture"

@ocornut
Copy link
Owner

ocornut commented Mar 4, 2016

this way I could switch the language by just moving the one I wanted to show into place 0.

What do you mean by "moving the one I wanted to show into place 0" ? If you copy around the pointers in that array ImGui will have duplicate pointers and of course it'll crash when attempting to free the structure.

You probably don't want to mess up with that array. You can change your font by using ImGui::PushFont(my_font); Just grab the pointer return from AddFont() functions and use that instead of messing up with indices.

that's why I came up with that idea to split them into 4 textures.
that is what I meant with "I solved it by merging all into one texture"

But none of your code is splitting anything into 4 textures. You are creating 4 fonts but it's still and always 1 texture. You cannot create multiple textures unless you create multiple ImFontAtlas which you haven't done.

( Ideally we'd need to work on a dynamic atlas to solve this issues with Chinese language, but allocating multiple days of work on ImGui is difficult. In the meanwhile compute your ranges based on your data or reduce oversampling )

@MINIONBOTS
Copy link
Author

Yes, that copying around was indeed the problem... -.-... I'm sorry for wasting your time...

ocornut added a commit that referenced this issue Mar 4, 2016
@ocornut
Copy link
Owner

ocornut commented Mar 4, 2016

No worry! At least I wanted to make sure there's wasn't a crashing bug in the library. I suppose when everything is public it is easy to make that sort of mistake. Thanks for helping to clarify! I have added a comment which might help other people.

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

No branches or pull requests

2 participants