From 439f72694596620257611a8ae46d5d4b2ab1174e Mon Sep 17 00:00:00 2001 From: omar Date: Sun, 24 Feb 2019 23:31:00 +0100 Subject: [PATCH 1/3] InputText; Disabled rendering selection when inactive (it kinda work but I'm not sure this is desirable especially for single-line input, was not intended to be active). --- imgui_widgets.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index 8ad51a28..7ff3d100 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -3143,7 +3143,7 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 ImGuiIO& io = g.IO; const ImGuiStyle& style = g.Style; - const bool RENDER_SELECTION_WHEN_INACTIVE = true; + const bool RENDER_SELECTION_WHEN_INACTIVE = false; const bool is_multiline = (flags & ImGuiInputTextFlags_Multiline) != 0; const bool is_readonly = (flags & ImGuiInputTextFlags_ReadOnly) != 0; const bool is_password = (flags & ImGuiInputTextFlags_Password) != 0; From b7b82520b4d8223d5d676f6bff05b34f7f58bf1f Mon Sep 17 00:00:00 2001 From: omar Date: Tue, 26 Feb 2019 12:22:58 +0100 Subject: [PATCH 2/3] Internal: InputText: Minor changes (intended to have side-effect but clarify next commit, however there is rarely such a thing as zero side effect in InputText land!) --- imgui_internal.h | 4 ++++ imgui_widgets.cpp | 41 +++++++++++++++++++++-------------------- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/imgui_internal.h b/imgui_internal.h index a76c386a..84420b99 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -104,6 +104,8 @@ namespace ImStb #define STB_TEXTEDIT_STRING ImGuiInputTextState #define STB_TEXTEDIT_CHARTYPE ImWchar #define STB_TEXTEDIT_GETWIDTH_NEWLINE -1.0f +#define STB_TEXTEDIT_UNDOSTATECOUNT 99 +#define STB_TEXTEDIT_UNDOCHARCOUNT 999 #include "imstb_textedit.h" } // namespace ImStb @@ -593,6 +595,8 @@ struct IMGUI_API ImGuiInputTextState bool HasSelection() const { return Stb.select_start != Stb.select_end; } void ClearSelection() { Stb.select_start = Stb.select_end = Stb.cursor; } void SelectAll() { Stb.select_start = 0; Stb.cursor = Stb.select_end = CurLenW; Stb.has_preferred_x = 0; } + int GetUndoAvailCount() const { return Stb.undostate.undo_point; } + int GetRedoAvailCount() const { return STB_TEXTEDIT_UNDOSTATECOUNT - Stb.undostate.redo_point; } void OnKeyPressed(int key); // Cannot be inline because we call in code in stb_textedit.h implementation }; diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index 7ff3d100..d1ecb088 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -3277,32 +3277,33 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 g.ActiveIdAllowNavDirFlags = ((1 << ImGuiDir_Up) | (1 << ImGuiDir_Down)); } + // We have an edge case if ActiveId was set through another widget (e.g. widget being swapped), clear id immediately (don't wait until the end of the function) + if (g.ActiveId == id && state == NULL) + ClearActiveID(); + // Release focus when we click outside if (!init_make_active && io.MouseClicked[0]) clear_active_id = true; - // We have an edge case if ActiveId was set through another widget (e.g. widget being swapped) - if (g.ActiveId == id && state == NULL) - ClearActiveID(); - bool value_changed = false; bool enter_pressed = false; int backup_current_text_length = 0; + // When read-only we always use the live data passed to the function + if (g.ActiveId == id && is_readonly && !g.ActiveIdIsJustActivated) + { + IM_ASSERT(state != NULL); + const char* buf_end = NULL; + state->TextW.resize(buf_size + 1); + state->CurLenW = ImTextStrFromUtf8(state->TextW.Data, state->TextW.Size, buf, NULL, &buf_end); + state->CurLenA = (int)(buf_end - buf); + state->CursorClamp(); + } + // Process mouse inputs and character inputs if (g.ActiveId == id) { IM_ASSERT(state != NULL); - if (is_readonly && !g.ActiveIdIsJustActivated) - { - // When read-only we always use the live data passed to the function - const char* buf_end = NULL; - state->TextW.resize(buf_size+1); - state->CurLenW = ImTextStrFromUtf8(state->TextW.Data, state->TextW.Size, buf, NULL, &buf_end); - state->CurLenA = (int)(buf_end - buf); - state->CursorClamp(); - } - backup_current_text_length = state->CurLenA; state->BufCapacityA = buf_size; state->UserFlags = flags; @@ -3653,7 +3654,7 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 // Render text. We currently only render selection when the widget is active or while scrolling. // FIXME: We could remove the '&& render_cursor' to keep rendering selection when inactive. - const bool render_cursor = (g.ActiveId == id) || user_scroll_active; + const bool render_cursor = (g.ActiveId == id) || (state && user_scroll_active); const bool render_selection = state && state->HasSelection() && (RENDER_SELECTION_WHEN_INACTIVE || render_cursor); if (render_cursor || render_selection) { @@ -3811,13 +3812,13 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 else { // Render text only (no selection, no cursor) - const char* buf_end = NULL; + const char* buf_display_end = NULL; if (is_multiline) - text_size = ImVec2(size.x, InputTextCalcTextLenAndLineCount(buf_display, &buf_end) * g.FontSize); // We don't need width + text_size = ImVec2(size.x, InputTextCalcTextLenAndLineCount(buf_display, &buf_display_end) * g.FontSize); // We don't need width else - buf_end = buf_display + strlen(buf_display); - if (is_multiline || (buf_end - buf_display) < buf_display_max_length) - draw_window->DrawList->AddText(g.Font, g.FontSize, draw_pos, GetColorU32(ImGuiCol_Text), buf_display, buf_end, 0.0f, is_multiline ? NULL : &clip_rect); + buf_display_end = buf_display + strlen(buf_display); + if (is_multiline || (buf_display_end - buf_display) < buf_display_max_length) + draw_window->DrawList->AddText(g.Font, g.FontSize, draw_pos, GetColorU32(ImGuiCol_Text), buf_display, buf_display_end, 0.0f, is_multiline ? NULL : &clip_rect); } if (is_multiline) From cf3cb7cf7ee01a066a20c3e47189b336c48fb776 Mon Sep 17 00:00:00 2001 From: omar Date: Tue, 26 Feb 2019 12:50:44 +0100 Subject: [PATCH 3/3] InputText: Fixed various display corruption related to swapping the underlying buffer while a input widget is active (both for writable and read-only paths). Often they would manifest when manipulating the scrollbar of a multi-line input text. --- docs/CHANGELOG.txt | 3 +++ imgui_internal.h | 3 ++- imgui_widgets.cpp | 54 ++++++++++++++++++++++++++-------------------- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index 9ef37fab..2511e4c0 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -41,6 +41,9 @@ Other Changes: meant to be square (to align with e.g. color button) we always use FramePadding.y. (#2367) - InputText: Fixed an edge case crash that would happen if another widget sharing the same ID is being swapped with an InputText that has yet to be activated. +- InputText: Fixed various display corruption related to swapping the underlying buffer while + a input widget is active (both for writable and read-only paths). Often they would manifest + when manipulating the scrollbar of a multi-line input text. - TabBar: Fixed a crash when using BeginTabBar() recursively (didn't affect docking). (#2371) - TabBar: Added extra mis-usage error recovery. Past the assert, common mis-usage don't lead to hard crashes any more, facilitating integration with scripting languages. (#1651) diff --git a/imgui_internal.h b/imgui_internal.h index 84420b99..40e10748 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -572,10 +572,11 @@ struct IMGUI_API ImGuiMenuColumns struct IMGUI_API ImGuiInputTextState { ImGuiID ID; // widget id owning the text state - int CurLenW, CurLenA; // we need to maintain our buffer length in both UTF-8 and wchar format. + int CurLenW, CurLenA; // we need to maintain our buffer length in both UTF-8 and wchar format. UTF-8 len is valid even if TextA is not. ImVector TextW; // edit buffer, we need to persist but can't guarantee the persistence of the user-provided buffer. so we copy into own buffer. ImVector TextA; // temporary UTF8 buffer for callbacks and other operations. this is not updated in every code-path! size=capacity. ImVector InitialTextA; // backup of end-user buffer at the time of focus (in UTF-8, unaltered) + bool TextAIsValid; // temporary UTF8 buffer is not initially valid before we make the widget active (until then we pull the data from user argument) int BufCapacityA; // end-user buffer capacity float ScrollX; // horizontal scrolling/offset ImStb::STB_TexteditState Stb; // state for stb_textedit.h diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index d1ecb088..946a5dbe 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -3224,7 +3224,8 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 bool select_all = (g.ActiveId != id) && ((flags & ImGuiInputTextFlags_AutoSelectAll) != 0 || user_nav_input_start) && (!is_multiline); const bool init_make_active = (focus_requested || user_clicked || user_scroll_finish || user_nav_input_start); - if (init_make_active && g.ActiveId != id) + const bool init_state = (init_make_active || user_scroll_active); + if (init_state && g.ActiveId != id) { // Access state even if we don't own it yet. state = &g.InputTextState; @@ -3237,15 +3238,16 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 memcpy(state->InitialTextA.Data, buf, buf_len + 1); // Start edition - const int prev_len_w = state->CurLenW; const char* buf_end = NULL; state->TextW.resize(buf_size + 1); // wchar count <= UTF-8 count. we use +1 to make sure that .Data is always pointing to at least an empty string. + state->TextA.resize(0); + state->TextAIsValid = false; // TextA is not valid yet (we will display buf until then) state->CurLenW = ImTextStrFromUtf8(state->TextW.Data, buf_size, buf, NULL, &buf_end); state->CurLenA = (int)(buf_end - buf); // We can't get the result from ImStrncpy() above because it is not UTF-8 aware. Here we'll cut off malformed UTF-8. // Preserve cursor position and undo/redo stack if we come back to same widget - // FIXME: We should probably compare the whole buffer to be on the safety side. Comparing buf (utf8) and edit_state.Text (wchar). - const bool recycle_state = (state->ID == id) && (prev_len_w == state->CurLenW); + // FIXME: For non-readonly widgets we might be able to require that TextAIsValid && TextA == buf ? (untested) and discard undo stack if user buffer has changed. + const bool recycle_state = (state->ID == id); if (recycle_state) { // Recycle existing cursor/selection/undo stack but clamp position @@ -3266,7 +3268,7 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 select_all = true; } - if (init_make_active) + if (g.ActiveId != id && init_make_active) { IM_ASSERT(state && state->ID == id); SetActiveID(id, window); @@ -3282,7 +3284,7 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 ClearActiveID(); // Release focus when we click outside - if (!init_make_active && io.MouseClicked[0]) + if (g.ActiveId == id && io.MouseClicked[0] && !init_state && !init_make_active) clear_active_id = true; bool value_changed = false; @@ -3290,14 +3292,19 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 int backup_current_text_length = 0; // When read-only we always use the live data passed to the function - if (g.ActiveId == id && is_readonly && !g.ActiveIdIsJustActivated) + // FIXME-OPT: Because our selection/cursor code currently needs the wide text we need to convert it when active, which is not ideal :( + if (is_readonly && state != NULL) { - IM_ASSERT(state != NULL); - const char* buf_end = NULL; - state->TextW.resize(buf_size + 1); - state->CurLenW = ImTextStrFromUtf8(state->TextW.Data, state->TextW.Size, buf, NULL, &buf_end); - state->CurLenA = (int)(buf_end - buf); - state->CursorClamp(); + const bool will_render_cursor = (g.ActiveId == id) || (state && user_scroll_active); + const bool will_render_selection = state && state->HasSelection() && (RENDER_SELECTION_WHEN_INACTIVE || will_render_cursor); + if (will_render_cursor || will_render_selection) + { + const char* buf_end = NULL; + state->TextW.resize(buf_size + 1); + state->CurLenW = ImTextStrFromUtf8(state->TextW.Data, state->TextW.Size, buf, NULL, &buf_end); + state->CurLenA = (int)(buf_end - buf); + state->CursorClamp(); + } } // Process mouse inputs and character inputs @@ -3516,6 +3523,7 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 // FIXME-OPT: CPU waste to do this every time the widget is active, should mark dirty state from the stb_textedit callbacks. if (!is_readonly) { + state->TextAIsValid = true; state->TextA.resize(state->TextW.Size * 4 + 1); ImTextStrToUtf8(state->TextA.Data, state->TextA.Size, state->TextW.Data, NULL); } @@ -3646,11 +3654,8 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 // without any carriage return, which would makes ImFont::RenderText() reserve too many vertices and probably crash. Avoid it altogether. // Note that we only use this limit on single-line InputText(), so a pathologically large line on a InputTextMultiline() would still crash. const int buf_display_max_length = 2 * 1024 * 1024; - - // Select which buffer we are going to display. We set buf to NULL to prevent accidental usage from now on. - const char* buf_display = (g.ActiveId == id && state && !is_readonly) ? state->TextA.Data : buf; - IM_ASSERT(buf_display); - buf = NULL; + const char* buf_display = NULL; + const char* buf_display_end = NULL; // Render text. We currently only render selection when the widget is active or while scrolling. // FIXME: We could remove the '&& render_cursor' to keep rendering selection when inactive. @@ -3790,9 +3795,10 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 } // We test for 'buf_display_max_length' as a way to avoid some pathological cases (e.g. single-line 1 MB string) which would make ImDrawList crash. - const int buf_display_len = state->CurLenA; - if (is_multiline || buf_display_len < buf_display_max_length) - draw_window->DrawList->AddText(g.Font, g.FontSize, draw_pos - draw_scroll, GetColorU32(ImGuiCol_Text), buf_display, buf_display + buf_display_len, 0.0f, is_multiline ? NULL : &clip_rect); + buf_display = (!is_readonly && state->TextAIsValid) ? state->TextA.Data : buf; + buf_display_end = buf_display + state->CurLenA; + if (is_multiline || (buf_display_end - buf_display) < buf_display_max_length) + draw_window->DrawList->AddText(g.Font, g.FontSize, draw_pos - draw_scroll, GetColorU32(ImGuiCol_Text), buf_display, buf_display_end, 0.0f, is_multiline ? NULL : &clip_rect); // Draw blinking cursor if (render_cursor) @@ -3812,9 +3818,11 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 else { // Render text only (no selection, no cursor) - const char* buf_display_end = NULL; + buf_display = (g.ActiveId == id && !is_readonly && state->TextAIsValid) ? state->TextA.Data : buf; if (is_multiline) text_size = ImVec2(size.x, InputTextCalcTextLenAndLineCount(buf_display, &buf_display_end) * g.FontSize); // We don't need width + else if (g.ActiveId == id) + buf_display_end = buf_display + state->CurLenA; else buf_display_end = buf_display + strlen(buf_display); if (is_multiline || (buf_display_end - buf_display) < buf_display_max_length) @@ -3833,7 +3841,7 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 // Log as text if (g.LogEnabled && !is_password) - LogRenderedText(&draw_pos, buf_display, NULL); + LogRenderedText(&draw_pos, buf_display, buf_display_end); if (label_size.x > 0) RenderText(ImVec2(frame_bb.Max.x + style.ItemInnerSpacing.x, frame_bb.Min.y + style.FramePadding.y), label);