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

Fix: Auto-resizing input buffer #291

Merged
merged 3 commits into from
Aug 10, 2022

Conversation

KinoxKlark
Copy link
Member

This has been issued in #278.

The auto-resizing functionality of DearImgui uses a callback, and the user is responsible for managing the underlying buffer (malloc, realloc, free). Since in pyimgui the string buffer is internally managed, it wasn't possible to implement the functionality via python.

Now the resizing of the buffer, if asked via the imgui.INPUT_TEXT_CALLBACK_RESIZE, is done internally and is fully transparent to python users. If the user provides a custom callback, it is still called properly after the internal buffer resizing.

Moreover, it is not very python friendly to ask for a buffer with a given size that is growable; one would have to create a variable buffer_size, globally access it in a provided callback function, and update it when the resize callback is called. This all comes from the c/c++ management of character strings and does not make a lot of sense for a python user. Thus I decided to simplify a bit the prototype of input_text, input_text_multiline, and input_text_with_hint. Now the argument buffer_length is optional and has a value of -1 by default. In this case, the buffer size sent to DearImgui is the length of the argument value and the flag imgui.INPUT_TEXT_CALLBACK_RESIZE is automatically set.

This allows writing input text field as

_, buf = imgui.input_text("Basic", buf)
_, buf = imgui.input_text_multiline("Multiline", buf)
_, buf = imgui.input_text_with_hint("With hint", "astuce", buf)

Of course, the old syntax with the buffer_length argument is still usable and would lead to a field with a maximum byte length

_, buf = imgui.input_text("With max", buf, 32)

I consider this PR a work in progress since working on this has highlighted some weird and suboptimal functioning of pyimgui that I wish to fix. DearImgui asks for a char* that we must create from the passed value argument. Currently for each call of input_text(), memory is dynamically allocated with malloc, the content is copied from value to the internal buffer, and this buffer is passed to DearImgui. The internal buffer is freed at the end of the function. This is problematic since all of this is done for each text field and for each frame (which is a lot). I am considering implementing a memory pool internally to pyimgui that would be allocated at the start and using it instead of dynamically allocating memory for each input_text() call.

Moreover, there is some cleaning to do in the methods of the class _ImGuiInputTextCallbackData.

Any feedback is welcome :)

@KinoxKlark
Copy link
Member Author

I changed the allocation strategy to be more reasonable.

Before, it was calling malloc/free every call to input_text() (and thus every frame for every field) and strncpy inside it.

Now we have a shared memory buffer that is kept inside pyimgui. If capacity isn't enough, it is doubled and reallocated. Thus now, memory allocation is done only a couple of times during runtime, which sounds way more sain to me.

It also considerably simplified the implementation of the auto-resize functionality, which is good.

@KinoxKlark
Copy link
Member Author

Lastly, I implemented the setter for the buffer attribute of the input text callback object, which allows for editing the content via the callback. For example, one can now do:

# Somewhere before
def callback_fn(data):
    if data.event_flag == imgui.INPUT_TEXT_CALLBACK_ALWAYS:
        buffer = [ c.upper() if c in "aeiou" else c for c in list(data.buffer) ]
        new_buffer = "".join(buffer)
        data.buffer = new_buffer

# ...
buf = "Some content..."

# In main loop
imgui.begin("Tests Callback")
_, buf = imgui.input_text("Callback", buf, flags=imgui.INPUT_TEXT_CALLBACK_ALWAYS, callback=callback_fn)
imgui.text("Output: %s" % buf)
imgui.end()

which would systematically replace every vowel with its upper case version.

This can only work for editing current characters; one cannot add more characters than the existing text length (but can do less). If you need to insert more characters, you can use data.insert_chars(pos, text), which properly resizes the internal buffer.

I believe this should finish the fix, and I'll merge it to dev/version-2.0 later if there are no objections.

@KinoxKlark KinoxKlark marked this pull request as ready for review August 9, 2022 10:28
@KinoxKlark KinoxKlark merged commit f9bdf38 into dev/version-2.0 Aug 10, 2022
@KinoxKlark KinoxKlark deleted the fix/auto-resizing-input-buffer branch August 10, 2022 08:44
@KinoxKlark KinoxKlark added the release pending Merged but still needs official release label Aug 10, 2022
@KinoxKlark KinoxKlark removed the release pending Merged but still needs official release label Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant