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

Why does ImGui attempt to free memory that I (the user) have created? #6916

Closed
johannesugb opened this issue Oct 11, 2023 · 2 comments
Closed

Comments

@johannesugb
Copy link

Version/Branch of Dear ImGui:

Version: v1.89.9
Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_glfw.cpp + imgui_impl_vulkan.cpp
Compiler: VS version v17.7.4, MSVC version 19.37.32824
Operating System: Windows 10, Windows 11

My Issue/Question:

I am using AddFontFromMemoryTTF to successfully load a TTF font from memory. What I don't understand is that I am not the one responsible for deleting the allocated memory, as it appears. This looks like a violation of responsibility principles.

Here's how I load the TTF font:

ImGuiIO& io = ImGui::GetIO();
io.Fonts->Clear();
auto  font_cfg = ImFontConfig();
ImFormatString(font_cfg.Name, IM_ARRAYSIZE(font_cfg.Name), "%s, %.0fpx", "MyFont.ttf", 13.f);
uint8_t* data= new uint8_t[] { 0x0, 0x1, 0x0, 0x0, 0x0, 0x11, 0x1, 0x0, 0x0, 0x4, ...  };
size_t data_size = 273900;
io.Fonts->AddFontFromMemoryTTF(data, (int)data_size, fontSize, &font_cfg, nullptr);

The crucial part here is the line where I allocate the font data: uint8_t* data= new uint8_t[] { ... };

Now, I would expect that I (the user of ImGui) has to delete data after uploading the fonts (which in my case means after ImGui_ImplVulkan_CreateFontsTexture and ImGui_ImplVulkan_DestroyFontUploadObjects and using a fence for these operations to have completed) and execute a delete [] data; afterwards.

But if I execute that delete [] data, then my compiler points out that that ImGui attempts to free that memory again on its own, namely here:

static void    FreeWrapper(void* ptr, void* user_data)        { IM_UNUSED(user_data); free(ptr); }

which is in imgui.cpp, line 1146.

The comments indicate that I could "SetAllocatorFunctions() to change them"... but why should I actually? I only want to free that data on my own and not have ImGui free my data.

What are your thoughts on this?
What is the intended way of handling this?

@ocornut
Copy link
Owner

ocornut commented Oct 11, 2023

See docs: Loading Font Data from Memory

ImFont* font = io.Fonts->AddFontFromMemoryTTF(data, data_size, size_pixels, ...);

IMPORTANT: AddFontFromMemoryTTF() by default transfer ownership of the data buffer to the font atlas, which will attempt to free it on destruction. This was to avoid an unnecessary copy, and is perhaps not a good API (a future version will redesign it). If you want to keep ownership of the data and free it yourself, you need to clear the FontDataOwnedByAtlas field:

ImFontConfig font_cfg;
font_cfg.FontDataOwnedByAtlas = false;
ImFont* font = io.Fonts->AddFontFromMemoryTTF(data, data_size, size_pixels, &font_cfg);

johannesugb added a commit to cg-tuwien/Auto-Vk-Toolkit that referenced this issue Oct 13, 2023
@johannesugb
Copy link
Author

Sorry for not reading the docs better & thank you. 🙏

johannesugb added a commit to cg-tuwien/Auto-Vk-Toolkit that referenced this issue Oct 15, 2023
johannesugb added a commit to cg-tuwien/Auto-Vk-Toolkit that referenced this issue Dec 6, 2023
… | also several bugfixes

* Fixed ImGui modifier keys (again)

ImGui mod keys (like Ctrl-clicking on a slider to enter a value manually) did not work anymore.
This presumably happened during the big ImGui update in commit b15af18.

* Tabs instead of spaces for consistency

* messing around with imgui

* Using JetBrainsMono-Regular.ttf as UI font and also accounting for display's content scale through GLFW

* Cleaning up font data only once (as instructed by Omar: ocornut/imgui#6916)

* Fixed ImGui modifier keys (again)

ImGui mod keys (like Ctrl-clicking on a slider to enter a value manually) did not work anymore.
This presumably happened during the big ImGui update in commit b15af18.

* Tabs instead of spaces for consistency

* messing around with imgui

* Using JetBrainsMono-Regular.ttf as UI font and also accounting for display's content scale through GLFW

* Cleaning up font data only once (as instructed by Omar: ocornut/imgui#6916)

* Added JetBrainsMono-Regular.cpp to CMakeLists.txt

* GCC needs more hand-holding.

* Using classic style again, using TTF font only if content scale != 1.0f (using ImGui's default font otherwise)

* Allow to change ImGui style and font settings

Added `aAlterSettingsBeforeCreation` to the imgui_manager constructor
for changing to an arbitrary style.
Also added `set_custom_font_mode` to influence whether the new font is
used.

* Font and style config refactoring

* Whoops

* Update imgui_manager.cpp

Whoops^2

* Update cross-platform-check.yml

* JetBrainsMono is now includes as files under assets/3rd_party/fonts/

* Custom fonts must be loaded explicitly

* fmt::format -> std::format, welcome to the future

fmt::format caused all kinds of deprecation warnings/errors with the latest MSVC.

# Conflicts:
#	auto_vk_toolkit/src/imgui_manager.cpp

* Renamed set_font_mode -> set_custom_font

* Update cross-platform-check.yml

* Using GCC-13 for Linux

* Using std::format also in examples, fmt::format no longer

* Using TTF in orca_loader example

---------

Co-authored-by: Johannes Unterguggenberger <johannes.unterguggenberger@gmail.com>
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