ImGuiNET / ImGui.NET

An ImGui wrapper for .NET.
MIT License
1.91k stars 307 forks source link

InputTextMultiline causes stack overflow with large strings #82

Open paavohuhtala opened 6 years ago

paavohuhtala commented 6 years ago

I'm working on a UI view which utilises InputTextMultiline as a part of OpenSAGE. The view is used for viewing INI configuration files. Some of these files are rather large - sometimes over a megabyte - which cause the method to throw a StackOverflowException. This is probably because the method stack allocates two large buffers.

I'm drawing the control like this:

ImGui.InputTextMultiline("ini string", ref _iniString, (uint) _iniString.Length, ImGui.GetContentRegionAvail(), ImGuiInputTextFlags.ReadOnly);

I'm not sure what the best solution would be. I only need to use the control in read-only mode, so I don't "need" the other buffer. I could manage a heap-allocated UTF8 encoded buffer myself, and pass it (or a pointer to it) manually.

TextUnformatted works as a workaround (even with large strings), but since it has no selection / copying capabilities it's not ideal.

mellinoe commented 6 years ago

Thanks for filing this. It's an issue I've been thinking about quite a bit, and I have some potential solutions to try out. For extremely large strings, it might always be best to store them in a dedicated "Utf8String"-like class, because it will avoid the need to convert back and forth from UTF8->UTF16. With such a large string, the conversion overhead will probably become noticeably large. Currently, there isn't an InputTextMultiline overload that takes a raw UTF8 buffer, but one can be added.

The string overloads should also have a cutoff for when they will use stack allocation, and fall back to a native allocation, or some sort of pooled native memory.

mellinoe commented 5 years ago

I have added a limit to stack allocation limit of 2048 bytes for all string marshalling. I'd still like to provide raw buffer overloads where it makes sense (because it will avoid costly copies), but for the time being the main scenario should be unblocked.