Allow for dynamic re-allocation of internal buffer while user is still typing in InputText?

[LAK132]

I'm currently running into an issue with using a ImGuiTextEditCallback for dynamic allocation of memory for an InputText. It seems to me that in order for the internal buffer to resize the user has to stop editing. This is extremely frustrating!

Would it be reasonable to include something like

if (edit_state.Text.size() < buf_size + 1)
    edit_state.Text.resize(buf_size + 1);

Inside of

if (is_editable)
{
    edit_state.TempTextBuffer.resize(edit_state.Text.Size * 4);
    ImTextStrToUtf8(edit_state.TempTextBuffer.Data, edit_state.TempTextBuffer.Size, edit_state.Text.Data, NULL);
}

I have tested that this works, and it is a lot smoother than having to hit enter all the time. I'm not 100% sure of the performance impact though

My current use case (not the best performance but it's a WIP):

struct ImGuiStringTextCallbackData
{
    void *UserData;
    ImGuiTextEditCallback callback;
    string* str;
};

int ImGuiStringTextCallback(ImGuiTextEditCallbackData *data)
{
    ImGuiStringTextCallbackData* udata = static_cast<ImGuiStringTextCallbackData*>(data->UserData);

    string* str = udata != NULL ? udata->str : nullptr;
    if (str != nullptr && data->BufTextLen >= data->BufSize - 2)
    {
        str->resize((data->BufSize + 1) * 2);
    }

    if (udata->callback != NULL) 
    {
        data->UserData = udata->UserData;
        return udata->callback(data);
    }
    return 0;
}

IMGUI_API bool InputText(const char *label, string *str, ImGuiInputTextFlags flags, ImGuiTextEditCallback callback, void *user_data)
{
    IM_ASSERT(!(flags & ImGuiInputTextFlags_Multiline)); // call InputTextMultiline()

    ImGuiStringTextCallbackData data;
    data.UserData = user_data;
    data.callback = callback;
    data.str = str;

    vector<char> vstr(str->begin(), str->end());
    if (vstr[vstr.size()-1] != 0)
        vstr.push_back(0);
    
    bool rtn = InputTextEx(label, &vstr[0], vstr.size(), ImVec2(0, 0), flags | ImGuiInputTextFlags_CallbackAlways, ImGuiStringTextCallback, &data);

    if (vstr.size() > str->size()) // just to be safe
        str->resize(vstr.size());
    size_t i = 0;
    for (char& c : vstr)
        (*str)[i++] = c;
    return rtn;
}

This InputText takes a string* instead of a char* and resizes when needed. The rest of my code uses strings a lot, so converting over to char* just for ImGui is a pain

Edit: I can do a pull request if this is wanted

[LAK132]

Alternatively an ImGuiInputTextFlags_ResizeTextBufferAlways style flag could be nice
ie:

if (is_editable)
{
    if (((flags & ImGuiInputTextFlags_ResizeTextBufferAlways) != 0) && edit_state.Text.size() < buf_size + 1)
        edit_state.Text.resize(buf_size + 1);
    edit_state.TempTextBuffer.resize(edit_state.Text.Size * 4);
    ImTextStrToUtf8(edit_state.TempTextBuffer.Data, edit_state.TempTextBuffer.Size, edit_state.Text.Data, NULL);
}

[ocornut]

1- The solution wouldn't work with clipboard pasting afaik?

2- You have a intermediary vector<char> involved which incurr a full alloc+copy+free every frame, whereas you could do the same directly on the string? I am not sure why the vector is necessary at all.

This is recurring issue, see #1008, #1443. There's probably a nice solution awaiting in the form of a change to the callback design.. I personally haven't taken the time to sit and figure it out yet. I'll give it a bit of time now.

[LAK132]

whereas you could do the same directly on the string?

std::string isn't guaranteed to have contiguous memory until C++11, and the pointer is invalidated after the resize. My method to overcome this issue is messy to say the least, but I desperately needed it working

Another option I had considered (but would require a fair bit more work) would be allowing the passed string and the internal buffer to be resized from the callback, rather than having to wait until the next time InputText is called. The buffer is already passed to the callback, so allowing it to be reallocated shouldn't be too hard? If I find the time I'll see if I can hack something together to test the idea

[ocornut]

I’m working on this, i’ll probably have a patch tomorrow.

[ocornut]

I'm going to push and describe my solution right below this post, but first I wanted to react on:

std::string isn't guaranteed to have contiguous memory until C++11, and the pointer is invalidated after the resize.

Pointer invalidation is handled by the solution described below. About contiguous memory guarantee: are you aware of any pre C++11 implementation that actually used non-contiguous strings? To honor c_str() such implementation would need to hold on a second pointer and recreate a fully contiguous string, which the added headache that the string wouldn't necessarily have a chance to release the later. To me it seems unlikely that any implementation ever did use non-contiguous storage for std::string.

It is just that the early standard wasn't overtly explicit about that requirement but this is where everyone went anyway and they decided to standardize it later.

Some ref:
https://debugfailure.wordpress.com/2009/11/20/is-stdstrings-storage-contiguous/
https://stackoverflow.com/questions/1986966/does-s0-point-to-contiguous-characters-in-a-stdstring
https://stackoverflow.com/questions/33124479/does-stdstring-need-to-store-its-character-in-a-contiguous-piece-of-memory/33124544

Either way, in core imgui we are solely providing a resize callback (no actual std::string support) and it would be fine if in theory the imgui_stl.h wrapper requires C++11.

[ocornut]

I have made various changes to InputText now and among there pushed what I propose to be a solution for this much requested issue.

I have added a ImGuiInputTextFlags_CallbackResize parameter to InputText() which require also specifying a callback function.

The callback function will be called with ImGuiInputTextFlags_CallbackResize whenever the string needs to be resized. That allows us to handle proper resizing of std::string-like types even if the capacity doesn't change: several string types have their Size cached (instead of performing a strlen call). And that allows us to handle capacity change as well.

This is how a regular callback function would look like:

static int InputTextCallback(ImGuiInputTextCallbackData* data)
{
    if (data->EventFlag == ImGuiInputTextFlags_CallbackResize)
    {
        // Resize string callback
        std::string* str = (std::string*)data->UserData;
        IM_ASSERT(data->Buf == str->c_str());
        str->resize(data->BufTextLen);
        data->Buf = (char*)str->c_str();
    }
}

bool ImGui::InputText(const char* label, std::string* str, ImGuiInputTextFlags flags)
{
    flags |= ImGuiInputTextFlags_CallbackResize;
    return InputText(label, (char*)str->c_str(), str->capacity() + 1, flags, InputTextCallback, (void*)str);
}

The actual function in the imgui_stl wrapper is a little more complicated (a few more lines) because we handle chaining callbacks, so it is possible for the end-user to use the other callbacks (completion/history, etc.) which you may or not need.

misc/stl/imgui_stl.h
https://github.com/ocornut/imgui/blob/master/misc/stl/imgui_stl.h
misc/stl/imgui_stl.cpp
https://github.com/ocornut/imgui/blob/master/misc/stl/imgui_stl.cpp

FEEDBACK HIGHLY WELCOME!

[ratchetfreak]

It's possible that std::string::resize bypasses the geometric growth that is typically used for std::string::append. This can cause O(n²) complexity with many small appends.

Though that will be spread over O(n) frames (each frame doing a small number of resizes, which means O(n) complexity per frame.

[LAK132]

About contiguous memory guarantee: are you aware of any pre C++11 implementation that actually used non-contiguous strings? To honor c_str() such implementation would need to hold on a second pointer and recreate a fully contiguous string, which the added headache that the string wouldn't necessarily have a chance to release the later. To me it seems unlikely that any implementation ever did use non-contiguous storage for std::string.

Yeah this is why the standard was changed IIRC, but I'd still prefer to not rely on it. c_str() guarantees contiguous memory, but only until the string is modified (resized), at which point the returned pointer is invalidated and (prior to C++11) the memory may no longer be contiguous.
Not that that is important anymore : )

char* Buf; // Current text buffer // Read-write // [Resize] Can replace pointer

This is perfect! Thank you!

[ocornut]

Though that will be spread over O(n) frames (each frame doing a small number of resizes, which means O(n) complexity per frame.

With the current system at maximum we'll have 1 resize call a frame on user interaction, so it should be inconsequential.

Note that it is up to the user to implement that resize callback, so people may do a combination or reserve+resize if they deem necessary. Arguably we can improve imgui_stl to do so, thought on my test bed MS STL the string grow by a factor of 2-ish so I didn't push it further.

With this feature done I'd be happy to tag and release 1.63 but I'll probably need feedback from a few people who have used and tested the feature (and also making sure there are no InputText regression related to the changes).

[LAK132]

Only slightly on topic, but can we get a flag to make InputText return true when the user clicks out of the text box? It seems to write to the buffer like pressing enter does, but it doesn't trigger ifs in cases like this:

string &str = dir.u8string();
if (ImGui::InputText("", &str, ImGuiInputTextFlags_EnterReturnsTrue))
    dir = str.c_str();

This wasn't a big deal when working with c-strings, but it is an issue now that we're dealing with std::strings

[ocornut]

This wasn't a big deal when working with c-strings, but it is an issue now that we're dealing with std::strings

I'm not sure I understand your issue and why/how it relate to using std::string.
Could you clarify in more details?
With ImGuiInputTextFlags_EnterReturnsTrue true is returned on pressing Enter, otherwise when the buffer is modified. Why would you want a true return value when user clicks out of the text box?

Will probably expose IsItemValueChanged() as discussed in #2034 which would facilitate using ImGuiInputTextFlags_EnterReturnsTrue so that's probably the answer you need here.

[ocornut]

Now added IsItemEdited() which is equivalent to the return value of InputText() without the ImGuiInputTextFlags_EnterReturnsTrue flag, so you can test for both separately.

[LAK132]

Does IsItemEdited() return true every time the string is changed or only once the user has finished typing? ImGuiInputTextFlags_EnterReturnsTrue is nice because you know the user has finished with that input. In the example I showed before

string &str = dir.u8string();
if (ImGui::InputText("", &str, ImGuiInputTextFlags_EnterReturnsTrue))
    dir = str.c_str();

I don't want to be calling dir = str.c_str() until the user has completely finished... which doesn't work if they click out of the box (despite still modifying str)

[ocornut]

Does IsItemEdited() return true every time the string is changed

It return true every time the string is changed.

I don't want to be calling dir = str.c_str() until the user has completely finished... which doesn't work if they click out of the box (despite still modifying str)

You may use IsItemDeactivatedAfterEdit() for this purpose.
(*EDIT* was IsItemDeactivatedAfterChange() ..)

[LAK132]

That works, but it seems like a very round about way of doing what ImGuiInputTextFlags_EnterReturnsTrue is effectively already doing ¯\_(ツ)_/¯

[ocornut]

?? Isn’t the whole point that it is NOT doing the same as EnterReturnTrue and handle cases such as clicking out of the item?

[LAK132]

I mean in terms of return-true-when-done (which EnterReturnsTrue kind of does). Having to use a second function to get that for clicking out is annoying, especially when I don't see any use for InputText's return value other than telling when the user is done.
What I'm trying to say is that something like EnterReturnsTrue would be perfect if it could also return true when the user clicked out. I understand there's possible situations where this would be unwanted, so a FinishedReturnsTrue flag would be perfect (imho).
Something like 6c6ddd5 (edit: I didn't get to test this, it may not actually work...)

[ocornut]

EnterReturnsTrue still has its uses (e.g. for a console entry validation).
That suggested new flag would a one-off-custom duplicate of a now standard feature - would rather focus on the now standard function call?

[LAK132]

I understand that having another function makes it a little more universal, but I'd still prefer

string &str = dir.u8string();
if (ImGui::InputText("", &str))
    dir = str.c_str();

rather than

string &str = dir.u8string();
ImGui::InputText("", &str);
if (ImGui::IsItemDeactivatedAfterChange())
    dir = str.c_str();

I find the current default return behaviour of InputText pretty useless anyway, if I want to get an update every single time it updates I'd probably be using a callback. But that might just be me

[ocornut]

In many engines I've seen, it was useful and preferable to have live edit for strings be the default default, and the use of Return in an exception for situation like command processing.

What/how are you using it for?

[LAK132]

I'm using it for a file browser where the directories shouldn't be selected till you're sure you've got it right. I've also used it for things similar to HTML forms.

[ocornut]

I see. Rather than a flag to detect clicked/tabbed out, I think we should aim at a more generic feature of being able to disable the immediate writing-back of values (see #701).

The difference with IsItemDeactivatedAfterEdit() would be that the value is not written back to the buffer until validated or unfocused. Using IsItemDeactivatedAfterEdit() or an equivalent hypothetical FinishedReturnsTrue flag only works in your situation because you have an indirection between the edited value and your real value, and that indirection/duplication is something undesirable in the first place that we'd like to get rid of.

So I think the focus should be on something like #701 here.

[LAK132]

Would it be possible to toggle this with something like PushStyleVar? Say if you did ImGui::PushReturnBehaviour(ReturnBehaviourFlags_ReturnTrueOnChange) it made widgets return true every change, but ImGui::PushReturnBehaviour(ReturnBehaviourFlags_ReturnTrueOnComplete) made them only return true on completion. This way you don't have to do this for every single widget

[ocornut]

@LAK132 Could you open a separate issue for this specifically?
(Going to close this issue.)

[swebyte]

Currently I do not have C++11 and an older version of ImGui. Until I can upgrade I manage to solve it like this. But this is terrible, doing resize on every frame, but it works for my debug purposes for now.

bool InputText(const char* label, std::string* str, ImGuiInputTextFlags flags)
{
std::vector temp(str->begin(), str->end());
temp.resize(128);
auto ret = InputText(label, static_cast<char*>(&temp[0]), 128, flags);
if (ret)
{
*str = std::string(temp.data());
}
return ret;
}

[ocornut]

@vincentkarlsson94 Which compiler do you use that doesn't support C++11?
You could do it only if GImGui->ActiveId == ImGui::GetID(label) otherwise call InputText with the current length without any copy.

[swebyte]

My compiler support C++11 but our project does not fully support it yet. We are working on converting our huge project!

Thanks that is a neat solution to use for now!