AddFontFromMemoryTTF transferring ownership

[petrihakkinen]

It took me a while to figure out why loading a font with AddFontFromMemoryTTF crashed but AddFontFromFileTTF worked ok with the same data. Turns out AddFontFromMemoryTTF transfers the ownership of the data to imgui, which is a little surprising. I suggest that you add a flag transfer_ownership: if the flag is off, AddFontFromMemoryTTF should copy the data.

[ocornut]

I am a little hesitant to add another parameter... thinking about it.
I assume you missed the comments in imgui.h to the right of the declaration? Is there any other locations where adding a comment would have saved you time?

ImFont* AddFontFromMemoryTTF(void* in_ttf_data, unsigned int in_ttf_data_size, float size_pixels, const ImWchar* glyph_ranges = NULL, int font_no = 0); // Pass ownership of 'in_ttf_data' memory, will be deleted after build

I'll change "pass" to "transfer". Evidently you can't really see the comment in github with that width but I'd assume you would see it in a code editor.

[ocornut]

Cosmetic change for now I made the declaration a little shorter to increase odds of reading the comment, reworded comment and made the comment in the implementation the same.

ImFont* AddFontFromMemoryTTF(void* ttf_data, int ttf_size, float size_pixels, const ImWchar* glyph_ranges = NULL, int font_no = 0); // Transfer ownership of 'ttf_data' to ImFontAtlas, will be deleted after Build()

If there was an extra flag already do you think you would have seen it ? That flag would be the last parameter and therefore just next to the comment.
Transferring ownership allows to avoid an extra alloc/copy but I realize it may be awkward if you have high-level constructs on your side that manage the ownership already.

[petrihakkinen]

Well, I don't read comments unless something goes terribly wrong :) I think extra parameter would make it more noticeable, but it's not a big deal if you'd rather not add it.

[ocornut]

Can't blame you for not reading the comments, at least they are here when you come and look again.

ImGui only needs the data until Build is called, generally via one of the GetTexData**() function is called. So it could also hold on the pointer without deleting it without making a copy of the data. Which is a third possibility. Will probably add a flag.

[ocornut]

The AA branch now support this with the new ImFontConfig* settings.
All the AddFontxxx() functions now have a ImFontConfig* pointer. If you pass one you can alter settings.

Alter with:

ImFontConfig font_cfg;
font_cfg.FontDataOwnedByAtlas = false;
io.AddFontFromMemoryTTF(data, data_size, 20.0f, &font_cfg);

Alternatively you can call the lower-level AddFont() function as well. AddFontFromMemoryTTF() doesn't do much anymore.

[ocornut]

This is now merged in master along with the entire new AA-branch. Note that it is a rather big update and you'll need to update your render function to use indexed vertex rendering.

[floooh]

I was also just stumbling over this. I'm doing an AddFontFromMemoryTTF() from static data, embedded as C array, and was a bit surprised that imgui tried to free the data on shutdown. I'll use the way described above to pass a ImFontConfig struct :)

[ElectroidDes]

Same thing - Imgi crashes.
I don't understand why Imgi is trying to free the font data if it is still in use.