From a35e8769780e7ef0a89678cac27b98a532357af5 Mon Sep 17 00:00:00 2001 From: ocornut Date: Tue, 7 Jun 2022 17:28:31 +0200 Subject: [PATCH] InputText: Amends and tidying up: Fixed undo/redo state corruption when editing buffer in user callback. (#4947, #4949) --- docs/CHANGELOG.txt | 4 ++- imgui.h | 1 + imgui_widgets.cpp | 63 +++++++++++++++++++++------------------------- 3 files changed, 32 insertions(+), 36 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index 7a59c214..23a55026 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -88,7 +88,9 @@ Other Changes: trickled with the new input queue (happened on some backends only). (#2467, #1336) - InputText: Fixed a one-frame display glitch where pressing Escape to revert after a deletion would lead to small garbage being displayed for one frame. Curiously a rather old bug! (#3008) -- InputText: Fixed an undo-state corruption issue when editing buffer before reactivating item. (#4947) +- InputText: Fixed an undo-state corruption issue when editing main buffer before reactivating item. (#4947) +- InputText: Fixed an undo-state corruption issue when editing in-flight buffer in user callback. + (#4947, #4949] [@JoshuaWebb] - Tables: Fixed incorrect border height used for logic when resizing one of several synchronized instance of a same table ID, when instances have a different height. (#3955). - Tables: Fixed incorrect auto-fit of parent windows when using non-resizable weighted columns. (#5276) diff --git a/imgui.h b/imgui.h index c9bdb2d7..20b6ea9b 100644 --- a/imgui.h +++ b/imgui.h @@ -1809,6 +1809,7 @@ struct ImVector inline void resize(int new_size, const T& v) { if (new_size > Capacity) reserve(_grow_capacity(new_size)); if (new_size > Size) for (int n = Size; n < new_size; n++) memcpy(&Data[n], &v, sizeof(v)); Size = new_size; } inline void shrink(int new_size) { IM_ASSERT(new_size <= Size); Size = new_size; } // Resize a vector to a smaller size, guaranteed not to cause a reallocation inline void reserve(int new_capacity) { if (new_capacity <= Capacity) return; T* new_data = (T*)IM_ALLOC((size_t)new_capacity * sizeof(T)); if (Data) { memcpy(new_data, Data, (size_t)Size * sizeof(T)); IM_FREE(Data); } Data = new_data; Capacity = new_capacity; } + inline void reserve_discard(int new_capacity) { if (new_capacity <= Capacity) return; if (Data) IM_FREE(Data); Data = (T*)IM_ALLOC((size_t)new_capacity * sizeof(T)); Capacity = new_capacity; } // NB: It is illegal to call push_back/push_front/insert with a reference pointing inside the ImVector data itself! e.g. v.push_back(v[10]) is forbidden. inline void push_back(const T& v) { if (Size == Capacity) reserve(_grow_capacity(Size + 1)); memcpy(&Data[Size], &v, sizeof(v)); Size++; } diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index 5d146da9..aaa42dcf 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -3929,46 +3929,39 @@ static bool InputTextFilterCharacter(unsigned int* p_char, ImGuiInputTextFlags f return true; } -static inline void InputTextUpdateUndoStateAfterUserCallback(ImGuiInputTextState *state, const char *new_buf) { - // Find the shortest single replacement we can make to get the new text - // from the old text. - ImWchar *old_buf = state->TextW.Data; - int old_length = state->CurLenW; - int new_length = ImTextCountCharsFromUtf8(new_buf, NULL); - int shorter_length = ImMin(old_length, new_length); +// Find the shortest single replacement we can make to get the new text from the old text. +// Important: needs to be run before TextW is rewritten with the new characters because calling STB_TEXTEDIT_GETCHAR() at the end. +// FIXME: Ideally we should transition toward (1) making InsertChars()/DeleteChars() update undo-stack (2) discourage (and keep reconcile) or obsolete (and remove reconcile) accessing buffer directly. +static void InputTextReconcileUndoStateAfterUserCallback(ImGuiInputTextState* state, const char* new_buf_a, int new_length_a) +{ + ImGuiContext& g = *GImGui; + const ImWchar* old_buf = state->TextW.Data; + const int old_length = state->CurLenW; + const int new_length = ImTextCountCharsFromUtf8(new_buf_a, new_buf_a + new_length_a); + g.TempBuffer.reserve_discard((new_length + 1) * sizeof(ImWchar)); + ImWchar* new_buf = (ImWchar*)(void*)g.TempBuffer.Data; + ImTextStrFromUtf8(new_buf, new_length + 1, new_buf_a, new_buf_a + new_length_a); - int where = 0; - while (where < shorter_length) { - unsigned int b; - ImTextCharFromUtf8(&b, new_buf + where, NULL); - if (old_buf[where] != b) + const int shorter_length = ImMin(old_length, new_length); + int first_diff; + for (first_diff = 0; first_diff < shorter_length; first_diff++) + if (old_buf[first_diff] != new_buf[first_diff]) break; - - where += 1; - } + if (first_diff == old_length && first_diff == new_length) + return; int old_last_diff = old_length - 1; int new_last_diff = new_length - 1; - while (old_last_diff >= where && new_last_diff >= 0) { - unsigned int b; - ImTextCharFromUtf8(&b, new_buf + new_last_diff, NULL); - if (old_buf[old_last_diff] != b) + for (; old_last_diff >= first_diff && new_last_diff >= first_diff; old_last_diff--, new_last_diff--) + if (old_buf[old_last_diff] != new_buf[new_last_diff]) break; - old_last_diff -= 1; - new_last_diff -= 1; - } - - int insert_len = new_last_diff - where + 1; - int delete_len = old_last_diff - where + 1; - - if (insert_len > 0 || delete_len > 0) { - STB_TEXTEDIT_CHARTYPE *p = stb_text_createundo(&state->Stb.undostate, where, delete_len, insert_len); - if (p) { - for (int i=0; i < delete_len; ++i) - p[i] = ImStb::STB_TEXTEDIT_GETCHAR(state, where + i); - } - } + const int insert_len = new_last_diff - first_diff + 1; + const int delete_len = old_last_diff - first_diff + 1; + if (insert_len > 0 || delete_len > 0) + if (STB_TEXTEDIT_CHARTYPE* p = stb_text_createundo(&state->Stb.undostate, first_diff, delete_len, insert_len)) + for (int i = 0; i < delete_len; i++) + p[i] = ImStb::STB_TEXTEDIT_GETCHAR(state, first_diff + i); } // Edit a string of text @@ -4558,9 +4551,9 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_ if (buf_dirty) { IM_ASSERT(callback_data.BufTextLen == (int)strlen(callback_data.Buf)); // You need to maintain BufTextLen if you change the text! - InputTextUpdateUndoStateAfterUserCallback(state, callback_data.Buf); + InputTextReconcileUndoStateAfterUserCallback(state, callback_data.Buf, callback_data.BufTextLen); // FIXME: Move the rest of this block inside function and rename to InputTextReconcileStateAfterUserCallback() ? if (callback_data.BufTextLen > backup_current_text_length && is_resizable) - state->TextW.resize(state->TextW.Size + (callback_data.BufTextLen - backup_current_text_length)); + state->TextW.resize(state->TextW.Size + (callback_data.BufTextLen - backup_current_text_length)); // Worse case scenario resize state->CurLenW = ImTextStrFromUtf8(state->TextW.Data, state->TextW.Size, callback_data.Buf, NULL); state->CurLenA = callback_data.BufTextLen; // Assume correct length and valid UTF-8 from user, saves us an extra strlen() state->CursorAnimReset();