Browse Source

InputText: Fixed issue when activating a ReadOnly field when the underlying value is being modified. (#8242)

ocornut 8 months ago
parent
commit
e900571ac2
3 changed files with 37 additions and 8 deletions
  1. 2 0
      docs/CHANGELOG.txt
  2. 1 1
      imgui.h
  3. 34 7
      imgui_widgets.cpp

+ 2 - 0
docs/CHANGELOG.txt

@@ -45,6 +45,8 @@ Other changes:
 
 
 - InputText: Fixed a bug where character replacements performed from a callback
 - InputText: Fixed a bug where character replacements performed from a callback
   were not applied when pasting from clipbard. (#8229)
   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
 - Drags: Added ImGuiSliderFlags_NoSpeedTweaks flag to disable keyboard
   modifiers altering the tweak speed. Useful if you want to alter tweak speed
   modifiers altering the tweak speed. Useful if you want to alter tweak speed
   yourself based on your own logic. (#8223)
   yourself based on your own logic. (#8223)

+ 1 - 1
imgui.h

@@ -29,7 +29,7 @@
 // Library Version
 // Library Version
 // (Integer encoded as XYYZZ for use in #if preprocessor conditionals, e.g. '#if IMGUI_VERSION_NUM >= 12345')
 // (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       "1.91.7 WIP"
-#define IMGUI_VERSION_NUM   19161
+#define IMGUI_VERSION_NUM   19162
 #define IMGUI_HAS_TABLE
 #define IMGUI_HAS_TABLE
 
 
 /*
 /*

+ 34 - 7
imgui_widgets.cpp

@@ -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_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_make_active = (user_clicked || user_scroll_finish || input_requested_by_nav);
     const bool init_state = (init_make_active || user_scroll_active);
     const bool init_state = (init_make_active || user_scroll_active);
+    bool readonly_swapped_text_data = false;
     if (init_reload_from_user_buf)
     if (init_reload_from_user_buf)
     {
     {
         int new_len = (int)strlen(buf);
         int new_len = (int)strlen(buf);
         state->WantReloadUserBuf = false;
         state->WantReloadUserBuf = false;
         InputTextReconcileUndoState(state, state->TextA.Data, state->TextLen, buf, new_len);
         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;
         state->TextLen = new_len;
         memcpy(state->TextA.Data, buf, state->TextLen + 1);
         memcpy(state->TextA.Data, buf, state->TextLen + 1);
         state->Stb->select_start = state->ReloadSelectionStart;
         state->Stb->select_start = state->ReloadSelectionStart;
@@ -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
         // 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?
         // 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);
         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;
             recycle_state = false;
 
 
         // Start edition
         // Start edition
         state->ID = id;
         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);
         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
         // Find initial scroll position for right alignment
         state->Scroll = ImVec2(0.0f, 0.0f);
         state->Scroll = ImVec2(0.0f, 0.0f);
@@ -4610,6 +4614,21 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
             state->Scroll.y = draw_window->Scroll.y;
             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)
     // 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)
     if (g.ActiveId == id && state == NULL)
         ClearActiveID();
         ClearActiveID();
@@ -5008,10 +5027,11 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
                     callback_data.UserData = callback_user_data;
                     callback_data.UserData = callback_user_data;
 
 
                     // FIXME-OPT: Undo stack reconcile needs a backup of the data until we rework API, see #7925
                     // 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);
                     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.EventKey = event_key;
                     callback_data.Buf = callback_buf;
                     callback_data.Buf = callback_buf;
                     callback_data.BufTextLen = state->TextLen;
                     callback_data.BufTextLen = state->TextLen;
@@ -5142,7 +5162,7 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
         // - Measure text height (for scrollbar)
         // - 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)
         // 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.
         // 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;
         const char* text_end = text_begin + state->TextLen;
         ImVec2 cursor_offset, select_start_offset;
         ImVec2 cursor_offset, select_start_offset;
 
 
@@ -5304,6 +5324,13 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
     if (is_password && !is_displaying_hint)
     if (is_password && !is_displaying_hint)
         PopFont();
         PopFont();
 
 
+    if (readonly_swapped_text_data)
+    {
+        IM_ASSERT(state->TextA.Data == buf);
+        state->TextA.Size = 0;
+        state->TextA.Data = NULL;
+    }
+
     if (is_multiline)
     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)...
         // For focus requests to work on our multiline we need to ensure our child ItemAdd() call specifies the ImGuiItemFlags_Inputable (see #4761, #7870)...