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

crash while building a font atlas from merged fonts #8081

Closed
db48x opened this issue Oct 21, 2024 · 5 comments
Closed

crash while building a font atlas from merged fonts #8081

db48x opened this issue Oct 21, 2024 · 5 comments

Comments

@db48x
Copy link

db48x commented Oct 21, 2024

Version/Branch of Dear ImGui:

Version 1.86, Branch: master

Back-ends:

SDL

Compiler, OS:

Linux + clang 18.1.8

Full config/build information:

Dear ImGui 1.89.6 (18960)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201703
define: IMGUI_DISABLE_OBSOLETE_KEYIO
define: __linux__
define: __GNUC__=4
define: __clang_version__=18.1.8 (Fedora 18.1.8-1.fc40)
--------------------------------
io.BackendPlatformName: imgui_impl_sdl2
io.BackendRendererName: imgui_impl_sdlrenderer2
io.ConfigFlags: 0x00000003
 NavEnableKeyboard
 NavEnableGamepad
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 2 fonts, Flags: 0x00000000, TexSize: 512,256
io.DisplaySize: 1920.00,1080.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

For some reason, ImGui is crashing while building the font atlas. I build a font range based on the user’s locale setting, then I loop through a list of fonts (configurable by the user). For the first font in the list I set MergeMode to false. For every other font in the list I set MergeMode to true so that these fonts will be used as a fallback in case a glyph is not found in a font earlier in the list. I do this first for a variable‐width font, then again for a monospaced font.

The crash happens when the locale is set to en and the font list contains a Japanese font. (Other combinations crash as well; the key seems to be that the Japanese font has no glyphs needed by the chosen locale). Notably it does not crash if the locale is set to ja, or if the Japanese font is removed from the font list.

Here is what the font atlas looks like when the locale is ja and the Japanese font is included in the list:
Screenshot from 2024-10-20 12-20-28

You can see that the atlas is still a fairly reasonable size and everything is working fine.

Here’s what the stack looks like:

    0x000000000543db61 in ImFont::AddGlyph (this=0x76de130, cfg=0x76c1e58, codepoint=32, x0=0, y0=0, x1=0, y1=0, u0=0.998046875, v0=0.2265625, u1=0.998046875, v1=0.2265625,
        advance_x=3) at src/third-party/imgui/imgui_draw.cpp:3318
    3318       float pad = ContainerAtlas->TexGlyphPadding + 0.99f;
    (gdb) p ContainerAtlas
    $1 = (ImFontAtlas *) 0x0
    (gdb) bt
    #0  0x000000000543db61 in ImFont::AddGlyph
        (this=0x76de130, cfg=0x76c1e58, codepoint=32, x0=0, y0=0, x1=0, y1=0, u0=0.998046875, v0=0.2265625, u1=0.998046875, v1=0.2265625, advance_x=3)
        at src/third-party/imgui/imgui_draw.cpp:3318
    #1  0x000000000544586f in ImFontAtlasBuildWithFreeTypeEx (ft_library=0x6e14c80, atlas=0x768e160, extra_flags=0) at src/third-party/imgui/imgui_freetype.cpp:668
    #2  0x0000000005446c21 in ImFontAtlasBuildWithFreeType (atlas=0x768e160) at src/third-party/imgui/imgui_freetype.cpp:761
    #3  0x000000000543abb2 in ImFontAtlas::Build (this=0x768e160) at src/third-party/imgui/imgui_draw.cpp:2280
    #4  0x0000000003127512 in cataimgui::client::load_fonts
        (this=0x7688570, gui_font=std::unique_ptr<Font> = {...}, mono_font=std::unique_ptr<Font> = {...}, windowsPalette=..., gui_typefaces=std::vector of length 4, capacity 4 = {...}, mono_typefaces=std::vector of length 2, capacity 2 = {...}) at src/cata_imgui.cpp:447
    #5  0x00000000051d329d in catacurses::init_interface () at src/sdltiles.cpp:3742
    #6  0x00000000042633a4 in main (argc=2, argv=0x7fffffffdff8) at src/main.cpp:780

