From 550f110354fc922d1a471906360558b14374c863 Mon Sep 17 00:00:00 2001 From: omar Date: Sun, 12 Jul 2020 23:51:13 +0200 Subject: [PATCH] InputText, ImDrawList: Fixed assert triggering when drawing single line of text with more than ~16 KB characters. (#3349) --- docs/CHANGELOG.txt | 5 +++++ docs/TODO.txt | 3 ++- imgui_draw.cpp | 5 ++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index 6ee96d784..0264e3147 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -37,6 +37,11 @@ HOW TO UPDATE? Other Changes: +- InputText, ImDrawList: Fixed assert triggering when drawing single line of text with more + than ~16 KB characters. (Note that current code is going to show corrupted display if after + clipping, more than 16 KB characters are visible in the same low-level ImDrawList::RenderText + call. ImGui-level functions such as TextUnformatted() are not affected. This is quite rare + but it will be addressed later). (#3349) - ImDrawList: Thick anti-aliased strokes (> 1.0f) with integer thickness now use a texture-based path, reducing the amount of vertices/indices and CPU/GPU usage. (#3245) [@Shironekoben] - This change will facilitate the wider use of thick borders in future style changes. diff --git a/docs/TODO.txt b/docs/TODO.txt index abb9d513e..9502e2646 100644 --- a/docs/TODO.txt +++ b/docs/TODO.txt @@ -308,7 +308,8 @@ It's mostly a bunch of personal notes, probably incomplete. Feel free to query i - font/atlas: dynamic font atlas to avoid baking huge ranges into bitmap and make scaling easier. - 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/draw: better reserve policy for large horizontal block of text (shouldn't reserve for all clipped lines). also see #3349. + - font/draw: fix for drawing 16k+ visible characters in same call. - font/draw: underline, squiggle line rendering helpers. - 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), would save on cache line. - 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? diff --git a/imgui_draw.cpp b/imgui_draw.cpp index 864364e93..09b5a413e 100644 --- a/imgui_draw.cpp +++ b/imgui_draw.cpp @@ -517,7 +517,7 @@ void ImDrawList::_OnChangedVtxOffset() // We don't need to compare curr_cmd->VtxOffset != _CmdHeader.VtxOffset because we know it'll be different at the time we call this. _VtxCurrentIdx = 0; ImDrawCmd* curr_cmd = &CmdBuffer.Data[CmdBuffer.Size - 1]; - IM_ASSERT(curr_cmd->VtxOffset != _CmdHeader.VtxOffset); + //IM_ASSERT(curr_cmd->VtxOffset != _CmdHeader.VtxOffset); // See #3349 if (curr_cmd->ElemCount != 0) { AddDrawCmd(); @@ -582,6 +582,9 @@ void ImDrawList::PrimReserve(int idx_count, int vtx_count) IM_ASSERT_PARANOID(idx_count >= 0 && vtx_count >= 0); if (sizeof(ImDrawIdx) == 2 && (_VtxCurrentIdx + vtx_count >= (1 << 16)) && (Flags & ImDrawListFlags_AllowVtxOffset)) { + // FIXME: In theory we should be testing that vtx_count <64k here. + // In practice, RenderText() relies on reserving ahead for a worst case scenario so it is currently useful for us + // to not make that check until we rework the text functions to handle clipping and large horizontal lines better. _CmdHeader.VtxOffset = VtxBuffer.Size; _OnChangedVtxOffset(); }