Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editing the buffer in InputTextCallback desyncs the undo/redo stack #4947

Closed
JoshuaWebb opened this issue Jan 27, 2022 · 5 comments
Closed

Comments

@JoshuaWebb
Copy link
Contributor

Version/Branch of Dear ImGui:

Version: 1.87
Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_win32.cpp + imgui_impl_dx11.cpp (although it doesn't actually matter for this bug)
Operating System: Windows 10

My Issue/Question:

Editing an InputText via ImGuiInputTextCallbackData doesn't register any entries in the undo/redo stack.

Screenshots/Video

Repro steps:

  1. Using the provided sample placed into one of the example main.cpp
  2. Type some characters into Input
  3. Optionally type some characters into Replacement
  4. Make a selection in Input to replace
  5. Press Ctrl+R to trigger the replacement callback
  6. Press Ctrl+Z (Undo) once. The input text is now incorrect, and the cursor is wrong.
  7. Press Ctrl+Y (Redo) once. The application crashes with an assertion (Assertion failed: pos <= text_len, file ..\..\imgui_widgets.cpp, line 3667)

edit_callback_repro

Standalone, minimal, complete and verifiable example:

struct MyData {
    char InputBuf[256];
    char Replacement[256];

    MyData() { memset(this, 0, sizeof(*this)); }
};
int TextEditCallback(ImGuiInputTextCallbackData* data)
{
    ImGuiIO& io = ImGui::GetIO();

    MyData *my_data = (MyData*)data->UserData;

    switch (data->EventFlag)
    {
    case ImGuiInputTextFlags_CallbackAlways:
        {
            if (io.KeyCtrl && ImGui::IsKeyPressed('R')) {

                int pos = data->SelectionStart < data->SelectionEnd
                          ? data->SelectionStart
                          : data->SelectionEnd;
                int del_count = abs(data->SelectionEnd - data->SelectionStart);
                if (del_count > 0) {
                    data->DeleteChars(pos, del_count);
                }

                data->InsertChars(pos, my_data->Replacement);
            }

            break;
        }
    }
    return 0;
}

int main(int, char**)
{
        // .... 

        // 4. Repro
        {
            static MyData my_data;
            ImGui::SetNextWindowSize(ImVec2(380,120), ImGuiCond_Appearing);
            ImGui::Begin("Example");
            ImGui::InputText("Replacement", my_data.Replacement, IM_ARRAYSIZE(my_data.Replacement));
            ImGui::InputText("Input", my_data.InputBuf, IM_ARRAYSIZE(my_data.InputBuf), ImGuiInputTextFlags_CallbackAlways, &TextEditCallback, &my_data);
            ImGui::End();
        }

        // Rendering
        // ...
}
JoshuaWebb added a commit to JoshuaWebb/imgui that referenced this issue Jan 27, 2022
Implement a function to update the undo/redo state for an `InputText`
after a user callback has modified the buffer.

This function is a bit weird in that it uses the wide string from the
state for the old text and a utf8 buffer for the new text. It does this
because these are the formats that are already available at the time we
need them without requiring additional storage.
@JoshuaWebb
Copy link
Contributor Author

I've added a pull request (#4949) with the code I've used to fix this problem for myself in case that's useful.

@nukeulater
Copy link

nukeulater commented May 31, 2022

there's another way to trigger the stack desync.
here's an example from the demo console that's provided with ImGui

  1. type and execute a command (e.g. clear)
  2. hold Ctrl+Z (notice how the cursor moves back to the last character position of the command)
  3. try to type something, assert() gets triggered

ImGuiBuilder_x64_PsbP5pdkc2-gif

image

image

@ocornut
Copy link
Owner

ocornut commented May 31, 2022

Thank you @JoshuaWebb and @nukeulater for those details and PR, will try to look soon.

@ocornut ocornut added this to the v1.88 milestone May 31, 2022
ocornut added a commit that referenced this issue Jun 7, 2022
…efore reactivating item. (#4947) + Metrics: Added "InputText" section.
@ocornut
Copy link
Owner

ocornut commented Jun 7, 2022

I have pushed a first fix for the second issue (submitted by @nukeulater) which is a different one than the first one (fixed by #4949). The fix include a new tool in Metrics to visualize the undo state:

image

@ocornut
Copy link
Owner

ocornut commented Jun 8, 2022

Fixed with 530332d + amends a35e876

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants