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

Dynamic Font Atlas and how to fix glitch? [solved] #4723

Closed
mandaxyzw opened this issue Nov 15, 2021 · 6 comments
Closed

Dynamic Font Atlas and how to fix glitch? [solved] #4723

mandaxyzw opened this issue Nov 15, 2021 · 6 comments

Comments

@mandaxyzw
Copy link

mandaxyzw commented Nov 15, 2021

Version/Branch of Dear ImGui:
Version: v1.84
Branch:

Back-end/Renderer/Compiler/OS
Back-ends: imgui_impl_sdl.cpp + imgui_impl_vulkan.cpp
Compiler: Visual Studio 2015
Operating System: Windows 10

Introduction of the glitch:

  • a. When I change groups[0].resolutionScale which is for ImGui Windows Text, then there's A LOT of glitch on ImGui Windows Text.
  • b. When I change groups[1].resolutionScale which is for the default font of my engine's gui, then there's A LITTLE chance that a glitch happens on ImGui Windows Text.
  • c. Conclusion: I believe that the Font Atlas Texture UVs of ImGui Windows Text changes very frequently for (a.), and the Font Atlas Texture UVs changes with smaller chance for (b.), check How to update only all UV(s) of texts in all drawlist(s) after fonts change? #4637 to get the idea that a Texture UV is always Square, (x, y) between ([0,1], [0,1])
    imgui_glitch_2groups

Edit: How the animation looks like after I solved the problem?

  • How I solved it? Always update Font Atlas with its Texture UVs before starting the Dear ImGui frame. But that logic idea never gets into my head if I don't analyze or if I don't work hard.
  • Go to the very bottom of the thread for more details.
    imgui_glitch_2groups_fixed
    But it's a luck that I found the problem, it's about philosophy because if you work hard then you will get reward with lucks like magic, and I believe in that.

My Issue/Question:
My engine has separated FontAtlas Texture and ImGui windows have separated FontAtlas Texture which is explained in the C++ code at bottom (paragraphs 2. and 3.). fontGroups[0] is for ImGui windows text, and when I change the font size of fontGroups[1] which is for my engine gui then there is a risk that the ImGui windows texts show glitch just like in the .jpg image below and in the animations. The reason I use many fontGroups is there can be many groups using the same font with same size, so they are all share the same ImFont's Texture.
The question is: "How to fix that glitch in the animation?".
screenshot
imgui_glitch_atlas imgui_glitch_gui
There's a glitch in the ImGui window texts when resizing fonts.

The reason I can build things so fast is I have high experience in my own sources, but when I don't have experience in other sources then I'm stuck. For example, I'm a beginner in Vulkan which is the hardest backend, and I have no idea how to fix the glitch by analyzing 'imgui_impl_vulkan.cpp', because Vulkan is too difficult, I'm only a beginner on it.

1. How I use the function buildFontAtlasIfNeeded_ImGui in my engine

class AppBase : public AppBase_Internal
{
public:
    AppBase() : AppBase_Internal(this) {}
    virtual void imGuiDebug() {}
    virtual void prepareGui(Window *window) {}
    virtual void destroyGui() {}
    void buildFontAtlasIfNeeded() {
        _engine->updateFontGroupsIfNeeded(); // <-- This is the FontGroups I'm talking about in the screenshot.
    }
    virtual void beforeFrameRender() {}
    int main(const char *title, const ivec2 &windowSize, const char *projectName);
};

class App : public AppBase
{
    ...
    void beforeFrameRender() // <-- This is called every frame
    {
        _engine->getDefaultFontGroup()->setScale(_debug_resolutionScale);
        buildFontAtlasIfNeeded();
    }
}

2. Old code. Upload Fonts code referenced from "examples/example_sdl_vulkan/main.cpp > in function main" in official ImGui v1.84

