From 19a1f2a5d2cb1366d7c702e905e6aa5ed4b1ab12 Mon Sep 17 00:00:00 2001 From: ocornut <omarcornut@gmail.com> Date: Fri, 29 Nov 2024 18:46:17 +0100 Subject: [PATCH] Fonts: fixed AddCustomRect() not being packed with TexGlyphPadding + not accounted in surface area. (#8107) --- docs/CHANGELOG.txt | 2 ++ imgui.h | 6 +++--- imgui_draw.cpp | 18 +++++++++++------- misc/freetype/imgui_freetype.cpp | 4 +++- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index b93888c85..6650debf0 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -62,6 +62,8 @@ Other changes: - Tools: binary_to_compressed_c: added -u8/-u32/-base85 export options. - Demo: example tree used by Property Editor & Selection demos properly freed on application closure. (#8158) [@Legulysse] +- Fonts: fixed AddCustomRect() not being packed with TexGlyphPadding + not accounted + for surface area used to determine best-guess texture size. (#8107) [@YarikTH, @ocornut] - Misc: use SSE 4.2 crc32 instructions when available. (#8169, #4933) [@Teselka] - Backends: Vulkan: Make user-provided descriptor pool optional. As a convenience, when setting init_info->DescriptorPoolSize then the backend will create and manage diff --git a/imgui.h b/imgui.h index 30115e810..423e2a8ba 100644 --- a/imgui.h +++ b/imgui.h @@ -3230,8 +3230,8 @@ struct ImFontConfig float SizePixels; // // Size in pixels for rasterizer (more or less maps to the resulting font height). int OversampleH; // 2 // Rasterize at higher quality for sub-pixel positioning. Note the difference between 2 and 3 is minimal. You can reduce this to 1 for large glyphs save memory. Read https://github.com/nothings/stb/blob/master/tests/oversample/README.md for details. int OversampleV; // 1 // Rasterize at higher quality for sub-pixel positioning. This is not really useful as we don't use sub-pixel positions on the Y axis. - bool PixelSnapH; // false // Align every glyph to pixel boundary. Useful e.g. if you are merging a non-pixel aligned font with the default font. If enabled, you can set OversampleH/V to 1. - ImVec2 GlyphExtraSpacing; // 0, 0 // Extra spacing (in pixels) between glyphs. Only X axis is supported for now. + bool PixelSnapH; // false // Align every glyph AdvanceX to pixel boundaries. Useful e.g. if you are merging a non-pixel aligned font with the default font. If enabled, you can set OversampleH/V to 1. + ImVec2 GlyphExtraSpacing; // 0, 0 // Extra spacing (in pixels) between glyphs when rendered: essentially add to glyph->AdvanceX. Only X axis is supported for now. ImVec2 GlyphOffset; // 0, 0 // Offset all glyphs from this font input. const ImWchar* GlyphRanges; // NULL // THE ARRAY DATA NEEDS TO PERSIST AS LONG AS THE FONT IS ALIVE. Pointer to a user-provided list of Unicode range (2 value per range, values are inclusive, zero-terminated list). float GlyphMinAdvanceX; // 0 // Minimum AdvanceX for glyphs, set Min to align font icons, set both Min/Max to enforce mono-space font @@ -3389,7 +3389,7 @@ struct ImFontAtlas ImFontAtlasFlags Flags; // Build flags (see ImFontAtlasFlags_) ImTextureID TexID; // User data to refer to the texture once it has been uploaded to user's graphic systems. It is passed back to you during rendering via the ImDrawCmd structure. int TexDesiredWidth; // Texture width desired by user before Build(). Must be a power-of-two. If have many glyphs your graphics API have texture size restrictions you may want to increase texture width to decrease height. - int TexGlyphPadding; // Padding between glyphs within texture in pixels. Defaults to 1. If your rendering method doesn't rely on bilinear filtering you may set this to 0 (will also need to set AntiAliasedLinesUseTex = false). + int TexGlyphPadding; // FIXME: Should be called "TexPackPadding". Padding between glyphs within texture in pixels. Defaults to 1. If your rendering method doesn't rely on bilinear filtering you may set this to 0 (will also need to set AntiAliasedLinesUseTex = false). bool Locked; // Marked as Locked by ImGui::NewFrame() so attempt to modify the atlas will assert. void* UserData; // Store your own atlas related user-data (if e.g. you have multiple font atlas). diff --git a/imgui_draw.cpp b/imgui_draw.cpp index fd1feb0a8..631472a71 100644 --- a/imgui_draw.cpp +++ b/imgui_draw.cpp @@ -2941,6 +2941,7 @@ static bool ImFontAtlasBuildWithStbTruetype(ImFontAtlas* atlas) int total_surface = 0; int buf_rects_out_n = 0; int buf_packedchars_out_n = 0; + const int pack_padding = atlas->TexGlyphPadding; for (int src_i = 0; src_i < src_tmp_array.Size; src_i++) { ImFontBuildSrcData& src_tmp = src_tmp_array[src_i]; @@ -2964,18 +2965,19 @@ static bool ImFontAtlasBuildWithStbTruetype(ImFontAtlas* atlas) // Gather the sizes of all rectangles we will need to pack (this loop is based on stbtt_PackFontRangesGatherRects) const float scale = (cfg.SizePixels > 0.0f) ? stbtt_ScaleForPixelHeight(&src_tmp.FontInfo, cfg.SizePixels * cfg.RasterizerDensity) : stbtt_ScaleForMappingEmToPixels(&src_tmp.FontInfo, -cfg.SizePixels * cfg.RasterizerDensity); - const int padding = atlas->TexGlyphPadding; for (int glyph_i = 0; glyph_i < src_tmp.GlyphsList.Size; glyph_i++) { int x0, y0, x1, y1; const int glyph_index_in_font = stbtt_FindGlyphIndex(&src_tmp.FontInfo, src_tmp.GlyphsList[glyph_i]); IM_ASSERT(glyph_index_in_font != 0); stbtt_GetGlyphBitmapBoxSubpixel(&src_tmp.FontInfo, glyph_index_in_font, scale * cfg.OversampleH, scale * cfg.OversampleV, 0, 0, &x0, &y0, &x1, &y1); - src_tmp.Rects[glyph_i].w = (stbrp_coord)(x1 - x0 + padding + cfg.OversampleH - 1); - src_tmp.Rects[glyph_i].h = (stbrp_coord)(y1 - y0 + padding + cfg.OversampleV - 1); + src_tmp.Rects[glyph_i].w = (stbrp_coord)(x1 - x0 + pack_padding + cfg.OversampleH - 1); + src_tmp.Rects[glyph_i].h = (stbrp_coord)(y1 - y0 + pack_padding + cfg.OversampleV - 1); total_surface += src_tmp.Rects[glyph_i].w * src_tmp.Rects[glyph_i].h; } } + for (int i = 0; i < atlas->CustomRects.Size; i++) + total_surface += (atlas->CustomRects[i].Width + pack_padding) * (atlas->CustomRects[i].Height + pack_padding); // We need a width for the skyline algorithm, any width! // The exact width doesn't really matter much, but some API/GPU have texture size limitations and increasing width can decrease height. @@ -2991,7 +2993,8 @@ static bool ImFontAtlasBuildWithStbTruetype(ImFontAtlas* atlas) // Pack our extra data rectangles first, so it will be on the upper-left corner of our texture (UV will have small values). const int TEX_HEIGHT_MAX = 1024 * 32; stbtt_pack_context spc = {}; - stbtt_PackBegin(&spc, NULL, atlas->TexWidth, TEX_HEIGHT_MAX, 0, atlas->TexGlyphPadding, NULL); + stbtt_PackBegin(&spc, NULL, atlas->TexWidth, TEX_HEIGHT_MAX, 0, 0, NULL); + spc.padding = atlas->TexGlyphPadding; // Because we mixup stbtt_PackXXX and stbrp_PackXXX there's a bit of a hack here, not passing the value to stbtt_PackBegin() allows us to still pack a TexWidth-1 wide item. (#8107) ImFontAtlasBuildPackCustomRects(atlas, spc.pack_info); // 6. Pack each source font. No rendering yet, we are working with rectangles in an infinitely tall texture at this point. @@ -3137,13 +3140,14 @@ void ImFontAtlasBuildPackCustomRects(ImFontAtlas* atlas, void* stbrp_context_opa if (user_rects.Size < 1) { __builtin_unreachable(); } // Workaround for GCC bug if IM_ASSERT() is defined to conditionally throw (see #5343) #endif + const int pack_padding = atlas->TexGlyphPadding; ImVector<stbrp_rect> pack_rects; pack_rects.resize(user_rects.Size); memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes()); for (int i = 0; i < user_rects.Size; i++) { - pack_rects[i].w = user_rects[i].Width; - pack_rects[i].h = user_rects[i].Height; + pack_rects[i].w = user_rects[i].Width + pack_padding; + pack_rects[i].h = user_rects[i].Height + pack_padding; } stbrp_pack_rects(pack_context, &pack_rects[0], pack_rects.Size); for (int i = 0; i < pack_rects.Size; i++) @@ -3151,7 +3155,7 @@ void ImFontAtlasBuildPackCustomRects(ImFontAtlas* atlas, void* stbrp_context_opa { user_rects[i].X = (unsigned short)pack_rects[i].x; user_rects[i].Y = (unsigned short)pack_rects[i].y; - IM_ASSERT(pack_rects[i].w == user_rects[i].Width && pack_rects[i].h == user_rects[i].Height); + IM_ASSERT(pack_rects[i].w == user_rects[i].Width + pack_padding && pack_rects[i].h == user_rects[i].Height + pack_padding); atlas->TexHeight = ImMax(atlas->TexHeight, pack_rects[i].y + pack_rects[i].h); } } diff --git a/misc/freetype/imgui_freetype.cpp b/misc/freetype/imgui_freetype.cpp index e68a2af40..bee15ba08 100644 --- a/misc/freetype/imgui_freetype.cpp +++ b/misc/freetype/imgui_freetype.cpp @@ -573,6 +573,7 @@ bool ImFontAtlasBuildWithFreeTypeEx(FT_Library ft_library, ImFontAtlas* atlas, u // 8. Render/rasterize font characters into the texture int total_surface = 0; int buf_rects_out_n = 0; + const int pack_padding = atlas->TexGlyphPadding; for (int src_i = 0; src_i < src_tmp_array.Size; src_i++) { ImFontBuildSrcDataFT& src_tmp = src_tmp_array[src_i]; @@ -590,7 +591,6 @@ bool ImFontAtlasBuildWithFreeTypeEx(FT_Library ft_library, ImFontAtlas* atlas, u ImFontAtlasBuildMultiplyCalcLookupTable(multiply_table, cfg.RasterizerMultiply); // Gather the sizes of all rectangles we will need to pack - const int padding = atlas->TexGlyphPadding; for (int glyph_i = 0; glyph_i < src_tmp.GlyphsList.Size; glyph_i++) { ImFontBuildSrcGlyphFT& src_glyph = src_tmp.GlyphsList[glyph_i]; @@ -623,6 +623,8 @@ bool ImFontAtlasBuildWithFreeTypeEx(FT_Library ft_library, ImFontAtlas* atlas, u total_surface += src_tmp.Rects[glyph_i].w * src_tmp.Rects[glyph_i].h; } } + for (int i = 0; i < atlas->CustomRects.Size; i++) + total_surface += (atlas->CustomRects[i].Width + pack_padding) * (atlas->CustomRects[i].Height + pack_padding); // We need a width for the skyline algorithm, any width! // The exact width doesn't really matter much, but some API/GPU have texture size limitations and increasing width can decrease height.