Skip to content

Commit

Permalink
InputText: Fixed issue when activating a ReadOnly field when the unde…
Browse files Browse the repository at this point in the history
…rlying value is being modified. (#8242)
  • Loading branch information
ocornut committed Dec 18, 2024
1 parent f31d530 commit e900571
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 8 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ Other changes:

- InputText: Fixed a bug where character replacements performed from a callback
were not applied when pasting from clipbard. (#8229)
- InputText: Fixed issue when activating a ReadOnly field when the underlying
value is being modified. (#8242)
- Drags: Added ImGuiSliderFlags_NoSpeedTweaks flag to disable keyboard
modifiers altering the tweak speed. Useful if you want to alter tweak speed
yourself based on your own logic. (#8223)
Expand Down
2 changes: 1 addition & 1 deletion imgui.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
// Library Version
// (Integer encoded as XYYZZ for use in #if preprocessor conditionals, e.g. '#if IMGUI_VERSION_NUM >= 12345')
#define IMGUI_VERSION "1.91.7 WIP"
#define IMGUI_VERSION_NUM 19161
#define IMGUI_VERSION_NUM 19162
#define IMGUI_HAS_TABLE

/*
Expand Down
41 changes: 34 additions & 7 deletions imgui_widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4507,12 +4507,13 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
const bool init_changed_specs = (state != NULL && state->Stb->single_line != !is_multiline); // state != NULL means its our state.
const bool init_make_active = (user_clicked || user_scroll_finish || input_requested_by_nav);
const bool init_state = (init_make_active || user_scroll_active);
bool readonly_swapped_text_data = false;
if (init_reload_from_user_buf)
{
int new_len = (int)strlen(buf);
state->WantReloadUserBuf = false;
InputTextReconcileUndoState(state, state->TextA.Data, state->TextLen, buf, new_len);
state->TextA.resize(buf_size + 1); // we use +1 to make sure that .Data is always pointing to at least an empty string.
state->TextA.resize(buf_size + 1); // we use +1 to make sure that .Data is always pointing to at least an empty string.
state->TextLen = new_len;
memcpy(state->TextA.Data, buf, state->TextLen + 1);
state->Stb->select_start = state->ReloadSelectionStart;
Expand All @@ -4537,14 +4538,17 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
// Preserve cursor position and undo/redo stack if we come back to same widget
// FIXME: Since we reworked this on 2022/06, may want to differentiate recycle_cursor vs recycle_undostate?
bool recycle_state = (state->ID == id && !init_changed_specs);
if (recycle_state && (state->TextLen != buf_len || (strncmp(state->TextA.Data, buf, buf_len) != 0)))
if (recycle_state && (state->TextLen != buf_len || (state->TextA.Data == NULL || strncmp(state->TextA.Data, buf, buf_len) != 0)))
recycle_state = false;

// Start edition
state->ID = id;
state->TextA.resize(buf_size + 1); // we use +1 to make sure that .Data is always pointing to at least an empty string.
state->TextLen = (int)strlen(buf);
memcpy(state->TextA.Data, buf, state->TextLen + 1);
if (!is_readonly)
{
state->TextA.resize(buf_size + 1); // we use +1 to make sure that .Data is always pointing to at least an empty string.
memcpy(state->TextA.Data, buf, state->TextLen + 1);
}

// Find initial scroll position for right alignment
state->Scroll = ImVec2(0.0f, 0.0f);
Expand Down Expand Up @@ -4610,6 +4614,21 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
state->Scroll.y = draw_window->Scroll.y;
}

if (g.ActiveId == id && is_readonly)
{
// FIXME: Refresh buffer because cursor/selection code uses that data (see repro #8242)
// The "simple" way would be to copy buf into state->TextA, like in the block above.
// But because we like to live dangerously, we do a little swap....
// Removing the swap and only doing a TextA.clear() is a way to identify who's using TextA.Data.
state->TextLen = (int)strlen(buf);
state->TextA.clear();
state->TextA.Data = buf; // Ouch
state->TextA.Size = state->TextLen + 1;
readonly_swapped_text_data = true; // Need to always ensure that every code path below lead to this being handled
//state->TextA.resize(buf_size + 1);
//memcpy(state->TextA.Data, buf, state->TextLen + 1);
}

// 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();
Expand Down Expand Up @@ -5008,10 +5027,11 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
callback_data.UserData = callback_user_data;

// FIXME-OPT: Undo stack reconcile needs a backup of the data until we rework API, see #7925
char* callback_buf = is_readonly ? buf : state->TextA.Data;

state->CallbackTextBackup.resize(state->TextLen + 1);
memcpy(state->CallbackTextBackup.Data, state->TextA.Data, state->TextLen + 1);
memcpy(state->CallbackTextBackup.Data, callback_buf, state->TextLen + 1);

char* callback_buf = is_readonly ? buf : state->TextA.Data;
callback_data.EventKey = event_key;
callback_data.Buf = callback_buf;
callback_data.BufTextLen = state->TextLen;
Expand Down Expand Up @@ -5142,7 +5162,7 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
// - Measure text height (for scrollbar)
// We are attempting to do most of that in **one main pass** to minimize the computation cost (non-negligible for large amount of text) + 2nd pass for selection rendering (we could merge them by an extra refactoring effort)
// FIXME: This should occur on buf_display but we'd need to maintain cursor/select_start/select_end for UTF-8.
const char* text_begin = state->TextA.Data;
const char* text_begin = buf_display;
const char* text_end = text_begin + state->TextLen;
ImVec2 cursor_offset, select_start_offset;

Expand Down Expand Up @@ -5304,6 +5324,13 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
if (is_password && !is_displaying_hint)
PopFont();

if (readonly_swapped_text_data)
{
IM_ASSERT(state->TextA.Data == buf);
state->TextA.Size = 0;
state->TextA.Data = NULL;
}

if (is_multiline)
{
// For focus requests to work on our multiline we need to ensure our child ItemAdd() call specifies the ImGuiItemFlags_Inputable (see #4761, #7870)...
Expand Down

0 comments on commit e900571

Please sign in to comment.