void my_engine::AppBase_Internal::buildFontAtlasIfNeeded_ImGui(ImGui_ImplVulkanH_Window* wd)
{
    if (_engine->_isFontAtlasBuilt_ImGui) {
        return;
    }
    _engine->_isFontAtlasBuilt_ImGui = true;

    VkResult err;

    // [Originated from "example_sdl_vulkan > main.cpp" in official ImGui v1.84]
    // Load Fonts
    // - If no fonts are loaded, dear imgui will use the default font. You can also load multiple fonts and use ImGui::PushFont()/PopFont() to select them.
    // - AddFontFromFileTTF() will return the ImFont* so you can store it if you need to select the font among multiple.
    // - If the file cannot be loaded, the function will return NULL. Please handle those errors in your application (e.g. use an assertion, or display an error and quit).
    // - The fonts will be rasterized at a given size (w/ oversampling) and stored into a texture when calling ImFontAtlas::Build()/GetTexDataAsXXXX(), which ImGui_ImplXXXX_NewFrame below will call.
    // - Read 'docs/FONTS.md' for more instructions and details.
    // - Remember that in C/C++ if you want to include a backslash \ in a string literal you need to write a double backslash \\ !
    //io.Fonts->AddFontDefault();
    //io.Fonts->AddFontFromFileTTF("../../misc/fonts/Roboto-Medium.ttf", 16.0f);
    //io.Fonts->AddFontFromFileTTF("../../misc/fonts/Cousine-Regular.ttf", 15.0f);
    //io.Fonts->AddFontFromFileTTF("../../misc/fonts/DroidSans.ttf", 16.0f);
    //io.Fonts->AddFontFromFileTTF("../../misc/fonts/ProggyTiny.ttf", 10.0f);
    //ImFont* font = io.Fonts->AddFontFromFileTTF("c:\\Windows\\Fonts\\ArialUni.ttf", 18.0f, NULL, io.Fonts->GetGlyphRangesJapanese());
    //IM_ASSERT(font != NULL);

    // Upload Fonts
    {
        // Use any command queue
        VkCommandPool command_pool = wd->Frames[wd->FrameIndex].CommandPool;
        VkCommandBuffer command_buffer = wd->Frames[wd->FrameIndex].CommandBuffer;

        err = vkResetCommandPool(g_Device, command_pool, 0);
        check_vk_result(err);
        VkCommandBufferBeginInfo begin_info = {};
        begin_info.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
        begin_info.flags |= VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;
        err = vkBeginCommandBuffer(command_buffer, &begin_info);
        check_vk_result(err);

        ImGui_ImplVulkan_CreateFontsTexture(command_buffer);

        VkSubmitInfo end_info = {};
        end_info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
        end_info.commandBufferCount = 1;
        end_info.pCommandBuffers = &command_buffer;
        err = vkEndCommandBuffer(command_buffer);
        check_vk_result(err);
        err = vkQueueSubmit(g_Queue, 1, &end_info, VK_NULL_HANDLE);
        check_vk_result(err);

        err = vkDeviceWaitIdle(g_Device);
        check_vk_result(err);
        ImGui_ImplVulkan_DestroyFontUploadObjects();
    }
}

3. I modified the function ImGui_ImplVulkan_CreateFontsTexture for multiple FontAtlas Texture updates reason

bool ImGui_ImplVulkan_CreateFontsTexture(VkCommandBuffer command_buffer)
{
    ImGuiIO& io = ImGui::GetIO();
    ImGui_ImplVulkan_Data* bd = ImGui_ImplVulkan_GetBackendData();
    ImGui_ImplVulkan_InitInfo* v = &bd->VulkanInitInfo;

    unsigned char* pixels;
    int width, height;
    io.Fonts->GetTexDataAsRGBA32(&pixels, &width, &height);
    size_t upload_size = width * height * 4 * sizeof(char);

    VkResult err;

    //_/--------------------------------------------------\_
    // EDIT: The reason is for multiple updates of FontAtlas Texture by calling this function multiple
    // times and to avoid memory leak.
    if (bd->FontImage) {
        vkDestroyImage(v->Device, bd->FontImage, v->Allocator);
        bd->FontImage = nullptr;
    }
    if (bd->FontMemory) {
        vkFreeMemory(v->Device, bd->FontMemory, v->Allocator);
        bd->FontMemory = nullptr;
    }
    if (bd->FontView) {
        vkDestroyImageView(v->Device, bd->FontView, v->Allocator);
        bd->FontView = nullptr;
    }
    // \--------------------------------------------------/

    // Create the Image:
    { ... }
    // Create the Image View:
    { ... }
    // Update the Descriptor Set:
    { ... }
    // Create the Upload Buffer:
    { ... }
    // Upload to Buffer:
    { ... }
    // Copy to Image:
    { ... }
    // Store our identifier
    io.Fonts->SetTexID((ImTextureID)(intptr_t)bd->FontImage);
    return true;
}

4. Advanced reading just to explain how I update FontAtlas in my engine

void my_engine::Engine::FontGroup::setScale(float scale)
{
    int old_isize = _iSizePixels;
    _iSizePixels = int(scale * _engine->_iDefaultFontSize);
    assert(_iSizePixels >= 1);
    if (old_isize != _iSizePixels) {
        _engine->_areFontGroupsUpdated = false;
    }
}

void my_engine::Engine::updateFontGroupsIfNeeded()
{
    if (_areFontGroupsUpdated) {
        return;
    }
    _areFontGroupsUpdated = true;

    clearFonts();
    for (auto it : _fontGroups) {
        switch (it->getType()) {
        case FontGroupType_Default: it->_font = loadFont("arial", it->_iSizePixels); break;
        case FontGroupType_Script:  it->_font = loadFont("consola", it->_iSizePixels); break;
        default: assert(0);
        }
    }
    buildFontAtlas(); // Not just skipped if there is no change in the FontAtlas because clearFonts forces to rebuild the FontAtlas, but don't worry, the update function is rarely performed.
}
@PathogenDavid
Copy link
Contributor

I didn't have a chance to read through all your code, but at a glance it doesn't look like you're ensuring the GPU is done with the old font texture before you destroy it in ImGui_ImplVulkan_CreateFontsTexture. (An easy way to quickly check would be to remove your code deleting the old font texture and just let it leak.)

If you don't have the Vulkan validation layers enabled you probably should.

@mandaxyzw
Copy link
Author

