From 5a2b1e84828f192d30c91cdd210db41de8ad3236 Mon Sep 17 00:00:00 2001 From: ocornut Date: Wed, 8 Dec 2021 20:54:04 +0100 Subject: [PATCH] InputText: Fixed a tricky edge case, ensuring value is always written back on the frame where IsItemDeactivated() returns true (#4714) Altered ItemAdd() clipping rule to keep previous-frame ActiveId unclipped to support that late commit. Also, MarkItemEdited() may in theory need to do: if (g.ActiveIdPreviousFrame == id) g.ActiveIdPreviousFrameHasBeenEditedBefore = true; But this should already be set so not adding now. --- docs/CHANGELOG.txt | 5 +++++ imgui.cpp | 39 ++++++++++++++++++++++++++++----------- imgui.h | 2 +- imgui_internal.h | 12 ++++++++++++ imgui_widgets.cpp | 34 +++++++++++++++++++++++++++++++++- 5 files changed, 79 insertions(+), 13 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index 01685f7ed..d08205454 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -39,6 +39,11 @@ Breaking Changes: Other changes: +- InputText: Fixed a tricky edge case, ensuring value is always written back on the + frame where IsItemDeactivated() returns true, in order to allow usage without user + retaining underlying data. While we don't really want to encourage user not retaining + underlying data, in the absence of a "late commit" behavior/flag we understand it may + be desirable to take advantage of this trick. (#4714) - Backends: OpenGL3: Fixed GL loader crash when GL_VERSION returns NULL. (#6154, #4445, #3530) - Backends: GLFW: Fixed key modifiers handling on secondary viewports. (#6248, #6034) [@aiekick] - Examples: Windows: Added 'misc/debuggers/imgui.natstepfilter' file to all Visual Studio projects, diff --git a/imgui.cpp b/imgui.cpp index bba56c799..e6614ccbe 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -3587,6 +3587,7 @@ void ImGui::Shutdown() g.ClipboardHandlerData.clear(); g.MenusIdSubmittedThisFrame.clear(); g.InputTextState.ClearFreeMemory(); + g.InputTextDeactivatedState.ClearFreeMemory(); g.SettingsWindows.clear(); g.SettingsHandlers.clear(); @@ -3759,13 +3760,23 @@ void ImGui::SetActiveID(ImGuiID id, ImGuiWindow* window) { ImGuiContext& g = *GImGui; - // While most behaved code would make an effort to not steal active id during window move/drag operations, - // we at least need to be resilient to it. Cancelling the move is rather aggressive and users of 'master' branch - // may prefer the weird ill-defined half working situation ('docking' did assert), so may need to rework that. - if (g.MovingWindow != NULL && g.ActiveId == g.MovingWindow->MoveId) + // Clear previous active id + if (g.ActiveId != 0) { - IMGUI_DEBUG_LOG_ACTIVEID("SetActiveID() cancel MovingWindow\n"); - g.MovingWindow = NULL; + // While most behaved code would make an effort to not steal active id during window move/drag operations, + // we at least need to be resilient to it. Canceling the move is rather aggressive and users of 'master' branch + // may prefer the weird ill-defined half working situation ('docking' did assert), so may need to rework that. + if (g.MovingWindow != NULL && g.ActiveId == g.MovingWindow->MoveId) + { + IMGUI_DEBUG_LOG_ACTIVEID("SetActiveID() cancel MovingWindow\n"); + g.MovingWindow = NULL; + } + + // This could be written in a more general way (e.g associate a hook to ActiveId), + // but since this is currently quite an exception we'll leave it as is. + // One common scenario leading to this is: pressing Key ->NavMoveRequestApplyResult() -> ClearActiveId() + if (g.InputTextState.ID == g.ActiveId) + InputTextDeactivateHook(g.ActiveId); } // Set active id @@ -3840,11 +3851,17 @@ void ImGui::MarkItemEdited(ImGuiID id) // This marking is solely to be able to provide info for IsItemDeactivatedAfterEdit(). // ActiveId might have been released by the time we call this (as in the typical press/release button behavior) but still need to fill the data. ImGuiContext& g = *GImGui; - IM_ASSERT(g.ActiveId == id || g.ActiveId == 0 || g.DragDropActive); - IM_UNUSED(id); // Avoid unused variable warnings when asserts are compiled out. + if (g.ActiveId == id || g.ActiveId == 0) + { + g.ActiveIdHasBeenEditedThisFrame = true; + g.ActiveIdHasBeenEditedBefore = true; + } + + // We accept a MarkItemEdited() on drag and drop targets (see https://github.com/ocornut/imgui/issues/1875#issuecomment-978243343) + // We accept 'ActiveIdPreviousFrame == id' for InputText() returning an edit after it has been taken ActiveId away (#4714) + IM_ASSERT(g.DragDropActive || g.ActiveId == id || g.ActiveId == 0 || g.ActiveIdPreviousFrame == id); + //IM_ASSERT(g.CurrentWindow->DC.LastItemId == id); - g.ActiveIdHasBeenEditedThisFrame = true; - g.ActiveIdHasBeenEditedBefore = true; g.LastItemData.StatusFlags |= ImGuiItemStatusFlags_Edited; } @@ -9293,7 +9310,7 @@ bool ImGui::ItemAdd(const ImRect& bb, ImGuiID id, const ImRect* nav_bb_arg, ImGu // return false; const bool is_rect_visible = bb.Overlaps(window->ClipRect); if (!is_rect_visible) - if (id == 0 || (id != g.ActiveId && id != g.NavId)) + if (id == 0 || (id != g.ActiveId && id != g.ActiveIdPreviousFrame && id != g.NavId)) if (!g.LogEnabled) return false; diff --git a/imgui.h b/imgui.h index 43dfc47a9..a2522aef6 100644 --- a/imgui.h +++ b/imgui.h @@ -23,7 +23,7 @@ // Library Version // (Integer encoded as XYYZZ for use in #if preprocessor conditionals, e.g. '#if IMGUI_VERSION_NUM > 12345') #define IMGUI_VERSION "1.89.5 WIP" -#define IMGUI_VERSION_NUM 18941 +#define IMGUI_VERSION_NUM 18942 #define IMGUI_HAS_TABLE /* diff --git a/imgui_internal.h b/imgui_internal.h index 627f7ab7f..d74b89ed3 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -130,6 +130,7 @@ struct ImGuiDataVarInfo; // Variable information (e.g. to avoid style struct ImGuiDataTypeInfo; // Type information associated to a ImGuiDataType enum struct ImGuiGroupData; // Stacked storage data for BeginGroup()/EndGroup() struct ImGuiInputTextState; // Internal state of the currently focused/edited text input box +struct ImGuiInputTextDeactivateData;// Short term storage to backup text of a deactivating InputText() while another is stealing active id struct ImGuiLastItemData; // Status storage for last submitted items struct ImGuiLocEntry; // A localization entry. struct ImGuiMenuColumns; // Simple column measurement, currently used for MenuItem() only @@ -1048,6 +1049,15 @@ struct IMGUI_API ImGuiMenuColumns void CalcNextTotalWidth(bool update_offsets); }; +// Internal temporary state for deactivating InputText() instances. +struct IMGUI_API ImGuiInputTextDeactivatedState +{ + ImGuiID ID; // widget id owning the text state (which just got deactivated) + ImVector TextA; // text buffer + + ImGuiInputTextDeactivatedState() { memset(this, 0, sizeof(*this)); } + void ClearFreeMemory() { ID = 0; TextA.clear(); } +}; // Internal state of the currently focused/edited text input box // For a given item ID, access with ImGui::GetInputTextState() struct IMGUI_API ImGuiInputTextState @@ -1935,6 +1945,7 @@ struct ImGuiContext // Widget state ImVec2 MouseLastValidPos; ImGuiInputTextState InputTextState; + ImGuiInputTextDeactivatedState InputTextDeactivatedState; ImFont InputTextPasswordFont; ImGuiID TempInputId; // Temporary text input when CTRL+clicking on a slider, etc. ImGuiColorEditFlags ColorEditOptions; // Store user options for color edit widgets @@ -3131,6 +3142,7 @@ namespace ImGui // InputText IMGUI_API bool InputTextEx(const char* label, const char* hint, char* buf, int buf_size, const ImVec2& size_arg, ImGuiInputTextFlags flags, ImGuiInputTextCallback callback = NULL, void* user_data = NULL); + IMGUI_API void InputTextDeactivateHook(ImGuiID id); IMGUI_API bool TempInputText(const ImRect& bb, ImGuiID id, const char* label, char* buf, int buf_size, ImGuiInputTextFlags flags); IMGUI_API bool TempInputScalar(const ImRect& bb, ImGuiID id, const char* label, ImGuiDataType data_type, void* p_data, const char* format, const void* p_clamp_min = NULL, const void* p_clamp_max = NULL); inline bool TempInputIsActive(ImGuiID id) { ImGuiContext& g = *GImGui; return (g.ActiveId == id && g.TempInputId == id); } diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index f25e2782e..326bbe1e8 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -3999,6 +3999,21 @@ static void InputTextReconcileUndoStateAfterUserCallback(ImGuiInputTextState* st p[i] = ImStb::STB_TEXTEDIT_GETCHAR(state, first_diff + i); } +// As InputText() retain textual data and we currently provide a path for user to not retain it (via local variables) +// we need some form of hook to reapply data back to user buffer on deactivation frame. (#4714) +// It would be more desirable that we discourage users from taking advantage of the "user not retaining data" trick, +// but that more likely be attractive when we do have _NoLiveEdit flag available. +void ImGui::InputTextDeactivateHook(ImGuiID id) +{ + ImGuiContext& g = *GImGui; + ImGuiInputTextState* state = &g.InputTextState; + if (id == 0 || state->ID != id) + return; + g.InputTextDeactivatedState.ID = state->ID; + g.InputTextDeactivatedState.TextA.resize(state->CurLenA + 1); + memcpy(g.InputTextDeactivatedState.TextA.Data, state->TextA.Data, state->CurLenA + 1); +} + // Edit a string of text // - buf_size account for the zero-terminator, so a buf_size of 6 can hold "Hello" but not "Hello!". // This is so we can easily call InputText() on static arrays using ARRAYSIZE() and to match @@ -4113,6 +4128,9 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_ state = &g.InputTextState; state->CursorAnimReset(); + // Backup state of deactivating item so they'll have a chance to do a write to output buffer on the same frame they report IsItemDeactivatedAfterEdit (#4714) + InputTextDeactivateHook(state->ID); + // Take a copy of the initial buffer value (both in original UTF-8 format and converted to wchar) // From the moment we focused we are ignoring the content of 'buf' (unless we are in read-only mode) const int buf_len = (int)strlen(buf); @@ -4525,6 +4543,7 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_ // Push records into the undo stack so we can CTRL+Z the revert operation itself apply_new_text = state->InitialTextA.Data; apply_new_text_length = state->InitialTextA.Size - 1; + value_changed = true; ImVector w_text; if (apply_new_text_length > 0) { @@ -4638,10 +4657,24 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_ { apply_new_text = state->TextA.Data; apply_new_text_length = state->CurLenA; + value_changed = true; } } } + // Handle reapplying final data on deactivation (see InputTextDeactivateHook() for details) + if (g.InputTextDeactivatedState.ID == id) + { + if (g.ActiveId != id && IsItemDeactivatedAfterEdit() && !is_readonly) + { + apply_new_text = g.InputTextDeactivatedState.TextA.Data; + apply_new_text_length = g.InputTextDeactivatedState.TextA.Size - 1; + value_changed |= (strcmp(g.InputTextDeactivatedState.TextA.Data, buf) != 0); + //IMGUI_DEBUG_LOG("InputText(): apply Deactivated data for 0x%08X: \"%.*s\".\n", id, apply_new_text_length, apply_new_text); + } + g.InputTextDeactivatedState.ID = 0; + } + // Copy result to user buffer. This can currently only happen when (g.ActiveId == id) if (apply_new_text != NULL) { @@ -4669,7 +4702,6 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_ // If the underlying buffer resize was denied or not carried to the next frame, apply_new_text_length+1 may be >= buf_size. ImStrncpy(buf, apply_new_text, ImMin(apply_new_text_length + 1, buf_size)); - value_changed = true; } // Release active ID at the end of the function (so e.g. pressing Return still does a final application of the value)