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

InputText CallbackResize does not work with EnterReturnsTrue if user code doesn't persist their string #3009

Closed
PeterJohnson opened this issue Feb 1, 2020 · 7 comments

Comments

@PeterJohnson
Copy link

Version/Branch of Dear ImGui:

Version: 1.72b (17202)
Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_opengl3.cpp + imgui_impl_glfw.cpp
Operating System: Linux

My Issue:

Using ImGuiInputTextFlags_CallbackResize (e.g. as used in imgui_stdlib for outputting to std::string) does not interoperate correctly with the ImGuiInputTextFlags_EnterReturnsTrue flag. When the EnterReturnsTrue flag is specified, the returned string data is correct, but the length is incorrect--it's the original length instead of the new length (if longer), and the original length-1 (if shorter).

Standalone, minimal, complete and verifiable example:

#include "imgui_stdlib.h"
ImGui::Begin("Example Bug");
std::string myInput = "abcd";
if (ImGui::InputText("bug", &myInput, ImGuiInputTextFlags_EnterReturnsTrue))
  std::cout << "result: '" << myInput << "'\n";
ImGui::End();

Edit the string to make it shorter or longer and hit Enter. If the input text is longer (e.g. "hello" is input), the result will be truncated at length=4 (e.g. "hell"). If the input text is shorter, the result will be length=3 and contain a mix of input text and original text depending on the input text length.

@rokups
Copy link
Contributor

rokups commented Feb 3, 2020

I am unable to reproduce the issue. Could you please test latest version from here? Also please clarify what field of ImGuiInputTextCallbackData you refer to by saying "size" "length"? data->BufTextLen is always equal to length of entered string.

@ocornut ocornut added the bug label Feb 3, 2020
@ocornut
Copy link
Owner

ocornut commented Feb 3, 2020

I can repro this and as mentioned the string data is correct but std::string Size member isn't:

printf("1 result: '%s'\n", myInput.c_str());
std::cout << "2 result: '" << myInput << "'\n";
1 result: 'hello'
2 result: 'hell'

EDIT some/all std::string implementation maintain Size which I presume is relied on by the << operator instead of the null terminator, so using printf you wouldn't notice the bug right away.

@ocornut
Copy link
Owner

ocornut commented Feb 4, 2020

Note that the bug doesn't happen if you actually preserve the input buffer as you are generally meant to, e.g.
std::string myInput = "abcd";

InputText is specific because it owns a copy of the data, it allows running without persisting user storage, but few people use that technique and this explain why nobody reported this issue before.

@ocornut ocornut changed the title InputText CallbackResize does not work with EnterReturnsTrue InputText CallbackResize does not work with EnterReturnsTrue if user code doesn't persist their string Feb 4, 2020
ocornut added a commit that referenced this issue Feb 4, 2020
@ocornut
Copy link
Owner

ocornut commented Feb 4, 2020

This is now fixed, however:

Using CallbackResize along with no persisting user storage (as implied by your repro) will trigger a likely allocation every single frame when editing the string. This is not ideal.

It's actually surprising that your repro doesn't save the string accross frames and I was wondering if it was how your real code was actually laid out? If it was NOT setup like that (= you actually persisted your copy of the string) and you encountered the bug it means we may have another unfixed bug to find.

Let us know!

@PeterJohnson
Copy link
Author

PeterJohnson commented Feb 4, 2020

Thanks for the fix! No, I'm not persisting the string, so I don't think there's another bug.

A bit of background: in the actual application, I'm interfacing with an underlying API that looks something like this:

int GetNumHandles();
const char *GetString(int handle, int* length);
void SetString(int handle, const char* data, int length);

And then I have a loop over the handles like:

for (int i=0; i<GetNumHandles(); ++i) {
  GetString(...);
  if (ImGui::InputText(...)) {
    SetString(...);
  }
}

So I have to copy the buffer contents. This works if I have a static char[] that I'm copying the data into, but I'd like to support unlimited size. I think I've tested (but need to double-check) that a static std::string doesn't work in this scenario either. About the only for-sure workaround I can think of would have been to create my own array on the GUI side so I can persist a copy of every string.

@ocornut
Copy link
Owner

ocornut commented Feb 4, 2020

Ideally it would be good to be able to access that underlying data with a less restrictive API...

But another thing you could do:

  1. Grab the InputText() identifier with ImGui::GetID(label).
#include "imgui_internal.h"
bool is_input_text_active = (g.ActiveId == id) && (g.InputTextState.ID == id)

Then if the bool is active, perform the copy, otherwise pass a raw char* to InputText() without any copy.

@ocornut
Copy link
Owner

ocornut commented Feb 9, 2020

I'll keep a note that I would like to solve this usage pattern better or more naturally.
Will post here when I do, in the meanwhile I'll close this issue.

@ocornut ocornut closed this as completed Feb 9, 2020
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