features/string_view: ImGui::TreeNodeEx() has different IDs when id is const char*/string literal

[Squareys]

Version/Branch of Dear ImGui:

Version: v1.87
Branch: docking + features/string_view

Back-end/Renderer/Compiler/OS

Back-ends: Magnum ImGuiIntegration
Compiler: MSVC19
Operating System: Windows 10

My Issue/Question:

Previously, when specifying the id of a tree node by string literal, the id would be pushed as string, now the same call leads to it being pushed as pointer (i.e. the const void* id overload seems to take precendence over the new ImStrv overload).

Standalone, minimal, complete and verifiable example:

    ImGuiTreeNodeFlags nodeFlags = ImGuiTreeNodeFlags_DefaultOpen|ImGuiTreeNodeFlags_OpenOnArrow;

    // Previously would push "root" to ID stack, now pushes some pointer, 0x1234ABCD...
    ImGui::TreeNodeEx("root", nodeFlags, "root");
    
    // Currently required workaround, correctly pushes "root"
    ImGui::TreeNodeEx(ImStrv{"root"}, nodeFlags, "root");

PS: I am aware that features/string_view is highly unstable and not production ready. I'm posting this issue only because you might find it useful/helpful, I'm not expecting a fix anytime soon.

[Squareys]

Same is the case for ImGui::PushID(const char*), which causes the id of PushMenuBar() to turn to a pointer rather than "##menubar".

Adding an overload as follows helps give precendence to the conversion to ImStrv over conversion to const void*:

    inline void          PushID(const char* ptr_id) { PushID(ImStrv(ptr_id)); }                                    // push pointer into the ID stack (will hash pointer).

[ocornut]

Hi Jonathan,

Yes we knew about this, sorry for not communicating on this branch well. From the pov of being used from a generated backend eg Rust I don’t think this would be a problem, but it is indeed a problem in C++ for which I don’t have a good solution now (other than adding extra char* versions to a bunch of Tree functions, which isn’t great).

The two other problems preventing this #3038 #175 to be merged are:

• standard sprintf functions don’t have non-zero terminated FORMAT. I think the best solution here is to switch to an embedded stb_printf, for which I have the patch ready (patch adds a bit of overhead over stb_printf, making its merge debatable, but from our view stb_printf is way faster than any standard version).

• Visual Studio don’t support natstepfilter files in projects, so the debug stepping experience will be harmed by ImStrv() adoption. I find it very annoying since VS IDE supports natvis in projects. There’s an open community issue on Microsoft website but it’s unlikely they are going to fix soon.

[Squareys]

Hey @ocornut!

Yes we knew about this, sorry for not communicating on this branch well.

No worries, I know it's more of WIP than e. g. docking might be. It saved us tons of time and bugs during our switch to string views, though, so we took the risk and so far do not regret it 👍

For later (post-vacation)

stb_printf is way faster than any standard version

We're using Corrade's format library for exactly that reason, it's way faster, has non-zero terminated output, all for switching to something fast there.
I'm wondering, though, it does seem to compile and run just fine with the standard printf. We briefly turned on the flag for stb_printf, but it didn't no-brain compile, so we postponed, but might try again.

support natstepfilter files in projects

Yeah, I did notice the debugging experience and I read about the natstepfilter issue in #3038.

One pitfall we ran into is that the varargs overloads of various widgets e.g. ::Text, ::TreeNodeEx do not support ImStrv args, which is an easy mistake to make (but not hard to fix once noticed). Edit: clang actually catches these, I recommend -Werror=format.

[ocornut]

FYI i pushed b9a8c5e which adds the workaround for those functions. I'm not happy about the solution but lacking a better one it'll do for now.

support natstepfilter files in projects
Yeah, I did notice the debugging experience and I read about the natstepfilter issue in #3038.

We got some good upvoting at https://developercommunity.visualstudio.com/t/allow-natstepfilter-and-natjmc-to-be-included-as-p/561718 but any extra vote would be useful (~10 more votes gets us on page 1 of open issues)