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

InsertChars with InputTextCallback resulting in assertion failure if resized. #4784

Closed
cajallen opened this issue Dec 4, 2021 · 3 comments
Closed

Comments

@cajallen
Copy link

cajallen commented Dec 4, 2021

Version/Branch of Dear ImGui:

Version: 1.85
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_opengl3.cpp + imgui_impl_glfw.cpp + imgui_stdlib.h

My Issue/Question:
Calling InsertChars() in a callback results in an assertion thrown.

// - To modify the text buffer in a callback, prefer using the InsertChars() / DeleteChars() function. InsertChars() will take care of calling the resize callback if necessary.
void ImGuiInputTextCallbackData::InsertChars(int pos, const char* new_text, const char* new_text_end)
{
    const bool is_resizable = (Flags & ImGuiInputTextFlags_CallbackResize) != 0;
    const int new_text_len = new_text_end ? (int)(new_text_end - new_text) : (int)strlen(new_text);
    if (new_text_len + BufTextLen >= BufSize)
    {
        if (!is_resizable)
            return;

        ImGuiContext& g = *GImGui;
        ImGuiInputTextState* edit_state = &g.InputTextState;
        IM_ASSERT(edit_state->ID != 0 && g.ActiveId == edit_state->ID);
        IM_ASSERT(Buf == edit_state->TextA.Data);
        int new_buf_size = BufTextLen + ImClamp(new_text_len * 4, 32, ImMax(256, new_text_len)) + 1;
        edit_state->TextA.reserve(new_buf_size + 1);
        Buf = edit_state->TextA.Data;
        BufSize = edit_state->BufCapacityA = new_buf_size;
    }

    if (BufTextLen != pos)
        memmove(Buf + pos + new_text_len, Buf + pos, (size_t)(BufTextLen - pos));
    memcpy(Buf + pos, new_text, (size_t)new_text_len * sizeof(char));
    Buf[BufTextLen + new_text_len] = '\0';

    if (CursorPos >= pos)
        CursorPos += new_text_len;
    SelectionStart = SelectionEnd = CursorPos;
    BufDirty = true;
    BufTextLen += new_text_len;
}
                    callback_data.Buf = callback_buf;
                    // Call user code
                    callback(&callback_data);

                    // Read back what user may have modified
                    IM_ASSERT(callback_data.Buf == callback_buf);  // Invalid to modify those fields

How is this supposed to work?
If it's valid to use InsertChars to modify text in a callback, and InsertChars sets Buf, then why is it invalid for Buf to be set in the callback?

Standalone, minimal, complete and verifiable example:

struct Console {
        bool input_reset;
	vector<string> input_history;
	vector<string>::iterator input_history_cursor;

	Console() {
		input_history = {"testing an input longer than 16 characters"};
		input_history_cursor = input_history.end();
	}
};

int console_input_callback(ImGuiInputTextCallbackData* data) {
	if (data->EventFlag == ImGuiInputTextFlags_CallbackHistory) {
		if (data->EventKey == ImGuiKey_UpArrow) {
			if (console.input_history_cursor == console.input_history.begin())
				console.input_history_cursor = console.input_history.end();
			else
				console.input_history_cursor--;

			data->DeleteChars(0, data->BufTextLen);

			if (console.input_history_cursor != console.input_history.end()) {
				data->InsertChars(0, console.input_history_cursor->c_str());
				data->SelectAll();
			}
		}
	} else if (data->EventFlag == ImGuiInputTextFlags_CallbackAlways) {
		if (console.input_reset) {
			data->DeleteChars(0, data->BufTextLen);
			console.input_reset = false;
		}
	}
}

void show_console(bool* open) {
	if (ImGui::Begin("Console", open)) {
		ImGui::SetNextItemWidth(ImGui::GetContentRegionAvail().x);
		static string console_input;
		auto flags = ImGuiInputTextFlags_EnterReturnsTrue | ImGuiInputTextFlags_CallbackHistory;
		if (ImGui::InputText("##ConsoleInput", &console_input, flags, console_input_callback)) {
			console.input_history.push_back(console_input);
	                console.input_history_cursor = console.input_history.end();
	                console.input_reset = true;
			ImGui::SetActiveID(ImGui::GetID("##ConsoleInput"), ImGui::GetCurrentWindow());
		}
	}
	ImGui::End();
}

This example might not show up in the Console demo, because as I understand, Buf is only resized when inputs are typed, so to set the contents to something you've previously entered isn't going to resize.

@ocornut
Copy link
Owner

ocornut commented Dec 5, 2021

Version: 1.85

Seems like a regression added last week by 5ac25e7, and afaik the code you quoted is not from 1.85 but from a more recent version. Can you confirm? I'll push a fix for current version.

ocornut added a commit that referenced this issue Dec 5, 2021
@ocornut
Copy link
Owner

ocornut commented Dec 5, 2021

Fix should be eea8361

@ocornut ocornut closed this as completed Dec 5, 2021
@cajallen
Copy link
Author

cajallen commented Dec 5, 2021

Version: 1.85

Seems like a regression added last week by 5ac25e7, and afaik the code you quoted is not from 1.85 but from a more recent version. Can you confirm? I'll push a fix for current version.

Yeah, my mistake, I forgot I switched when I moved to the docking branch. Thanks for the fix!

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