CustomRect API problems (AddCustomRectRegular)

[YarikTH]

Version/Branch of Dear ImGui:

Version 1.91.4, Branch: master

Back-ends:

does not apply

Compiler, OS:

does not apply

Full config/build information:

Dear ImGui 1.91.4 (19140)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201103
define: __linux__
define: __GNUC__=13
--------------------------------
io.BackendPlatformName: imgui_impl_sdl2
io.BackendRendererName: imgui_impl_opengl3
io.ConfigFlags: 0x00000003
 NavEnableKeyboard
 NavEnableGamepad
io.ConfigNavCaptureKeyboard
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,128
io.DisplaySize: 1280.00,720.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:

My Issue:

I use ImGui to draw a custom service menu with some additional graphics like button texture, company logo etc. So far I used additional texture atlas for custom graphics. But for performance reasons I have tried to pack custom graphics in ImGui atlas using custom rect API as described in using-custom-colorful-icons.

It worked fine most of the time, but I found several limitations:

1. Custom rects are not added in total_surface value in ImFontAtlasBuildWithStbTruetype(), so auto calculated width is not optimal in case surface of custom rects is comparable with total surface of all fonts.
2. In case of very wide custom rects we have several problems:
   1. Custom rect is silently not added in the texture atlas in case selected width or calculated width is not enough to pack it. The only indication it that packing has failed is 0xFFFF value in X and Y of ImFontAtlasCustomRect. There is no assertion or other obvious indication of a problem. I wasn't ready for it based on "using-custom-colorful-icons" example and unexpectedly bumped into write out of memory error because of it.
   2. Maybe it is a rare case, but custom rect api allows adding rects of any size, so in my case width of the added rect was the same as autodetected width of a font atlas texture. Considering additional 1 pixel padding (TexGlyphPadding) it doesn't fit. Such case can be easily checked in texture width autodetect algorithm. Obviously minimal width should be pot(max(custom rects widths) + TexGlyphPadding). It can be optionally disabled with ImFontAtlasFlags_ like other things, but it is nice to have consideration.
3. It seems that TexGlyphPadding is applied only for... tex glyphs, but not for custom rects. So as a result all custom rects are clumped with each other seamlessly without any option to pad them from each other. It can be seen on the upscaled screenshot below. Colored rects are not separated, while font glyphs are.
   

MCVE is not enough for demonstration, so, I prepared the patch file for example_sdl2_opengl3 instead. Unfortunately github doesn't want to attach .patch files, so I provide its content below (it might be easier to make a fork and add link to the commit, but I'm too lazy to make a fork for it at this point).

0001-custom-rect-example.patch

From c41efcc58a36abcd2fc9d92a9166f8c8c10d6854 Mon Sep 17 00:00:00 2001
From: John Doeh <[email protected]>
Date: Mon, 28 Oct 2024 17:26:09 +0100
Subject: [PATCH] custom rect example

---
 backends/imgui_impl_opengl3.cpp        |   5 +
 backends/imgui_impl_opengl3.h          |   4 +
 examples/example_sdl2_opengl3/main.cpp | 123 ++++++++++++++++++++++---
 3 files changed, 118 insertions(+), 14 deletions(-)

diff --git a/backends/imgui_impl_opengl3.cpp b/backends/imgui_impl_opengl3.cpp
index f5235096..91e46950 100644
--- a/backends/imgui_impl_opengl3.cpp
+++ b/backends/imgui_impl_opengl3.cpp
@@ -664,6 +664,8 @@ void    ImGui_ImplOpenGL3_RenderDrawData(ImDrawData* draw_data)
     (void)bd; // Not all compilation paths use this
 }
 