mandaxyzw commented Nov 15, 2021

Thank you so much, I will think about it and work on it. Maybe, I will come back after several hours because I must think, it makes things perfect. Also, I already enabled the Validation Layers, it's about Vulkan Errors Debugging.

@ocornut
Copy link
Owner

ocornut commented Nov 15, 2021

Note that the change for #3761 (we have transitioned this to a private WIP branch) will make it possible for backends to have multiple in-flight textures, as this it will required to allows full rebuilding or GC-ing of atlas. In 99% cases adding new glyphs should fill empty spaces and not require multiple in-flight textures but mid-frame resizing or full rebuilding or atlas will.

@mandaxyzw
Copy link
Author

mandaxyzw commented Nov 15, 2021

As @PathogenDavid explained, I tried to remove the code deleting by letting my program leak, then the glitch is still there. When I comment the code deleting, then there is so many memory leak.

As @ocornut explained, from that link #3761, I download the latest "dear imgui, v1.86 WIP" and I searched for the term ImGui_ImplVulkan_UpdateFontsTexture in the entire solution and the text was not found. That thread was posted on Jan 26.
image

The problem is still not solved, and I have no idea how to fix it.

I just sleep on bed and an idea comes to my head that it's about multi-threading problem, like another thread erases a part of another thread due to bad synchronization. I'm just telling hint if anyone can help because I'm a very beginner in Vulkan.

@mandaxyzw
Copy link
Author

mandaxyzw commented Nov 16, 2021

Conclusion, the glitch has nothing to do with ImGui_ImplVulkan_UpdateFontsTexture or ImGui_ImplVulkan_CreateFontsTexture because I completely modify the function using my engine font texture's variables and the problem is still not solved. The good thing is the code of my project becomes much simpler, and I like it.

void ImGui_ImplVulkan_UpdateFontsTexture(VkSampler fontSampler, VkImageView fontView)
{
    ImGui_ImplVulkan_Data *bd = ImGui_ImplVulkan_GetBackendData();
    bd->FontSampler = fontSampler;
    bd->FontMemory = nullptr;
    bd->FontImage = nullptr;
    bd->FontView = fontView;
    bd->UploadBufferMemory = nullptr;
    bd->UploadBuffer = nullptr;

    ImGui_ImplVulkan_InitInfo* v = &bd->VulkanInitInfo;

    // Update the Descriptor Set:
    {
        VkDescriptorImageInfo desc_image[1] = {};
        desc_image[0].sampler = bd->FontSampler;
        desc_image[0].imageView = bd->FontView;
        desc_image[0].imageLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
        VkWriteDescriptorSet write_desc[1] = {};
        write_desc[0].sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET;
        write_desc[0].dstSet = bd->DescriptorSet;
        write_desc[0].descriptorCount = 1;
        write_desc[0].descriptorType = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER;
        write_desc[0].pImageInfo = desc_image;
        vkUpdateDescriptorSets(v->Device, 1, write_desc, 0, NULL);
    }

    // Store our identifier
    ImGuiIO& io = ImGui::GetIO();
    io.Fonts->SetTexID((ImTextureID)(intptr_t)bd->FontImage);
}

@mandaxyzw
Copy link
Author

mandaxyzw commented Nov 16, 2021

That bug fix is important, and I hope someone will fix it soon because it's not in my engine, it's inside Dear ImGui, that glitch is very popular, people call it "Draw call explosion for dynamically font change". Another conclusion, the glitch is not in the function FrameRender in examples/example_sdl_vulkan/main.cpp because when I replace ImGui_ImplVulkan_RenderDrawData with my own code and I change the font size dynamically then there's still the exact same glitch.

What am I doing now is to explore the link that administrator gave me: #3471 --> #797 (which is awesome, I don't know how did you do that? I can do that with my own engine without problem, but there's a glitch on ImGui windows when change font size, and it's annoying because it's like ImGui is forever with me as a debugging tool unless I will replace it in the future with my own tools, that's the only solution, so sad).

I downloaded the example and I compiled it and it runs, and I notice that when I zoom the node editor then the font texture doesn't change, just the vertices are scaled up, and I like that idea because it's smooth (only for node editor), as font size is only integer (only for gui).
Blueprints
The good thing is this is free open source, so I can take ideas from it, and thank you.

I keep silence for now because I post too much, I cannot ask question that's impossible for others to answer.

Solution of the main problem

// Update Font Atlas must be before starting Dear ImGui frame
_appBase->beforeFrameRender(); // This must be before updating buffers because it rebuilds the Font Atlas Texture UVs.
_window->updateBuffers(g_Queue); // This requires the updated Font Atlas Texture UVs.

// Start the Dear ImGui frame
ImGui_ImplVulkan_NewFrame();
ImGui_ImplSDL2_NewFrame();
ImGui::NewFrame();

@mandaxyzw mandaxyzw changed the title Build Font Atlas to update its Texture and how to fix glitch? Dynamic Font Atlas and how to fix glitch? [solved] Nov 17, 2021
@ocornut ocornut closed this as completed Nov 17, 2021
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

3 participants