And here’s what’s in my load_fonts function:

bool first = true;
for( auto &face : io_typefaces ) {
    if( file_exist( face ) ) {
        ImFontConfig config = ImFontConfig();
        config.MergeMode = !first;
        if( face.find( "Terminus.ttf" ) != std::string::npos ||
            face.find( "unifont.ttf" ) != std::string::npos ) {
            config.FontBuilderFlags = ImGuiFreeTypeBuilderFlags_ForceAutoHint;
        }
        io.Fonts->AddFontFromFileTTF( face.c_str(), fontheight, &config, ranges );
        if( first ) {
            first = false;
        }
    }
}

For each language I have pulled a set of characters from the CLDR database

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

First, define these functions:

static void AddGlyphRangesFromCLDRForJA( ImFontGlyphRangesBuilder *b )
{
    b->AddChar( 0x3005 );
    b->AddChar( 0x3041 );
    b->AddChar( 0x3042 );
    b->AddChar( 0x3043 );
    b->AddChar( 0x3044 );
    b->AddChar( 0x3045 );
    b->AddChar( 0x3046 );
    b->AddChar( 0x3047 );
    b->AddChar( 0x3048 );
    b->AddChar( 0x3049 );
    b->AddChar( 0x304a );
    b->AddChar( 0x304b );
    b->AddChar( 0x304c );
    b->AddChar( 0x304d );
    b->AddChar( 0x304e );
    b->AddChar( 0x304f );
    // greatly truncated…
}

static void AddGlyphRangesFromCLDR( ImFontGlyphRangesBuilder *b, const std::string &lang )
{
    // NOLINTBEGIN(bugprone-branch-clone)
    if( lang == "ja" ) {
        AddGlyphRangesFromCLDRForJA( b );
    }
    // bunch of other locales elided…
    // NOLINTEND(bugprone-branch-clone)
}

static void AddGlyphRangesMisc( ImFontGlyphRangesBuilder *b )
{
    // NOLINTNEXTLINE(modernize-avoid-c-arrays)
    static ImWchar superscripts[] = { 0x00B9, 0x00B9, 0x00B2, 0x00B3, 0x2070, 0x208E, 0x0000 };
    b->AddRanges( &superscripts[0] );
    static ImWchar arrows[] = { 0x2190, 0x2199, 0x21D0, 0x21D9, 0x21E6, 0x21E9, 0x21F3, 0x21F3, 0x2B00, 0x2B0D, 0x2B95, 0x2B95, 0x0000 };
    b->AddRanges( &arrows[0] );
}

static void load_font(ImGuiIO &io, const std::vector<std::string> &typefaces, const ImWchar *ranges)
{
    bool first = true;
    for (auto &face : typefaces) {
        ImFontConfig config = ImFontConfig();
        config.MergeMode = !first;
#ifdef IMGUI_ENABLE_FREETYPE
        if (face.find( "Terminus.ttf" ) != std::string::npos ||
            face.find( "unifont.ttf" ) != std::string::npos) {
            config.FontBuilderFlags = ImGuiFreeTypeBuilderFlags_ForceAutoHint;
        }
#endif
        io.Fonts->AddFontFromFileTTF(face.c_str(), 16, &config, ranges);
        if (first) {
            first = false;
        }
    }
}

static void check_font( const ImFont *font )
{
    if( !font || !font->IsLoaded() ) {
        // we can’t use debugmsg or cata_fatal because they trigger a new ImGui frame
        // NOLINTNEXTLINE(cert-err33-c)
        fprintf( stderr,
                 "Failed to create font atlas!  Make sure that your chosen "
                 "font exists, can be read, and has glyphs for your chosen "
                 "language.\n" );
        // NOLINTNEXTLINE(cata-assert)
        std::abort();
    }
}

