Browse Source

ImDrawList: Large text passed to AddText() are being scanned for their end in order to avoid pre-reserving too many vertices.

omar 7 years ago
parent
commit
68448c5faa
4 changed files with 23 additions and 6 deletions
  1. 2 1
      CHANGELOG.txt
  2. 4 3
      TODO.txt
  3. 0 1
      imgui.cpp
  4. 17 1
      imgui_draw.cpp

+ 2 - 1
CHANGELOG.txt

@@ -73,7 +73,8 @@ Other Changes:
  - Fixed PushID() from keeping alive the new ID Stack top value (if a previously active widget shared the ID it would be erroneously kept alive).  
  - Fixed horizontal mouse wheel not forwarding the request to the parent window if ImGuiWindowFlags_NoScrollWithMouse is set. (#1463, #1380, #1502)
  - Fixed a include build issue for Cygwin in non-POSIX (Win32) mode. (#1917, #1319, #276)
- - ImDrawList: Fixed clipping of leading lines above the clipping rectangle from counting in the worst-case vertices reservation. (fix code added in #200!)
+ - ImDrawList: Improved handling for worst-case vertices reservation policy when large amount of text (e.g. ~1 million character strings)
+   are being submitted in a single call. It would typically have crashed InputTextMultiline(). (#200)
  - OS/Windows: Fixed missing ImmReleaseContext() call in the default Win32 IME handler. (#1932) [@vby]
  - Metrics: Changed io.MetricsActiveWindows to reflect the number of active windows (!= from visible windows), which is useful
    for lazy/idle render mechanisms as new windows are typically not visible for one frame.

+ 4 - 3
TODO.txt

@@ -240,13 +240,14 @@ It's mostly a bunch of personal notes, probably incomplete. Feel free to query i
  - font: enforce monospace through ImFontConfig (for icons?) + create dual ImFont output from same input, reusing rasterized data but with different glyphs/AdvanceX
  - font: finish CustomRectRegister() to allow mapping Unicode codepoint to custom texture data 
  - font: PushFontSize API (#1018)
+ - font: MemoryTTF taking ownership confusing/not obvious, maybe default should be opposite?
  - font/atlas: add a missing Glyphs.reserve()
  - font/atlas: incremental updates
  - font/atlas: dynamic font atlas to avoid baking huge ranges into bitmap and make scaling easier.
  - font/atlas: allow user to submit its own primitive to be rectpacked, and allow to map them on a Unicode point.
- - font: MemoryTTF taking ownership confusing/not obvious, maybe default should be opposite?
- - font draw: vertical and/or rotated text renderer (#705) - vertical is easier clipping wise
- - font draw: need to be able to specify wrap start position.
+ - font/draw: vertical and/or rotated text renderer (#705) - vertical is easier clipping wise
+ - font/draw: need to be able to specify wrap start position.
+ - font/draw: better reserve policy for large horizontal block of text (shouldn't reserve for all clipped lines)
  - font: imgui_freetype.h alternative renderer (#618)
  - font: optimization: for monospace font (like the default one) we can trim IndexXAdvance as long as trailing value is == FallbackXAdvance (need to make sure TAB is still correct).
  - font: add support for kerning, probably optional. A) perhaps default to (32..128)^2 matrix ~ 9K entries = 36KB, then hash for non-ascii?. B) or sparse lookup into per-char list?

+ 0 - 1
imgui.cpp

@@ -11303,7 +11303,6 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2
             }
         }
 
-        // FIXME-OPT: We should coarse clip very large text on this end (even though the low-level ImFont::RenderText perform line-based will do it, it will lead us to temporary vertex allocations) 
         draw_window->DrawList->AddText(g.Font, g.FontSize, render_pos - render_scroll, GetColorU32(ImGuiCol_Text), buf_display, buf_display + edit_state.CurLenA, 0.0f, is_multiline ? NULL : &clip_rect);
 
         // Draw blinking cursor

+ 17 - 1
imgui_draw.cpp

@@ -2634,7 +2634,7 @@ void ImFont::RenderText(ImDrawList* draw_list, float size, ImVec2 pos, ImU32 col
 
     // Fast-forward to first visible line
     const char* s = text_begin;
-    if (!word_wrap_enabled && y + line_height < clip_rect.y)
+    if (y + line_height < clip_rect.y && !word_wrap_enabled)
         while (y + line_height < clip_rect.y)
         {
             while (s < text_end)
@@ -2643,6 +2643,22 @@ void ImFont::RenderText(ImDrawList* draw_list, float size, ImVec2 pos, ImU32 col
             y += line_height;
         }
 
+    // For large text, scan for the last visible line in order to avoid over-reserving in the call to PrimReserve()
+    // Note that very large horizontal line will still be affected by the issue (e.g. a one megabyte string buffer without a newline will likely crash atm)
+    if (text_end - s > 10000 && !word_wrap_enabled)
+    {
+        const char* s_end = s;
+        float y_end = y;
+        while (y_end < clip_rect.w)
+        {
+            while (s_end < text_end)
+                if (*s_end++ == '\n')
+                    break;
+            y_end += line_height;
+        }
+        text_end = s_end;
+    }
+
     // Reserve vertices for remaining worse case (over-reserving is useful and easily amortized)
     const int vtx_count_max = (int)(text_end - s) * 4;
     const int idx_count_max = (int)(text_end - s) * 6;