Browse Source

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.
pull/6254/head
ocornut 3 years ago
parent
commit
5a2b1e8482
  1. 5
      docs/CHANGELOG.txt
  2. 39
      imgui.cpp
  3. 2
      imgui.h
  4. 12
      imgui_internal.h
  5. 34
      imgui_widgets.cpp

5
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,

39
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;

2
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
/*

12
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<char> 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); }

34
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<ImWchar> 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)

Loading…
Cancel
Save