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

Bugged ImGuiInputTextCallbackData::DeleteChars? #3454

Closed
ryobg opened this issue Sep 3, 2020 · 3 comments
Closed

Bugged ImGuiInputTextCallbackData::DeleteChars? #3454

ryobg opened this issue Sep 3, 2020 · 3 comments

Comments

@ryobg
Copy link

ryobg commented Sep 3, 2020

What happens if we call, for example, DeleteChars (0, 5) when our CursorPos is somewhere in the middle of the string i.e. less than byte_count, but after pos? For example 2.

Answer: Out of this function we are left with dangling CursorPos == -3. Which may or may not trigger an assert out of the callback, depending how more we messed up with this out of order state. Selection is also going off btw.

Excerpt of the more important piece of code:

void ImGuiInputTextCallbackData::DeleteChars(int pos, int bytes_count)
{
    IM_ASSERT(pos + bytes_count <= BufTextLen);
    // ...
     if (CursorPos + bytes_count >= pos)
        CursorPos -= bytes_count;
    else if (CursorPos >= pos)
        CursorPos = pos;
    SelectionStart = SelectionEnd = CursorPos;
    //..
}

Direct link:

void ImGuiInputTextCallbackData::DeleteChars(int pos, int bytes_count)

@ryobg
Copy link
Author

ryobg commented Sep 3, 2020

Possible solution is to clamp it within a safe range during the first condition? Say:

CursorPos = ImMax (CursorPos - bytes_count, 0);

@ocornut ocornut added the bug label Sep 3, 2020
ocornut added a commit that referenced this issue Sep 3, 2020
@ocornut
Copy link
Owner

ocornut commented Sep 3, 2020

Hello @ryobg, thanks a lot for reporting this!
I believe this code is incorrect and should always have been:

    if (CursorPos >= pos + bytes_count)
        CursorPos -= bytes_count;
    else if (CursorPos >= pos)
        CursorPos = pos;
   SelectionStart = SelectionEnd = CursorPos;

Fixed it now.

@ocornut ocornut closed this as completed Sep 3, 2020
@ryobg
Copy link
Author

ryobg commented Sep 3, 2020

Quick! Patched (its old) my code this way too. Good luck!

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

2 participants