void load_fonts(std::vector<std::string> typefaces, std::string lang) {
    auto io = ImGui::GetIO();
    ImFontGlyphRangesBuilder b = {};
    b.AddRanges(io.Fonts->GetGlyphRangesDefault());
    AddGlyphRangesFromCLDR(&b, lang);
    AddGlyphRangesMisc(&b);
    ImVector<ImWchar> ranges;
    b.BuildRanges(&ranges);

    load_font(io, typefaces, ranges.begin());
    io.Fonts->Build();
    check_font(io.Fonts->Fonts[0]);
    ImGui::SetCurrentFont(ImGui::GetDefaultFont());
}

Check the args:

    if (argc != 2 || *argv[1] == '\0') {
        fprintf(stderr, "specify a language code on the command line (“en” or “ja” for this crash testcase)");
        exit(1);
    }

Then, call them from main just after the renderer is initialized:

    static std::vector<std::string> typefaces = { "/usr/share/fonts/google-droid-sans-fonts/DroidSansJapanese.ttf",
                                                  "../../misc/fonts/Roboto-Medium.ttf",
                                                  "../../misc/fonts/ProggyClean.ttf" };
    load_fonts(typefaces, std::string(argv[1]));

./example_sdl_sdlrenderer "en" crashes, ./example_sdl_sdlrenderer "ja" does not.

@db48x
Copy link
Author

db48x commented Oct 21, 2024

I updated the example code to make it easier to run.

Also, I attempted to fix the crash. The result is a small patch:

diff --git a/imgui_draw.cpp b/imgui_draw.cpp
index bf1da15b..56f64072 100644
--- a/imgui_draw.cpp
+++ b/imgui_draw.cpp
@@ -2607,13 +2607,13 @@ void ImFontAtlasBuildSetupFont(ImFontAtlas* atlas, ImFont* font, ImFontConfig* f
     if (!font_config->MergeMode)
     {
         font->ClearOutputData();
-        font->FontSize = font_config->SizePixels;
         font->ConfigData = font_config;
         font->ConfigDataCount = 0;
-        font->ContainerAtlas = atlas;
-        font->Ascent = ascent;
-        font->Descent = descent;
     }
+    font->ContainerAtlas = atlas;
+    font->FontSize = font_config->SizePixels;
+    font->Ascent = ascent;
+    font->Descent = descent;
     font->ConfigDataCount++;
 }

This avoids the crash, but when in the English locale it loses the names of the constituent fonts. I haven't dug deeper into how that information is tracked.

@ocornut
Copy link
Owner

ocornut commented Nov 7, 2024

Hmmm,
I think the reason might be this check in imgui_freetype.cpp :

    // 8. Copy rasterized font characters back into the main texture
    // 9. Setup ImFont and glyphs for runtime
    bool tex_use_colors = false;
    for (int src_i = 0; src_i < src_tmp_array.Size; src_i++)
    {
        ImFontBuildSrcDataFT& src_tmp = src_tmp_array[src_i];
        if (src_tmp.GlyphsCount == 0)
            continue;

Can you try to move this block

        if (src_tmp.GlyphsCount == 0)
            continue;

AFTER the ImFontAtlasBuildSetupFont() call ?

@ocornut ocornut added the bug label Nov 7, 2024
ocornut added a commit that referenced this issue Nov 7, 2024
…onts and the first font in a merged set has no loaded glyph. (#8081)
@ocornut
Copy link
Owner

ocornut commented Nov 7, 2024

I went ahead and pushed this 3b68392 because it seems like the only possible cause for this.
Your confirmation would be good.

@ocornut ocornut closed this as completed Nov 7, 2024
@db48x
Copy link
Author

db48x commented Nov 11, 2024

My first attempt to confirm the fix was a failure; it crashed in both cases. I will try again.

@db48x
Copy link
Author

db48x commented Nov 11, 2024

It is fixed! Thank you!

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