+extern ImGui_ImplOpenGL3_PostProcessFontTextureCallback_Signature ImGui_ImplOpenGL3_PostProcessFontTextureCallback = nullptr;
+
 bool ImGui_ImplOpenGL3_CreateFontsTexture()
 {
     ImGuiIO& io = ImGui::GetIO();
@@ -674,6 +676,9 @@ bool ImGui_ImplOpenGL3_CreateFontsTexture()
     int width, height;
     io.Fonts->GetTexDataAsRGBA32(&pixels, &width, &height);   // Load as RGBA 32-bit (75% of the memory is wasted, but default font is so small) because it is more likely to be compatible with user's existing shaders. If your ImTextureId represent a higher-level concept than just a GL texture id, consider calling GetTexDataAsAlpha8() instead to save on GPU memory.
 
+    if(ImGui_ImplOpenGL3_PostProcessFontTextureCallback)
+        ImGui_ImplOpenGL3_PostProcessFontTextureCallback(pixels, width, height);
+
     // Upload texture to graphics system
     // (Bilinear sampling is required by default. Set 'io.Fonts->Flags |= ImFontAtlasFlags_NoBakedLines' or 'style.AntiAliasedLinesUseTex = false' to allow point/nearest sampling)
     GLint last_texture;
diff --git a/backends/imgui_impl_opengl3.h b/backends/imgui_impl_opengl3.h
index 54545f95..7bd164fc 100644
--- a/backends/imgui_impl_opengl3.h
+++ b/backends/imgui_impl_opengl3.h
@@ -29,6 +29,10 @@
 #include "imgui.h"      // IMGUI_IMPL_API
 #ifndef IMGUI_DISABLE
 
+typedef void (*ImGui_ImplOpenGL3_PostProcessFontTextureCallback_Signature)(unsigned char* pixels, int width, int height);
+
+extern ImGui_ImplOpenGL3_PostProcessFontTextureCallback_Signature ImGui_ImplOpenGL3_PostProcessFontTextureCallback;
+
 // Follow "Getting Started" link and check examples/ folder to learn about using backends!
 IMGUI_IMPL_API bool     ImGui_ImplOpenGL3_Init(const char* glsl_version = nullptr);
 IMGUI_IMPL_API void     ImGui_ImplOpenGL3_Shutdown();
diff --git a/examples/example_sdl2_opengl3/main.cpp b/examples/example_sdl2_opengl3/main.cpp
index 2a4d7b9e..fe20c969 100644
--- a/examples/example_sdl2_opengl3/main.cpp
+++ b/examples/example_sdl2_opengl3/main.cpp
@@ -23,6 +23,29 @@
 #include "../libs/emscripten/emscripten_mainloop_stub.h"
 #endif
 
+enum CustomRectNames
+{
+    CR_RED,
+    CR_GREEN,
+    CR_BLUE,
+    CR_BIG_MINUS_ONE,
+    CR_BIG,
+    CR_BIG_PLUS_ONE,
+
+    CR_AMOUNT
+};
+
+const char* rect_names[] = {
+    "CR_RED",
+    "CR_GREEN",
+    "CR_BLUE",
+    "CR_BIG_MINUS_ONE",
+    "CR_BIG",
+    "CR_BIG_PLUS_ONE",
+};
+
+int rect_ids[CR_AMOUNT];
+
 // Main code
 int main(int, char**)
 {
@@ -110,6 +133,45 @@ int main(int, char**)
     //ImFont* font = io.Fonts->AddFontFromFileTTF("c:\\Windows\\Fonts\\ArialUni.ttf", 18.0f, nullptr, io.Fonts->GetGlyphRangesJapanese());
     //IM_ASSERT(font != nullptr);
 
+    rect_ids[CR_RED]            = io.Fonts->AddCustomRectRegular(20, 20);
+    rect_ids[CR_GREEN]          = io.Fonts->AddCustomRectRegular(20, 20);
+    rect_ids[CR_BLUE]           = io.Fonts->AddCustomRectRegular(20, 20);
+    rect_ids[CR_BIG_MINUS_ONE]  = io.Fonts->AddCustomRectRegular(511, 20);
+    rect_ids[CR_BIG]            = io.Fonts->AddCustomRectRegular(512, 20);
+    rect_ids[CR_BIG_PLUS_ONE]   = io.Fonts->AddCustomRectRegular(513, 20);
+
+    ImGui_ImplOpenGL3_PostProcessFontTextureCallback = [](unsigned char* pixels, int width, int height){
+        const ImU32 rect_colors[CR_AMOUNT] = {
+            IM_COL32(255, 0,   0,   255),
+            IM_COL32(0,   255, 0,   255),
+            IM_COL32(0,   0,   255, 255),
+            IM_COL32(255, 255, 0,   255),
+            IM_COL32(0,   255, 255, 255),
+            IM_COL32(255, 0,   255, 255),
+        };
+
+        ImGuiIO& io = ImGui::GetIO();
+
+        for (int rect_n = 0; rect_n < CR_AMOUNT; rect_n++)
+        {
+            const int rect_id = rect_ids[rect_n];
+            const ImU32 rect_color = rect_colors[rect_n];
+            if (const ImFontAtlasCustomRect* rect = io.Fonts->GetCustomRectByIndex(rect_id))
+            {
+                if (rect->IsPacked())
+                {
+                    // Fill the custom rectangle with red pixels (in reality you would draw/copy your bitmap data here!)
+                    for (int y = 0; y < rect->Height; y++)
+                    {
+                        ImU32* p = (ImU32*)pixels + (rect->Y + y) * width + (rect->X);
+                        for (int x = rect->Width; x > 0; x--)
+                            *p++ = rect_color;
+                    }
+                }
+            }
+        }
+    };
+
     // Our state
     bool show_demo_window = true;
     bool show_another_window = false;
@@ -157,24 +219,57 @@ int main(int, char**)
 
         // 2. Show a simple window that we create ourselves. We use a Begin/End pair to create a named window.
         {
-            static float f = 0.0f;
-            static int counter = 0;
-
-            ImGui::Begin("Hello, world!");                          // Create a window called "Hello, world!" and append into it.
+            ImGui::SetNextWindowSize(ImVec2(544, 440), ImGuiCond_FirstUseEver);
+            ImGui::Begin("Custom rect test");
 
-            ImGui::Text("This is some useful text.");               // Display some text (you can use a format strings too)
-            ImGui::Checkbox("Demo Window", &show_demo_window);      // Edit bools storing our window open/close state
-            ImGui::Checkbox("Another Window", &show_another_window);
+            // Copy from "Widgets/Images"
+            ImTextureID my_tex_id = io.Fonts->TexID;
+            float my_tex_w = (float)io.Fonts->TexWidth;
+            float my_tex_h = (float)io.Fonts->TexHeight;
+            {
+                static bool use_text_color_for_tint = false;
+                ImGui::Checkbox("Use Text Color for Tint", &use_text_color_for_tint);
+                ImGui::Text("%.0fx%.0f", my_tex_w, my_tex_h);
+                ImVec2 pos = ImGui::GetCursorScreenPos();
+                ImVec2 uv_min = ImVec2(0.0f, 0.0f);                 // Top-left
+                ImVec2 uv_max = ImVec2(1.0f, 1.0f);                 // Lower-right
+                ImVec4 tint_col = use_text_color_for_tint ? ImGui::GetStyleColorVec4(ImGuiCol_Text) : ImVec4(1.0f, 1.0f, 1.0f, 1.0f); // No tint
+                ImVec4 border_col = ImGui::GetStyleColorVec4(ImGuiCol_Border);
+                ImGui::Image(my_tex_id, ImVec2(my_tex_w, my_tex_h), uv_min, uv_max, tint_col, border_col);
+                if (ImGui::BeginItemTooltip())
+                {
+                    float region_sz = 32.0f;
+                    float region_x = io.MousePos.x - pos.x - region_sz * 0.5f;
+                    float region_y = io.MousePos.y - pos.y - region_sz * 0.5f;
+                    float zoom = 4.0f;
+                    if (region_x < 0.0f) { region_x = 0.0f; }
+                    else if (region_x > my_tex_w - region_sz) { region_x = my_tex_w - region_sz; }
+                    if (region_y < 0.0f) { region_y = 0.0f; }
+                    else if (region_y > my_tex_h - region_sz) { region_y = my_tex_h - region_sz; }
+                    ImGui::Text("Min: (%.2f, %.2f)", region_x, region_y);
+                    ImGui::Text("Max: (%.2f, %.2f)", region_x + region_sz, region_y + region_sz);
+                    ImVec2 uv0 = ImVec2((region_x) / my_tex_w, (region_y) / my_tex_h);
+                    ImVec2 uv1 = ImVec2((region_x + region_sz) / my_tex_w, (region_y + region_sz) / my_tex_h);
+                    ImGui::Image(my_tex_id, ImVec2(region_sz * zoom, region_sz * zoom), uv0, uv1, tint_col, border_col);
+                    ImGui::EndTooltip();
+                }
+            }
 
-            ImGui::SliderFloat("float", &f, 0.0f, 1.0f);            // Edit 1 float using a slider from 0.0f to 1.0f
-            ImGui::ColorEdit3("clear color", (float*)&clear_color); // Edit 3 floats representing a color
+            const auto describeCustomRect = [&](CustomRectNames rect_id, const char* description)
+            {
+                ImFontAtlasCustomRect* customRect = io.Fonts->GetCustomRectByIndex(rect_id);
+                ImGui::PushStyleColor(ImGuiCol_Text, customRect->IsPacked() ? ImVec4(152.f/255, 251.f/255, 152.f/255, 1.0f) : ImVec4(255.f/255, 192.f/255, 203.f/255, 1.0f));
+                ImGui::Text("%s: {Width: %d, Height: %d, X: %d, Y: %d}", rect_names[rect_id], customRect->Width, customRect->Height, customRect->X, customRect->Y);
+                ImGui::Text("%s", description);
+                ImGui::PopStyleColor();
+            };
 
-            if (ImGui::Button("Button"))                            // Buttons return true when clicked (most widgets return true when edited/activated)
-                counter++;
-            ImGui::SameLine();
-            ImGui::Text("counter = %d", counter);
+            describeCustomRect( CR_BIG_MINUS_ONE, "Width a little less than atlas texture width.\nWorks fine." );
+            ImGui::NewLine();
+            describeCustomRect( CR_BIG, "Width the same atlas texture width.\nFor some reason can't pack such rect.\nThe only indication that packing is failed is 65535 value in X and Y" );
+            ImGui::NewLine();
+            describeCustomRect( CR_BIG_PLUS_ONE, "Width a little more than atlas texture width.\nFor some reason this value is not considered as minimal\natlas width on auto detection of atlas texture width.\nThe only indication that packing is failed is 65535 value in X and Y" );
 
-            ImGui::Text("Application average %.3f ms/frame (%.1f FPS)", 1000.0f / io.Framerate, io.Framerate);
             ImGui::End();
         }
 
-- 
2.43.0

Result looks like this:

requested: 6 colored rects
result: 4 colored rects were added next to each other, 2 rects were silently skipped

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

No response

[ocornut]

But for performance reasons I have tried to pack

Before everything: have you thoroughly measured the performance cost of texture changes?
It should be wholly negligible for a large majority of use. If you have, would you mind sharing your findings because that would be useful for us. If you haven't, I reckon you shouldn't be using this API for this.

[ocornut]

I've been working on completely rewriting this system, so it's going to change and I am unlikely to want to put much energy into fixing the existing one, but I will investigate your 3 issues and at minimum try to make sure the new system doesn't have them. I'll keep this open in the meanwhile. Thanks!

EDIT I believe if we at least fix (1) which is very easy, it should reduce the occurrences of (2).

[YarikTH]

Before

After

In case of drawing a lot of buttons with custom image and text (Tabs) amount of draw calls was reduced from 27 and 5 to 1 and 1.

It improved FPS a little.

                                before      after
LOGIN page (only logout page)   28.01       33.52 FPS
PARTIAL STATISTICS page         17.18       19.19 FPS
TOTAL STATISTICS page           16.67       18.36 FPS
EVENTS page                     18.75       21.28 FPS

It is not a common use case though. It is webgl version on a very outdated hardware. There are other things to improve, but apart from some minor problems, merging of atlases gave me a small visible improvement.

[YarikTH]

But for performance reasons I have tried to pack

Before everything: have you thoroughly measured the performance cost of texture changes? It should be wholly negligible for a large majority of use. If you have, would you mind sharing your findings because that would be useful for us. If you haven't, I reckon you shouldn't be using this API for this.

I forgot to make previous message as a reply. But tl;dr I believe that on more or less modern hardware it wouldn't be a problem, but I have a very specific use case where every small optimization matters.

[ocornut]

Thank you for your thoughtful initial post and answer.
I am treating it as low-priority as I assume that given your known content you can set a desired texture width and not have any immediate problem?

[YarikTH]

Thank you for your thoughtful initial post and answer. I am treating it as low-priority as I assume that given your known content you can set a desired texture width and not have any immediate problem?

Yes, it is low priority. I can set texture width and in case of problems I can patch my fork of ImGui to fix at least some mentioned problems. But I believe that hardcoding of width and making manual padding will solve all my issues without touching imgui.

[axeldavy]

I faced the issue (3) described here, and it gives visible artifacts when rendering:

The current workaround I found is to add a padding of 1 when filling custom rects, and use the following imgui patch:

@@ -2687,7 +2689,7 @@ void ImFontAtlas::CalcCustomRectUV(const ImFontAtlasCustomRect* rect, ImVec2* ou
     IM_ASSERT(TexWidth > 0 && TexHeight > 0);   // Font atlas needs to be built before we can calculate UV coordinates
     IM_ASSERT(rect->IsPacked());                // Make sure the rectangle has been packed
     *out_uv_min = ImVec2((float)rect->X * TexUvScale.x, (float)rect->Y * TexUvScale.y);
-    *out_uv_max = ImVec2((float)(rect->X + rect->Width) * TexUvScale.x, (float)(rect->Y + rect->Height) * TexUvScale.y);
+    *out_uv_max = ImVec2((float)(rect->X + rect->Width-1) * TexUvScale.x, (float)(rect->Y + rect->Height-1) * TexUvScale.y);
 }
 
 bool ImFontAtlas::GetMouseCursorTexData(ImGuiMouseCursor cursor_type, ImVec2* out_offset, ImVec2* out_size, ImVec2 out_uv_border[2], ImVec2 out_uv_fill[2])
@@ -3284,7 +3286,7 @@ void ImFontAtlasBuildFinish(ImFontAtlas* atlas)
         IM_ASSERT(r->Font->ContainerAtlas == atlas);
         ImVec2 uv0, uv1;
         atlas->CalcCustomRectUV(r, &uv0, &uv1);
-        r->Font->AddGlyph(NULL, (ImWchar)r->GlyphID, r->GlyphOffset.x, r->GlyphOffset.y, r->GlyphOffset.x + r->Width, r->GlyphOffset.y + r->Height, uv0.x, uv0.y, uv1.x, uv1.y, r->GlyphAdvanceX);
+        r->Font->AddGlyph(NULL, (ImWchar)r->GlyphID, r->GlyphOffset.x, r->GlyphOffset.y, r->GlyphOffset.x + r->Width-1, r->GlyphOffset.y + r->Height-1, uv0.x, uv0.y, uv1.x, uv1.y, r->GlyphAdvanceX);
     }
 

Basically in addition to adding the padding in the filled rects, you need Imgui to be aware of it. This replicates the behaviour of the normal font path.

I might add an issue (4), is that I encountered crashes if I try to build a custom-rect only font (appending custom rect to a font created with AddFont() and no font file attached), thus I have to add a minimal font file for it to work.